fix(datastore): observe API mutation event decode to model successfully #2684
Conversation
| if isEagerLoad && ModelRegistry.modelType(from: modelSchema.name)?.rootPath != nil { | ||
| self.isEagerLoad = false | ||
| } |
There was a problem hiding this comment.
We need to control the eagerload or lazyloading, used by the queries that result in models encoded in MutationEvents. If the Model types do not support lazy loading (LazyReference encapsulation) then we need to continue eager loading on queries, much like the DataStore.query API path.
| do { | ||
| let models = try await withCheckedThrowingContinuation { continuation in | ||
| storageAdapter.query(modelSchema: modelSchema, predicate: groupedQueryPredicates) { result in | ||
| storageAdapter.query(modelSchema: modelSchema, predicate: groupedQueryPredicates, eagerLoad: true) { result in |
There was a problem hiding this comment.
CascadeDelete occurs when DataStore.delete is called for a parent model with has-many connected models. For example when deleting a Post, we query for the Comments, sync the deletions from children to parents, queuing up the comments then the post itself. The results of these queries are internal to the CascadeDeleteOperation so it doesn't matter if it's eager loaded or lazy loaded for models which support lazy loading, but have to be eager load for previously generated models. We can control this flag like how we do this for queries from other places (AWSDataStorePlugin and ReconcileAndLocalSaveOperation) which could be an optimization on the query against the local store. I would prefer to keep optimizations and testing that out of the scope of this PR though
| case .notLoaded(let associatedIdentifiers, | ||
| let associatedField): | ||
|
|
||
| if let associatedId = associatedIdentifiers.first { | ||
| let metadata = DataStoreListDecoder.Meetadata(associatedId: associatedId, | ||
| associatedField: associatedField) | ||
| var container = encoder.singleValueContainer() | ||
| try container.encode(metadata) | ||
| } |
There was a problem hiding this comment.
By delegating the encoding logic down to the provider, the provider encodes the metadata back into a DataStoreListDecoder.Meetadata so that if the LazyReference gets decoded from this object, it can create the Provider again through DataStoreListDecoder.decode method
| query(modelSchema: modelSchema, | ||
| predicate: untypedModel.identifier(schema: modelSchema).predicate, | ||
| eagerLoad: eagerLoad) { |
There was a problem hiding this comment.
This method save used to return a model from the input, which is now inaccurate in the lazy loading cases. The input model untypedModel may be a model that was from the response of an Amplify.API mutation call. Imagine returning this model back to the caller in a MutationEvent on the DataStore Observe API. This will mean that the lazy loading functionality will delegate to the AppSyncModelProvider and AppSyncListProvider, calling into API directly. To avoid this, we do a query to get the latest data from the local store and return that data to the caller (model -> MutationEvent -> emit to Observe API)
| // Reset is used in internal testing only. Some operations get kicked off at this point and do not finish | ||
| // We're sometimes hitting a deadlock when waiting for them to finish. Commenting this out and letting | ||
| // the tests continue onto the next works pretty well, but ideally ReconcileAndLocalSaveOperation's should | ||
| // always finish. We can uncomment this to explore a better fix that will still gives us test stability. | ||
| //reconcileAndSaveQueue.waitUntilOperationsAreFinished() |
There was a problem hiding this comment.
See comment above, this seemed to help with test stability in the LazyLoading tests. A deadlock happens when we call DataStore.save, wait for first mutation event, and then finish by calling reset. Reset calls reconcileAndSaveQueue.waitUntilOperationsAreFinished() while another subscription event comes in which causes a ReconcileAndLocalSaveOperation to start, but never finishes, while this thread is waiting for all operations to finish. I think this must have been happening in the DataStoreIntegrationTests as well.
lawmicha
temporarily deployed
to
IntegrationTest
GitHub Actions
Inactive
lawmicha
temporarily deployed
to
IntegrationTest
GitHub Actions
Inactive
lawmicha
temporarily deployed
to
IntegrationTest
GitHub Actions
Inactive
lawmicha
temporarily deployed
to
IntegrationTest
GitHub Actions
Inactive
|
Workflow dispatch w/ this branch |
lawmicha
temporarily deployed
to
IntegrationTest
GitHub Actions
Inactive
lawmicha
had a problem deploying
to
IntegrationTest
GitHub Actions
Failure
lawmicha
temporarily deployed
to
IntegrationTest
GitHub Actions
Inactive
AmplifyPlugins/DataStore/Sources/AWSDataStorePlugin/Core/DataStoreListProvider.swift
Outdated
Show resolved
Hide resolved
…ly (#2684) * chore: enable lazy loading integration tests * fix(datastore): observe API mutation event decode to model successfully * update branches for GH workflows * fix typo
…ly (#2684) * chore: enable lazy loading integration tests * fix(datastore): observe API mutation event decode to model successfully * update branches for GH workflows * fix typo
…ly (#2684) * chore: enable lazy loading integration tests * fix(datastore): observe API mutation event decode to model successfully * update branches for GH workflows * fix typo
2 participants
Issue #
Description
This PR addresses a bug related to DataStore's observe API. When observing data through this API, a MutationEvent is received on the async sequence. The MutationEvent contains data about the mutation that occurred, version of the model, and the model encoded. To retrieve the model back, the developer can decode the mutation event's
jsonproperty to a model type:The decoded model should have the lazy loading functionality available, for example.
receivedPostshould be able to lazy load its comments.receivedCommentshould be able to lazy load the posts. The resulting decoded model should have the same functionality as if a query occurred. This is essentially what's happening when the model gets reconciled, in the last step before emitting the MutationEvent, in ReconcileAndLocalSaveOperation. A query is performed to retrieve the model and is emitted to the Observe API subscribers as an encoded model inside a MutationEvent object.This PR fixes the problem with encoding the queried model, for the MutationEvent, so that when the caller decodes the model from the MutationEvent, the lazy loading functionality will work.
The bug is that the encoding logic for LazyReference and List did not delegate it to the modelProvider, which contains the state of the object. Delegating the encoding functionality to the underlying modelProvider allows the modelProvider to control the shape of the encoded object, thus when the custom decoding logic runs, it will decode successfully back into a modelProvider though the ModelListDecoder/ModelProviderDecoders (DataStoreListDecoder/AppSyncListDecoder/DataStoreModelDecoder/AppSyncModelDecoder).
General Checklist
Given When Theninline code documentation and are named accordinglytestThing_condition_expectation()By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.