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(@aws-amplify/datastore): add Selective Sync #7001

Merged
merged 11 commits into from
Oct 28, 2020

Conversation

iartemiev
Copy link
Contributor

@iartemiev iartemiev commented Oct 19, 2020

Works in concert with these CLI changes: aws-amplify/amplify-cli#5586

DataStore - Selective Sync Feature

Enables developers to filter data on the back end before it gets synced down into their users' local stores, as opposed to syncing the entire contents of the associated data source.

Additional details:

  • Provides code completion ("Intellisense") for returning a valid expression (just like Query predicates).
  • Applies the same filtering to inbound AppSync subscriptions.
  • Adds new DataStore.stop() method for stopping the sync & subscriptions without clearing out the contents of the local store.

Misc.:

  • renames asyncstorage.ts -> AsyncStorageAdapter.ts
  • renames indexeddb.ts -> IndexedDBAdapter.ts
  • make Reachability.ts more WebWorker-friendly

Addresses the following feature requests/issues:

Basic usage

Selective Sync is configured by specifying syncExpressions, a newly added optional configuration parameter.
Developers can choose which model(s) they would like to apply an expression to.

import { DataStore, syncExpression } from 'aws-amplify';
import { Post, Comment } from './models';

DataStore.configure({
  syncExpressions: [
    syncExpression(Post, () => {
      return (c) => c.rating('gt', 5);
    }),
    syncExpression(Comment, () => {
      return (c) => c.status('eq', 'active');
    }),
  ]
});

When DataStore starts syncing, only Posts with rating > 5 and Comments with status === 'active' will be synced down to the user's local store.
Developers should only specify a single syncExpression per model. Any subsequent expressions for the same model will be ignored.

Reevalute expressions at runtime

Sync expressions get evaluated in DataStore.start() so it's possible to reevaluate them at runtime.

let rating = 5;

DataStore.configure({
  syncExpressions: [
    syncExpression(Post, () => {
      return (c) => c.rating('gt', rating));
    })
  ]
});

If you want to start syncing down Posts with rating > 1, you can do:

async function changeSync() {
  rating = 1;
  await DataStore.stop(); // or DataStore.clear() if you also want to empty the contents of the local store
  await DataStore.start();
}

Upon calling start() (or executing a DataStore operation, e.g., query, save, delete, or observe), DataStore will reevaluate the syncExpressions.
In the above case, the predicate will contain the value 1, so all Posts with rating > 1 will get synced down.

Using async expressions

You can pass a Promise to syncExpression:

DataStore.configure({
  syncExpressions: [
    syncExpression(Post, async () => {
      const ratingValue = await getRatingValue();
      return (c) => c.rating('gt', ratingValue);
    })
  ]
});

DataStore will wait for the Promise to resolve before applying the expression to sync. Async expressions can also be reevaluated at runtime, just like synchronous expressions (see previous section).

Shorthand

If you don't need to add any logic to your syncExpression, you can use the following shorthand (returning the predicate, as opposed to a function or promise that in turn returns a predicate like in the above examples):

DataStore.configure({
  syncExpressions: [
    syncExpression(Post, (c) => c.rating('gt', 5))
  ]
});

Advanced use case

You can utilize Selective Sync in order to have your base sync retrieve items from DynamoDB via query against a table, LSI, or GSI. By default, the base sync will perform a scan.
In order to do that, your syncExpression should return a predicate that maps to a queryable expression.

For example, for the following schema:

type User @model
  @key(name: "byLastName", fields: ["lastName", "createdAt"]) {
  id: ID!
  firstName: String!
  lastName: String!
  createdAt: AWSDateTime!
}

This sync expression will produce a query operation:

DataStore.configure({
  syncExpressions: [
    syncExpression(User, async () => {
      const lastName = await getLastNameForSync();
      return (c) => c.lastName('eq', lastName).createdAt('gt', '2020-10-10')
    })
  ]
});

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@iartemiev iartemiev added the DataStore Related to DataStore category label Oct 19, 2020
@iartemiev iartemiev self-assigned this Oct 19, 2020
@lgtm-com
Copy link

lgtm-com bot commented Oct 19, 2020

