Skip to content

Commit

Permalink
FIX: sort strings in UTF-8 encoded byte order (#2275)
Browse files Browse the repository at this point in the history
  • Loading branch information
milaGGL authored Jan 15, 2025
1 parent 1e714a8 commit a2950e0
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 14 deletions.
19 changes: 17 additions & 2 deletions dev/src/order.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ function compareObjects(left: ApiMapValue, right: ApiMapValue): number {
leftKeys.sort();
rightKeys.sort();
for (let i = 0; i < leftKeys.length && i < rightKeys.length; i++) {
const keyComparison = primitiveComparator(leftKeys[i], rightKeys[i]);
const keyComparison = compareUtf8Strings(leftKeys[i], rightKeys[i]);
if (keyComparison !== 0) {
return keyComparison;
}
Expand Down Expand Up @@ -248,6 +248,21 @@ function compareVectors(left: ApiMapValue, right: ApiMapValue): number {
return compareArrays(leftArray, rightArray);
}

function stringToUtf8Bytes(str: string): Uint8Array {
return new TextEncoder().encode(str);
}

/*!
* Compare strings in UTF-8 encoded byte order
* @private
* @internal
*/
export function compareUtf8Strings(left: string, right: string): number {
const leftBytes = stringToUtf8Bytes(left);
const rightBytes = stringToUtf8Bytes(right);
return compareBlobs(Buffer.from(leftBytes), Buffer.from(rightBytes));
}

/*!
* @private
* @internal
Expand All @@ -269,7 +284,7 @@ export function compare(left: api.IValue, right: api.IValue): number {
case TypeOrder.BOOLEAN:
return primitiveComparator(left.booleanValue!, right.booleanValue!);
case TypeOrder.STRING:
return primitiveComparator(left.stringValue!, right.stringValue!);
return compareUtf8Strings(left.stringValue!, right.stringValue!);
case TypeOrder.NUMBER:
return compareNumberProtos(left, right);
case TypeOrder.TIMESTAMP:
Expand Down
15 changes: 3 additions & 12 deletions dev/src/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import * as firestore from '@google-cloud/firestore';

import {google} from '../protos/firestore_v1_proto_api';
import {compareUtf8Strings, primitiveComparator} from './order';

import {isObject} from './util';
import {
Expand Down Expand Up @@ -170,7 +171,7 @@ abstract class Path<T> {
return comparison;
}
}
return Math.sign(this.segments.length - other.segments.length);
return primitiveComparator(this.segments.length, other.segments.length);
}

private compareSegments(lhs: string, rhs: string): number {
Expand All @@ -191,7 +192,7 @@ abstract class Path<T> {
);
} else {
// both non-numeric
return this.compareStrings(lhs, rhs);
return compareUtf8Strings(lhs, rhs);
}
}

Expand All @@ -215,16 +216,6 @@ abstract class Path<T> {
}
}

private compareStrings(lhs: string, rhs: string): number {
if (lhs < rhs) {
return -1;
} else if (lhs > rhs) {
return 1;
} else {
return 0;
}
}

/**
* Returns a copy of the underlying segments.
*
Expand Down
150 changes: 150 additions & 0 deletions dev/system-test/firestore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4084,6 +4084,156 @@ describe('Query class', () => {
docChanges: expectedChanges,
});
});

describe('sort unicode strings', () => {
it('snapshot listener sorts unicode strings same as server', async () => {
const collection = await testCollectionWithDocs({
a: {value: 'Łukasiewicz'},
b: {value: 'Sierpiński'},
c: {value: '岩澤'},
d: {value: '🄟'},
e: {value: 'P'},
f: {value: '︒'},
g: {value: '🐵'},
});

const query = collection.orderBy('value');
const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];

const getSnapshot = await query.get();
expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);

const unsubscribe = query.onSnapshot(snapshot =>
currentDeferred.resolve(snapshot)
);
const watchSnapshot = await waitForSnapshot();
snapshotsEqual(watchSnapshot, {
docs: getSnapshot.docs,
docChanges: getSnapshot.docChanges(),
});
unsubscribe();
});

it('snapshot listener sorts unicode strings in array same as server', async () => {
const collection = await testCollectionWithDocs({
a: {value: ['Łukasiewicz']},
b: {value: ['Sierpiński']},
c: {value: ['岩澤']},
d: {value: ['🄟']},
e: {value: ['P']},
f: {value: ['︒']},
g: {value: ['🐵']},
});

const query = collection.orderBy('value');
const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];

const getSnapshot = await query.get();
expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);

const unsubscribe = query.onSnapshot(snapshot =>
currentDeferred.resolve(snapshot)
);
const watchSnapshot = await waitForSnapshot();
snapshotsEqual(watchSnapshot, {
docs: getSnapshot.docs,
docChanges: getSnapshot.docChanges(),
});
unsubscribe();
});

it('snapshot listener sorts unicode strings in map same as server', async () => {
const collection = await testCollectionWithDocs({
a: {value: {foo: 'Łukasiewicz'}},
b: {value: {foo: 'Sierpiński'}},
c: {value: {foo: '岩澤'}},
d: {value: {foo: '🄟'}},
e: {value: {foo: 'P'}},
f: {value: {foo: '︒'}},
g: {value: {foo: '🐵'}},
});

const query = collection.orderBy('value');
const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];

const getSnapshot = await query.get();
expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);

const unsubscribe = query.onSnapshot(snapshot =>
currentDeferred.resolve(snapshot)
);
const watchSnapshot = await waitForSnapshot();
snapshotsEqual(watchSnapshot, {
docs: getSnapshot.docs,
docChanges: getSnapshot.docChanges(),
});
unsubscribe();
});

it('snapshot listener sorts unicode strings in map key same as server', async () => {
const collection = await testCollectionWithDocs({
a: {value: {Łukasiewicz: true}},
b: {value: {Sierpiński: true}},
c: {value: {岩澤: true}},
d: {value: {'🄟': true}},
e: {value: {: true}},
f: {value: {'︒': true}},
g: {value: {'🐵': true}},
});

const query = collection.orderBy('value');
const expectedDocs = ['b', 'a', 'c', 'f', 'e', 'd', 'g'];

const getSnapshot = await query.get();
expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);

const unsubscribe = query.onSnapshot(snapshot =>
currentDeferred.resolve(snapshot)
);
const watchSnapshot = await waitForSnapshot();
snapshotsEqual(watchSnapshot, {
docs: getSnapshot.docs,
docChanges: getSnapshot.docChanges(),
});
unsubscribe();
});

it('snapshot listener sorts unicode strings in document key same as server', async () => {
const collection = await testCollectionWithDocs({
Łukasiewicz: {value: true},
Sierpiński: {value: true},
岩澤: {value: true},
'🄟': {value: true},
: {value: true},
'︒': {value: true},
'🐵': {value: true},
});

const query = collection.orderBy(FieldPath.documentId());
const expectedDocs = [
'Sierpiński',
'Łukasiewicz',
'岩澤',
'︒',
'P',
'🄟',
'🐵',
];

const getSnapshot = await query.get();
expect(getSnapshot.docs.map(d => d.id)).to.deep.equal(expectedDocs);

const unsubscribe = query.onSnapshot(snapshot =>
currentDeferred.resolve(snapshot)
);
const watchSnapshot = await waitForSnapshot();
snapshotsEqual(watchSnapshot, {
docs: getSnapshot.docs,
docChanges: getSnapshot.docChanges(),
});
unsubscribe();
});
});
});

(process.env.FIRESTORE_EMULATOR_HOST === undefined
Expand Down

0 comments on commit a2950e0

Please sign in to comment.