Skip to content

Commit 0d569cf

Browse files
authored
Fix: sort document reference by long type id (#14248)
1 parent 3a470b0 commit 0d569cf

File tree

2 files changed

+135
-1
lines changed

2 files changed

+135
-1
lines changed

Firestore/Example/Tests/Integration/API/FIRQueryTests.mm

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,98 @@ - (void)testCollectionGroupQueriesWithStartAtEndAtWithArbitraryDocumentIDs {
804804
XCTAssertEqualObjects(ids, (@[ @"cg-doc2" ]));
805805
}
806806

807+
- (void)testSnapshotListenerSortsQueryByDocumentIdInTheSameOrderAsServer {
808+
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
809+
@"A" : @{@"a" : @1},
810+
@"a" : @{@"a" : @1},
811+
@"Aa" : @{@"a" : @1},
812+
@"7" : @{@"a" : @1},
813+
@"12" : @{@"a" : @1},
814+
@"__id7__" : @{@"a" : @1},
815+
@"__id12__" : @{@"a" : @1},
816+
@"__id-2__" : @{@"a" : @1},
817+
@"__id1_" : @{@"a" : @1},
818+
@"_id1__" : @{@"a" : @1},
819+
@"__id" : @{@"a" : @1},
820+
@"__id9223372036854775807__" : @{@"a" : @1},
821+
@"__id-9223372036854775808__" : @{@"a" : @1},
822+
}];
823+
824+
FIRQuery *query = [collRef queryOrderedByFieldPath:[FIRFieldPath documentID]];
825+
NSArray<NSString *> *expectedDocs = @[
826+
@"__id-9223372036854775808__", @"__id-2__", @"__id7__", @"__id12__",
827+
@"__id9223372036854775807__", @"12", @"7", @"A", @"Aa", @"__id", @"__id1_", @"_id1__", @"a"
828+
];
829+
FIRQuerySnapshot *getSnapshot = [self readDocumentSetForRef:query];
830+
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(getSnapshot), expectedDocs);
831+
832+
id<FIRListenerRegistration> registration =
833+
[query addSnapshotListener:self.eventAccumulator.valueEventHandler];
834+
FIRQuerySnapshot *watchSnapshot = [self.eventAccumulator awaitEventWithName:@"Snapshot"];
835+
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(watchSnapshot), expectedDocs);
836+
837+
[registration remove];
838+
}
839+
840+
- (void)testSnapshotListenerSortsFilteredQueryByDocumentIdInTheSameOrderAsServer {
841+
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
842+
@"A" : @{@"a" : @1},
843+
@"a" : @{@"a" : @1},
844+
@"Aa" : @{@"a" : @1},
845+
@"7" : @{@"a" : @1},
846+
@"12" : @{@"a" : @1},
847+
@"__id7__" : @{@"a" : @1},
848+
@"__id12__" : @{@"a" : @1},
849+
@"__id-2__" : @{@"a" : @1},
850+
@"__id1_" : @{@"a" : @1},
851+
@"_id1__" : @{@"a" : @1},
852+
@"__id" : @{@"a" : @1},
853+
@"__id9223372036854775807__" : @{@"a" : @1},
854+
@"__id-9223372036854775808__" : @{@"a" : @1},
855+
}];
856+
857+
FIRQuery *query = [[[collRef queryWhereFieldPath:[FIRFieldPath documentID]
858+
isGreaterThan:@"__id7__"]
859+
queryWhereFieldPath:[FIRFieldPath documentID]
860+
isLessThanOrEqualTo:@"A"] queryOrderedByFieldPath:[FIRFieldPath documentID]];
861+
NSArray<NSString *> *expectedDocs =
862+
@[ @"__id12__", @"__id9223372036854775807__", @"12", @"7", @"A" ];
863+
FIRQuerySnapshot *getSnapshot = [self readDocumentSetForRef:query];
864+
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(getSnapshot), expectedDocs);
865+
866+
id<FIRListenerRegistration> registration =
867+
[query addSnapshotListener:self.eventAccumulator.valueEventHandler];
868+
FIRQuerySnapshot *watchSnapshot = [self.eventAccumulator awaitEventWithName:@"Snapshot"];
869+
XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(watchSnapshot), expectedDocs);
870+
871+
[registration remove];
872+
}
873+
874+
- (void)testSdkOrdersQueryByDocumentIdTheSameWayOnlineAndOffline {
875+
FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
876+
@"A" : @{@"a" : @1},
877+
@"a" : @{@"a" : @1},
878+
@"Aa" : @{@"a" : @1},
879+
@"7" : @{@"a" : @1},
880+
@"12" : @{@"a" : @1},
881+
@"__id7__" : @{@"a" : @1},
882+
@"__id12__" : @{@"a" : @1},
883+
@"__id-2__" : @{@"a" : @1},
884+
@"__id1_" : @{@"a" : @1},
885+
@"_id1__" : @{@"a" : @1},
886+
@"__id" : @{@"a" : @1},
887+
@"__id9223372036854775807__" : @{@"a" : @1},
888+
@"__id-9223372036854775808__" : @{@"a" : @1},
889+
}];
890+
891+
[self checkOnlineAndOfflineQuery:[collRef queryOrderedByFieldPath:[FIRFieldPath documentID]]
892+
matchesResult:@[
893+
@"__id-9223372036854775808__", @"__id-2__", @"__id7__", @"__id12__",
894+
@"__id9223372036854775807__", @"12", @"7", @"A", @"Aa", @"__id", @"__id1_",
895+
@"_id1__", @"a"
896+
]];
897+
}
898+
807899
- (void)testCollectionGroupQueriesWithWhereFiltersOnArbitraryDocumentIDs {
808900
// Use .document() to get a random collection group name to use but ensure it starts with 'b'
809901
// for predictable ordering.

Firestore/core/src/model/base_path.h

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,18 @@ class BasePath {
150150
std::equal(begin(), end(), potential_child.begin());
151151
}
152152

153+
/**
154+
* Compares the current path against another Path object. Paths are compared
155+
* segment by segment, prioritizing numeric IDs (e.g., "__id123__") in numeric
156+
* ascending order, followed by string segments in lexicographical order.
157+
*/
153158
util::ComparisonResult CompareTo(const T& rhs) const {
154-
return util::CompareContainer(segments_, rhs.segments_);
159+
size_t min_size = std::min(size(), rhs.size());
160+
for (size_t i = 0; i < min_size; ++i) {
161+
auto cmp = CompareSegments(segments_[i], rhs.segments_[i]);
162+
if (!util::Same(cmp)) return cmp;
163+
}
164+
return util::Compare(size(), rhs.size());
155165
}
156166

157167
friend bool operator==(const BasePath& lhs, const BasePath& rhs) {
@@ -174,6 +184,38 @@ class BasePath {
174184

175185
private:
176186
SegmentsT segments_;
187+
188+
static const size_t kNumericIdPrefixLength = 4;
189+
static const size_t kNumericIdSuffixLength = 2;
190+
static const size_t kNumericIdTotalOverhead =
191+
kNumericIdPrefixLength + kNumericIdSuffixLength;
192+
193+
static util::ComparisonResult CompareSegments(const std::string& lhs,
194+
const std::string& rhs) {
195+
bool isLhsNumeric = IsNumericId(lhs);
196+
bool isRhsNumeric = IsNumericId(rhs);
197+
198+
if (isLhsNumeric && !isRhsNumeric) {
199+
return util::ComparisonResult::Ascending;
200+
} else if (!isLhsNumeric && isRhsNumeric) {
201+
return util::ComparisonResult::Descending;
202+
} else if (isLhsNumeric && isRhsNumeric) {
203+
return util::Compare(ExtractNumericId(lhs), ExtractNumericId(rhs));
204+
} else {
205+
return util::Compare(lhs, rhs);
206+
}
207+
}
208+
209+
static bool IsNumericId(const std::string& segment) {
210+
return segment.size() > kNumericIdTotalOverhead &&
211+
segment.substr(0, kNumericIdPrefixLength) == "__id" &&
212+
segment.substr(segment.size() - kNumericIdSuffixLength) == "__";
213+
}
214+
215+
static int64_t ExtractNumericId(const std::string& segment) {
216+
return std::stol(segment.substr(kNumericIdPrefixLength,
217+
segment.size() - kNumericIdSuffixLength));
218+
}
177219
};
178220

179221
} // namespace impl

0 commit comments

Comments
 (0)