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

[DataStore] Model cannot be synced to cloud due to index field is null #1390

Closed
HuiSF opened this issue Aug 24, 2021 · 8 comments
Closed

[DataStore] Model cannot be synced to cloud due to index field is null #1390

HuiSF opened this issue Aug 24, 2021 · 8 comments
Labels
bug Something isn't working datastore Issues related to the DataStore category pending-community-response Issue is pending response from the issue requestor

Comments

@HuiSF
Copy link
Member

HuiSF commented Aug 24, 2021

Describe the bug

When saving a model with below schema, if the index field is not assigned, model can only be saved into local DB, but failed syncing to DynamoDB. In addition, amplify-ios doesn't report the underlying GraphQL error on invoking DataStore.save API.

type TestModel @model @key(name: "parent-id-index", fields: ["parentId"]) {
  id: ID!
  content: String!
  parentId: ID
}

Original debugging please refer to aws-amplify/amplify-flutter#306 (comment)
Same issue in amplify-android: aws-amplify/amplify-android#1461

Steps To Reproduce

Steps to reproduce the behavior:
1. Create a minimal iOS App with above mentioned model schema
2. Create a model without assigning the `parentId` field
3. Save model
4. Observe

Expected behavior

  1. Model should be saved into DynamoDB
  2. amplify-ios should report underlying GraphQL error if item 1 is unavoidable

Amplify Framework Version

1.13.3

Amplify Categories

DataStore

Dependency manager

Swift PM

Swift version

5

CLI version

5.3.0

Xcode version

12.5.1

Relevant log output

No response

Is this a regression? (i.e. was this working before a version upgrade)

No response

Device

iPhone 12 simulators

iOS Version

iOS 14.5

Specific to simulators

No response

Additional context

Swift code used for testing

  let testModel = TestModel(content: "Test model 1")
        Amplify.DataStore.save(testModel) {
            switch $0 {
            case .success:
                print("saved test model")
            case .failure:
                print("failed")
            }
        }

Above code executed successfully, there is error message observed in debug console.

GraphQL request

{
	"query": "mutation CreateTestModel($input: CreateTestModelInput!) {\n  createTestModel(input: $input) {\n    id\n    content\n    parentId\n    __typename\n    _version\n    _deleted\n    _lastChangedAt\n  }\n}",
	"variables": {
		"input": {
			"content": "Test content",
			"id": "902cd218-799e-4b66-9d24-20a0d7406d22",
			"parentId": null
		}
	}
}

GraphQL response

{
	"data": {
		"createTestModel": null
	},
	"errors": [{
		"path": ["createTestModel"],
		"data": null,
		"errorType": "DynamoDB:DynamoDbException",
		"errorInfo": null,
		"locations": [{
			"line": 2,
			"column": 3,
			"sourceName": null
		}],
		"message": "One or more parameter values were invalid: Type mismatch for Index Key parentId Expected: S Actual: NULL IndexName: parent-id-index (Service: DynamoDb, Status Code: 400, Request ID: B3AHK1Q1ESMHO7635VKKGQHP9JVV4KQNSO5AEMVJF66Q9ASUAAJG, Extended Request ID: null)"
	}]
}

After removing parentId: null from variables, model can be synced into DynamoDB.

@diegocstn diegocstn added datastore Issues related to the DataStore category bug Something isn't working labels Aug 25, 2021
@lawmicha
Copy link
Member

the generated model and schema (amplify CLI 5.0.1)

// swiftlint:disable all
import Amplify
import Foundation

public struct TestModel: Model {
  public let id: String
  public var content: String
  public var parentId: String?
  public var createdAt: Temporal.DateTime?
  public var updatedAt: Temporal.DateTime?
  
  public init(id: String = UUID().uuidString,
      content: String,
      parentId: String? = nil) {
    self.init(id: id,
      content: content,
      parentId: parentId,
      createdAt: nil,
      updatedAt: nil)
  }
  internal init(id: String = UUID().uuidString,
      content: String,
      parentId: String? = nil,
      createdAt: Temporal.DateTime? = nil,
      updatedAt: Temporal.DateTime? = nil) {
      self.id = id
      self.content = content
      self.parentId = parentId
      self.createdAt = createdAt
      self.updatedAt = updatedAt
  }
}
// swiftlint:disable all
import Amplify
import Foundation

extension TestModel {
  // MARK: - CodingKeys 
   public enum CodingKeys: String, ModelKey {
    case id
    case content
    case parentId
    case createdAt
    case updatedAt
  }
  
  public static let keys = CodingKeys.self
  //  MARK: - ModelSchema 
  
  public static let schema = defineSchema { model in
    let testModel = TestModel.keys
    
    model.pluralName = "TestModels"
    
    model.fields(
      .id(),
      .field(testModel.content, is: .required, ofType: .string),
      .field(testModel.parentId, is: .optional, ofType: .string),
      .field(testModel.createdAt, is: .optional, isReadOnly: true, ofType: .dateTime),
      .field(testModel.updatedAt, is: .optional, isReadOnly: true, ofType: .dateTime)
    )
    }
}

@lawmicha
Copy link
Member

lawmicha commented Aug 31, 2021

Is the expectation that when no parentId is passed in, then parentId should be empty string ""? are you able to get a successful sync to cloud if you create the TestModel with empty string for the parentId? perhaps there's a codegen change we could do here to change the init to default to empty string instead of nil when it knows that the personId is a secondary index.

amplify-ios doesn't report the underlying GraphQL error on invoking DataStore.save API.

can you clarify this as well? do you mean the errorHandler registered and is not called? as I recall, if this is being built for flutter, the errorHandler is not exposed (yet)

After removing parentId: null from variables, model can be synced into DynamoDB.

So what happens if you try to retrieve it again from AppSync? parentId is not added on the create mutation, TestModel is saved to DynamoDB. When you retrieve it, is the value of personId an empty string or null?

@lawmicha
Copy link
Member

lawmicha commented Aug 31, 2021

currently iOS DataStore will sync all the fields from the model instance, so when there is no personId field, null is passed in the GraphQL variables.

ideally, we do per field syncing such that when a create mutation is created, since personId is optional and is not in the model we would then create a GraphQL variables object that doesn't contain the personId key. This would also avoid the problem of sending null while it is a secondary index, and avoid having to work around this by sending empty string.

we should also consider data manipulation afterwards: If the model is then synced to the local store, and then retrieved using DataStore.query, and updated using DataStore.save(), then what will the update mutation have when syncing to the cloud?null personId? if we have per field updates then the "personId" field would not have been marked as dirty and should not be added to the graphql variables

@HuiSF
Copy link
Member Author

HuiSF commented Aug 31, 2021

Is the expectation that when no parentId is passed in, then parentId should be empty string ""?

For this case, parentId should be omitted from the GraphQL document when sending the request to AppSync, therefore the parentId field in DynamoDB remains "empty" (not a empty string, just has no value), when I omit the field, syncing succeeded

can you clarify this as well?

Yes Flutter is missing errorHandler implementation at this moment. It may be the reason Flutter didn't get any kind of error on this use case. (I haven't tested errorHandler in iOS or Android libraries)

When you retrieve it, is the value of personId an empty string or null?

As the model now cannot be synced to cloud, there was no retrieving happened. But you possibly can test the list queries via AppSync console.

query MyQuery {
  listTestModels {
    items {
      parentId
      id
      content
    }
  }
}

Returns below result

{
  "data": {
    "listTestModels": {
      "items": [
        {
          "parentId": "1234",
          "id": "b2595335-b118-440b-aac3-0fc8a07592e5",
          "content": "Test model with parentID null"
        },
        {
          "parentId": null,
          "id": "1bbdba5f-7dcd-450e-ac8d-de54885dd47f",
          "content": "Test content"
        },
        {
          "parentId": null,
          "id": "b2595225-b118-440b-aac3-0fc8a07592e5",
          "content": "Test model with parentID null"
        }
      ]
    }
  }
}

For the empty parentId fields it returns null. (empty != empty string)

@lawmicha
Copy link
Member

lawmicha commented Aug 31, 2021

thanks for the quick response, I think field-level saves and updates may end up fixing this. Alternatively we could be more explicit either in the library or Appsync service. If secondary index at the DynamoDB level can be empty but not null, then can AppSync API accept null values and ignore it? As we saw, the list query returns null values when it is empty in DynamoDB. Otherwise, the library can be more explicit in 1. having this information in the code generated field that the field is a secondary index, and 2. when creating the graphQL variables, omit the key/value in the GraphQL variables if the model instance contains nil personId

@HuiSF
Copy link
Member Author

HuiSF commented Feb 11, 2022

This is no longer an issue with GraphQL Transformer v2
example schema

type SecondIndexModel @model {
  id: ID! @primaryKey
  parentID: ID @index(name: "parent-id-index")
}

When parentID is unset, the underlying GraphQL document contains parentID: null and AppSync no longer rejects the request.

If there is no requirement to fix for v1, please close accordingly, thanks!

Update: this issue is still reproducible with transform v2

@HuiSF
Copy link
Member Author

HuiSF commented Mar 4, 2022

Amplify CLI opened an issue for looking for a solution aws-amplify/amplify-cli#9915

@lawmicha lawmicha added the pending-community-response Issue is pending response from the issue requestor label Mar 15, 2022
@HuiSF
Copy link
Member Author

HuiSF commented May 12, 2022

The underlying issue has been fixed on Amplify CLI side.

Please ensure upgrade to Amplify CLI version >=8.0.2, and re-deploy schema (regenerate resolvers).

@HuiSF HuiSF closed this as completed May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working datastore Issues related to the DataStore category pending-community-response Issue is pending response from the issue requestor
Projects
None yet
Development

No branches or pull requests

3 participants