Skip to content

Commit 8529ae7

Browse files
committed
update constructor validation
1 parent 687a5b6 commit 8529ae7

File tree

2 files changed

+33
-10
lines changed

2 files changed

+33
-10
lines changed

packages/firestore/src/remote/bloom_filter.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ export class BloomFilter {
2929
this.bitSize = this.bitmap.length * 8 - padding;
3030
if (this.bitSize === 0) {
3131
debugAssert(
32-
this.hashCount === 0,
33-
'An empty bitmap should correspond to 0 hashCount.'
32+
this.hashCount === 0 && padding === 0,
33+
'A valid empty bloom filter should have all 3 fields empty.'
3434
);
3535
} else {
3636
debugAssert(padding >= 0, 'Padding is negative.');
37-
debugAssert(this.bitSize >= 0, 'Bitmap size is negative.');
38-
debugAssert(this.hashCount >= 0, 'Hash count is negative.');
37+
debugAssert(this.bitSize > 0, 'Bitmap size is negative.');
38+
debugAssert(this.hashCount > 0, 'Hash count is 0 or negative');
3939
}
4040
}
4141

packages/firestore/test/unit/remote/bloom_filter.test.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ import {
4747
} from './bloom_filter_golden_test_data';
4848

4949
describe('BloomFilter', () => {
50-
it('can initiate an empty BloomFilter', () => {
50+
it('can initiate an empty bloom filter', () => {
5151
const bloomFilter = new BloomFilter(new Uint8Array(0), 0, 0);
5252
expect(bloomFilter.getBitSize()).to.equal(0);
5353
});
5454

55-
it('can initiate a non empty BloomFilter', () => {
55+
it('can initiate a non empty bloom filter', () => {
5656
const bloomFilter = new BloomFilter(
5757
new Uint8Array([151, 153, 236, 116, 7]),
5858
3,
@@ -61,14 +61,24 @@ describe('BloomFilter', () => {
6161
expect(bloomFilter.getBitSize()).to.equal(37);
6262
});
6363

64-
it('should throw error if empty BloomFilter has hash count', () => {
64+
it('should throw error if empty bloom filter inputs are invalid', () => {
6565
try {
6666
new BloomFilter(new Uint8Array(0), 0, 1);
6767
expect.fail();
6868
} catch (error) {
6969
expect(
7070
(error as Error)?.message.includes(
71-
'INTERNAL ASSERTION FAILED: An empty bitmap should correspond to 0 hashCount.'
71+
'INTERNAL ASSERTION FAILED: A valid empty bloom filter should have all 3 fields empty.'
72+
)
73+
).to.be.true;
74+
}
75+
try {
76+
new BloomFilter(new Uint8Array(1), 8, 0);
77+
expect.fail();
78+
} catch (error) {
79+
expect(
80+
(error as Error)?.message.includes(
81+
'INTERNAL ASSERTION FAILED: A valid empty bloom filter should have all 3 fields empty.'
7282
)
7383
).to.be.true;
7484
}
@@ -78,7 +88,7 @@ describe('BloomFilter', () => {
7888
} catch (error) {
7989
expect(
8090
(error as Error)?.message.includes(
81-
'INTERNAL ASSERTION FAILED: An empty bitmap should correspond to 0 hashCount.'
91+
'INTERNAL ASSERTION FAILED: A valid empty bloom filter should have all 3 fields empty.'
8292
)
8393
).to.be.true;
8494
}
@@ -117,7 +127,20 @@ describe('BloomFilter', () => {
117127
} catch (error) {
118128
expect(
119129
(error as Error)?.message.includes(
120-
'INTERNAL ASSERTION FAILED: Hash count is negative.'
130+
'INTERNAL ASSERTION FAILED: Hash count is 0 or negative'
131+
)
132+
).to.be.true;
133+
}
134+
});
135+
136+
it('should throw error if hash count is 0 for non empty bloom filter', () => {
137+
try {
138+
new BloomFilter(new Uint8Array(1), 1, 0);
139+
expect.fail();
140+
} catch (error) {
141+
expect(
142+
(error as Error)?.message.includes(
143+
'INTERNAL ASSERTION FAILED: Hash count is 0 or negative'
121144
)
122145
).to.be.true;
123146
}

0 commit comments

Comments
 (0)