-
-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Partially updating an object sends the full object to the server #242
Comments
Thanks for opening this issue!
|
Posting my responses to your comments in #241:
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 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 = "myNewEmail@parse.com"
myObject.save { _ in } This also can be easily achieved by using Parse-Swift/ParseSwift.playground/Pages/13 - Operations.xcplaygroundpage/Contents.swift Lines 36 to 58 in 087227e
These examples should address most of your comments below:
When it comes to:
"Listening to property changes" is typically associated to |
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 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 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:
A few minutes later, user two calls:
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
|
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.
ParseSwift doesn't have local storage (besides using the Keychain for 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. |
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 ( |
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 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 |
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 |
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 |
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:
Let me know what you think... |
That sounds like a good solution to me! |
What are the risks for the developer when using |
My understanding is that if a developer uses E.g,
|
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? |
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 #243 (comment) and examples are in the playground files: Regular ParseObject: Parse-Swift/ParseSwift.playground/Pages/1 - Your first Object.xcplaygroundpage/Contents.swift Lines 81 to 123 in 5451e5c
Current User: Parse-Swift/ParseSwift.playground/Pages/4 - User - Continued.xcplaygroundpage/Contents.swift Lines 102 to 122 in 5451e5c
Current Installation: Parse-Swift/ParseSwift.playground/Pages/6 - Installation.xcplaygroundpage/Contents.swift Lines 57 to 74 in 5451e5c
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 |
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. |
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. |
Maybe if you can give a specific problem statement, we can post it on Stack Overflow to get advice on how to implement the change from a wider audience. If we can't find a solution, I could approach someone from the Apple engineering team to discuss this. |
I'm assuming you are asking for yourself and @dblythy to come up with a problem statement since you both opened/labeled the issue. If you are asking me, not sure why I would make one when I don't see a current problem with the SDK design. Since you are both proposing the change, you should come up with it. |
I mean a problem statement for why we cannot change the SDK to send only modified fields on |
That should be on you all, since you both believe the current solutions don't address the problem. |
Ok, this is good constructive feedback as to why So, it seems that if someone wanted to pick up this issue and submit a PR, the recommended approach would be to store objects on fetch? As the Swift expert here Corey your view on the most optimal solution and downsides of any suggested changes is valuable, I've already stated how much I appreciate your work on this SDK and how easy and clean it is to used when compared to other SDKs. The focus here is on focusing on what is best for the community, and gracefully accepting constructive criticism (as per the code of conduct).
Our discussions here are canvassing different options to make the SDK more efficient. We're trying to get to a solution as easy and quick to implement as possible, and @mtrezza and I are simply only discussing potential ideas to solve the problem at this stage. Technical complexity aside, I think we all can agree that a more efficient |
For context, it's also worth mentioning that using However, if one client calls This as well as scalable network costing concerns (especially for developers in regions with slower bandwidth capacity / networking costs), in my view, is the reason why this issue is relevant and the discussion is open. There's no where that says our SDKs should function the same, but simplicity across SDKs makes Parse Server attractive to developers. It's reasonable to assume each SDK has its own caveats, but I don't think developers expect the core interaction with the REST API to change across SDKs (where one SDK replaces the whole object using In summary: Pros of changing
Cons of changing
I'm happy for |
@cbaker6 You mentioned in #392 (comment) that this issue has been fixed, which would be great news. It may help to untangle this discussion if you could provide a simple example here that shows what's needed to make this SDK only send the updated keys. This may help to condense the result of #315, which was merged after @dblythy's last comment here. As we know, in other SDKs it's as simple as this:
What would the code look like for an equivalent example using the Swift SDK? |
SynchronousParse-Swift/ParseSwift.playground/Pages/1 - Your first Object.xcplaygroundpage/Contents.swift Lines 198 to 230 in 89e5e69
AsynchronousParse-Swift/ParseSwift.playground/Pages/1 - Your first Object.xcplaygroundpage/Contents.swift Lines 107 to 143 in 89e5e69
UserParse-Swift/ParseSwift.playground/Pages/4 - User - Continued.xcplaygroundpage/Contents.swift Lines 136 to 156 in 89e5e69
InstallationParse-Swift/ParseSwift.playground/Pages/6 - Installation.xcplaygroundpage/Contents.swift Lines 68 to 84 in 89e5e69
|
So looking again at the JS example:
Would this be the complete equivalent in Swift or is there anything else needed?
|
Parse-Swift/ParseSwift.playground/Pages/1 - Your first Object.xcplaygroundpage/Contents.swift Lines 32 to 53 in 675168e
|
So let me try again,
Is that now the complete equivalent of this:
|
It looks right |
Is it correct that in order to send only updated keys to the server the developer must:
Is your conclusion in #315 that there is no way to prevent that the developer has to implement and maintain all this overhead and instead build this into the SDK? |
The developers override the default implementation of Parse-Swift/ParseSwift.playground/Pages/1 - Your first Object.xcplaygroundpage/Contents.swift Lines 44 to 53 in 675168e
If the developer doesn't override If a developer prefers, they can also use operations which looks more like your JS example, #268 (comment) |
Can you confirm that you see no technical way to avoid the developer having to override the |
If I knew of a better way it would have been added in PR 316-400 |
Thanks, I've updated the Migration guide with a condensed version of this thread. Could you please review the code examples and suggest any changes you may want to see? My suggestion would be to close this issue, as the bug has been addressed and it's now possible to send only the updated properties to the server, albeit with additional code and not by default. I would also move the point in the migration guide from "Known Issues" to "Behavioral Differences", as it's just the required difference in code we want to point out. At the same time I would open a "feature request" issue to track efforts to reduce the code overhead. PR #263 can then be linked to it as it suggests some approaches to achieve that. |
I’m fine with this |
The examples are to explain the behavioral difference between ObjC and Swift SDK. I don't intend on writing DocC tutorials (because of time), but anyone is free to do so. For now it's important to have a central migration guide, rather than bits of information scattered across issues and PR threads. Even a simple txt file would be better than nothing at this point.
This is a migration guide not a programming guide. It's not intended to teach when to use async/sync. There are use cases for both. There's a clear note that sync calls are used for simplicity. Feel free to open a PR to add async calls to the examples.
The goal of the guide is to help developers transition their way of thinking coming from the ObjC SDK. Each variant gives context to lead them to the final recommended approach. That is described in the code comments. If you find a way to convey that info in 1 condensed example please feel free to open a PR.
We'll analyze why if that happens. |
If there's no objection from @dblythy I'll go ahead and close this issue and open a feature request as commented above. |
Closing as limitation has been addressed; further improvement tracked in #401. |
New Issue Checklist
Issue Description
When fetching a saved object and then calling
.save
, all keys are marked asdirty
and saved to the backend, causing unnecessary traffic and database ops.Steps to reproduce
object.dirtyKeys()
.Actual Outcome
All fields are dirty.
Expected Outcome
No fields should be dirty unless mutated.
Environment
Client
1.9.10
10.15.7
iOS
14.7
Server
4.5.0
macOS
localhost
Database
mongodb
4.2
Atlas
Logs
The text was updated successfully, but these errors were encountered: