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 operation to allow for single key updates #248

Merged
merged 12 commits into from
Oct 9, 2021

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Oct 2, 2021

New Pull Request Checklist

Issue Description

Currently, the solution to only updating one field on an object is to use emptyObject #249, which requires the developer to add the emptyObject property to their ParseObject manually.

Related issue: #242

Approach

This adds a new simple operation of .set, so that individual keys can be updated without manually adding a property.

Although .set is perhaps an artifact of the old SDK, this would allow developers to use .save to save the whole object, and .operation.set to update the object, which in my opinion, is a good balance, and provides the reasonable distinction between object.save and object.operation.

If we agree on this approach, and I haven't missed any side-effects such as #247, I think we can revert emptyObject.

You can set multiple values at once (score and levels) and only save modified properties by doing the following:

struct GameScore: ParseObject {
        // Those are required for Object
        var objectId: String?
        var createdAt: Date?
        var updatedAt: Date?
        var ACL: ParseACL?

        // Your own properties
        var score: Int
        var members = [String]()
        var levels: [String]?
        var previous: [Level]?

        // custom initializers
        init() {
            self.score = 5
        }

        init(score: Int) {
            self.score = score
        }
}

let score = GameScore(score: 10)
do {
    // Perform any number of operations.
    let operations = try score.operation
            .set(("score", \.score), value: 15)
            .set(("levels", \.levels), value: ["hello"])

    // Use new async/await to save operations.
    let saved = try await operations.save()
} catch {
  print(error)
}

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 Oct 2, 2021

Thanks for opening this pull request!

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

@codecov
Copy link

codecov bot commented Oct 2, 2021

Codecov Report

Merging #248 (43d5e3f) into main (7932ba7) will increase coverage by 0.04%.
The diff coverage is 91.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
+ Coverage   76.63%   76.67%   +0.04%     
==========================================
  Files          99       99              
  Lines       10351    10374      +23     
==========================================
+ Hits         7932     7954      +22     
- Misses       2419     2420       +1     
Impacted Files Coverage Δ
Sources/ParseSwift/Operations/ParseOperation.swift 74.08% <91.30%> (+1.76%) ⬆️
Sources/ParseSwift/Coding/ParseEncoder.swift 73.25% <0.00%> (-0.36%) ⬇️
Sources/ParseSwift/Objects/ParseUser.swift 79.44% <0.00%> (-0.22%) ⬇️
Sources/ParseSwift/Types/Query.swift 93.84% <0.00%> (+0.22%) ⬆️
Sources/ParseSwift/Coding/AnyEncodable.swift 37.79% <0.00%> (+2.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7932ba7...43d5e3f. Read the comment docs.

@dblythy
Copy link
Member Author

dblythy commented Oct 3, 2021

Adds two new options to operation:

  • .set, which sends the field if it has been updated since last fetch
  • .forceSet, which force sets the field, regardless of last fetch (similar to current behaviour on struct)

@dblythy
Copy link
Member Author

dblythy commented Oct 8, 2021

@cbaker6 @mtrezza do you have any thoughts around this approach for an OOTB solution for dirtyKeys ? 😊

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 8, 2021

I updated your code to use keyPath's instead. Feel free to keep making updates and let me know when it's ready for review.

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 8, 2021

Can you rebase with the main branch? That should get rid of the codecov failure.

@dblythy
Copy link
Member Author

dblythy commented Oct 8, 2021

Thanks @cbaker6. I appreciate your help 😊

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 8, 2021

np, I think this should be good to go after merging with main and adding the objectId test case. For the objectId test case, I'm guessing the ParseEncoder should ignore changes in the standard cases.

@dblythy
Copy link
Member Author

dblythy commented Oct 8, 2021

One quick question- does operation pass values back to target after save is called?

edit: I’m assuming your additions solve this

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 8, 2021

One quick question- does operation pass values back to target after save is called?

edit: I’m assuming your additions solve this

Yes, mutates target, you can add a test case to be sure

@dblythy
Copy link
Member Author

dblythy commented Oct 8, 2021

I think i've added the test cases that you were after - can you let me know if this is what you have in mind?

Also, I added a XCTAssertEqual to check if target is updated after an operation is called, and it seems to be failing. Do you have any pointers here?

Tests/ParseSwiftTests/ParseOperationTests.swift Outdated Show resolved Hide resolved
Tests/ParseSwiftTests/ParseOperationTests.swift Outdated Show resolved Hide resolved
@cbaker6
Copy link
Contributor

cbaker6 commented Oct 8, 2021

I think i've added the test cases that you were after - can you let me know if this is what you have in mind?

Also, I added a XCTAssertEqual to check if target is updated after an operation is called, and it seems to be failing. Do you have any pointers here?

Here it returns the mutated target, so you want to compare against, operations.target?.score instead of the original score.

@dblythy
Copy link
Member Author

dblythy commented Oct 8, 2021

Right, thanks!! So if you use operations you should assign score to operations.target after save?

I’m assuming that because the SDK isn’t reference types, you have to do this yourself

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 8, 2021

Right, thanks!! So if you use operations you should assign score to operations.target after save?

I added two more test cases. The mutations on the target is returned after saving, so the developer doesn't need to handle target directly.

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 8, 2021

Don't forget to add the change log entry

@dblythy dblythy requested a review from cbaker6 October 9, 2021 00:55
@cbaker6
Copy link
Contributor

cbaker6 commented Oct 9, 2021

@dblythy I updated the description some. Feel free to change if you see something that's incorrect.

I moved you to the front of the change log since you began the PR.

Also, did #248 (comment) answer your question?

I'm going to make one more edit to prepare for release. We will wait to your other PR is finished to actually make the release.

@dblythy
Copy link
Member Author

dblythy commented Oct 9, 2021

Yep, question answered. I just couldn't figure out how to write the test case to emulate the save op, so thanks for that!

No worries, I'll get to the other PR in the next few hours. Thanks again!!

@cbaker6 cbaker6 linked an issue Oct 9, 2021 that may be closed by this pull request
4 tasks
@cbaker6 cbaker6 merged commit 7aeef06 into parse-community:main Oct 9, 2021
@dblythy dblythy deleted the dirty-keys branch October 9, 2021 07:12
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.

Partially updating an object sends the full object to the server
2 participants