Skip to content

Commit 0b9aaf1

Browse files
committed
try hand-creating the ExistenceFilterMismatchInfo to see if it survives minification
1 parent 85ed7ab commit 0b9aaf1

File tree

5 files changed

+96
-112
lines changed

5 files changed

+96
-112
lines changed

packages/firestore/src/api.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,4 +197,3 @@ export { logWarn as _logWarn } from './util/log';
197197
export { EmptyAuthCredentialsProvider as _EmptyAuthCredentialsProvider } from './api/credentials';
198198
export { EmptyAppCheckTokenProvider as _EmptyAppCheckTokenProvider } from './api/credentials';
199199
export { TestingHooks as _TestingHooks } from './util/testing_hooks';
200-
export { ExistenceFilterMismatchCallback as _TestingHooksExistenceFilterMismatchCallback } from './util/testing_hooks';

packages/firestore/src/remote/watch_change.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ import { logDebug, logWarn } from '../util/log';
3737
import { primitiveComparator } from '../util/misc';
3838
import { SortedMap } from '../util/sorted_map';
3939
import { SortedSet } from '../util/sorted_set';
40-
import { TestingHooks } from '../util/testing_hooks';
40+
import {
41+
ExistenceFilterMismatchInfo,
42+
TestingHooks
43+
} from '../util/testing_hooks';
4144

4245
import { BloomFilter, BloomFilterError } from './bloom_filter';
4346
import { ExistenceFilter } from './existence_filter';
@@ -801,19 +804,22 @@ function notifyTestingHooksOnExistenceFilterMismatch(
801804
actualCount: number,
802805
existenceFilter: ExistenceFilter
803806
): void {
804-
const expectedCount = existenceFilter.count;
807+
const existenceFilterMismatchInfo: ExistenceFilterMismatchInfo = {
808+
actualCount,
809+
expectedCount: existenceFilter.count
810+
};
811+
805812
const unchangedNames = existenceFilter.unchangedNames;
806-
const bloomFilterSentFromWatch = !!unchangedNames;
807-
const bloomFilterHashCount = unchangedNames?.hashCount ?? 0;
808-
const bloomFilterBitmapLength = unchangedNames?.bits?.bitmap?.length ?? 0;
809-
const bloomFilterPadding = unchangedNames?.bits?.padding ?? 0;
813+
if (unchangedNames) {
814+
existenceFilterMismatchInfo.bloomFilter = {
815+
applied: bloomFilterApplied,
816+
hashCount: unchangedNames?.hashCount ?? 0,
817+
bitmapLength: unchangedNames?.bits?.bitmap?.length ?? 0,
818+
padding: unchangedNames?.bits?.padding ?? 0
819+
};
820+
}
821+
810822
TestingHooks.instance?.notifyOnExistenceFilterMismatch(
811-
actualCount,
812-
expectedCount,
813-
bloomFilterSentFromWatch,
814-
bloomFilterApplied,
815-
bloomFilterHashCount,
816-
bloomFilterBitmapLength,
817-
bloomFilterPadding
823+
existenceFilterMismatchInfo
818824
);
819825
}

packages/firestore/src/util/testing_hooks.ts

