Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
// Documents are ordered by key, so we can use a prefix scan to narrow down
// the documents we need to match the query against.
const collectionPath = query.path;
const prefix = new DocumentKey(collectionPath.child(''));
// DocumentId can be numeric ("__id<Long>__") or a plain string. Numeric IDs ordered before strings, sorted numerically.
const prefix = new DocumentKey(
collectionPath.child('__id' + Number.MIN_SAFE_INTEGER + '__')
);
const iterator = this.docs.getIteratorFrom(prefix);
while (iterator.hasNext()) {
const {
Expand Down
53 changes: 42 additions & 11 deletions packages/firestore/src/model/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,28 +163,59 @@ abstract class BasePath<B extends BasePath<B>> {
return this.segments.slice(this.offset, this.limit());
}

/**
* Compare 2 paths compared segment by segment, prioritizing numeric IDs
* (e.g., "__id123__") in numeric ascending order, followed by string
* segments in lexicographical order.
*/
static comparator<T extends BasePath<T>>(
p1: BasePath<T>,
p2: BasePath<T>
): number {
const len = Math.min(p1.length, p2.length);
for (let i = 0; i < len; i++) {
const left = p1.get(i);
const right = p2.get(i);
if (left < right) {
return -1;
}
if (left > right) {
return 1;
const comparison = BasePath.compareSegments(p1.get(i), p2.get(i));
if (comparison !== 0) {
return comparison;
}
}
if (p1.length < p2.length) {
return Math.sign(p1.length - p2.length);
}

private static compareSegments(lhs: string, rhs: string): number {
const isLhsNumeric = BasePath.isNumericId(lhs);
const isRhsNumeric = BasePath.isNumericId(rhs);

if (isLhsNumeric && !isRhsNumeric) {
// Only lhs is numeric
return -1;
}
if (p1.length > p2.length) {
} else if (!isLhsNumeric && isRhsNumeric) {
// Only rhs is numeric
return 1;
} else if (isLhsNumeric && isRhsNumeric) {
// both numeric
return Math.sign(
BasePath.extractNumericId(lhs) - BasePath.extractNumericId(rhs)
);
} else {
// both non-numeric
if (lhs < rhs) {
return -1;
}
if (lhs > rhs) {
return 1;
}
return 0;
}
return 0;
}

// Checks if a segment is a numeric ID (starts with "__id" and ends with "__").
private static isNumericId(segment: string): boolean {
return segment.startsWith('__id') && segment.endsWith('__');
}

private static extractNumericId(segment: string): number {
return parseInt(segment.substring(4, segment.length - 2), 10);
}
}

Expand Down
133 changes: 132 additions & 1 deletion packages/firestore/test/integration/api/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ import {
withTestDocAndInitialData,
withNamedTestDbsOrSkipUnlessUsingEmulator,
toDataArray,
checkOnlineAndOfflineResultsMatch
checkOnlineAndOfflineResultsMatch,
toIds
} from '../util/helpers';
import { DEFAULT_SETTINGS, DEFAULT_PROJECT_ID } from '../util/settings';

Expand Down Expand Up @@ -2245,4 +2246,134 @@ apiDescribe('Database', persistence => {
});
});
});

describe('sort documents by DocumentId', () => {
it('snapshot listener sorts query by DocumentId same way as get query', async () => {
const testDocs = {
A: { a: 1 },
a: { a: 1 },
Aa: { a: 1 },
'7': { a: 1 },
12: { a: 1 },
'__id7__': { a: 1 },
__id12__: { a: 1 },
'__id-2__': { a: 1 },
'_id1__': { a: 1 },
'__id1_': { a: 1 }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const orderedQuery = query(collectionRef, orderBy(documentId()));
const expectedDocs = [
'__id-2__',
'__id7__',
'__id12__',
'12',
'7',
'A',
'Aa',
'__id1_',
'_id1__',
'a'
];

const getSnapshot = await getDocsFromServer(orderedQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(orderedQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs);
unsubscribe();
});
});

it('snapshot listener sorts filtered query by DocumentId same way as get query', async () => {
const testDocs = {
A: { a: 1 },
a: { a: 1 },
Aa: { a: 1 },
'7': { a: 1 },
12: { a: 1 },
'__id7__': { a: 1 },
__id12__: { a: 1 },
'__id-2__': { a: 1 },
'_id1__': { a: 1 },
'__id1_': { a: 1 }
};

return withTestCollection(persistence, testDocs, async collectionRef => {
const filteredQuery = query(
collectionRef,
orderBy(documentId()),
where(documentId(), '>', '__id7__'),
where(documentId(), '<=', 'Aa')
);
const expectedDocs = ['__id12__', '12', '7', 'A', 'Aa'];

const getSnapshot = await getDocsFromServer(filteredQuery);
expect(toIds(getSnapshot)).to.deep.equal(expectedDocs);

const storeEvent = new EventsAccumulator<QuerySnapshot>();
const unsubscribe = onSnapshot(filteredQuery, storeEvent.storeEvent);
const watchSnapshot = await storeEvent.awaitEvent();
expect(toIds(watchSnapshot)).to.deep.equal(expectedDocs);
unsubscribe();
});
});

// eslint-disable-next-line no-restricted-properties
(persistence.gc === 'lru' ? describe : describe.skip)('offline', () => {
it('SDK orders query the same way online and offline', async () => {
const testDocs = {
A: { a: 1 },
a: { a: 1 },
Aa: { a: 1 },
'7': { a: 1 },
12: { a: 1 },
'__id7__': { a: 1 },
__id12__: { a: 1 },
'__id-2__': { a: 1 },
'_id1__': { a: 1 },
'__id1_': { a: 1 }
};

return withTestCollection(
persistence,
testDocs,
async collectionRef => {
const orderedQuery = query(collectionRef, orderBy(documentId()));
let expectedDocs = [
'__id-2__',
'__id7__',
'__id12__',
'12',
'7',
'A',
'Aa',
'__id1_',
'_id1__',
'a'
];
await checkOnlineAndOfflineResultsMatch(
orderedQuery,
...expectedDocs
);

const filteredQuery = query(
collectionRef,
orderBy(documentId()),
where(documentId(), '>', '__id7__'),
where(documentId(), '<=', 'Aa')
);
expectedDocs = ['__id12__', '12', '7', 'A', 'Aa'];
await checkOnlineAndOfflineResultsMatch(
filteredQuery,
...expectedDocs
);
}
);
});
});
});
});
Loading