This pull request fixes 1 alert when merging 462305b into 377acd0 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 19, 2020

This pull request fixes 1 alert when merging 4c8e9ce into 377acd0 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #7001 into main will decrease coverage by 0.30%.
The diff coverage is 39.43%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7001      +/-   ##
==========================================
- Coverage   73.28%   72.97%   -0.31%     
==========================================
  Files         213      213              
  Lines       13197    13295      +98     
  Branches     2585     2604      +19     
==========================================
+ Hits         9671     9702      +31     
- Misses       3332     3397      +65     
- Partials      194      196       +2     
Impacted Files Coverage Δ
packages/core/src/Util/Reachability.ts 33.33% <0.00%> (+1.19%) ⬆️
packages/datastore/src/sync/index.ts 15.07% <0.00%> (-0.34%) ⬇️
packages/datastore/src/datastore/datastore.ts 77.43% <18.86%> (-7.82%) ⬇️
packages/datastore/src/sync/processors/sync.ts 16.66% <21.05%> (ø)
...ages/datastore/src/sync/processors/subscription.ts 39.65% <23.07%> (-0.59%) ⬇️
...ore/src/storage/adapter/getDefaultAdapter/index.ts 75.00% <50.00%> (ø)
packages/datastore/src/types.ts 83.60% <50.00%> (-1.14%) ⬇️
...tastore/src/storage/adapter/AsyncStorageAdapter.ts 77.21% <90.90%> (ø)
packages/datastore/src/sync/utils.ts 82.28% <94.44%> (+1.27%) ⬆️
.../datastore/src/storage/adapter/IndexedDBAdapter.ts 77.84% <100.00%> (ø)
... and 2 more

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 958f61e...374d241. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Oct 19, 2020

This pull request fixes 1 alert when merging c237a19 into 7bdb09f - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 19, 2020

This pull request fixes 1 alert when merging 7375ba7 into 59fdb08 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request fixes 1 alert when merging 9ffe730 into a85c724 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2020

This pull request fixes 1 alert when merging 2bb1d06 into 96fda2a - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

