Skip to content
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

feat: add set() and auto merge updated properties with original #406

Merged
merged 5 commits into from
Sep 9, 2022

Conversation

cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Sep 7, 2022

New Pull Request Checklist

Issue Description

Developers need to call the mergeable instance property when they start to make updates to an ParseObject in order to use the Parse Servers "quirky patch". To have a local copy of the updated ParseObject merged with original, developers also need to implement the merge() method. If merge() is not implemented, the developer can call fetch() to get the merged updates.

Related issue: #401

Approach

Build onto #315 and add a set method that developers can call on their objects which automatically creates a mergeable instance that can be merged with the original object without the developer implementing merge(). An example is below:

/*
Assume you have a ParseObject named GameScore that has
properties named points and levels.
*/ 
var score = GameScore()
score.points = 10
let savedScore = try await score.save() // Create ParseObject on server
/* 
Now you want to update the saved object.
Multiple operations can be chained together.
*/
var updatedScore = try savedScore
      .set(\.points, to: 15)
      .set(\.levels, to: [1])
let savedUpdatedScore = try await updatedScore.save()
print(savedUpdatedScore) // Updated score

If a developer chooses to not implement the merge() and depends on the new default merging in this PR they are choosing to incur more computational overhead on the respective client after the update is already saved. This is due to the extra encoding/decoding needed to discover and complete the merge. The overhead will most likely increase the larger the ParseObject. If a developer wants to skip the overhead, they should implement merge() on each of their ParseObjects. The new set method is below:

/**
     Set the value of a specific `KeyPath` on a `ParseObject`.
     - parameter key: The `KeyPath` of the value to set.
     - parameter to: The value to set the `KeyPath` to.
     - returns: The updated `ParseObject`.
     - important: This method should be used when updating a `ParseObject` that has already been saved to
     a Parse Server. You can also use this method on a new `ParseObject`'s that has not been saved to a Parse Server
     as long as the `objectId` of the respective `ParseObject` is **nil**.
     - attention: If you are using the `set()` method, you do not need to implement `merge()`. Using `set()`
     may perform slower than implementing `merge()` after saving the updated `ParseObject` to a Parse Server.
     This is due to neccesary overhead required to determine what keys have been updated. If a developer finds decoding
     updated `ParseObjects`'s to be slow, implementing `merge()` may speed up the process.
     - warning: This method should always be used when making the very first update/mutation to your `ParseObject`.
     Any subsequent mutations can modify the `ParseObject` property directly or use the `set()` method.
 */
    func set<W>(_ keyPath: WritableKeyPath<Self, W?>, to value: W) -> Self where W: Equatable

TODOs before merging

  • Add tests
  • Add entry to changelog
  • Add changes to documentation (guides, repository pages, in-code descriptions)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Sep 7, 2022

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@cbaker6 cbaker6 linked an issue Sep 7, 2022 that may be closed by this pull request
3 tasks
@codecov
Copy link

codecov bot commented Sep 7, 2022

Codecov Report

Merging #406 (ec71155) into main (a94ac8c) will decrease coverage by 0.02%.
The diff coverage is 91.08%.

@@            Coverage Diff             @@
##             main     #406      +/-   ##
==========================================
- Coverage   89.98%   89.95%   -0.03%     
==========================================
  Files         159      158       -1     
  Lines       15130    15175      +45     
==========================================
+ Hits        13615    13651      +36     
- Misses       1515     1524       +9     
Impacted Files Coverage Δ
Sources/ParseSwift/Objects/ParseInstallation.swift 85.65% <86.66%> (-0.22%) ⬇️
Sources/ParseSwift/Objects/ParseUser.swift 87.09% <86.66%> (-0.37%) ⬇️
Sources/ParseSwift/Objects/ParseObject.swift 89.45% <87.17%> (-0.25%) ⬇️
Sources/ParseSwift/API/API+Command.swift 89.40% <100.00%> (ø)
Sources/ParseSwift/Types/ParseOperation.swift 100.00% <100.00%> (ø)
Sources/ParseSwift/LiveQuery/ParseLiveQuery.swift 76.75% <0.00%> (+0.58%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mtrezza
Copy link
Member

mtrezza commented Sep 8, 2022

If you have add properties to your ParseObjectthat are of type Date and need the precise time of
the date, you should implement merge() or call fetch() on your object after updating.

What does that mean? Could you give an example what the effect on a date value is?

@cbaker6
Copy link
Contributor Author

cbaker6 commented Sep 9, 2022

What does that mean? Could you give an example what the effect on a date value is?

This is has been updated in the docs, it's not an issue.

@mtrezza
Copy link
Member

mtrezza commented Sep 9, 2022

Could you point to where in the docs that is explained? Or do you mean it's not an issue anymore because date values are not affected anymore?

@cbaker6
Copy link
Contributor Author

cbaker6 commented Sep 9, 2022

Could you point to where in the docs that is explained? Or do you mean it's not an issue anymore because date values are not affected anymore?

The latter, there’s no issue with encoding/decoding dates

@cbaker6 cbaker6 merged commit 8245f9e into parse-community:main Sep 9, 2022
@cbaker6 cbaker6 deleted the setMethod branch September 9, 2022 18:31
@mtrezza mtrezza mentioned this pull request Sep 11, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send only changed properties to cloud by default when updating object
2 participants