Parse-Swift
Parse-Swift copied to clipboard
Partially updating an object sends the full object to the server
New Issue Checklist
- [x] I am not disclosing a vulnerability.
- [x] I am not just asking a question.
- [x] I have searched through existing issues.
- [x] I can reproduce the issue with the latest versions of Parse Server and the Parse Swift SDK.
Issue Description
When fetching a saved object and then calling .save, all keys are marked as dirty and saved to the backend, causing unnecessary traffic and database ops.
Steps to reproduce
- Fetch an existing object
- Have a Parse Cloud beforeSave trigger that logs
object.dirtyKeys().
Actual Outcome
All fields are dirty.
Expected Outcome
No fields should be dirty unless mutated.
Environment
Client
- Parse Swift SDK version:
1.9.10 - Xcode version:
10.15.7 - Operating system (iOS, macOS, watchOS, etc.):
iOS - Operating system version:
14.7
Server
- Parse Server version:
4.5.0 - Operating system:
macOS - Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc):
localhost
Database
- System (MongoDB or Postgres):
mongodb - Database version:
4.2 - Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc):
Atlas
Logs
Thanks for opening this issue!
- 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.
Posting my responses to your comments in #241:
So there's no way to only send mutated keys to the server using Parse Swift?
There are a few ways, but the developer has to take some action here. For example, developers can add the following method (or a similar one) to their ParseObject's:
struct MyUser: ParseUser {
var authData: [String: [String: String]?]?
var username: String?
var email: String?
var emailVerified: Bool?
var password: String?
var objectId: String?
var createdAt: Date?
var updatedAt: Date?
var ACL: ParseACL?
// method that creates empty versions of self
func emptyObject() -> Self {
var object = Self()
object.objectId = objectId
return object
}
}
Then add properties what will be mutated and saved. Only these get sent to the server:
var myObject = MyUser()
myObject.email = "[email protected]"
myObject.save { _ in }
This also can be easily achieved by using ParseOperation: https://github.com/parse-community/Parse-Swift/blob/087227e340d61276fef604152517b7746b19a035/ParseSwift.playground/Pages/13%20-%20Operations.xcplaygroundpage/Contents.swift#L36-L58
These examples should address most of your comments below:
Just a quick comment - whilst this will likely fix the issue, the core problem of
dirtyKeysstill remains. This is problematic because:
- unnecessary data is sent to parse server, which in developing countries may result in unbearable latency, or, on a high scaled system, result in additional data expenses
- unnecessary database ops will happen when no data is actually changing
- Cloud functions will tell developers to assume keys are dirty when they are not
- email being set to dirty is just an example of an unpredictable consequences of all the keys being sent, there could be other cases which we don’t know about.
When it comes to:
@cbaker6 is there a way we could perhaps store the serverData from the initial fetch of the object, and then compare changes to keys onSave if we can’t directly listen to property changes on a struct?
"Listening to property changes" is typically associated to reference types or "instances of Classes" in many languages, not just Swift. Most/All of the Parse SDK's use "classes" with the exception of the Swift SDK. value types or "structs" don't have this ability and are one of the distinct difference between a struct and a class. The Swift SDK doesn't need to listen property changes and instead should use a method similar to what I mentioned at the beginning of this comment. It's important not to think or compare of the Swift SDK to the other SDKs as it has a completely different design philosophy. Flo's comments and thread provide good insight into the design philosophy and the reasoning. For these reasons, the Swift SDK can exist with zero dependencies, not run into the threading issues the other client SDKs encounter, along with having ParseObject's play nicely with MVVM and SwiftUI (from what I see reference types is what's causing a major barrier when using MVVM in the issues you and others mention with the JS SDK).
Thank you for your reply here @cbaker6.
In my view, it shouldn't be up to the developer to custom code a solution for the SDK to work as expected with the core Parse Server. One of our attractive features is simple SDKs which offer a broad range of consistent features. I understand that there are tradeoffs with the current implementation, but I still think it's problematic that at the end of the day, Parse Server has no distinction for dirtyKeys in the Swift SDK, which is different from any other SDK.
Although you've said in terms of philosophy it's not valuable to compare across SDKs, I think from both a developer and Server team perspective, it would be expected that beforeSave functions would function the same, regardless of the SDK. It just seems a little odd to me to say "if you use .dirtyKeys in Parse Cloud, make sure you customise ParseSwift so it doesn't send all the data".
And on the race condition, let's say you have an object that is being managed by two Swift SDK users at the same time. They both fetch the object at the same time.
The object looks like:
{
foo: "bar",
yolo: "xyz"
}
Both users query this object at the same time.
User one calls:
obj.foo = "aaa", and then saves.
A few minutes later, user two calls:
obj.yolo = "aaa", and then saves.
The result that will be saved in Parse Server would be:
{
foo: "bar",
yolo: "aaa"
}
As all keys saved by user two will be overriden. If this was any other SDK, the result would be:
{
foo: "aaa",
yolo: "aaa"
}
Meaning again, if you have a multi tenant solution (iOS App, Web App, Android App), using the Swift SDK in its purest form could be problematic and deliver unexpected results.
Could we perhaps store a clone of the fetched object somewhere, and then compare keys as part of the save operation? So when building the save operation, Parse Swift runs something like:
const fetchedObj // this has the fetched state
const updatedObj // this has all the user mutations
const bodyToSend = {};
for (const key in updatedObject) {
if (fetchedObj[key] !== updatedObj[key]) {
bodyToSend[key] = updateObj[key]
}
}
Forgive the JS as said I am not a Swift dev 😊
If this technically can't be solved on the SDK side because the use of struct, maybe the solution is to override dirtyKeys in RestWrite if object.key === original.key. I've discussed this with @mtrezza, however he has stated that believes:
Adding this to the server may be a mitigating step, but that doesn’t fully solve the cost and efficiency issue
In my view, it shouldn't be up to the developer to custom code a solution for the SDK to work as expected with the core Parse Server. One of our attractive features is simple SDKs which offer a broad range of consistent features. I understand that there are tradeoffs with the current implementation, but I still think it's problematic that at the end of the day, Parse Server has no distinction for dirtyKeys in the Swift SDK, which is different from any other SDK.
Although you've said in terms of philosophy it's not valuable to compare across SDKs, I think from both a developer and Server team perspective, it would be expected that beforeSave functions would function the same, regardless of the SDK. It just seems a little odd to me to say "if you use .dirtyKeys in Parse Cloud, make sure you customise ParseSwift so it doesn't send all the data".
It seems you are thinking in terms of the other SDKs, OOP, and Classes. I pointed to Flo's comment because the philosophy eludes to a number of things traditional users of the other SDKs may be unfamiliar with and the Swift SDK wasn't designed with the restrictions of OOP and Classes. I recommend looking into protocol oriented programing (POP) as it already forces the developer to customize and add their own keys for their ParseObjects, even for the Parse provided keys. The addition of the method I mentioned is simple and straightforward. In POP, developers should feel comfortable using the provided methods, replacing with their own, and adding extensions. As I mentioned in my previous comment, the design patterns of the other SDKs in terms of using OOP and Classes lead to other issues that are not present in the Swift SDK.
Could we perhaps store a clone of the fetched object somewhere, and then compare keys as part of the save operation? So when building the save operation, Parse Swift runs something like:
const fetchedObj // this has the fetched state const updatedObj // this has all the user mutations const bodyToSend = {}; for (const key in updatedObject) { if (fetchedObj[key] !== updatedObj[key]) { bodyToSend[key] = updateObj[key] } }
ParseSwift doesn't have local storage (besides using the Keychain for current) and if it did have storage, it would be protocol based, meaning the developer would implement it or use some 3rd party storage implementation that conformed to the "parse storage protocol". The 3rd party storage can have some notion of "dirty" if it felt it was needed, but depending on the implementation, it may not be necessary. The JS code you mentioned is in the style of the other SDKs which all use runtime encoders/decoders to dynamically access key/value pairs. Those encoders/decoders are also slower and prone to runtime bugs. ParseSwift uses compile time encoders/decoders and doesn't/shouldn't be doing anything like toJSON like the other SDKs to access/modify properties of an object.
Of course, a developer doesn't have to use the Swift SDK and can use the iOS SDK if they want OOP and Class based parse objects.
As all keys saved by user two will be overriden. If this was any other SDK, the result would be:
{ foo: "aaa", yolo: "aaa" } Meaning again, if you have a multi tenant solution (iOS App, Web App, Android App), using the Swift SDK in its purest form could be problematic and deliver unexpected results.
This looks like a synchronization issue as a developer can run into the same issue with any of the SDKs if 2 clients attempted to modify/save the same property of an object at the same time. The race condition you mentioned is because Parse doesn't have a real synchronization mechanism and out-of-the-box asks developers to depend on a wall-clock (createdAt and updatedAt). If developers implement their storage in a decentralized way as I suggested, they will never run into the issue you mentioned with the Swift SDK or any of the other SDKs.
Right, you're much more familiar with Swift and POP than I am, so I can't confidently argue either way. It just seems strange to me that updating an object with save fundamentally interacts differently with Parse Server than any other SDK. Creating, deleting objects is great with Parse Swift, but I'm having to create a custom method to update an existing object - a core feature of Parse. As I've said previously, I think this should be a consideration developers should be aware of, especially if they use cloud functions.
The synchronization issue will happen for the clients in the other SDKs, but it won't happen on the Parse Server as with the other sdks, only mutated keys are dirty. When a user calls .fetch in the JS SDK, they will get an object with both keys set by each respective user, whereas in the swift sdk they will only get the second users' save.
As I've said previously, I think this should be a consideration developers should be aware of, especially if they use cloud functions.
Agreed, they should definitely be made aware and see the workarounds I mentioned earlier.
Right, so the consideration here to be aware of, is that if you use Parse Swift, calling .save on an existing object, will always send all keys to be updated, regardless of whether they are mutated? And this can't be avoided internally on the SDK side because of the way that POP works? If this is the case, I think we should mitigate the dirtyKeys on the server to minimise the potential confusion with cloud code, or unpredictable consequences of internal read-only keys becoming modified.
If this is the case, I think we should mitigate the dirtyKeys on the server to minimise the potential confusion with cloud code, or unpredictable consequences of internal read-only keys becoming modified.
Im not sure what you mean here. Internal parse keys such as objectId, createdAt, etc. are always skipped when using the Swift SDK (unless when using customObjectId), if they get modified, it’s not because of the Swift SDK. Some of the other Client SDKs send the internal keys and depends on the server to strip or ignore them.
I see your server PR, makes sense and my guess is the trade off will be negligible when compared to the DB writes. The server does a lot of key checking already, the only possible improvement I can think of there is if you added your check to a place that was already checking keys to avoid another for loop, but I’m not sure if/where that place exists
The main concern for the server PR is to make sure cloud functions that use .dirty and dirtyKeys can be use as expected with ParseSwift
I think the PR I just opened with respect to this issue solves this problem and makes it pretty easy for the developer, server, and cloud code to handle dirty keys properly (server and cloud code due what they normally do). Essentially, the developer just has to:
- add
.emptyObjectwhen turning theirParseObjectinto a mutable object - make modifications/mutations as they normally would do
- save as they normally would do
Let me know what you think...
That sounds like a good solution to me!
What are the risks for the developer when using emptyObject? I assume the risks mentioned in previous comments have not been eliminated.
My understanding is that if a developer uses emptyObject, they still need to compare the updated fields to the fields on fetch.
E.g,
- object is fetched
- text fields are entered, save is pressed
- Check if text !== originalObject.key
- Assign to
emptyObjectif the key is dirty - Call .save on
emptyObject - Enumerate through
emptyObjectkeys and set them to the original object
What are the risks for the developer when using emptyObject? I assume the risks mentioned in previous comments have not been eliminated.
what risk has emptyObject not eliminated?
I am referring to the comments against offering a mutable object. If the Parse SDK is now offering a way for a developer to implement this, do the arguments against a mutable object still stand?
My understanding is that if a developer uses
emptyObject, they still need to compare the updated fields to the fields on fetch.E.g,
- object is fetched
- text fields are entered, save is pressed
- Check if text !== originalObject.key
- Assign to
emptyObjectif the key is dirty- Call .save on
emptyObject- Enumerate through
emptyObjectkeys and set them to the original object
The Swift SDK doesn't work like this. If a developer is "enumerating through keys" they are using the SDK incorrectly, introducing unnecessary client overhead and processing, and possibly introducing errors that are unrelated to the SDK. The proper process of using emptyObject correctly is outlined here https://github.com/parse-community/Parse-Swift/pull/243#issue-1005536964 and examples are in the playground files:
Regular ParseObject: https://github.com/parse-community/Parse-Swift/blob/5451e5cb39ddfb51425fe89c20a54e5d5ba19538/ParseSwift.playground/Pages/1%20-%20Your%20first%20Object.xcplaygroundpage/Contents.swift#L81-L123
Current User: https://github.com/parse-community/Parse-Swift/blob/5451e5cb39ddfb51425fe89c20a54e5d5ba19538/ParseSwift.playground/Pages/4%20-%20User%20-%20Continued.xcplaygroundpage/Contents.swift#L102-L122
Current Installation: https://github.com/parse-community/Parse-Swift/blob/5451e5cb39ddfb51425fe89c20a54e5d5ba19538/ParseSwift.playground/Pages/6%20-%20Installation.xcplaygroundpage/Contents.swift#L57-L74
There's no checking for dirty on the client-side or checking for any keys. You simply mutate the keys on emptyObject you want changed on the server and save. On the server side, you should do everything the same as you normally would. No special handling is needed unless you are creating your own way to handle data.
The Swift SDK doesn't work like this. If a developer is "enumerating through keys"
in the first example you showed, you set the required fields to emptyObject, and then after save set those fields on originalObject, which is the process I was explaining. If a developer wanted only updated keys to be assigned to emptyObject, they would add a simple if statement comparing new value to original object value.
If the developer doesn’t use an if statement to selectively assign the keys to emptyObject, then they may as well send the full object to the server.
I am referring to the comments against offering a mutable object. If the Parse SDK is now offering a way for a developer to implement this, do the arguments against a mutable object still stand?
Can you clarify what you are referring to? The Swift SDK has always offered mutable objects. The original concern from @dblythy was all keys were always sent to the server, no matter if their values were dirty/modified or not. My original response was developers could add a simple method to their ParseObjects to address this. The PR added the method for the developer to make the process easy. The Swift SDK still doesn't have a notion of dirty, because it isn't needed from the SDK. If a developer wants/needs dirty, they could do so when they implement their local storage option.
Is local storage needed to implement a notion of dirty or is that possible without it?
The Swift SDK doesn't work like this. If a developer is "enumerating through keys"
in the first example you showed, you set the required fields to emptyObject, and then after save set those fields on originalObject, which is the process I was explaining. If a developer wanted only updated keys to be assigned to emptyObject, they would add a simple if statement comparing new value to original object value.
I understand your points, if the developer wanted to update the original object locally after saving the mutations on emptyObject they could use something similar to what you mentioned. This is all use-case dependent though and if they have implemented their own local storage, they may or may not need this.
If a developer wants/needs dirty
Purely from an optimisation perspective, I don’t understand where a developer wouldn’t need dirty. For newer devs, sending the whole object all the time might seem fine as it doesn’t have any side effects, but in terms of saving server costs, using emptyObject or a local storage with the notion of dirty to minimise the amount of data sent seems essential to me
Purely from an optimisation perspective, I don’t understand where a developer wouldn’t need dirty.
This is using the mindset of the old SDKs which this SDK doesn't follow. In addition, this is all use-case dependent, I personally would never use dirty and the way the other SDKs implement in conjunction with the Parse Server promotes bad synchronization, but to each as on for design...
For newer devs, sending the whole object all the time might seem fine as it doesn’t have any side effects, but in terms of saving server costs, using emptyObject or a local storage with the notion of dirty to minimise the amount of data sent seems essential to me
The PR of emptyObject already solved this problem completely, so I'm not sure why this particular issue was reopened. The PR doesn't need "dirty" to accomplish this.
Reopening because I think @dblythy makes an important point. This can be a major concern in a real-word business environment, at least as I understand the discussion so far.
-
As long as sending only dirty fields is not yet the default behavior of the SDK, there should be a warning for developers that this SDK does not behave like all other Parse SDKs. Otherwise someone may switch from the ObjC to Swift SDK and get a surprise with their next cloud provider bill. Maybe there is a warning already somewhere?
-
From reading this and flovimart's thread I don't understand yet what the specific technical constraint are that do not allow the SDK to have a fully functional dirty implementation. How would the SDK make a http spec compliant PATCH request where only modified fields are sent, once Parse Server supports that? Maybe we can just talk about the specific technical challenge and put our heads together to see if we can find a solution together?
From reading this and flovimart's thread I don't understand yet what the specific technical constraint are that do not allow the SDK to have a fully functional dirty implementation. Maybe we can just talk about the specific technical challenge and put our heads together to see if we can find a solution together?
Sounds like a time-sync that I don't see going anywhere. I've either commented or linked all my reasoning. If you all feel you want to discuss/design/develop a solution, feel free, but I won't be able to commit time to this topic as I don't see it leading to anything productive.
The topic, "All keys are marked as dirty using Parse Swift, full body is sent when only one key is updated" has been addressed in its entirety.
You don't have to commit any time, maybe someone else will. I am asking because you are the expert here and maybe you can help me understand this:
How would the SDK make a http spec compliant PATCH request where only modified fields are sent, once Parse Server supports that? Wouldn't that require a dirty implementation?
How would the SDK make a http spec compliant PATCH request where only modified fields are sent, once Parse Server supports that? Wouldn't that require a dirty implementation?
The developer would use emptyObject or have something in their local storage implementation that only produces the modified fields to send to the server.
I’m sure I’ve said this before, but if a key is set to increment, and a developer uses operations to increment it, if they save without emptyObject it will override its incremented sync (if two users are calling increment and then one calls save).
I like the solution of emptyObject for now, but it seems according to @cbaker6, a hard-coded dirty option will require a storage option out of the box.
I think the discussion here is how do we make an emptyObject like solution the default behaviour.