Take 2: Move input validation logic from FIRFieldPath to model::FieldPath. (#2727) (#2790)

* Move input validation logic from FIRFieldPath to model::FieldPath. (#2727)

Author: mikelehen

Firestore/Source/API/FIRFieldPath.mm

@@ -51,13 +51,10 @@ - (instancetype)initWithFields:(NSArray<NSString *> *)fieldNames {
   std::vector<std::string> field_names;
   field_names.reserve(fieldNames.count);
   for (int i = 0; i < fieldNames.count; ++i) {
-    if (fieldNames[i].length == 0) {
-      ThrowInvalidArgument("Invalid field name at index %s. Field names must not be empty.", i);
-    }
     field_names.emplace_back(util::MakeString(fieldNames[i]));
   }
 
-  return [self initPrivate:FieldPath(std::move(field_names))];
+  return [self initPrivate:FieldPath::FromSegments(std::move(field_names))];
 }
 
 + (instancetype)documentID {
@@ -72,21 +69,8 @@ - (instancetype)initPrivate:(FieldPath)fieldPath {
 }
 
 + (instancetype)pathWithDotSeparatedString:(NSString *)path {
-  if ([[FIRFieldPath reservedCharactersRegex]
-          numberOfMatchesInString:path
-                          options:0
-                            range:NSMakeRange(0, path.length)] > 0) {
-    ThrowInvalidArgument(
-        "Invalid field path (%s). Paths must not contain '~', '*', '/', '[', or ']'", path);
-  }
-  @try {
-    return [[FIRFieldPath alloc] initWithFields:[path componentsSeparatedByString:@"."]];
-  } @catch (NSException *exception) {
-    ThrowInvalidArgument(
-        "Invalid field path (%s). Paths must not be empty, begin with '.', end with '.', or "
-        "contain '..'",
-        path);
-  }
+  return
+      [[FIRFieldPath alloc] initPrivate:FieldPath::FromDotSeparatedString(util::MakeString(path))];
 }
 
 /** Matches any characters in a field path string that are reserved. */

Firestore/core/src/firebase/firestore/model/CMakeLists.txt

@@ -50,6 +50,7 @@ cc_library(
   DEPENDS
     absl_optional
     absl_strings
+    firebase_firestore_api_input_validation
     firebase_firestore_immutable
     firebase_firestore_util
     firebase_firestore_types

Firestore/core/src/firebase/firestore/model/field_path.cc

@@ -19,6 +19,7 @@
 #include <algorithm>
 #include <utility>
 
+#include "Firestore/core/src/firebase/firestore/api/input_validation.h"
 #include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
 #include "absl/strings/str_join.h"
 #include "absl/strings/str_replace.h"
@@ -28,6 +29,8 @@ namespace firebase {
 namespace firestore {
 namespace model {
 
+using api::ThrowInvalidArgument;
+
 namespace {
 
 /**
@@ -78,6 +81,28 @@ struct JoinEscaped {
 };
 }  // namespace
 
+FieldPath FieldPath::FromDotSeparatedString(absl::string_view path) {
+  if (path.find_first_of("~*/[]") != absl::string_view::npos) {
+    ThrowInvalidArgument(
+        "Invalid field path (%s). Paths must not contain '~', '*', '/', '[', "
+        "or ']'",
+        path);
+  }
+
+  SegmentsT segments =
+      absl::StrSplit(path, '.', [path](absl::string_view segment) {
+        if (segment.empty()) {
+          ThrowInvalidArgument(
+              "Invalid field path (%s). Paths must not be empty, begin with "
+              "'.', end with '.', or contain '..'",
+              path);
+        }
+        return true;
+      });
+
+  return FieldPath(std::move(segments));
+}
+
 FieldPath FieldPath::FromServerFormat(const absl::string_view path) {
   SegmentsT segments;
   std::string segment;

Firestore/core/src/firebase/firestore/model/field_path.h

@@ -21,6 +21,7 @@
 #include <string>
 #include <utility>
 
+#include "Firestore/core/src/firebase/firestore/api/input_validation.h"
 #include "Firestore/core/src/firebase/firestore/model/base_path.h"
 #include "absl/strings/string_view.h"
 
@@ -51,6 +52,25 @@ class FieldPath : public impl::BasePath<FieldPath> {
   explicit FieldPath(SegmentsT&& segments) : BasePath{std::move(segments)} {
   }
 
+  /**
+   * Creates and returns a new path from a dot-separated field-path string,
+   * where path segments are separated by a dot ".".
+   *
+   * PORTING NOTE: We define this on the model class to avoid having a tiny
+   * api::FieldPath wrapper class.
+   */
+  static FieldPath FromDotSeparatedString(absl::string_view path);
+
+  /**
+   * Creates and returns a new path from a set of segments received from the
+   * public API.
+   */
+  static FieldPath FromSegments(SegmentsT&& segments) {
+    ValidateSegments(segments);
+    FieldPath path(std::move(segments));
+    return path;
+  }
+
   /**
    * Creates and returns a new path from the server formatted field-path string,
    * where path segments are separated by a dot "." and optionally encoded using
@@ -85,6 +105,22 @@ class FieldPath : public impl::BasePath<FieldPath> {
   bool operator>=(const FieldPath& rhs) const {
     return BasePath::operator>=(rhs);
   }
+
+ private:
+  static void ValidateSegments(const SegmentsT& segments) {
+    if (segments.empty()) {
+      api::ThrowInvalidArgument(
+          "Invalid field path. Provided names must not be empty.");
+    }
+
+    for (size_t i = 0; i < segments.size(); i++) {
+      if (segments[i].empty()) {
+        api::ThrowInvalidArgument(
+            "Invalid field name at index %s. Field names must not be empty.",
+            i);
+      }
+    }
+  }
 };
 
 }  // namespace model