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

IndexedDB Tile Cache #6720

Merged
merged 34 commits into from
May 28, 2024
Merged

IndexedDB Tile Cache #6720

merged 34 commits into from
May 28, 2024

Conversation

andremig-bentley
Copy link
Contributor

@andremig-bentley andremig-bentley commented May 13, 2024

Create a local cache with IndexedDB which will be used to store tiles from the mesh export service. The IndexedDBCache class is a standalone class which interfaces with IndexedDB and can be used widely. The class can store, delete, and retrieve content from an IndexedDB database, and has the option to have the content in the database expire.

If the useIndexedDBCache conditional is set true, tiles being fetched through the itwinjs-batched-models channel will first attempt to fetch their content from an IndexedDB database. If that content cannot be found or is expired, the channel will fetch the content normally and store the content for future requests.

@andremig-bentley
Copy link
Contributor Author

@danieliborra

Updates based on your feedback. I believe the IDBStorage class should now be testable on its own, so I'm working on getting that going.

Copy link
Contributor

@danieliborra danieliborra left a comment

Choose a reason for hiding this comment

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

Good progress, but we're still not there 😉
As I commented below, I'd suggest branching this branch and see how it looks when removing the new channel.
It won't take you long, and then we can discuss it further, I think it is worth a try.

core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
extensions/frontend-tiles/src/BatchedTile.ts Outdated Show resolved Hide resolved
extensions/frontend-tiles/src/BatchedTile.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IDBTileStorage.ts Outdated Show resolved Hide resolved
@andremig-bentley
Copy link
Contributor Author

IDBTileRequestChannel has now been fully removed, IDBStorage has been renamed to IndexedDBCacheManager (copilot's suggestion). The interface IndexedDBCache has been created with a fetch function, and IndexedDBCacheManager and IndexedDBCachePassThrough implement the interface. Within IndexedDBCacheManager, all functions except for the fetch function have been made private. Within BatchedTiles.ts, the IDBTileRequestChannel has been replaced with _localCache, an instance of IndexedDBCacheManager.

@danieliborra
Copy link
Contributor

IDBTileRequestChannel has now been fully removed, IDBStorage has been renamed to IndexedDBCacheManager (copilot's suggestion). The interface IndexedDBCache has been created with a fetch function, and IndexedDBCacheManager and IndexedDBCachePassThrough implement the interface. Within IndexedDBCacheManager, all functions except for the fetch function have been made private. Within BatchedTiles.ts, the IDBTileRequestChannel has been replaced with _localCache, an instance of IndexedDBCacheManager.

It looks way better! All changes are good.
In this case, co-pilot didn't do great job 😄
What about:

  • IndexedDBCache => ILocalCache or ICloudContentCache or IFetchContentCache (generic name, has nothing to do with IndexDB)
  • IndexedDBCacheManager => IndexedDBCache
  • IndexedDBCachePassThrough => PassThroughCache

@andremig-bentley
Copy link
Contributor Author

Updates from most recent commit:

  • Fixed all async issues/issues with database functions happening out of order, etc. Confirmed now that all database functionality is working as intended (searching, retrieving, adding, deleting, all working how/when they're supposed to).
  • General cleanup, changed IDBTileStorage.ts to IndexedDBCache.ts
  • Moved all changes dealing with IModelTileRequestChannels to a new branch and removed them from this PR

Copy link
Contributor

@markschlosseratbentley markschlosseratbentley left a comment

Choose a reason for hiding this comment

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

extract-api

@andremig-bentley
Copy link
Contributor Author

extract-api

Taken care of.

@markschlosseratbentley
Copy link
Contributor

is extract-api failing again?

@andremig-bentley
Copy link
Contributor Author

is extract-api failing again?

Yeah it is, not sure why. I re-ran it and re-committed everything applicable with my latest commit, but it doesn’t seem to be working. Looking into that this morning

@markschlosseratbentley
Copy link
Contributor

is extract-api failing again?

Yeah it is, not sure why. I re-ran it and re-committed everything applicable with my latest commit, but it doesn’t seem to be working. Looking into that this morning

All good. Approving

core/frontend/src/tile/IndexedDBCache.ts Outdated Show resolved Hide resolved
common/api/core-frontend.api.md Outdated Show resolved Hide resolved
core/frontend/src/test/tile/IndexedDBCache.test.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IndexedDBCache.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IndexedDBCache.ts Outdated Show resolved Hide resolved
core/frontend/src/tile/IndexedDBCache.ts Outdated Show resolved Hide resolved
core/frontend/src/test/tile/IndexedDBCache.test.ts Outdated Show resolved Hide resolved
Co-authored-by: Paul Connelly <22944042+pmconne@users.noreply.github.com>
@pmconne
Copy link
Member

pmconne commented May 28, 2024

Force-merging because unrelated failing reality data integration test.

@pmconne pmconne merged commit f52531c into master May 28, 2024
15 of 16 checks passed
@pmconne pmconne deleted the andremig/idbtilecache branch May 28, 2024 19:06
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.

None yet

4 participants