Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: sort strings in UTF-8 encoded byte order #2275

Merged
merged 6 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
milaGGL marked this conversation as resolved.
Show resolved Hide resolved
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
17 changes: 4 additions & 13 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 @@ -190,8 +191,8 @@ abstract class Path<T> {
this.extractNumericId(rhs)
);
} else {
// both non-numeric
return this.compareStrings(lhs, rhs);
// both string
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();
ehsannas marked this conversation as resolved.
Show resolved Hide resolved
});

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: {P: 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},
P: {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
Loading