Add IndexManager CRUD operations #5970

schmidt-sebastian · 2022-02-03T04:16:39Z

Ports the IndexManager from Android. Web doesn't have in-memory backing b/c of Multi-Tab. Some of the tests are not present on Android (yet).

changeset-bot · 2022-02-03T04:16:42Z

⚠️ No Changeset found

Latest commit: df5edb3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

google-oss-bot · 2022-02-03T22:44:46Z

Size Report ¹

Affected Products

`@firebase/firestore`

Type	Base (`00916e7`)	Merge (`c9cff76`)	Diff
browser	231 kB	233 kB	+2.16 kB (+0.9%)
esm5	288 kB	291 kB	+2.53 kB (+0.9%)
main	460 kB	463 kB	+3.65 kB (+0.8%)
module	231 kB	233 kB	+2.16 kB (+0.9%)
react-native	231 kB	233 kB	+2.16 kB (+0.9%)

`bundle`

Type	Base (`00916e7`)	Merge (`c9cff76`)	Diff
firestore (Persistence)	232 kB	234 kB	+2.58 kB (+1.1%)
firestore (Query Cursors)	190 kB	190 kB	+91 B (+0.0%)
firestore (Query)	191 kB	191 kB	+91 B (+0.0%)
firestore (Read data once)	180 kB	180 kB	+91 B (+0.1%)
firestore (Realtime updates)	182 kB	182 kB	+91 B (+0.1%)
firestore (Transaction)	164 kB	164 kB	+85 B (+0.1%)
firestore (Write data)	163 kB	163 kB	+85 B (+0.1%)

firebase
Type Base (00916e7) Merge (c9cff76) Diff
firebase-compat.js 755 kB 758 kB +2.52 kB (+0.3%)
firebase-firestore-compat.js 283 kB 285 kB +2.52 kB (+0.9%)
firebase-firestore.js 780 kB 784 kB +4.01 kB (+0.5%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/ncOw0tT7Cl.html

google-oss-bot · 2022-02-03T23:00:31Z

Size Analysis Report ¹

This report is too large (445,136 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/2RBlJBYC2Y.html

github-actions · 2022-02-04T18:55:31Z

Changeset File Check ✅

No modified packages are missing from the changeset file.
No changeset formatting errors detected.

wu-hui

Sorry for the delay.

wu-hui · 2022-02-09T20:16:36Z

packages/firestore/src/local/indexeddb_schema.ts

@@ -708,13 +708,20 @@ export class DbIndexConfiguration {

  static keyPath = 'indexId';

+  static collectionGroupIndex = 'collectionGroupIndex';


This does not exist in Android?

I see why now. Please add comment like you did below.

wu-hui · 2022-02-09T20:17:11Z

packages/firestore/src/local/indexeddb_schema.ts

+   * An index that provides access to documents in a collection sorted by last
+   * update time. Used by the backfiller.
+   *
+   * PORTING NOTE: iOS and Android maintain this index in-memory.


Maybe also explain why JS is different here?

wu-hui · 2022-02-09T20:25:18Z

packages/firestore/src/local/indexeddb_remote_document_cache.ts


+  setIndexManager(indexManager: IndexManager): void {
+    this.indexManager = indexManager;
+  }


Empty line after here?

wu-hui · 2022-02-09T21:12:52Z

packages/firestore/src/local/indexeddb_index_manager.ts

+            IDBKeyRange.bound(collectionGroup, collectionGroup)
+          )
+        : indexes.loadAll()
+    ).next(indexEntries => {


indexEntry has different meaning..maybe indexConfig?

Makes sense. Done.

wu-hui · 2022-02-09T21:39:55Z

packages/firestore/test/unit/local/index_manager.test.ts

+
+    let fieldIndexes = await indexManager.getFieldIndexes();
+    expect(fieldIndexes[0].indexState).to.deep.equal(
+      new IndexState(0, IndexOffset.min())


Perhaps explain what 0 and 1 means here.

schmidt-sebastian

Thanks for the review

schmidt-sebastian · 2022-02-10T15:56:22Z

packages/firestore/src/local/indexeddb_index_manager.ts

+            IDBKeyRange.bound(collectionGroup, collectionGroup)
+          )
+        : indexes.loadAll()
+    ).next(indexEntries => {


Makes sense. Done.

schmidt-sebastian · 2022-02-10T15:56:44Z

packages/firestore/src/local/indexeddb_remote_document_cache.ts


+  setIndexManager(indexManager: IndexManager): void {
+    this.indexManager = indexManager;
+  }


schmidt-sebastian · 2022-02-10T15:58:49Z

packages/firestore/src/local/indexeddb_schema.ts

@@ -708,13 +708,20 @@ export class DbIndexConfiguration {

  static keyPath = 'indexId';

+  static collectionGroupIndex = 'collectionGroupIndex';


schmidt-sebastian · 2022-02-10T15:58:54Z

packages/firestore/src/local/indexeddb_schema.ts

+   * An index that provides access to documents in a collection sorted by last
+   * update time. Used by the backfiller.
+   *
+   * PORTING NOTE: iOS and Android maintain this index in-memory.


schmidt-sebastian · 2022-02-10T16:00:06Z

packages/firestore/test/unit/local/index_manager.test.ts

+
+    let fieldIndexes = await indexManager.getFieldIndexes();
+    expect(fieldIndexes[0].indexState).to.deep.equal(
+      new IndexState(0, IndexOffset.min())


Add IndexManager CRUD operations

f18792a

schmidt-sebastian force-pushed the mrschmidt/indexmanager branch from cef59a7 to f18792a Compare February 3, 2022 22:25

schmidt-sebastian changed the title ~~Mrschmidt/indexmanager~~ Add IndexManager CRUD operations Feb 3, 2022

schmidt-sebastian marked this pull request as ready for review February 3, 2022 22:27

schmidt-sebastian requested a review from wu-hui February 3, 2022 22:27

schmidt-sebastian assigned wu-hui Feb 3, 2022

schmidt-sebastian added the api: firestore label Feb 3, 2022

schmidt-sebastian added 3 commits February 4, 2022 11:53

Fix tests

ef5ac48

Remove only

d987143

Remove only

e783bcf

schmidt-sebastian added 2 commits February 4, 2022 11:56

More test fixes

e6fbeb7

Merge

d821727

wu-hui reviewed Feb 9, 2022

View reviewed changes

wu-hui assigned schmidt-sebastian and unassigned wu-hui Feb 9, 2022

wu-hui approved these changes Feb 9, 2022

View reviewed changes

Review

df5edb3

schmidt-sebastian commented Feb 10, 2022

View reviewed changes

schmidt-sebastian merged commit 434d87e into master Feb 10, 2022

schmidt-sebastian deleted the mrschmidt/indexmanager branch February 10, 2022 16:29

firebase locked and limited conversation to collaborators Mar 13, 2022

		@@ -708,13 +708,20 @@ export class DbIndexConfiguration {

		static keyPath = 'indexId';

		static collectionGroupIndex = 'collectionGroupIndex';

Add IndexManager CRUD operations #5970

Add IndexManager CRUD operations #5970

Uh oh!

Conversation

schmidt-sebastian commented Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

google-oss-bot commented Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report 1

Affected Products

@firebase/firestore

bundle

firebase

Test Logs

Uh oh!

google-oss-bot commented Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Analysis Report 1

Test Logs

Uh oh!

github-actions bot commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changeset File Check ✅

Uh oh!

wu-hui left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schmidt-sebastian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

schmidt-sebastian commented Feb 3, 2022 •

edited

Loading

changeset-bot bot commented Feb 3, 2022 •

edited

Loading

google-oss-bot commented Feb 3, 2022 •

edited

Loading

Size Report ¹

`@firebase/firestore`

`bundle`

`firebase`

google-oss-bot commented Feb 3, 2022 •

edited

Loading

Size Analysis Report ¹

github-actions bot commented Feb 4, 2022 •

edited

Loading