Comment on lines +5 to +7
import {
ModelPredicateCreator,
ModelSortPredicateCreator,
Copy link
Contributor Author

@iartemiev iartemiev Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI to reviewers, the sort code is showing up in the diff for AsyncStorageAdapter and IndexedDBAdatper due to a file rename and delete. It's not actually new code, it's just that we had duplicate files (e.g., asyncstorage.ts and AsyncStorageAdapter.ts and the latter was out of date).

I deleted AsyncStorageAdapter.ts and then renamed asyncstorage.ts -> AsyncStorageAdapter.ts. Same for IDB

Copy link
Contributor

@amhinson amhinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 👍

@iartemiev iartemiev marked this pull request as ready for review October 22, 2020 02:24
Copy link
Contributor

@manueliglesias manueliglesias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I am very excited for this feature 😄

I added minor comments.

packages/datastore/src/types.ts Outdated Show resolved Hide resolved
};
conflictHandler?: ConflictHandler; // default : retry until client wins up to x times
errorHandler?: (error: SyncError) => void; // default : logger.warn
maxRecordsToSync?: number; // merge
syncPageSize?: number;
fullSyncInterval?: number;
};
syncExpressions?: SyncExpression<any>[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Comment on lines 447 to 453
export type SyncExpression<T extends PersistentModel> = Promise<{
modelConstructor: PersistentModelConstructor<T>;
conditionProducer:
| ProducerModelPredicate<T>
| (() => ProducerModelPredicate<T>)
| (() => Promise<ProducerModelPredicate<T>>);
}>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this type works better (is more flexible for the future) if you drop the Promise from here and instead add it as needed where you consume it.

Comment on lines 403 to 411
result[type] = predicates.map(p => {
if (isPredicateObj(p)) {
const { field, operator, operand } = p;

return { [field]: { [operator]: operand } };
} else {
result[p.type] = predicateToGraphQLCondition(p);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This map operation is confusing, it is only returning a mapped element in one circumstance and modifying an object from the outside scope in the other. This should probably be a forEach or re-written as something clearer.

packages/datastore/src/sync/utils.ts Show resolved Hide resolved
packages/datastore/src/storage/adapter/IndexedDBAdapter.ts Outdated Show resolved Hide resolved
packages/datastore/src/datastore/datastore.ts Outdated Show resolved Hide resolved
packages/datastore/src/datastore/datastore.ts Show resolved Hide resolved
packages/datastore/src/datastore/datastore.ts Outdated Show resolved Hide resolved
packages/datastore/src/datastore/datastore.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 23, 2020

This pull request fixes 2 alerts when merging d60587d into abb5e36 - view on LGTM.com

fixed alerts:

  • 2 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 23, 2020

This pull request introduces 1 alert and fixes 2 when merging 5104d0e into abb5e36 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

fixed alerts:

  • 2 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Oct 23, 2020

This pull request fixes 2 alerts when merging a2146c0 into 958f61e - view on LGTM.com

fixed alerts:

  • 2 for Unused variable, import, function or class

Copy link
Contributor

@manueliglesias manueliglesias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just wait for @elorzafe 's review

👍

Thanks @iartemiev !

Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @iartemiev this looks great!

I would double check stop function to see there is no race condition in case the app invokes start and stop repeatedly

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2020

This pull request fixes 2 alerts when merging 374d241 into 958f61e - view on LGTM.com

fixed alerts:

  • 2 for Unused variable, import, function or class

Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @iartemiev this is awesome!! 🌮 🎉 🥇

packages/datastore/src/datastore/datastore.ts Show resolved Hide resolved
@LaurentEscalier
Copy link

LaurentEscalier commented Nov 25, 2020

Hey there 👋
Hey @iartemiev

I've updated aws-amplify to the latest version in my project and have since then been facing the following issue.

Datastore now adds a $filter parameter of type Model${typeName}FilterInput to the GraphQL sync operations that my app is sending:

{
   "query":"query operation($limit: Int, $nextToken: String, $lastSync: AWSTimestamp, $filter: ModelMyModelFilterInput) {\n  syncMyModels(limit: $limit, nextToken: $nextToken, lastSync: $lastSync, filter: $filter) {\n    items {\n      id\n      data\n      _version\n      _lastChangedAt\n      _deleted\n    }\n    nextToken\n    startedAt\n  }\n}\n",
   "variables":{
      "limit":1000,
      "nextToken":null,
      "lastSync":0,
      "filter":null
   }
}

Which results in the following errors in the response.

{
   "data":null,
   "errors":[
      {
         "path":null,
         "locations":[
            {
               "line":1,
               "column":84,
               "sourceName":null
            }
         ],
         "message":"Validation error of type UnknownType: Unknown type ModelMyModelFilterInput"
      },
      {
         "path":null,
         "locations":[
            {
               "line":2,
               "column":84,
               "sourceName":null
            }
         ],
         "message":"Validation error of type UnknownArgument: Unknown field argument filter @ 'syncMyModels'"
      }
   ]
}

Now, I understand why this happens: the related AppSync Backend schema does not expect any $filter parameter on this operation and also doesn't know about a type named ModelMyModelFilterInput.

The way I see it is that this Pull Request has added a change (somewhere around here) that forces me to update my AppSync Backend.

How should I proceed with that ? Where is this required Backend change documented ?

[Edit] Note that my team has previously decided to work without the Amplify CLI. So we have generated the related AppSync Backend by using a by-AWS-provided CloudFormation Stack template.

@iartemiev
Copy link
Contributor Author

@LaurentEscalier there is no backend change required for those using the CLI - which is the recommended approach for DataStore - as we've always included an optional filter argument in the generated query input type. It was just unused until this change.

In your case, you can run amplify api gql-compile in the root of your project and then copy the generated schema from amplify/backend/api/your-app-name/build/schema.graphql to the schema location referenced in your CloudFormation template. Or you can copy-paste the input types piecemeal from the generated schema into the schema you're using. See this doc for more info on how to update the schema in a CF stack.

CryogenicPlanet pushed a commit to MLH-Fellowship/amplify-js that referenced this pull request Jan 20, 2021
@github-actions
Copy link

github-actions bot commented Jun 2, 2022

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DataStore Related to DataStore category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants