Update driver cache entries to contain the file version #25337
Conversation
github-actions
bot
added
area: driver
There was a problem hiding this comment.
Pull Request Overview
This PR updates driver cache entries to include file version information in the cache key to prevent ops from appearing on incorrect document versions. The change addresses a bug where operations from the main document would incorrectly appear on previous versions.
Key changes:
- Enhanced cache key generation to include file version when available
- Restructured cache key format to better organize version, type, and key components
- Fixed typos in comments and documentation
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/drivers/odsp-driver/src/odspDocumentService.ts | Fixed typo in comment ("re receive" → "we receive") |
| packages/drivers/odsp-driver-definitions/src/odspCache.ts | Updated cache key generation logic to include file version and fixed typo in documentation |
| ? `_${entry.file.resolvedUrl.fileVersion}` | ||
| : ""; | ||
| const suffix = entry.type === snapshotKey ? "" : `_${entry.key}`; | ||
| return `${entry.file.docId}${version}_${entry.type}${suffix}`; |
There was a problem hiding this comment.
This will cause a regression in load performance as we are changing the key corresponding to the stored snapshot. So the previous entries won't be found and instead of loading from cache, we will load from network for each document.
There was a problem hiding this comment.
oh, the version will "" for snapshot key.
There was a problem hiding this comment.
Can you before and after example of how would the key look for snapshot and ops?
There was a problem hiding this comment.
Yes, the goal is to only change the cache entries for versioned snapshots/ops and leave the entries for the main document untouched.
There was a problem hiding this comment.
ok got it. So, when they will start supplying the version, then they will face some regression. You can let them know before hand, when they will start the rollout.
There was a problem hiding this comment.
Examples with versions:
before:
docId_snapshot_4.0
docId_ops_100_2
after:
docId_4.0_snapshot
docId_4.0_ops_100_2
If there is a version value in entry.key for a snapshot entry, it will also be accessible from the resolved url, and will end up in the version variable in the updated function.
| : ""; | ||
| const suffix = | ||
| entry.type === snapshotKey || entry.type === snapshotWithLoadingGroupIdKey | ||
| ? "" |
There was a problem hiding this comment.
This should be "_" instead of "" to keep the suffix same as before.
Previously, only snapshot entries for a given document would contain version information to indicate which version of the document the entry referred to. This caused a bug where ops from the main document would appear on previous versions of the document when they should not yet exist. This PR updates the cache entries of snapshots and ops to both contain the version information in the cache entry, if it exists.
AB#47218
Additionally, fixes a few typos I noticed while working in the area.