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 document reference by long type id #14248

Merged
merged 3 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
89 changes: 89 additions & 0 deletions Firestore/Example/Tests/Integration/API/FIRQueryTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,95 @@ - (void)testCollectionGroupQueriesWithStartAtEndAtWithArbitraryDocumentIDs {
XCTAssertEqualObjects(ids, (@[ @"cg-doc2" ]));
}

- (void)testSnapshotListenerSortsQueryByDocumentIdInTheSameOrderAsServer {
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
@"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},
@"__id9223372036854775807__" : @{@"a" : @1},
@"__id-9223372036854775808__" : @{@"a" : @1},
}];

FIRQuery *query = [collRef queryOrderedByFieldPath:[FIRFieldPath documentID]];
NSArray<NSString *> *expectedDocs = @[
@"__id-9223372036854775808__", @"__id-2__", @"__id7__", @"__id12__",
@"__id9223372036854775807__", @"12", @"7", @"A", @"Aa", @"__id1_", @"_id1__", @"a"
];
FIRQuerySnapshot *getSnapshot = [self readDocumentSetForRef:query];
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(getSnapshot), expectedDocs);

id<FIRListenerRegistration> registration =
[query addSnapshotListener:self.eventAccumulator.valueEventHandler];
FIRQuerySnapshot *watchSnapshot = [self.eventAccumulator awaitEventWithName:@"Snapshot"];
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(watchSnapshot), expectedDocs);

[registration remove];
}

- (void)testSnapshotListenerSortsFilteredQueryByDocumentIdInTheSameOrderAsServer {
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
@"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},
@"__id9223372036854775807__" : @{@"a" : @1},
@"__id-9223372036854775808__" : @{@"a" : @1},
}];

FIRQuery *query = [[[collRef queryWhereFieldPath:[FIRFieldPath documentID]
isGreaterThan:@"__id7__"]
queryWhereFieldPath:[FIRFieldPath documentID]
isLessThanOrEqualTo:@"A"] queryOrderedByFieldPath:[FIRFieldPath documentID]];
NSArray<NSString *> *expectedDocs =
@[ @"__id12__", @"__id9223372036854775807__", @"12", @"7", @"A" ];
FIRQuerySnapshot *getSnapshot = [self readDocumentSetForRef:query];
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(getSnapshot), expectedDocs);

id<FIRListenerRegistration> registration =
[query addSnapshotListener:self.eventAccumulator.valueEventHandler];
FIRQuerySnapshot *watchSnapshot = [self.eventAccumulator awaitEventWithName:@"Snapshot"];
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(watchSnapshot), expectedDocs);

[registration remove];
}

- (void)testSdkOrdersQueryByDocumentIdTheSameWayOnlineAndOffline {
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
@"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},
@"__id9223372036854775807__" : @{@"a" : @1},
@"__id-9223372036854775808__" : @{@"a" : @1},
}];

[self checkOnlineAndOfflineQuery:[collRef queryOrderedByFieldPath:[FIRFieldPath documentID]]
matchesResult:@[
@"__id-9223372036854775808__", @"__id-2__", @"__id7__", @"__id12__",
@"__id9223372036854775807__", @"12", @"7", @"A", @"Aa", @"__id1_", @"_id1__",
@"a"
]];
}

- (void)testCollectionGroupQueriesWithWhereFiltersOnArbitraryDocumentIDs {
// Use .document() to get a random collection group name to use but ensure it starts with 'b'
// for predictable ordering.
Expand Down
37 changes: 36 additions & 1 deletion Firestore/core/src/model/base_path.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,18 @@ class BasePath {
std::equal(begin(), end(), potential_child.begin());
}

/**
* Compares the current path against another Path object. Paths are compared
* segment by segment, prioritizing numeric IDs (e.g., "__id123__") in numeric
* ascending order, followed by string segments in lexicographical order.
*/
util::ComparisonResult CompareTo(const T& rhs) const {
return util::CompareContainer(segments_, rhs.segments_);
milaGGL marked this conversation as resolved.
Show resolved Hide resolved
size_t min_size = std::min(size(), rhs.size());
for (size_t i = 0; i < min_size; ++i) {
auto cmp = compareSegments(segments_[i], rhs.segments_[i]);
if (!util::Same(cmp)) return cmp;
}
return util::Compare(size(), rhs.size());
}

friend bool operator==(const BasePath& lhs, const BasePath& rhs) {
Expand All @@ -174,6 +184,31 @@ class BasePath {

private:
SegmentsT segments_;

static util::ComparisonResult compareSegments(const std::string& lhs,
ehsannas marked this conversation as resolved.
Show resolved Hide resolved
const std::string& rhs) {
bool isLhsNumeric = isNumericId(lhs);
bool isRhsNumeric = isNumericId(rhs);

if (isLhsNumeric && !isRhsNumeric) {
return util::ComparisonResult::Ascending;
} else if (!isLhsNumeric && isRhsNumeric) {
return util::ComparisonResult::Descending;
} else if (isLhsNumeric && isRhsNumeric) {
return util::Compare(extractNumericId(lhs), extractNumericId(rhs));
} else {
return util::Compare(lhs, rhs);
}
}

static bool isNumericId(const std::string& segment) {
return segment.substr(0, 4) == "__id" &&
segment.substr(segment.size() - 2) == "__";
ehsannas marked this conversation as resolved.
Show resolved Hide resolved
}

static int64_t extractNumericId(const std::string& segment) {
return std::stol(segment.substr(4, segment.size() - 2));
ehsannas marked this conversation as resolved.
Show resolved Hide resolved
}
};

} // namespace impl
Expand Down
Loading