Skip to content

Commit 9a11614

Browse files
milaGGLmilaGGL
committed
resolve comments
1 parent 9321eec commit 9a11614

File tree

4 files changed

+70
-45
lines changed

4 files changed

+70
-45
lines changed

‎packages/firestore/src/remote/bloom_filter.ts

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,28 @@ import { Md5, Integer } from '@firebase/webchannel-wrapper';
1919
import { newTextEncoder } from '../platform/serializer';
2020
import { debugAssert } from '../util/assert';
2121

22-
const MAX_64_BIT_UNSIGNED_INTEGER = Integer.fromNumber(Math.pow(2, 64));
22+
const MAX_64_BIT_UNSIGNED_INTEGER = new Integer([0xffffffff, 0xffffffff], 0);
23+
24+
// Hash a string using md5 hashing algorithm.
25+
function getMd5HashValue(value: string): Uint8Array {
26+
const encodedValue = newTextEncoder().encode(value);
27+
const md5 = new Md5();
28+
md5.update(encodedValue);
29+
return new Uint8Array(md5.digest());
30+
}
31+
32+
// Interpret the 16 bytes array as two 64-bit unsigned integers, encoded using
33+
// 2’s complement using little endian.
34+
function get64BitUints(Bytes: Uint8Array): [Integer, Integer] {
35+
const dataView = new DataView(Bytes.buffer);
36+
const chunk1 = dataView.getUint32(0, /* littleEndian= */ true);
37+
const chunk2 = dataView.getUint32(4, /* littleEndian= */ true);
38+
const chunk3 = dataView.getUint32(8, /* littleEndian= */ true);
39+
const chunk4 = dataView.getUint32(12, /* littleEndian= */ true);
40+
const integer1 = new Integer([chunk1, chunk2], 0);
41+
const integer2 = new Integer([chunk3, chunk4], 0);
42+
return [integer1, integer2];
43+
}
2344

2445
export class BloomFilter {
2546
readonly size: number;
@@ -45,30 +66,10 @@ export class BloomFilter {
4566
}
4667

4768
this.size = bitmap.length * 8 - padding;
69+
// Set the size in Integer to avoid repeated calculation in mightContain().
4870
this.sizeInInteger = Integer.fromNumber(this.size);
4971
}
5072

