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

Persist Record arguments/metadata with the SQL Json backend #4211

Merged
merged 23 commits into from
Jun 27, 2022

Conversation

BoD
Copy link
Contributor

@BoD BoD commented Jun 20, 2022

Also, add a test/sample for offset based pagination.

@netlify
Copy link

netlify bot commented Jun 20, 2022

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit c284a24
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/62b96a30b4d64400088f4cc0

@BoD BoD force-pushed the metadata-on-records-json branch from 66415a6 to c7f0c68 Compare June 20, 2022 14:57
@BoD
Copy link
Contributor Author

BoD commented Jun 21, 2022

Marking as Draft for now, because the tests won't work until we have a way to store the fields omitting pagination arguments e.g. just users instead ofusers(offset: 42, limit: 2)

@BoD BoD marked this pull request as ready for review June 22, 2022 08:38
recordMerger = CursorPaginationRecordMerger()
)
.serverUrl("unused")
.build()
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you're not using any of the ApolloClient functionality here? If that's the case, you can instanciate an ApolloStore directly:

ApolloStore(
          normalizedCacheFactory = normalizedCacheFactory,
          cacheKeyGenerator = cacheKeyGenerator,
          metadataGenerator = metadataGenerator,
          apolloResolver = apolloResolver,
          recordMerger = recordMerger
      )

val incomingList = incomingFieldValue as List<*>
val mergedList = mergeLists(existingList, incomingList, existingOffset, incomingOffset)
mergedFields[fieldKey] = mergedList
mergedMetadata[fieldKey] = mapOf("offset" to min(existingOffset, incomingOffset))
Copy link
Contributor

Choose a reason for hiding this comment

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

Using min doesn't take into account the case where existing and incoming do not join (the early returns in mergeLists)

UsersCursorBasedQuery.Edge("xx44", UsersCursorBasedQuery.Node("44", "Peter", "peter@a.com", "User")),
UsersCursorBasedQuery.Edge("xx45", UsersCursorBasedQuery.Node("45", "Alice", "alice@a.com", "User")),
UsersCursorBasedQuery.Edge("xx46", UsersCursorBasedQuery.Node("46", "Bob", "bob@a.com", "User")),
UsersCursorBasedQuery.Edge("xx47", UsersCursorBasedQuery.Node("47", "Charlie", "charlie@a.com", "User")),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you 🐶 🥫 test builders here?

@BoD BoD force-pushed the metadata-on-records-json branch from fa2fde3 to 6b18de4 Compare June 27, 2022 07:10
* Introduce FieldRecordMerger

* Fix badly named FieldMerger
@BoD BoD merged commit 34adae4 into main Jun 27, 2022
@BoD BoD deleted the metadata-on-records-json branch June 27, 2022 12:26
martinbonnin added a commit that referenced this pull request Jun 29, 2022
Co-authored-by: Martin Bonnin <martin@mbonnin.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants