resolve comments

Author: milaGGL

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

@@ -816,14 +816,15 @@ - (void)testSnapshotListenerSortsQueryByDocumentIdInTheSameOrderAsServer {
     @"__id-2__" : @{@"a" : @1},
     @"__id1_" : @{@"a" : @1},
     @"_id1__" : @{@"a" : @1},
+    @"__id" : @{@"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"
+    @"__id9223372036854775807__", @"12", @"7", @"A", @"Aa", @"__id", @"__id1_", @"_id1__", @"a"
   ];
   FIRQuerySnapshot *getSnapshot = [self readDocumentSetForRef:query];
   XCTAssertEqualObjects(FIRQuerySnapshotGetIDs(getSnapshot), expectedDocs);
@@ -848,6 +849,7 @@ - (void)testSnapshotListenerSortsFilteredQueryByDocumentIdInTheSameOrderAsServer
     @"__id-2__" : @{@"a" : @1},
     @"__id1_" : @{@"a" : @1},
     @"_id1__" : @{@"a" : @1},
+    @"__id" : @{@"a" : @1},
     @"__id9223372036854775807__" : @{@"a" : @1},
     @"__id-9223372036854775808__" : @{@"a" : @1},
   }];
@@ -881,15 +883,16 @@ - (void)testSdkOrdersQueryByDocumentIdTheSameWayOnlineAndOffline {
     @"__id-2__" : @{@"a" : @1},
     @"__id1_" : @{@"a" : @1},
     @"_id1__" : @{@"a" : @1},
+    @"__id" : @{@"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"
+                       @"__id9223372036854775807__", @"12", @"7", @"A", @"Aa", @"__id", @"__id1_",
+                       @"_id1__", @"a"
                      ]];
 }
 

Firestore/core/src/model/base_path.h

@@ -158,7 +158,7 @@ class BasePath {
   util::ComparisonResult CompareTo(const T& rhs) const {
     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]);
+      auto cmp = CompareSegments(segments_[i], rhs.segments_[i]);
       if (!util::Same(cmp)) return cmp;
     }
     return util::Compare(size(), rhs.size());
@@ -185,29 +185,36 @@ class BasePath {
  private:
   SegmentsT segments_;
 
-  static util::ComparisonResult compareSegments(const std::string& lhs,
+  static const size_t kNumericIdPrefixLength = 4;
+  static const size_t kNumericIdSuffixLength = 2;
+  static const size_t kNumericIdTotalOverhead =
+      kNumericIdPrefixLength + kNumericIdSuffixLength;
+
+  static util::ComparisonResult CompareSegments(const std::string& lhs,
                                                 const std::string& rhs) {
-    bool isLhsNumeric = isNumericId(lhs);
-    bool isRhsNumeric = isNumericId(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));
+      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) == "__";
+  static bool IsNumericId(const std::string& segment) {
+    return segment.size() > kNumericIdTotalOverhead &&
+           segment.substr(0, kNumericIdPrefixLength) == "__id" &&
+           segment.substr(segment.size() - kNumericIdSuffixLength) == "__";
   }
 
-  static int64_t extractNumericId(const std::string& segment) {
-    return std::stol(segment.substr(4, segment.size() - 2));
+  static int64_t ExtractNumericId(const std::string& segment) {
+    return std::stol(segment.substr(kNumericIdPrefixLength,
+                                    segment.size() - kNumericIdSuffixLength));
   }
 };