Amplify-cli: Allow specifying strategy to implement @model id

[onlybakam]

Do you want to request a feature or report a bug?

Feature Request

What is the current behavior?

currently when using @model, resolver for create mutation sets id to $util.autoId().
There are situations where this is not wanted. e.g.: we may want to use the current user identity information as the ID (use case: build user setting store).

could the directive be updated to something like this:

directive @model(
    id: idStrategy
    queries: ModelQueryMap,
    mutations: ModelMutationMap,
    subscriptions: ModelSubscriptionMap
) on OBJECT
enum idStrategy {auto sub username cognitoIdentityId}
input ModelMutationMap { create: String, update: String, delete: String }
input ModelQueryMap { get: String, list: String }
input ModelSubscriptionMap {
    onCreate: [String]
    onUpdate: [String]
    onDelete: [String]
}

where:

auto: util.autoId()
sub: ctx.identity.sub
username: ctx.identity.username
cognitoIdentityId: ctx.identity.cognitoIdentityId

with auto as the default

[undefobj]

"we may want to use the current user identity information as the ID, like for a setting store" - Would you still need to do this? A better design would be to use that userID as a sort key or an index. The problem with what you suggest is now you get into the necessity for mobile and web developers to understand distributed systems concepts in databases, which is exactly what we want to avoid. This also has implications to offline storage and persistence in the clients. AutoId was far and away one of the most requested features from customers to be the default for provisioning over the past ~9 months, and this use case doesn't tell me it shouldn't be as you'll end up with hotkeys.

[onlybakam]

The problem with what you suggest is now you get into the necessity for mobile and web developers to understand distributed systems concepts in databases, which is exactly what we want to avoid.

i agree we should try to avoid this if possible

This also has implications to offline storage and persistence in the clients

not necessarily

The identity of a user can be used as a key to retrieve a single object. This is made available by CUP and does not require persistence beyond what is already done. Suppose i want to store information about a User in dynamodb table and want to put auth around it. right now i could do

type User @model @auth(rules: [{ allow: owner }]) {
id: ID!
owner: String!
}

there is no direct way to get this a User. i would have to use listUsers and filter on owner field. If there are 100s of users, that means scanning the all the rows before filtering the returned data. seems costly.

this might be a limited use case, but i could see this coming up for collaborative applications where user information is maintained with appsync.

thoughts?

[undefobj]

Not sure I understand that use case directly because you could just have a query to get that user's specific data, either with an expression or an index on the userId per my previous post. And this does impact offline storage tracking as the semantics by which we perform CRUD operations in local DBs is dependent on the key schema and references.

I'm still not convinced of your use case, but if we were to do something like this it would have to be an override and not the default, with caveats that it can break some things (hotkeys, offline).

[mikeparisstuff]

I think the more general form of this feature request is to allow overriding the default @model key schema which I definitely see a use case for. As @undefobj said, it would have to be an opt-in change and the default behavior would be the simple "id" hash key case that is currently used. Here are a few options we have talked about in the past:

1) A new @primary/@pk directive (and similar @sort/@sk):

# By default the key is autoId() but this is configurable.
directive @pk(expression: String = "$util.autoId()") on FIELD_DEFINITION

# This could even be expanded to allow custom index names. The transform would then
# find the @pk & @sk with the same index name and manage that GSI or LSI.
directive @pk(expression: String = "$util.autoId()", index: String) on FIELD_DEFINITION

The simple case would look like this and would override the name of the primary key to be "userId"
and resolvers will still use $util.autoId().

type Post @model {
  userId: ID! @pk # Users must opt-in by default we assume "id"
  ...
}

Or for a more advanced use case:

type Post @model {
  userId: ID! @pk(expression: "$ctx.identity.sub")
  ...
}

A similar approach could be taken for @sort or @sk...

The pros of this approach are that we could use a similar construct to allow people to bind @model types to existing tables that do not adhere to the "id" hash key assumption.

2) Allow client's to override the value of "id" on create

This would mean adding an optional "id" field to each @model type's CreateXInput and then updating the createX resolver to use a $util.defaultIfNull($ctx.args.input.id, $util.autoId()). This would leave the "id" as the single hash key but you would have the ability to override that value. You are still not able to define a custom sort key with this approach.

3) Add config to @model

Similar to how you said we could add another argument to @model that allows us to override key information. Something like this maybe.