51-
// Hash a string using md5 hashing algorithm.
52-
private static getMd5HashValue(value: string): Uint8Array {
53-
const md5 = new Md5();
54-
const encodedValue = newTextEncoder().encode(value);
55-
md5.update(encodedValue);
56-
return new Uint8Array(md5.digest());
57-
}
58-
59-
// Interpret the 16 bytes array as two 64-bit unsigned integers, encoded using 2’s
60-
// complement using little endian.
61-
private static get64BitUints(Bytes: Uint8Array): [Integer, Integer] {
62-
const dataView = new DataView(Bytes.buffer);
63-
const chunk1 = dataView.getUint32(0, /* littleEndian= */ true);
64-
const chunk2 = dataView.getUint32(4, /* littleEndian= */ true);
65-
const chunk3 = dataView.getUint32(8, /* littleEndian= */ true);
66-
const chunk4 = dataView.getUint32(12, /* littleEndian= */ true);
67-
const integer1 = new Integer([chunk1, chunk2], 0);
68-
const integer2 = new Integer([chunk3, chunk4], 0);
69-
return [integer1, integer2];
70-
}
71-
7273
// Calculate the ith hash value based on the hashed 64bit integers,
7374
// and calculate its corresponding bit index in the bitmap to be checked.
7475
private getBitIndex(num1: Integer, num2: Integer, index: number): number {
@@ -85,17 +86,19 @@ export class BloomFilter {
8586
private isBitSet(index: number): boolean {
8687
// To retrieve bit n, calculate: (bitmap[n / 8] & (0x01 << (n % 8))).
8788
const byte = this.bitmap[Math.floor(index / 8)];
88-
return (byte & (0x01 << index % 8)) !== 0;
89+
const offset = index % 8;
90+
return (byte & (0x01 << offset)) !== 0;
8991
}
9092

9193
mightContain(value: string): boolean {
92-
// Empty bitmap and empty value should always return false on membership check.
94+
// Empty bitmap and empty value should always return false on membership
95+
// check.
9396
if (this.size === 0 || value === '') {
9497
return false;
9598
}
9699

97-
const md5HashedValue = BloomFilter.getMd5HashValue(value);
98-
const [hash1, hash2] = BloomFilter.get64BitUints(md5HashedValue);
100+
const md5HashedValue = getMd5HashValue(value);
101+
const [hash1, hash2] = get64BitUints(md5HashedValue);
99102
for (let i = 0; i < this.hashCount; i++) {
100103
const index = this.getBitIndex(hash1, hash2, i);
101104
if (!this.isBitSet(index)) {

‎packages/firestore/test/unit/core/webchannel_wrapper.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,4 +348,17 @@ describe('Integer', () => {
348348
expect(Integer.fromString('125715', 8).toNumber()).equals(43981);
349349
expect(Integer.fromString('1010101111001101', 2).toNumber()).equals(43981);
350350
});
351+
352+
it('getBits() create a new Integer with the given value', () => {
353+
expect(new Integer([1, 2], 0).getBits(0)).equals(1);
354+
expect(new Integer([1, 2], 0).getBits(1)).equals(2);
355+
expect(new Integer([-1, -2], -1).getBits(0)).equals(-1);
356+
expect(new Integer([-1, -2], -1).getBits(1)).equals(-2);
357+
expect(new Integer([0xff, 0xffff], 0).getBits(0)).equals(
358+
Integer.fromNumber(0xff).toNumber()
359+
);
360+
expect(new Integer([0xff, 0xffff], 0).getBits(1)).equals(
361+
Integer.fromNumber(0xffff).toNumber()
362+
);
363+
});
351364
});

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

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,31 @@ describe('BloomFilter', () => {
3737
});
3838

3939
it('can instantiate a non empty bloom filter', () => {
40-
const bloomFilter = new BloomFilter(new Uint8Array([255, 255, 124]), 3, 13);
41-
expect(bloomFilter.size).to.equal(21);
40+
const bloomFilter0 = new BloomFilter(new Uint8Array(1), 0, 1);
41+
const bloomFilter1 = new BloomFilter(new Uint8Array(1), 1, 1);
42+
const bloomFilter2 = new BloomFilter(new Uint8Array(1), 2, 1);
43+
const bloomFilter3 = new BloomFilter(new Uint8Array(1), 3, 1);
44+
const bloomFilter4 = new BloomFilter(new Uint8Array(1), 4, 1);
45+
const bloomFilter5 = new BloomFilter(new Uint8Array(1), 5, 1);
46+
const bloomFilter6 = new BloomFilter(new Uint8Array(1), 6, 1);
47+
const bloomFilter7 = new BloomFilter(new Uint8Array(1), 7, 1);
48+
49+
expect(bloomFilter0.size).to.equal(8);
50+
expect(bloomFilter1.size).to.equal(7);
51+
expect(bloomFilter2.size).to.equal(6);
52+
expect(bloomFilter3.size).to.equal(5);
53+
expect(bloomFilter4.size).to.equal(4);
54+
expect(bloomFilter5.size).to.equal(3);
55+
expect(bloomFilter6.size).to.equal(2);
56+
expect(bloomFilter7.size).to.equal(1);
4257
});
4358

4459
it('should throw error if padding is invalid', () => {
4560
expect(() => new BloomFilter(new Uint8Array(1), -1, 1)).to.throw(
4661
'Invalid padding: -1'
4762
);
48-
expect(() => new BloomFilter(new Uint8Array(1), 9, 1)).to.throw(
49-
'Invalid padding: 9'
63+
expect(() => new BloomFilter(new Uint8Array(1), 8, 1)).to.throw(
64+
'Invalid padding: 8'
5065
);
5166
});
5267

@@ -102,40 +117,32 @@ describe('BloomFilter', () => {
102117
const documentPrefix =
103118
'projects/project-1/databases/database-1/documents/coll/doc';
104119

105-
interface TestDataType {
120+
interface GoldenTestInput {
106121
bits: {
107122
bitmap: string;
108123
padding: number;
109124
};
110125
hashCount: number;
111126
}
112127

113-
interface TestResultType {
128+
interface GoldenTestExpectedResult {
114129
membershipTestResults: string;
115130
}
116131

117-
function decodeBase64ToUint8Array(encoded: string): Uint8Array {
118-
return ByteString.fromBase64String(encoded).toUint8Array();
119-
}
120-
121132
function testBloomFilterAgainstExpectedResult(
122-
bloomFilterInputs: TestDataType,
123-
expectedResult: TestResultType
133+
bloomFilterInputs: GoldenTestInput,
134+
expectedResult: GoldenTestExpectedResult
124135
): void {
125136
const {
126137
bits: { bitmap, padding },
127138
hashCount
128139
} = bloomFilterInputs;
129140
const { membershipTestResults } = expectedResult;
130141

131-
const bloomFilter = new BloomFilter(
132-
decodeBase64ToUint8Array(bitmap),
133-
padding,
134-
hashCount
135-
);
142+
const byteArray = ByteString.fromBase64String(bitmap).toUint8Array();
143+
const bloomFilter = new BloomFilter(byteArray, padding, hashCount);
136144
for (let i = 0; i < membershipTestResults.length; i++) {
137-
const expectedMembershipResult =
138-
membershipTestResults[i] === '1' ? true : false;
145+
const expectedMembershipResult = membershipTestResults[i] === '1';
139146
const mightContain = bloomFilter.mightContain(documentPrefix + i);
140147
expect(mightContain).to.equal(expectedMembershipResult);
141148
}

‎packages/webchannel-wrapper/src/index.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ export class FetchXmlHttpFactory {
138138
}
139139

140140
// See https://google.github.io/closure-library/api/goog.crypt.Md5.html
141+
// Unit test are written in
142+
// packages/firestore/test/unit/core/webchannel_wrapper.test.ts
141143
export class Md5 {
142144
reset(): void;
143145
digest(): Array<number>;

0 commit comments

Comments
 (0)