-
Notifications
You must be signed in to change notification settings - Fork 938
Update query-document mapping from bundles. #3620
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
Changes from 5 commits
7d808c1
d62f9cc
b00494e
e2f17ca
322e27d
1d5a4a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1373,7 +1373,8 @@ export function getNamedQuery( | |
*/ | ||
export async function saveNamedQuery( | ||
localStore: LocalStore, | ||
query: bundleProto.NamedQuery | ||
query: bundleProto.NamedQuery, | ||
documents: DocumentKeySet = documentKeySet() | ||
): Promise<void> { | ||
// Allocate a target for the named query such that it can be resumed | ||
// from associated read time if users use it to listen. | ||
|
@@ -1383,27 +1384,46 @@ export async function saveNamedQuery( | |
const allocated = await localStore.allocateTarget( | ||
queryToTarget(fromBundledQuery(query.bundledQuery!)) | ||
); | ||
|
||
const localStoreImpl = debugCast(localStore, LocalStoreImpl); | ||
return localStoreImpl.persistence.runTransaction( | ||
'Save named query', | ||
'readwrite', | ||
transaction => { | ||
// Update allocated target's read time, if the bundle's read time is newer. | ||
let updateReadTime = PersistencePromise.resolve(); | ||
const readTime = fromVersion(query.readTime!); | ||
if (allocated.snapshotVersion.compareTo(readTime) < 0) { | ||
const newTargetData = allocated.withResumeToken( | ||
ByteString.EMPTY_BYTE_STRING, | ||
readTime | ||
); | ||
updateReadTime = localStoreImpl.targetCache.updateTargetData( | ||
transaction, | ||
newTargetData | ||
); | ||
// Simply save the query itself if it is older than what the SDK already | ||
// has. | ||
if (allocated.snapshotVersion.compareTo(readTime) >= 0) { | ||
return localStoreImpl.bundleCache.saveNamedQuery(transaction, query); | ||
} | ||
return updateReadTime.next(() => | ||
localStoreImpl.bundleCache.saveNamedQuery(transaction, query) | ||
|
||
// Update existing target data because the query from the bundle is newer. | ||
const newTargetData = allocated.withResumeToken( | ||
ByteString.EMPTY_BYTE_STRING, | ||
readTime | ||
); | ||
localStoreImpl.targetDataByTarget = localStoreImpl.targetDataByTarget.insert( | ||
newTargetData.targetId, | ||
newTargetData | ||
); | ||
return localStoreImpl.targetCache | ||
.updateTargetData(transaction, newTargetData) | ||
.next(() => | ||
localStoreImpl.targetCache.removeMatchingKeysForTargetId( | ||
transaction, | ||
allocated.targetId | ||
) | ||
) | ||
.next(() => | ||
localStoreImpl.targetCache.addMatchingKeys( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you should only do this if you updated the readTime above. On top of that, you need to also remove all existing keys, otherwise your mapping will not match the backend's mapping. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree to both. |
||
transaction, | ||
documents, | ||
allocated.targetId | ||
) | ||
) | ||
.next(() => | ||
localStoreImpl.bundleCache.saveNamedQuery(transaction, query) | ||
); | ||
} | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This style of code is pretty hard to debug (a lot of the magic happens inline). Do you mind extracting a local variable for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.