WIP: Strip out unchanged keys on object update #263
Conversation
Thanks for opening this pull request!
|
|
Let me know once you add some test cases and get it to pass the test suite and I can provide a review |
| // | ||
|
|
||
| import Foundation | ||
| struct JSONStorage { |
There was a problem hiding this comment.
This should conform to ParseKeyValueStorage so it can be accessed like the other storage mediums when necessary:
|
I've followed along the original issue to see the reasoning for needing this, and frankly, I need some mild "object memory" so my PR works to unset values that are explicitly unset, instead of just trying to unset any nil values. (see #264) I am a bit fearful about having an in-memory cache to compare against, but it's possible I'm just not understanding everything correctly. Take this example: You fetch the same object (same object ID) in two different places in the app, we'll call those A and B, and you fetch them at the same time. Then after a few minutes, you change all the values in object A, but object B isn't currently being used so it's not changed. The user goes to the place object B is used and changes one property there but not the others. My understanding is that object B being older would compare itself against the cache, see that all the properties are different even though all the data is technically out of date, and on save overwrites everything instead of only the one changed value which is the original goal. Obviously, this probably isn't the best data structure and could potentially be solved in other ways, but I think it illustrates the potential issue with having a global shared cache. I could also see a lot of less experienced developers (or even some more experienced ones) running into this exact issue. Would it be better for each Parse object to store a snapshot of its own values at the time of each fetch? Then on save it would only compare changes against its own snapshot, instead of a global cache that may change elsewhere. It seems to me this would be a safer way to achieve the stated goal. Am I missing a requirement or otherwise misunderstanding the goal or the proposed solution? |
This approach could possibly work, the storage should be part of each object as a property. Centralized cache will attempt to make the SDK work like its using reference types and will cause a ton of issues. The key (name of the property) of the local storage (let's say it's the |
| let previousStore = store[id], | ||
| let json = try JSONSerialization.jsonObject(with: encoded, options: []) as? [String: AnyObject] { | ||
| for keyName in json.keys where | ||
| (json[keyName] as? NSObject) == (previousStore[keyName] as? NSObject) |
|
Re-opened as this PR suggests improvements tracked in #401; not sure why this has been closed. |
|
Closing due to inactivity since 10/21. @dblythy feel free to reopen if you can address the issues mentioned in #263 (comment), #263 (comment), and #263 (comment) and if you think the contribution of this PR will be an improvement over #406 |
|
Since you closed this, we'll need to open another issue #408 to track this discussion about further improvement. |
|
Yes - sorry - I closed this as hearing the valid comments from @Vortec4800, I decided this approach probably wasn't the best way to go. I might reconsider / retry having a crack again in the future |
New Pull Request Checklist
Issue Description
Draft PR for ideas / suggestions for improvements around this approach.
If this isn't suitable for this project, I'll be happy to work on this branch for my own usage. I would welcome feedback so I can make sure my fork is as good as it can be 😊
Related issue: #242
Approach
Stores objects in JSON form in memory. When objects are saved, adds unchanged keys to SkipKeys.
TODOs before merging