aws-amplify / amplify-swift

A declarative library for application development using cloud services.
Apache License 2.0
455 stars 196 forks source link

Amplify.API.mutate specific attributes of a model. #1272

Open bumbleparrot opened 3 years ago

bumbleparrot commented 3 years ago

Is your feature request related to a problem? Please describe. When using Amplify.API.mutate(.update, model) do I always have to supply the full model? I attempted to create an empty model and only fill in the ID and the 1 attribute I wanted to update, "primaryNeighborhood". When I updated the model, it wiped out all fields that were left nil.

Describe the solution you'd like I would like to send partial attributes of model and have only those attributes updated. This should leave all other attributes as is in the update operation.

Describe alternatives you've considered The alternative is that I have to fetch a current and fresh copy of the record, then update the specific field and then send the entire model back over to be updated.

bumbleparrot commented 3 years ago

Actually, i think the solution to this is to create an extension to the GraphQLRequest.

I don't think there is an "Easy" generic solution.

This approach is specific to the data update use cases you need.

extension GraphQLRequest {
    static func updateProfilePrimaryNeighborhood(byId id: String, primaryNeighborhood: String) -> GraphQLRequest<Profile> {
        let operationName = "updateProfile"
        let document = """
        mutation MyMutation ($id: ID!, $primaryNeighborhood: String) {
          updateProfile(input: {id: $id, primaryNeighborhood: $primaryNeighborhood}) {
            id
            primaryNeighborhood
          }
        }
        """

        return GraphQLRequest<Profile>(document: document,
                                    variables:
                                        ["id": id,
                                         "primaryNeighborhood": primaryNeighborhood],
                                    responseType: Profile.self,
                                    decodePath: operationName)
    }
}

Thoughts on if this approach is going to be okay for the future?

It will break if the GraphQLRequest implementation changes drastically.

bumbleparrot commented 3 years ago

Bonus, doing it this way above automatically updates the "updatedAt" timestamp.

palpatim commented 3 years ago

@bumbleparrot As you discovered, there is a difference between "not specifying" a value and "specifying null", and we don't have a great way to distinguish between those two cases. If you use a model-based approach, every field of that model is included in the GraphQL document, which means that fields you don't specify are set to nil, which means that AppSync interprets the request to mean "set the values to null. In your solution above, you're specifying a partial document, which means that only the id and primaryNeighborhood fields are specified, and the mutation wouldn't affect any other field in the record. (Except of course any that might be auto-updated like "updatedAt", "version", etc.)

Bear in mind that a partially-specified mutation has implications for subscribers as well: if any field of the model is required, but not specified in the mutation's selection set, subscribers will not be able to deserialize a valid model. (There may also be restrictions around the mutation data you have to specify for required fields, but I can't remember off the top of my head.)

We don't have any current plans to change this behavior, so your solution above should work as long as your model constraints, app code, and subscribers work well together. Even if we did change, any breaks would be denoted by appropriate semantic version bumps. But my guess is that if we update, we'd do so either on the local request or the Swift model side to let us generate a GraphQL document with better control over "null" vs. "unspecified" fields. A change to the way AppSync interprets GraphQL documents would break for more than just iOS customers, so I don't see that as a reasonable path forward with the little bit of thought I've given this.

palpatim commented 3 years ago

For future clarification: We are leaving this open as a feature request for local mutations to distinguish between "null values" and "don't change the value". Currently, null in the Swift model means "set the value to null" and there is no way to say "don't change the value".

bumbleparrot commented 3 years ago

Thanks @palpatim for explaining those considerations.

I think we are good for now and can get by via the extension approach.

Thanks for keeping it open as a future request! I'm sure others do not always want to retrieve a full record just to update 1 field when you may already know a primary key for the full session of the app (like userId).