directive @model(
    keys: ModelKeyMap,
    queries: ModelQueryMap,
    mutations: ModelMutationMap,
    subscriptions: ModelSubscriptionMap
) on OBJECT
# By default this is just { primary: "id" }.
# If changed then the create input type would ask the client to provide the IDs
input ModelKeyMap { primary: String, sort: String }

I think this is a worth while feature to explore but let's make sure the discussion is driven by use cases. Let me know what you think.

[onlybakam]

i actually like option 1) as an advanced configuration without impact the current @model directive

[mikeparisstuff]

I agree. I think this is a useful feature to have and could use a little more design. Let's use this issue to track the work. Also as a general note, a lot of these tasks like this one, https://github.com/aws-amplify/amplify-cli/issues/84, and https://github.com/aws-amplify/amplify-cli/issues/83 are just waiting for available hands if anyone is looking for a way to contribute 👍

[undefobj]

I still would like to see a good use case for this, so far all the ones I've heard are hypothetical or can be solved with proper indexing on the database. I'm very concerned of leaking implementation details into an abstraction that is supposed to allow customers to focus on the job to be done rather than how it's being done. Also the data reconciliation process for offline mutations in the cache with the backend could be impacted negatively by this, so we'd need to test that. The key indices are what we use to perform object synchronization, including dependent mutations with unique keys (e.g. blogs and comments).

If there is strong customer demand for this let's look at supporting but I'd really like to get more clarity on use cases and upvotes from developers that need this to meet a use case. It might very well be that there are some non-caching or non-Mobile/Web use cases that this helps with.

[psycura]

@mikeparisstuff
I also prefer variant 1

[blbradley]

This is a great issue. I don't think it is currently possible to create a one-to-one relationship between models and Cognito users without this.

[undefobj]

@blbradley That's a pretty interesting use case and we've chatted with some people about it in the past. Can you go into detail about what you're trying to build between AppSync, DynamoDB and Cognito and what data you'd be looking to store & access using GraphQL at different parts of the lifecycle?

[blbradley]

Sure! I'm building an app that will be doing integrations with various Git providers (GitHub/GitLab/others) and need to store the particular service's auth token per user.

[blbradley]

After more analysis, I believe my use case could be covered by Cognito custom user attributes.

[undefobj]

Sure! I'm building an app that will be doing integrations with various Git providers (GitHub/GitLab/others) and need to store the particular service's auth token per user.

If this was storing tokens in DynamoDB, that would be discouraged. Tokens should be ephemeral and on the device not stored centrally.

[blbradley]

Sorry, I'm talking about server-to-server interactions. Does your statement still apply to that?

[blbradley]

I suppose that it still applies, with the server being the device. Thanks!

[undefobj]

Sorry, I'm talking about server-to-server interactions. Does your statement still apply to that?

I think we'd need to investigate that more, however it's worth noting that use case isn't the primary scope for Amplify or the GraphQL transformer. This project is for client development using web and mobile apps.

[undefobj]

I want to find a way to localization and I'm thinking "locale" as sortKey might be one way for that.
But, since this is the first time for me to use dynamoDB and AppSync and I don't know it will work.

This is not the something to ask here though, would you let me know if you know how to do localization.
Thank you in advance!.

Can you give an example of the types of queries and mutations you would like to run as a use case in your app flow? Then the data model will fall out of that.

[blbradley]

My server-to-server interactions would happen in an Amplify managed Lambda and would be retrieving data for display in my Amplify managed React app.

[blbradley]

I wanted to report back. I had another piece of data, a GitHub App installation ID, that was needed per-user. Cognito custom user attributes worked well! The only problem is the limitation of maximum 25 custom user attributes. So, only a problem as an app grows.

[hisham]

fyi, just to add a vote, I'm also running into this, exact same scenario as https://github.com/aws-amplify/amplify-cli/issues/134. Storing users and so it seems to make sense that id of users should be cognito sub. Also the appsync client demos such as https://github.com/aws-samples/aws-mobile-appsync-chat-starter-angular (written by @onlybakam) do this so it's confusing to see an aws app demo doing this but then the amplify cli team saying this is not a good idea and will break things in the future.

So really my question is, if we're storing Users in dynamo through appsync, and that corresponds to Cognito users, what is the best practice of what the id should be? autoId() and then use the owner field like how amplify cli sets things up now or just set id to cognito sub?

Also why does amplify gql transformer set "owner" to "context.identity.username", shouldn't it be "context.identity.sub".

[CodySwannGT]

So, keep in mind that AppSync has offline enabled by default.

If offline is enabled, you MUST provide an optimistic response with your mutations.

In order to provide an optimistic response, you must provide the pk locally.

That is the use case we're currently hitting and I think the only way to solve that would be with the #2 proposed solution from @mikeparisstuff

[undefobj]

fyi, just to add a vote, I'm also running into this, exact same scenario as #134. Storing users and so it seems to make sense that id of users should be cognito sub. Also the appsync client demos such as https://github.com/aws-samples/aws-mobile-appsync-chat-starter-angular (written by @onlybakam) do this so it's confusing to see an aws app demo doing this but then the amplify cli team saying this is not a good idea and will break things in the future.

So really my question is, if we're storing Users in dynamo through appsync, and that corresponds to Cognito users, what is the best practice of what the id should be? autoId() and then use the owner field like how amplify cli sets things up now or just set id to cognito sub?

Also why does amplify gql transformer set "owner" to "context.identity.username", shouldn't it be "context.identity.sub".

@hisham we're not necessarily saying that it's a "bad idea" more that we're trying to find the best interface to maintain the abstraction for ease of use as well as flexibility. The GraphQL transformer was designed to remove the need (or greatly reduce) to write resolvers or worry about the underlying cloud implementation, so we just want to have thorough discussions around pros vs. cons for certain designs. As @CodySwannGT says part of our client implementations is to do offline by default and we have certain offline helpers that will create ephemeral local IDs when creating objects offline first and then use bookeeping techniques to reconcile them with the backend once the write is committed to the cloud. These client libraries also do optimistic responses, version control, and conflict resolution. The design allows us to do this regardless of the data source so while this is DynamoDB specific it could be other things in the future.

We just want to ensure that the designs of the interfaces are consistent, simple, and keep the proper abstractions while working seamlessly with our clients as well as allowing you the level of control you're looking for.

[kaustavghosh06]

This issue is fixed and would be a part of the next CLI version (0.1.30).

[ChenLi0830]

@kaustavghosh06 it's great to know the feature is added, where can we find docs about it?

[oste]

@kaustavghosh06 @mikeparisstuff did the sort key @sk/@sort end up getting implemented? Was getting an error trying these.

How would one add a sort key to @model?

Thanks

[strazeo]

I'm also interested in option 1 mentioned as I've got an existing DynamoDB table with a key different from 'id' and therefore the auto-generated resolvers don't match my schema.

Looking at #356 I can see that it implemented option 2 and mentions that option 1 would be a larger piece of work.

Would you consider an initial first step towards option 1 of enforcing zero/one 'ID' type per @model and modifying the dynamodb transformer and resolver generation to use that field name if provided or 'id' otherwise?

An example would be:

#model defined with custom ID field
type UserCustomId @model {
    userName: ID! 
    firstname: String!
    lastname: String!
}

#Auto generated delete input example 
input DeleteUserCustomIdInput {
  userName: ID
}

#model defined with no ID field
type UserNoId @model {
    firstname: String!
    lastname: String!
}

#Auto generated delete input example 
input DeleteUserNoIdInput {
  id: ID
}

I've already mocked this up locally (without sufficient testing / validations in place yet) but not sure if this is an approach you want to go with or just hold off on this until work can be done on the @pk/@sk/@sort functionality.

Happy to hear your plans for this piece of work / thoughts on the initial option.

[et304383]

As per https://github.com/aws-amplify/amplify-cli/issues/56#issuecomment-437635042

Where is the documentation on how to us this? What do I put into schema.graphql to specify that my user table ID field is to be populated by AppSync with the Cognito sub or identity ID?

Also, @undefobj all of your replies in this thread seem to indicate you don't understand that the way to restrict access to DynamoDB for a Cognito user to ONLY their record has historically been based on federated identities where the IAM permissions restrict the Dynamo access to a primary (hash) key being equal to the Cognito sub. THAT is why people are asking for this.

Suggesting putting a GSI on the owner column and looking up that way is ridiculous. There's no reason in a proper data model to not establish a hard 1 to 1 link between the Cognito record and the Dynamo record.

Edit: adding another point. If you don't support enforcing a one to one relationship between Cognito user and Dynamo record by the ID field, then nothing stops the user from calling create user as many times as they want and ending up with multiple records, all with the owner field matching. We are talking about input from client side here - you have to assume someone is going to misbehave.