feat(datastore): selective sync on initial sync & incoming subscription models

[wooj2]

Smoke tested with a basic case.

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

[codecov]

Codecov Report

Merging #884 (ed035e9) into main (840fc3e) will increase coverage by 0.09%.
The diff coverage is 90.80%.

@@            Coverage Diff             @@
##             main     #884      +/-   ##
==========================================
+ Coverage   66.00%   66.09%   +0.09%     
==========================================
  Files         874      875       +1     
  Lines       34608    34769     +161     
==========================================
+ Hits        22843    22982     +139     
- Misses      11765    11787      +22     

Flag	Coverage Δ
API_plugin_unit_test	62.49% <ø> (-0.16%)	⬇️
Analytics_plugin_unit_test	72.38% <ø> (ø)
Auth_plugin_unit_test	39.11% <ø> (ø)
DataStore_plugin_unit_test	83.14% <90.80%> (+0.11%)	⬆️
Predictions_plugin_unit_test	42.11% <ø> (ø)
Storage_plugin_unit_test	74.74% <ø> (ø)
build_test_amplify	63.11% <ø> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...Sync/IncomingAsyncSubscriptionEventPublisher.swift	45.98% <ø> (ø)
...ionSync/Support/MockModelReconciliationQueue.swift	75.00% <ø> (ø)
...tionSync/Support/ReconciliationQueueTestBase.swift	83.33% <ø> (ø)
...tionSync/AWSIncomingEventReconciliationQueue.swift	50.00% <69.23%> (+2.22%)	⬆️
...s/Sync/InitialSync/InitialSyncOperationTests.swift	95.66% <89.55%> (-0.89%)	⬇️
...onSync/ModelReconciliationQueueBehaviorTests.swift	93.30% <91.07%> (-0.55%)	⬇️
...taStoreCategoryPlugin/DataStoreConfiguration.swift	86.36% <100.00%> (+1.36%)	⬆️
...aStoreCategoryPlugin/DataStoreSyncExpression.swift	100.00% <100.00%> (ø)
...Plugin/Sync/InitialSync/InitialSyncOperation.swift	80.00% <100.00%> (+0.63%)	⬆️
...ataStoreCategoryPlugin/Sync/RemoteSyncEngine.swift	92.89% <100.00%> (+0.17%)	⬆️
... and 8 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 840fc3e...ed035e9. Read the comment docs.

[wooj2]

@palpatim mentioned to combine a lot of these sync config parameters, but doing it this way seems to align better with android:
https://github.com/aws-amplify/amplify-android/blob/baa798bda6c97e9c8415792c4efcdc9dffb7ee3d/aws-datastore/src/main/java/com/amplifyframework/datastore/DataStoreConfiguration.java#L60

[palpatim]

As long as our ultimate behavior is the same, I'm not too interested in hewing closely to Android platform conventions. Even with named arguments and defaults, a pattern like:

let foo = DataStoreConfiguration(
    errorHandler: self.handleDataStoreError,
    conflictHandler: self.handleDataStoreConflict,
    syncInterval: 86_400,
    syncMaxRecords: 10_000,
    syncPageSize: 100,
    syncExpressions: [(Post.foo == getLatestValue()), (Post.bar < getSomeOtherValue()) ]
)

is pretty unwieldy. Even if we provide defaults, the named arguments must be provided in the same order, which adds friction to the customer's implementation.

Consider encapsulating the initializer arguments in purpose-built types (we have pluginOptions on our config types), or adopting a different construction pattern (Builder, for example) to let the customer have an easier time of initializing these.

[wooj2]

Regrading the configuraiton object being unwieldy, that is the worst case scenario. For customers only wanting to specify a selective sync, it would look something like this:

            let syncExpression1 = DataStoreSyncExpression.syncExpression(Todo.schema, where: {
                Todo.keys.tag == AppDelegate.tagToSearch
            })
            let dataStorePlugin = AWSDataStorePlugin(modelRegistration: AmplifyModels(),
                                                     configuration: .custom(syncExpressions: [syncExpression1]))

Are you still concerned about bloating this interface?

[palpatim]

Error handler and conflict handler seem like reasonable things to specify as well, but I'm guessing we don't have any data on how many customers are using which of the config options. It seems to me that 4 free-floating and yet sync-related arguments is excessive, but it's ultimately a judgement call, so consider this as my vote against it. I'm willing to be outvoted. :)

[palpatim]

The overall design makes sense, but I'm not joyful about the growth of the DataStoreConfiguration interface, and I don't like passing that whole configuration object around to the underlying components. Comments inline

[palpatim]

As long as our ultimate behavior is the same, I'm not too interested in hewing closely to Android platform conventions. Even with named arguments and defaults, a pattern like:

let foo = DataStoreConfiguration(
    errorHandler: self.handleDataStoreError,
    conflictHandler: self.handleDataStoreConflict,
    syncInterval: 86_400,
    syncMaxRecords: 10_000,
    syncPageSize: 100,
    syncExpressions: [(Post.foo == getLatestValue()), (Post.bar < getSomeOtherValue()) ]
)

is pretty unwieldy. Even if we provide defaults, the named arguments must be provided in the same order, which adds friction to the customer's implementation.

Consider encapsulating the initializer arguments in purpose-built types (we have pluginOptions on our config types), or adopting a different construction pattern (Builder, for example) to let the customer have an easier time of initializing these.