Lines changed: 43 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,7 @@ export class TestingHooks {
6565
* The relative order in which callbacks are notified is unspecified; do not
6666
* rely on any particular ordering.
6767
*
68-
* @param callback the callback to invoke upon existence filter mismatch; see
69-
* the documentation for `notifyOnExistenceFilterMismatch()` for the meaning
70-
* of these arguments.
68+
* @param callback the callback to invoke upon existence filter mismatch.
7169
*
7270
* @return a function that, when called, unregisters the given callback; only
7371
* the first invocation of the returned function does anything; all subsequent
@@ -83,64 +81,57 @@ export class TestingHooks {
8381

8482
/**
8583
* Invokes all currently-registered `onExistenceFilterMismatch` callbacks.
86-
* @param bloomFilterApplied true if a full requery was averted because the
87-
* limbo documents were successfully deduced using the bloom filter, or false
88-
* if a full requery had to be performed, such as due to a false positive in
89-
* the bloom filter.
90-
* @param actualCount the number of documents that matched the query in the
91-
* local cache.
92-
* @param expectedCount the number of documents that matched the query on the
93-
* server, as specified in the ExistenceFilter message's `count` field.
94-
* @param bloomFilterSentFromWatch whether the ExistenceFilter message
95-
* included the `unchangedNames` bloom filter. If false, the remaining
96-
* arguments are not applicable and may have any values.
97-
* @param bloomFilterApplied whether a full requery was averted by using the
98-
* bloom filter; if false, then a false positive occurred, requiring a full
99-
* requery to deduce which documents should go into limbo.
100-
* @param bloomFilterHashCount the number of hash functions used in the bloom
101-
* filter.
102-
* @param bloomFilterBitmapLength the number of bytes used by the bloom
103-
* filter.
104-
* @param bloomFilterPadding the number of bits of padding in the last byte
105-
* of the bloom filter.
84+
* @param info Information about the existence filter mismatch.
10685
*/
107-
notifyOnExistenceFilterMismatch(
108-
actualCount: number,
109-
expectedCount: number,
110-
bloomFilterSentFromWatch: boolean,
111-
bloomFilterApplied: boolean,
112-
bloomFilterHashCount: number,
113-
bloomFilterBitmapLength: number,
114-
bloomFilterPadding: number
115-
): void {
116-
this.onExistenceFilterMismatchCallbacks.forEach(callback => {
117-
callback(
118-
actualCount,
119-
expectedCount,
120-
bloomFilterSentFromWatch,
121-
bloomFilterApplied,
122-
bloomFilterHashCount,
123-
bloomFilterBitmapLength,
124-
bloomFilterPadding
125-
);
126-
});
86+
notifyOnExistenceFilterMismatch(info: ExistenceFilterMismatchInfo): void {
87+
this.onExistenceFilterMismatchCallbacks.forEach(callback => callback(info));
12788
}
12889
}
12990

13091
/**
13192
* The signature of callbacks registered with
13293
* `TestingUtils.onExistenceFilterMismatch()`.
133-
*
134-
* @internal
94+
*/
95+
export interface ExistenceFilterMismatchInfo {
96+
/** The number of documents that matched the query in the local cache. */
97+
actualCount: number;
98+
99+
/**
100+
* The number of documents that matched the query on the server, as specified
101+
* in the ExistenceFilter message's `count` field.
102+
*/
103+
expectedCount: number;
104+
105+
/**
106+
* Information about the bloom filter provided by Watch in the ExistenceFilter
107+
* message's `unchangedNames` field. If this property is omitted or undefined
108+
* then that means that Watch did _not_ provide a bloom filter.
109+
*/
110+
bloomFilter?: {
111+
/**
112+
* Whether a full requery was averted by using the bloom filter. If false,
113+
* then something happened, such as a false positive, to prevent using the
114+
* bloom filter to avoid a full requery.
115+
*/
116+
applied: boolean;
117+
118+
/** The number of hash functions used in the bloom filter. */
119+
hashCount: number;
120+
121+
/** The number of bytes in the bloom filter's bitmask. */
122+
bitmapLength: number;
123+
124+
/** The number of bits of padding in the last byte of the bloom filter. */
125+
padding: number;
126+
};
127+
}
128+
129+
/**
130+
* The signature of callbacks registered with
131+
* `TestingUtils.onExistenceFilterMismatch()`.
135132
*/
136133
export type ExistenceFilterMismatchCallback = (
137-
actualCount: number,
138-
expectedCount: number,
139-
bloomFilterSentFromWatch: boolean,
140-
bloomFilterApplied: boolean,
141-
bloomFilterHashCount: number,
142-
bloomFilterBitmapLength: number,
143-
bloomFilterPadding: number
134+
info: ExistenceFilterMismatchInfo
144135
) => void;
145136

146137
/** The global singleton instance of `TestingHooks`. */

packages/firestore/test/integration/api/query.test.ts

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2122,41 +2122,40 @@ apiDescribe('Queries', (persistence: boolean) => {
21222122
existenceFilterMismatches,
21232123
'existenceFilterMismatches'
21242124
).to.have.length(1);
2125-
const {
2126-
actualCount,
2127-
expectedCount,
2128-
bloomFilterSentFromWatch,
2129-
bloomFilterApplied,
2130-
bloomFilterHashCount,
2131-
bloomFilterBitmapLength,
2132-
bloomFilterPadding
2133-
} = existenceFilterMismatches[0];
2125+
const { actualCount, expectedCount, bloomFilter } =
2126+
existenceFilterMismatches[0];
21342127

21352128
expect(actualCount, 'actualCount').to.equal(100);
21362129
expect(expectedCount, 'expectedCount').to.equal(50);
2137-
expect(bloomFilterSentFromWatch, 'bloomFilterSentFromWatch').to.be
2138-
.true;
2130+
if (!bloomFilter) {
2131+
expect.fail(
2132+
'The existence filter should have specified ' +
2133+
'a bloom filter in its `unchanged_names` field.'
2134+
);
2135+
throw new Error('should never get here');
2136+
}
2137+
2138+
expect(
2139+
bloomFilter.hashCount,
2140+
'bloomFilter.hashCount'
2141+
).to.be.above(0);
2142+
expect(
2143+
bloomFilter.bitmapLength,
2144+
'bloomFilter.bitmapLength'
2145+
).to.be.above(0);
2146+
expect(bloomFilter.padding, 'bloomFilterPadding').to.be.above(0);
2147+
expect(bloomFilter.padding, 'bloomFilterPadding').to.be.below(8);
21392148

21402149
// Retry the entire test if a bloom filter false positive occurs.
21412150
// Although statistically rare, false positives are expected to
21422151
// happen occasionally. When a false positive _does_ happen, just
21432152
// retry the test with a different set of documents. If that retry
21442153
// _also_ experiences a false positive, then fail the test because
21452154
// that is so improbable that something must have gone wrong.
2146-
if (attemptNumber > 1 && !bloomFilterApplied) {
2155+
if (attemptNumber > 1 && !bloomFilter.applied) {
21472156
return 'retry';
21482157
}
2149-
2150-
expect(bloomFilterApplied, 'bloomFilterApplied').to.be.true;
2151-
expect(bloomFilterHashCount, 'bloomFilterHashCount').to.be.above(
2152-
0
2153-
);
2154-
expect(
2155-
bloomFilterBitmapLength,
2156-
'bloomFilterBitmapLength'
2157-
).to.be.above(0);
2158-
expect(bloomFilterPadding, 'bloomFilterPadding').to.be.above(0);
2159-
expect(bloomFilterPadding, 'bloomFilterPadding').to.be.below(8);
2158+
expect(bloomFilter.applied, 'bloomFilter.applied').to.be.true;
21602159
}
21612160

21622161
return 'passed';

packages/firestore/test/integration/util/testing_hooks_util.ts

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { _TestingHooks as TestingHooks } from './firebase_export';
18+
import { _TestingHooks as TestingHooks, _logWarn } from './firebase_export';
1919

2020
/**
2121
* Captures all existence filter mismatches in the Watch 'Listen' stream that
@@ -29,23 +29,11 @@ export async function captureExistenceFilterMismatches(
2929
): Promise<ExistenceFilterMismatchInfo[]> {
3030
const results: ExistenceFilterMismatchInfo[] = [];
3131
const onExistenceFilterMismatchCallback = (
32-
actualCount: number,
33-
expectedCount: number,
34-
bloomFilterSentFromWatch: boolean,
35-
bloomFilterApplied: boolean,
36-
bloomFilterHashCount: number,
37-
bloomFilterBitmapLength: number,
38-
bloomFilterPadding: number
39-
) =>
40-
results.push({
41-
actualCount,
42-
expectedCount,
43-
bloomFilterSentFromWatch,
44-
bloomFilterApplied,
45-
bloomFilterHashCount,
46-
bloomFilterBitmapLength,
47-
bloomFilterPadding
48-
});
32+
info: ExistenceFilterMismatchInfo
33+
) => {
34+
_logWarn('zzyzx onExistenceFilterMismatchCallback', info);
35+
results.push(info);
36+
};
4937

5038
const unregister =
5139
TestingHooks.getOrCreateInstance().onExistenceFilterMismatch(
@@ -71,9 +59,10 @@ export async function captureExistenceFilterMismatches(
7159
export interface ExistenceFilterMismatchInfo {
7260
actualCount: number;
7361
expectedCount: number;
74-
bloomFilterSentFromWatch: boolean;
75-
bloomFilterApplied: boolean;
76-
bloomFilterHashCount: number;
77-
bloomFilterBitmapLength: number;
78-
bloomFilterPadding: number;
62+
bloomFilter?: {
63+
applied: boolean;
64+
hashCount: number;
65+
bitmapLength: number;
66+
padding: number;
67+
};
7968
}

0 commit comments

Comments
 (0)