Merge remote-tracking branch 'origin/master' into rsgowman/protobuf_cpp

Also "fixed" BadFieldValueTagWithOtherValidTagsPresent test by changing
'false' to 'true'. Details: Depending on the version of nanopb, nanopb
would explicitly encode 'false', which shouldn't be done in proto3. When
it's explicitly encoded, the test worked properly. But when it was
(properly) dropped, the invalid tag is the only field that's actually
encoded, thus violating the assumptions of the test, leading to a test
failure. s/false/true fixes it, as now the boolean_value field is
(properly) encoded regardless of version.

6 files changed, 22 insertions, 19 deletions

diff --git a/Firestore/Source/API/FIRFirestore.mm b/Firestore/Source/API/FIRFirestore.mm
index 3671b51..fb4a1ec 100644
--- a/Firestore/Source/API/FIRFirestore.mm
+++ b/Firestore/Source/API/FIRFirestore.mm
@@ -128,11 +128,11 @@ extern "C" NSString *const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain";
                          @"Failed to get FirebaseApp instance. Please call FirebaseApp.configure() "
                          @"before using Firestore");
   }
-  return [self firestoreForApp:app database:util::WrapNSStringNoCopy(DatabaseId::kDefault)];
+  return [self firestoreForApp:app database:util::WrapNSString(DatabaseId::kDefault)];
 }
 
 + (instancetype)firestoreForApp:(FIRApp *)app {
-  return [self firestoreForApp:app database:util::WrapNSStringNoCopy(DatabaseId::kDefault)];
+  return [self firestoreForApp:app database:util::WrapNSString(DatabaseId::kDefault)];
 }
 
 // TODO(b/62410906): make this public
diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm
index 44e46da..d73a870 100644
--- a/Firestore/Source/API/FSTUserDataConverter.mm
+++ b/Firestore/Source/API/FSTUserDataConverter.mm
@@ -17,6 +17,7 @@
 #import "Firestore/Source/API/FSTUserDataConverter.h"
 
 #include <memory>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -41,6 +42,7 @@
 #include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 #include "absl/memory/memory.h"
+#include "absl/strings/match.h"
 
 namespace util = firebase::firestore::util;
 using firebase::firestore::model::ArrayTransform;
@@ -55,7 +57,7 @@ using firebase::firestore::model::TransformOperation;
 
 NS_ASSUME_NONNULL_BEGIN
 
-static NSString *const RESERVED_FIELD_DESIGNATOR = @"__";
+static const char *RESERVED_FIELD_DESIGNATOR = "__";
 
 #pragma mark - FSTParsedSetData
 
@@ -277,7 +279,7 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) {
                                                         arrayElement:NO
                                                      fieldTransforms:_fieldTransforms
                                                            fieldMask:_fieldMask];
-  [context validatePathSegment:fieldName];
+  [context validatePathSegment:util::MakeStringView(fieldName)];
   return context;
 }
 
@@ -334,15 +336,16 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) {
   if (_path == nullptr) {
     return;
   }
-  for (const auto &segment : *_path) {
-    [self validatePathSegment:util::WrapNSStringNoCopy(segment)];
+  for (const std::string &segment : *_path) {
+    [self validatePathSegment:segment];
   }
 }
 
-- (void)validatePathSegment:(NSString *)segment {
-  if ([self isWrite] && [segment hasPrefix:RESERVED_FIELD_DESIGNATOR] &&
-      [segment hasSuffix:RESERVED_FIELD_DESIGNATOR]) {
-    FSTThrowInvalidArgument(@"Document fields cannot begin and end with %@%@",
+- (void)validatePathSegment:(absl::string_view)segment {
+  absl::string_view designator{RESERVED_FIELD_DESIGNATOR};
+  if ([self isWrite] && absl::StartsWith(segment, designator) &&
+      absl::EndsWith(segment, designator)) {
+    FSTThrowInvalidArgument(@"Document fields cannot begin and end with %s%@",
                             RESERVED_FIELD_DESIGNATOR, [self fieldDescription]);
   }
 }
diff --git a/Firestore/Source/Core/FSTQuery.mm b/Firestore/Source/Core/FSTQuery.mm
index eb6d087..abec474 100644
--- a/Firestore/Source/Core/FSTQuery.mm
+++ b/Firestore/Source/Core/FSTQuery.mm
@@ -753,7 +753,8 @@ NSString *FSTStringFromQueryRelationOperator(FSTRelationFilterOperator filterOpe
     return _canonicalID;
   }
 
-  NSMutableString *canonicalID = [util::WrapNSStringNoCopy(_path.CanonicalString()) mutableCopy];
+  NSMutableString *canonicalID = [NSMutableString string];
+  [canonicalID appendFormat:@"%s", _path.CanonicalString().c_str()];
 
   // Add filters.
   [canonicalID appendString:@"|f:"];
diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm
index 321d47a..9f75a3e 100644
--- a/Firestore/Source/Local/FSTLevelDB.mm
+++ b/Firestore/Source/Local/FSTLevelDB.mm
@@ -114,10 +114,10 @@ using leveldb::WriteOptions;
   // projectIDs are DNS-compatible names and cannot contain dots so there's
   // no danger of collisions.
   NSString *directory = documentsDirectory;
-  directory = [directory
-      stringByAppendingPathComponent:util::WrapNSStringNoCopy(databaseInfo.persistence_key())];
+  directory =
+      [directory stringByAppendingPathComponent:util::WrapNSString(databaseInfo.persistence_key())];
 
-  NSString *segment = util::WrapNSStringNoCopy(databaseInfo.database_id().project_id());
+  NSString *segment = util::WrapNSString(databaseInfo.database_id().project_id());
   if (!databaseInfo.database_id().IsDefaultDatabase()) {
     segment = [NSString
         stringWithFormat:@"%@.%s", segment, databaseInfo.database_id().database_id().c_str()];
diff --git a/Firestore/Source/Model/FSTFieldValue.mm b/Firestore/Source/Model/FSTFieldValue.mm
index 6a13511..4acae7d 100644
--- a/Firestore/Source/Model/FSTFieldValue.mm
+++ b/Firestore/Source/Model/FSTFieldValue.mm
@@ -690,13 +690,12 @@ static NSComparisonResult CompareBytes(NSData *left, NSData *right) {
 - (NSComparisonResult)compare:(FSTFieldValue *)other {
   if ([other isKindOfClass:[FSTReferenceValue class]]) {
     FSTReferenceValue *ref = (FSTReferenceValue *)other;
-    NSComparisonResult cmp = [util::WrapNSStringNoCopy(self.databaseID->project_id())
-        compare:util::WrapNSStringNoCopy(ref.databaseID->project_id())];
+    NSComparisonResult cmp =
+        WrapCompare(self.databaseID->project_id(), ref.databaseID->project_id());
     if (cmp != NSOrderedSame) {
       return cmp;
     }
-    cmp = [util::WrapNSStringNoCopy(self.databaseID->database_id())
-        compare:util::WrapNSStringNoCopy(ref.databaseID->database_id())];
+    cmp = WrapCompare(self.databaseID->database_id(), ref.databaseID->database_id());
     return cmp != NSOrderedSame ? cmp : [self.key compare:ref.key];
   } else {
     return [self defaultCompare:other];
diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm
index ab40dd6..263fe6d 100644
--- a/Firestore/Source/Remote/FSTSerializerBeta.mm
+++ b/Firestore/Source/Remote/FSTSerializerBeta.mm
@@ -921,7 +921,7 @@ NS_ASSUME_NONNULL_BEGIN
   } else if ([filter isKindOfClass:[FSTNullFilter class]]) {
     proto.unaryFilter.op = GCFSStructuredQuery_UnaryFilter_Operator_IsNull;
   } else {
-    HARD_FAIL("Unrecognized filter: %s", static_cast<id>(filter));
+    HARD_FAIL("Unrecognized filter: %s", filter);
   }
   return proto;
 }