diff options
11 files changed, 48 insertions, 39 deletions
diff --git a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt index 8bdbe18..df602f9 100644 --- a/Firestore/core/src/firebase/firestore/model/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/model/CMakeLists.txt @@ -22,6 +22,8 @@ cc_library( field_path.h field_value.cc field_value.h + resource_path.cc + resource_path.h timestamp.cc timestamp.h DEPENDS diff --git a/Firestore/core/src/firebase/firestore/model/base_path.h b/Firestore/core/src/firebase/firestore/model/base_path.h index f5a8ab7..accce27 100644 --- a/Firestore/core/src/firebase/firestore/model/base_path.h +++ b/Firestore/core/src/firebase/firestore/model/base_path.h @@ -166,7 +166,7 @@ class BasePath { } BasePath(std::initializer_list<std::string> list) : segments_{list} { } - BasePath(SegmentsT&& segments) : segments_{std::move(segments)} { + explicit BasePath(SegmentsT&& segments) : segments_{std::move(segments)} { } private: diff --git a/Firestore/core/src/firebase/firestore/model/field_path.cc b/Firestore/core/src/firebase/firestore/model/field_path.cc index 6c40600..0da2319 100644 --- a/Firestore/core/src/firebase/firestore/model/field_path.cc +++ b/Firestore/core/src/firebase/firestore/model/field_path.cc @@ -51,7 +51,7 @@ bool IsValidIdentifier(const std::string& segment) { (first < 'A' || first > 'Z')) { return false; } - for (int i = 1; i != segment.size(); ++i) { + for (size_t i = 1; i != segment.size(); ++i) { const unsigned char c = segment[i]; if (c != '_' && (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') && (c < '0' || c > '9')) { @@ -93,7 +93,7 @@ FieldPath FieldPath::FromServerFormat(const absl::string_view path) { // Inside backticks, dots are treated literally. bool inside_backticks = false; - int i = 0; + size_t i = 0; while (i < path.size()) { const char c = path[i]; // std::string (and string_view) may contain embedded nulls. For full diff --git a/Firestore/core/src/firebase/firestore/model/field_path.h b/Firestore/core/src/firebase/firestore/model/field_path.h index 00b658a..ba6e2bd 100644 --- a/Firestore/core/src/firebase/firestore/model/field_path.h +++ b/Firestore/core/src/firebase/firestore/model/field_path.h @@ -82,7 +82,7 @@ class FieldPath : public impl::BasePath<FieldPath> { } private: - FieldPath(SegmentsT&& segments) : BasePath{std::move(segments)} { + explicit FieldPath(SegmentsT&& segments) : BasePath{std::move(segments)} { } // So that methods of base can construct FieldPath using the private diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h index 4cd0b3d..d43b23f 100644 --- a/Firestore/core/src/firebase/firestore/model/field_value.h +++ b/Firestore/core/src/firebase/firestore/model/field_value.h @@ -69,7 +69,7 @@ class FieldValue { // position instead, see the doc comment above. }; - FieldValue() : tag_(Type::Null) { + FieldValue() { } // Do not inline these ctor/dtor below, which contain call to non-trivial @@ -123,7 +123,7 @@ class FieldValue { */ void SwitchTo(const Type type); - Type tag_; + Type tag_ = Type::Null; union { // There is no null type as tag_ alone is enough for Null FieldValue. bool boolean_value_; diff --git a/Firestore/core/src/firebase/firestore/model/resource_path.cc b/Firestore/core/src/firebase/firestore/model/resource_path.cc index 36218e9..a4f921f 100644 --- a/Firestore/core/src/firebase/firestore/model/resource_path.cc +++ b/Firestore/core/src/firebase/firestore/model/resource_path.cc @@ -18,6 +18,7 @@ #include <algorithm> #include <utility> +#include <vector> #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" #include "absl/strings/str_join.h" diff --git a/Firestore/core/src/firebase/firestore/model/resource_path.h b/Firestore/core/src/firebase/firestore/model/resource_path.h index 481d32f..b0853c6 100644 --- a/Firestore/core/src/firebase/firestore/model/resource_path.h +++ b/Firestore/core/src/firebase/firestore/model/resource_path.h @@ -19,6 +19,7 @@ #include <initializer_list> #include <string> +#include <utility> #include "Firestore/core/src/firebase/firestore/model/base_path.h" #include "absl/strings/string_view.h" @@ -69,7 +70,7 @@ class ResourcePath : public impl::BasePath<ResourcePath> { } private: - ResourcePath(SegmentsT&& segments) : BasePath{std::move(segments)} { + explicit ResourcePath(SegmentsT&& segments) : BasePath{std::move(segments)} { } // So that methods of base can construct ResourcePath using the private @@ -81,4 +82,4 @@ class ResourcePath : public impl::BasePath<ResourcePath> { } // namespace firestore } // namespace firebase -#endif +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_RESOURCE_PATH_H_ diff --git a/Firestore/core/test/firebase/firestore/model/field_path_test.cc b/Firestore/core/test/firebase/firestore/model/field_path_test.cc index 7c7e0a3..a5ae3b2 100644 --- a/Firestore/core/test/firebase/firestore/model/field_path_test.cc +++ b/Firestore/core/test/firebase/firestore/model/field_path_test.cc @@ -29,18 +29,18 @@ namespace model { TEST(FieldPath, Constructors) { const FieldPath empty_path; EXPECT_TRUE(empty_path.empty()); - EXPECT_EQ(0, empty_path.size()); + EXPECT_EQ(0u, empty_path.size()); EXPECT_TRUE(empty_path.begin() == empty_path.end()); const FieldPath path_from_list = {"rooms", "Eros", "messages"}; EXPECT_FALSE(path_from_list.empty()); - EXPECT_EQ(3, path_from_list.size()); + EXPECT_EQ(3u, path_from_list.size()); EXPECT_TRUE(path_from_list.begin() + 3 == path_from_list.end()); std::vector<std::string> segments{"rooms", "Eros", "messages"}; const FieldPath path_from_segments{segments.begin(), segments.end()}; EXPECT_FALSE(path_from_segments.empty()); - EXPECT_EQ(3, path_from_segments.size()); + EXPECT_EQ(3u, path_from_segments.size()); EXPECT_TRUE(path_from_segments.begin() + 3 == path_from_segments.end()); FieldPath copied = path_from_list; @@ -168,13 +168,13 @@ TEST(FieldPath, IsPrefixOf) { TEST(FieldPath, AccessFailures) { const FieldPath path; - ASSERT_DEATH_IF_SUPPORTED(path.first_segment(), ""); - ASSERT_DEATH_IF_SUPPORTED(path.last_segment(), ""); - ASSERT_DEATH_IF_SUPPORTED(path[0], ""); - ASSERT_DEATH_IF_SUPPORTED(path[1], ""); - ASSERT_DEATH_IF_SUPPORTED(path.PopFirst(), ""); - ASSERT_DEATH_IF_SUPPORTED(path.PopFirst(2), ""); - ASSERT_DEATH_IF_SUPPORTED(path.PopLast(), ""); + ASSERT_ANY_THROW(path.first_segment()); + ASSERT_ANY_THROW(path.last_segment()); + ASSERT_ANY_THROW(path[0]); + ASSERT_ANY_THROW(path[1]); + ASSERT_ANY_THROW(path.PopFirst()); + ASSERT_ANY_THROW(path.PopFirst(2)); + ASSERT_ANY_THROW(path.PopLast()); } TEST(FieldPath, Parsing) { @@ -201,7 +201,7 @@ TEST(FieldPath, Parsing) { const auto path_with_dot = FieldPath::FromServerFormat(R"(foo\.bar)"); EXPECT_EQ(path_with_dot.CanonicalString(), "`foo.bar`"); - EXPECT_EQ(path_with_dot.size(), 1); + EXPECT_EQ(path_with_dot.size(), 1u); } // This is a special case in C++: std::string may contain embedded nulls. To @@ -213,22 +213,22 @@ TEST(FieldPath, ParseEmbeddedNull) { str += ".bar"; const auto path = FieldPath::FromServerFormat(str); - EXPECT_EQ(path.size(), 1); + EXPECT_EQ(path.size(), 1u); EXPECT_EQ(path.CanonicalString(), "foo"); } TEST(FieldPath, ParseFailures) { - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(""), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("."), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(".."), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo."), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(".bar"), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo..bar"), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(R"(foo\)"), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(R"(foo.\)"), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo`"), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo```"), ""); - ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("`foo"), ""); + ASSERT_ANY_THROW(FieldPath::FromServerFormat("")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat(".")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat("..")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo.")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat(".bar")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo..bar")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat(R"(foo\)")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat(R"(foo.\)")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo`")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo```")); + ASSERT_ANY_THROW(FieldPath::FromServerFormat("`foo")); } TEST(FieldPath, CanonicalStringOfSubstring) { diff --git a/Firestore/core/test/firebase/firestore/model/resource_path_test.cc b/Firestore/core/test/firebase/firestore/model/resource_path_test.cc index 317a1db..378a5e3 100644 --- a/Firestore/core/test/firebase/firestore/model/resource_path_test.cc +++ b/Firestore/core/test/firebase/firestore/model/resource_path_test.cc @@ -29,18 +29,18 @@ namespace model { TEST(ResourcePath, Constructor) { const ResourcePath empty_path; EXPECT_TRUE(empty_path.empty()); - EXPECT_EQ(0, empty_path.size()); + EXPECT_EQ(0u, empty_path.size()); EXPECT_TRUE(empty_path.begin() == empty_path.end()); const ResourcePath path_from_list{{"rooms", "Eros", "messages"}}; EXPECT_FALSE(path_from_list.empty()); - EXPECT_EQ(3, path_from_list.size()); + EXPECT_EQ(3u, path_from_list.size()); EXPECT_TRUE(path_from_list.begin() + 3 == path_from_list.end()); std::vector<std::string> segments{"rooms", "Eros", "messages"}; const ResourcePath path_from_segments{segments.begin(), segments.end()}; EXPECT_FALSE(path_from_segments.empty()); - EXPECT_EQ(3, path_from_segments.size()); + EXPECT_EQ(3u, path_from_segments.size()); EXPECT_TRUE(path_from_segments.begin() + 3 == path_from_segments.end()); ResourcePath copied = path_from_list; @@ -96,8 +96,8 @@ TEST(ResourcePath, Parsing) { } TEST(ResourcePath, ParseFailures) { - ASSERT_DEATH_IF_SUPPORTED(ResourcePath::Parse("//"), ""); - ASSERT_DEATH_IF_SUPPORTED(ResourcePath::Parse("foo//bar"), ""); + ASSERT_ANY_THROW(ResourcePath::Parse("//")); + ASSERT_ANY_THROW(ResourcePath::Parse("foo//bar")); } } // namespace model diff --git a/cmake/FindNanopb.cmake b/cmake/FindNanopb.cmake index fb22aef..43c2886 100644 --- a/cmake/FindNanopb.cmake +++ b/cmake/FindNanopb.cmake @@ -10,7 +10,7 @@ find_path( find_library( NANOPB_LIBRARY NAMES protobuf-nanopb protobuf-nanopbd - HINTS ${BINARY_DIR}/src/nanopb-build + HINTS ${BINARY_DIR}/src/nanopb ) find_package_handle_standard_args( diff --git a/cmake/external/nanopb.cmake b/cmake/external/nanopb.cmake index 5df0cf5..d09c668 100644 --- a/cmake/external/nanopb.cmake +++ b/cmake/external/nanopb.cmake @@ -55,9 +55,14 @@ ExternalProject_Add( # nanopb relies on $PATH for the location of protoc. cmake makes it difficult # to adjust the path, so we'll just patch the build files with the exact # location of protoc. + # + # NB: cmake sometimes runs the patch command multiple times in the same src + # dir, so we need to make sure this is idempotent. (eg 'make && make clean && + # make') PATCH_COMMAND - perl -i -pe s,protoc,${NANOPB_PROTOC_BIN},g - ./CMakeLists.txt ./generator/proto/Makefile + grep ${NANOPB_PROTOC_BIN} ./generator/proto/Makefile + || perl -i -pe s,protoc,${NANOPB_PROTOC_BIN},g + ./CMakeLists.txt ./generator/proto/Makefile UPDATE_COMMAND "" INSTALL_COMMAND "" |