From 4713ddf8dfec0be33c581fb977d32941b0ca7ff8 Mon Sep 17 00:00:00 2001 From: davidair Date: Wed, 30 May 2018 11:53:51 -0400 Subject: Version updates for 5.2.0 (#1355) --- Firestore/Example/Podfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Firestore') diff --git a/Firestore/Example/Podfile b/Firestore/Example/Podfile index 88cf93a..b7f30b8 100644 --- a/Firestore/Example/Podfile +++ b/Firestore/Example/Podfile @@ -10,7 +10,7 @@ target 'Firestore_Example_iOS' do # The next line is the forcing function for the Firebase pod. The Firebase # version's subspecs should depend on the component versions in their # corresponding podspec's. - pod 'Firebase/Core', '5.1.0' + pod 'Firebase/Core', '5.2.0' pod 'FirebaseAuth', :path => '../../' pod 'FirebaseCore', :path => '../../' -- cgit v1.2.3 From 4c84d5f5469ec469732a5656e3b601bd54c2d2a7 Mon Sep 17 00:00:00 2001 From: Gil Date: Tue, 5 Jun 2018 14:31:36 -0700 Subject: Add pathname manipulation utilities (#1376) --- .../Example/Firestore.xcodeproj/project.pbxproj | 4 + .../src/firebase/firestore/util/CMakeLists.txt | 2 + Firestore/core/src/firebase/firestore/util/path.cc | 84 ++++++++ Firestore/core/src/firebase/firestore/util/path.h | 101 +++++++++ .../test/firebase/firestore/util/CMakeLists.txt | 1 + .../core/test/firebase/firestore/util/path_test.cc | 240 +++++++++++++++++++++ 6 files changed, 432 insertions(+) create mode 100644 Firestore/core/src/firebase/firestore/util/path.cc create mode 100644 Firestore/core/src/firebase/firestore/util/path.h create mode 100644 Firestore/core/test/firebase/firestore/util/path_test.cc (limited to 'Firestore') diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index b366cb9..0ce2716 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -131,6 +131,7 @@ 54DA12AE1F315EE100DD57A1 /* resume_token_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA12A41F315EE100DD57A1 /* resume_token_spec_test.json */; }; 54DA12AF1F315EE100DD57A1 /* write_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA12A51F315EE100DD57A1 /* write_spec_test.json */; }; 54EB764D202277B30088B8F3 /* array_sorted_map_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54EB764C202277B30088B8F3 /* array_sorted_map_test.cc */; }; + 5A080105CCBFDB6BF3F3772D /* path_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 403DBF6EFB541DFD01582AA3 /* path_test.cc */; }; 5D405BE298CE4692CB00790A /* Pods_Firestore_Tests_iOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 2B50B3A0DF77100EEE887891 /* Pods_Firestore_Tests_iOS.framework */; }; 6003F58E195388D20070C39A /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6003F58D195388D20070C39A /* Foundation.framework */; }; 6003F590195388D20070C39A /* CoreGraphics.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6003F58F195388D20070C39A /* CoreGraphics.framework */; }; @@ -271,6 +272,7 @@ 3B843E4A1F3930A400548890 /* remote_store_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = remote_store_spec_test.json; sourceTree = ""; }; 3C81DE3772628FE297055662 /* Pods-Firestore_Example_iOS.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Example_iOS.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS.debug.xcconfig"; sourceTree = ""; }; 3F0992A4B83C60841C52E960 /* Pods-Firestore_Example_iOS.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Example_iOS.release.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS.release.xcconfig"; sourceTree = ""; }; + 403DBF6EFB541DFD01582AA3 /* path_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = path_test.cc; sourceTree = ""; }; 444B7AB3F5A2929070CB1363 /* hard_assert_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = hard_assert_test.cc; sourceTree = ""; }; 54131E9620ADE678001DF3FF /* string_format_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = string_format_test.cc; sourceTree = ""; }; 54511E8D209805F8005BD28F /* hashing_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = hashing_test.cc; sourceTree = ""; }; @@ -587,6 +589,7 @@ 54A0353420A3D8CB003E0143 /* iterator_adaptors_test.cc */, 54C2294E1FECABAE007D065B /* log_test.cc */, AB380D03201BC6E400D97691 /* ordered_code_test.cc */, + 403DBF6EFB541DFD01582AA3 /* path_test.cc */, 54740A531FC913E500713A1A /* secure_random_test.cc */, 54A0352C20A3B3D7003E0143 /* status_test.cc */, 54A0352B20A3B3D7003E0143 /* status_test_util.h */, @@ -1665,6 +1668,7 @@ AB6B908620322E6D00CC290A /* maybe_document_test.cc in Sources */, AB6B908820322E8800CC290A /* no_document_test.cc in Sources */, AB380D04201BC6E400D97691 /* ordered_code_test.cc in Sources */, + 5A080105CCBFDB6BF3F3772D /* path_test.cc in Sources */, 549CCA5920A36E1F00BCEB75 /* precondition_test.cc in Sources */, B686F2B22025000D0028D6BE /* resource_path_test.cc in Sources */, 54740A571FC914BA00713A1A /* secure_random_test.cc in Sources */, diff --git a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt index 30589a0..b2c4195 100644 --- a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt @@ -184,6 +184,8 @@ cc_library( iterator_adaptors.h ordered_code.cc ordered_code.h + path.cc + path.h range.h secure_random.h status.cc diff --git a/Firestore/core/src/firebase/firestore/util/path.cc b/Firestore/core/src/firebase/firestore/util/path.cc new file mode 100644 index 0000000..940f12a --- /dev/null +++ b/Firestore/core/src/firebase/firestore/util/path.cc @@ -0,0 +1,84 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Firestore/core/src/firebase/firestore/util/path.h" + +namespace firebase { +namespace firestore { +namespace util { + +absl::string_view Path::Basename(absl::string_view pathname) { + size_t slash = pathname.find_last_of('/'); + + if (slash == absl::string_view::npos) { + // No path separator found => the whole string. + return pathname; + } + + // Otherwise everything after the slash is the basename (even if empty string) + return pathname.substr(slash + 1); +} + +absl::string_view Path::Dirname(absl::string_view pathname) { + size_t last_slash = pathname.find_last_of('/'); + + if (last_slash == absl::string_view::npos) { + // No path separator found => empty string. Conformance with POSIX would + // have us return "." here. + return pathname.substr(0, 0); + } + + // Collapse runs of slashes. + size_t nonslash = pathname.find_last_not_of('/', last_slash); + if (nonslash == absl::string_view::npos) { + // All characters preceding the last path separator are slashes + return pathname.substr(0, 1); + } + + last_slash = nonslash + 1; + + // Otherwise everything up to the slash is the parent directory + return pathname.substr(0, last_slash); +} + +bool Path::IsAbsolute(absl::string_view path) { +#if defined(_WIN32) +#error "Handle drive letters" + +#else + return !path.empty() && path.front() == '/'; +#endif +} + +void Path::JoinAppend(std::string* base, absl::string_view path) { + if (IsAbsolute(path)) { + base->assign(path.data(), path.size()); + + } else { + size_t nonslash = base->find_last_not_of('/'); + if (nonslash != std::string::npos) { + base->resize(nonslash + 1); + base->push_back('/'); + } + + // If path started with a slash we'd treat it as absolute above + base->append(path.data(), path.size()); + } +} + +} // namespace util +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/util/path.h b/Firestore/core/src/firebase/firestore/util/path.h new file mode 100644 index 0000000..3eda40a --- /dev/null +++ b/Firestore/core/src/firebase/firestore/util/path.h @@ -0,0 +1,101 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_PATH_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_PATH_H_ + +#include +#include + +#include "absl/strings/string_view.h" + +namespace firebase { +namespace firestore { +namespace util { + +struct Path { + /** + * Returns the unqualified trailing part of the pathname, e.g. "c" for + * "/a/b/c". + */ + static absl::string_view Basename(absl::string_view pathname); + + /** + * Returns the parent directory name, e.g. "/a/b" for "/a/b/c". + * + * Note: + * * Trailing slashes are treated as a separator between an empty path + * segment and the dirname, so Dirname("/a/b/c/") is "/a/b/c". + * * Runs of more than one slash are treated as a single separator, so + * Dirname("/a/b//c") is "/a/b". + * * Paths are not canonicalized, so Dirname("/a//b//c") is "/a//b". + * * Presently only UNIX style paths are supported (but compilation + * intentionally fails on Windows to prompt implementation there). + */ + static absl::string_view Dirname(absl::string_view pathname); + + /** + * Returns true if the given `pathname` is an absolute path. + */ + static bool IsAbsolute(absl::string_view pathname); + + /** + * Returns the paths separated by path separators. + * + * @param base If base is of type std::string&& the result is moved from this + * value. Otherwise the first argument is copied. + * @param paths The rest of the path segments. + */ + template + static std::string Join(S1&& base, const SA&... paths) { + std::string result{std::forward(base)}; + JoinAppend(&result, paths...); + return result; + } + + /** + * Returns the paths separated by path separators. + */ + static std::string Join() { + return {}; + } + + private: + /** + * Joins the given base path with a suffix. If `path` is relative, appends it + * to the given base path. If `path` is absolute, replaces `base`. + */ + static void JoinAppend(std::string* base, absl::string_view path); + + template + static void JoinAppend(std::string* base, + absl::string_view path, + const S&... rest) { + JoinAppend(base, path); + JoinAppend(base, rest...); + } + + static void JoinAppend(std::string* base) { + // Recursive base case; nothing to do. + (void)base; + } +}; + +} // namespace util +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_PATH_H_ diff --git a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt index 44a3b61..f540d7c 100644 --- a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt @@ -126,6 +126,7 @@ cc_test( hashing_test.cc iterator_adaptors_test.cc ordered_code_test.cc + path_test.cc status_test.cc status_test_util.h statusor_test.cc diff --git a/Firestore/core/test/firebase/firestore/util/path_test.cc b/Firestore/core/test/firebase/firestore/util/path_test.cc new file mode 100644 index 0000000..be8be1e --- /dev/null +++ b/Firestore/core/test/firebase/firestore/util/path_test.cc @@ -0,0 +1,240 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "Firestore/core/src/firebase/firestore/util/path.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace util { + +// There are several potential sources of inspiration for what is correct +// behavior for these functions. +// +// Python: test with +// +// python -c 'import os.path; print(os.path.basename("a/b/c//"))' +// +// POSIX shell: test with +// +// dirname "a/b/c//" +// +// libc++: std::filesystem does not yet ship with Xcode (as of 9.4). Test with a +// new (non-default installed) llvm, e.g. llvm-6.0: +// +// brew install llvm +// llvm=$(brew --prefix)/opt/llvm +// $llvm/bin/clang++ -I$llvm/include -I$llvm/include/c++/v1 -L$llvm/lib \ +// -Wl,-rpath,$llvm/lib test.cc -lc++experimental && ./a.out +// +// test.cc contains something like: +// #include +// #include +// namespace fs = std::experimental::filesystem; +// int main() { +// std::cout << fs::path("/a/b/c//").parent_path() << std::endl; +// } +// +// cppreference: look up example output in functions declared here: +// https://en.cppreference.com/w/cpp/filesystem/path +// +// This implementation mostly follows python's example: +// +// * It's pretty simple to implement +// * POSIX is more complicated than we need +// * std::filesystem is still too experimental (as of 2018-06-05) + +#define EXPECT_BASENAME_EQ(expected, source) \ + do { \ + EXPECT_EQ(std::string{expected}, Path::Basename(source)); \ + } while (0) + +TEST(Path, Basename_NoSeparator) { + // POSIX would require all of these to be ".". + // python and libc++ agree this is "". + EXPECT_BASENAME_EQ("", ""); + EXPECT_BASENAME_EQ("a", "a"); + EXPECT_BASENAME_EQ("foo", "foo"); + EXPECT_BASENAME_EQ(".", "."); + EXPECT_BASENAME_EQ("..", ".."); +} + +TEST(Path, Basename_LeadingSlash) { + EXPECT_BASENAME_EQ("", "/"); + EXPECT_BASENAME_EQ("", "///"); + EXPECT_BASENAME_EQ("a", "/a"); + EXPECT_BASENAME_EQ("a", "//a"); + + EXPECT_BASENAME_EQ(".", "/."); + EXPECT_BASENAME_EQ("..", "/.."); + EXPECT_BASENAME_EQ("..", "//.."); +} + +TEST(Path, Basename_IntermediateSlash) { + EXPECT_BASENAME_EQ("b", "/a/b"); + EXPECT_BASENAME_EQ("b", "/a//b"); + EXPECT_BASENAME_EQ("b", "//a/b"); + EXPECT_BASENAME_EQ("b", "//a//b"); + + EXPECT_BASENAME_EQ("b", "//..//b"); + EXPECT_BASENAME_EQ("b", "//a/./b"); + EXPECT_BASENAME_EQ("b", "//a/.//b"); +} + +TEST(Path, Basename_TrailingSlash) { + // python: "a/b//" => "" + // POSIX: "a/b//" => "b" + // libc++ path::filename(): "a/b//" => "." (cppreference suggests "") + EXPECT_BASENAME_EQ("", "/a/"); + EXPECT_BASENAME_EQ("", "/a///"); + + EXPECT_BASENAME_EQ("", "/a/b/"); + EXPECT_BASENAME_EQ("", "/a/b//"); + EXPECT_BASENAME_EQ("", "/a//b//"); + EXPECT_BASENAME_EQ("", "//a//b//"); +} + +TEST(Path, Basename_RelativePath) { + EXPECT_BASENAME_EQ("b", "a/b"); + EXPECT_BASENAME_EQ("b", "a//b"); + + EXPECT_BASENAME_EQ("b", "..//b"); + EXPECT_BASENAME_EQ("b", "a/./b"); + EXPECT_BASENAME_EQ("b", "a/.//b"); + EXPECT_BASENAME_EQ("b", "a//.//b"); +} + +#define EXPECT_DIRNAME_EQ(expected, source) \ + do { \ + EXPECT_EQ(std::string{expected}, Path::Dirname(source)); \ + } while (0) + +TEST(Path, Dirname_NoSeparator) { + // POSIX would require all of these to be ".". + // python and libc++ agree this is "". + EXPECT_DIRNAME_EQ("", ""); + EXPECT_DIRNAME_EQ("", "a"); + EXPECT_DIRNAME_EQ("", "foo"); + EXPECT_DIRNAME_EQ("", "."); + EXPECT_DIRNAME_EQ("", ".."); +} + +TEST(Path, Dirname_LeadingSlash) { + // POSIX says all "/". + // python starts with "/" but does not strip trailing slashes. + // libc++ path::parent_path() considers all of these be "", though + // cppreference.com indicates this should be "/" in example output so this is + // likely a bug. + EXPECT_DIRNAME_EQ("/", "/"); + EXPECT_DIRNAME_EQ("/", "///"); + EXPECT_DIRNAME_EQ("/", "/a"); + EXPECT_DIRNAME_EQ("/", "//a"); + + EXPECT_DIRNAME_EQ("/", "/."); + EXPECT_DIRNAME_EQ("/", "/.."); + EXPECT_DIRNAME_EQ("/", "//.."); +} + +TEST(Path, Dirname_IntermediateSlash) { + EXPECT_DIRNAME_EQ("/a", "/a/b"); + EXPECT_DIRNAME_EQ("/a", "/a//b"); + EXPECT_DIRNAME_EQ("//a", "//a/b"); + EXPECT_DIRNAME_EQ("//a", "//a//b"); + + EXPECT_DIRNAME_EQ("//..", "//..//b"); + EXPECT_DIRNAME_EQ("//a/.", "//a/./b"); + EXPECT_DIRNAME_EQ("//a/.", "//a/.//b"); +} + +TEST(Path, Dirname_TrailingSlash) { + // POSIX demands stripping trailing slashes before computing dirname, while + // python and libc++ effectively seem to consider the path to contain an empty + // path segment there. + EXPECT_DIRNAME_EQ("/a", "/a/"); + EXPECT_DIRNAME_EQ("/a", "/a///"); + + EXPECT_DIRNAME_EQ("/a/b", "/a/b/"); + EXPECT_DIRNAME_EQ("/a/b", "/a/b//"); + EXPECT_DIRNAME_EQ("/a//b", "/a//b//"); + EXPECT_DIRNAME_EQ("//a//b", "//a//b//"); +} + +TEST(Path, Dirname_RelativePath) { + EXPECT_DIRNAME_EQ("a", "a/b"); + EXPECT_DIRNAME_EQ("a", "a//b"); + + EXPECT_DIRNAME_EQ("..", "..//b"); + EXPECT_DIRNAME_EQ("a/.", "a/./b"); + EXPECT_DIRNAME_EQ("a/.", "a/.//b"); + EXPECT_DIRNAME_EQ("a//.", "a//.//b"); +} + +TEST(Path, IsAbsolute) { + EXPECT_FALSE(Path::IsAbsolute("")); + EXPECT_TRUE(Path::IsAbsolute("/")); + EXPECT_TRUE(Path::IsAbsolute("//")); + EXPECT_TRUE(Path::IsAbsolute("/foo")); + EXPECT_FALSE(Path::IsAbsolute("foo")); + EXPECT_FALSE(Path::IsAbsolute("foo/bar")); +} + +TEST(Path, Join_Absolute) { + EXPECT_EQ("/", Path::Join("/")); + + EXPECT_EQ("/", Path::Join("", "/")); + EXPECT_EQ("/", Path::Join("a", "/")); + EXPECT_EQ("/b", Path::Join("a", "/b")); + + // Alternate root names should be preserved. + EXPECT_EQ("//", Path::Join("a", "//")); + EXPECT_EQ("//b", Path::Join("a", "//b")); + EXPECT_EQ("///b///", Path::Join("a", "///b///")); + + EXPECT_EQ("/", Path::Join("/", "/")); + EXPECT_EQ("/b", Path::Join("/", "/b")); + EXPECT_EQ("//b", Path::Join("//host/a", "//b")); + EXPECT_EQ("//b", Path::Join("//host/a/", "//b")); + + EXPECT_EQ("/", Path::Join("/", "")); + EXPECT_EQ("/a", Path::Join("/", "a")); + EXPECT_EQ("/a/b/c", Path::Join("/", "a", "b", "c")); + EXPECT_EQ("/a/", Path::Join("/", "a/")); + EXPECT_EQ("/.", Path::Join("/", ".")); + EXPECT_EQ("/..", Path::Join("/", "..")); +} + +TEST(Path, Join_Relative) { + EXPECT_EQ("", Path::Join("")); + + EXPECT_EQ("", Path::Join("", "", "", "")); + EXPECT_EQ("a/b/c", Path::Join("a/b", "c")); + EXPECT_EQ("/c/d", Path::Join("a/b", "/c", "d")); + EXPECT_EQ("/c/d", Path::Join("a/b/", "/c", "d")); +} + +TEST(Path, Join_Types) { + EXPECT_EQ("a/b", Path::Join(absl::string_view{"a"}, "b")); + EXPECT_EQ("a/b", Path::Join(std::string{"a"}, "b")); + + std::string a_string{"a"}; + EXPECT_EQ("a/b", Path::Join(a_string, "b")); + EXPECT_EQ("a", a_string); +} + +} // namespace util +} // namespace firestore +} // namespace firebase -- cgit v1.2.3 From 2b0f4627383574b09414117985b4d204312b9cc0 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Tue, 5 Jun 2018 19:29:14 -0400 Subject: Fix undefined behaviour in test. (#1381) PB_LAST_FIELD is required by nanopb, but was missed in the test. Without this, undefined behaviour results. (Either manifesting itself as fine (i.e. if there's zeros following) or segfaults.) --- Firestore/core/test/firebase/firestore/remote/serializer_test.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'Firestore') diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc index a902b5d..f3773b9 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -502,13 +502,14 @@ TEST_F(SerializerTest, EncodesFieldValuesWithRepeatedEntries) { // Copy of the real one (from the nanopb generated document.pb.c), but with // only boolean_value and integer_value. - const pb_field_t google_firestore_v1beta1_Value_fields_Fake[2] = { + const pb_field_t google_firestore_v1beta1_Value_fields_Fake[3] = { PB_FIELD(1, BOOL, SINGULAR, STATIC, FIRST, google_firestore_v1beta1_Value_Fake, boolean_value, boolean_value, 0), PB_FIELD(2, INT64, SINGULAR, STATIC, OTHER, google_firestore_v1beta1_Value_Fake, integer_value, boolean_value, 0), + PB_LAST_FIELD, }; // Craft the bytes. boolean_value has a smaller tag, so it'll get encoded @@ -642,13 +643,14 @@ TEST_F(SerializerTest, BadFieldValueTagWithOtherValidTagsPresent) { // only boolean_value and integer_value. Also modified such that integer_value // now has an invalid tag (instead of 2). const int invalid_tag = 31; - const pb_field_t google_firestore_v1beta1_Value_fields_Fake[2] = { + const pb_field_t google_firestore_v1beta1_Value_fields_Fake[3] = { PB_FIELD(1, BOOL, SINGULAR, STATIC, FIRST, google_firestore_v1beta1_Value_Fake, boolean_value, boolean_value, 0), PB_FIELD(invalid_tag, INT64, SINGULAR, STATIC, OTHER, google_firestore_v1beta1_Value_Fake, integer_value, boolean_value, 0), + PB_LAST_FIELD, }; // Craft the bytes. boolean_value has a smaller tag, so it'll get encoded @@ -824,13 +826,14 @@ TEST_F(SerializerTest, // now has a tag of 31. const int invalid_tag = 31; const pb_field_t - google_firestore_v1beta1_BatchGetDocumentsResponse_fields_Fake[2] = { + google_firestore_v1beta1_BatchGetDocumentsResponse_fields_Fake[3] = { PB_FIELD(2, STRING, SINGULAR, CALLBACK, FIRST, google_firestore_v1beta1_BatchGetDocumentsResponse_Fake, missing, missing, 0), PB_FIELD(invalid_tag, INT64, SINGULAR, STATIC, OTHER, google_firestore_v1beta1_BatchGetDocumentsResponse_Fake, extra_field, missing, 0), + PB_LAST_FIELD, }; const char* missing_value = "projects/p/databases/d/documents/one/two"; -- cgit v1.2.3 From 7dbf5caa5f25551658aee614247aed10012166e2 Mon Sep 17 00:00:00 2001 From: Gil Date: Wed, 6 Jun 2018 12:31:37 -0700 Subject: Fix support scripts on Linux (#1385) * Make it possible to run style.sh on Linux Needs clang-format in a nonstandard location, but can be made to work. * Fix lint.sh on Linux * Fix multiline comment error --- .../core/test/firebase/firestore/util/path_test.cc | 2 +- scripts/lint.sh | 18 ++++++++++-- scripts/style.sh | 34 +++++++++++++++++----- 3 files changed, 43 insertions(+), 11 deletions(-) (limited to 'Firestore') diff --git a/Firestore/core/test/firebase/firestore/util/path_test.cc b/Firestore/core/test/firebase/firestore/util/path_test.cc index be8be1e..a60e839 100644 --- a/Firestore/core/test/firebase/firestore/util/path_test.cc +++ b/Firestore/core/test/firebase/firestore/util/path_test.cc @@ -38,7 +38,7 @@ namespace util { // // brew install llvm // llvm=$(brew --prefix)/opt/llvm -// $llvm/bin/clang++ -I$llvm/include -I$llvm/include/c++/v1 -L$llvm/lib \ +// $llvm/bin/clang++ -I$llvm/include -I$llvm/include/c++/v1 -L$llvm/lib // -Wl,-rpath,$llvm/lib test.cc -lc++experimental && ./a.out // // test.cc contains something like: diff --git a/scripts/lint.sh b/scripts/lint.sh index d474129..3d4f453 100755 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -68,10 +68,21 @@ else command=(git ls-files) fi +# POSIX xargs is required to run commands at least once, but cpplint.py fails +# (with huge help text) if no files are supplied. Apple xargs avoids invocation +# if there are no arguments. Use a temporary file to avoid depending on/testing +# for this feature. +TEMP=$(mktemp -t lint-files-$$.XXXXXXXXXX) +trap "rm '$TEMP'" EXIT + # Straight C++ files get regular cpplint "${command[@]}" "${git_options[@]}" \ -- 'Firestore/core/**/*.'{h,cc} \ - | xargs -0 python scripts/cpplint.py --quiet 2>&1 + > "$TEMP" + +if [[ -s "$TEMP" ]]; then + xargs -0 python scripts/cpplint.py --quiet 2>&1 < "$TEMP" +fi CPP_STATUS=$? # Objective-C++ files get a looser cpplint @@ -79,7 +90,10 @@ CPP_STATUS=$? -- 'Firestore/Source/**/*.'{h,m,mm} \ 'Firestore/Example/Tests/**/*.'{h,m,mm} \ 'Firestore/core/**/*.mm' \ - | xargs -0 python scripts/cpplint.py "${objc_lint_options[@]}" --quiet 2>&1 + > "$TEMP" +if [[ -s "$TEMP" ]]; then + xargs -0 python scripts/cpplint.py "${objc_lint_options[@]}" --quiet 2>&1 < "$TEMP" +fi OBJC_STATUS=$? if [[ $CPP_STATUS != 0 || $OBJC_STATUS != 0 ]]; then diff --git a/scripts/style.sh b/scripts/style.sh index 8eff1e8..f82af77 100755 --- a/scripts/style.sh +++ b/scripts/style.sh @@ -21,18 +21,36 @@ # Commonly # ./scripts/style.sh master -system=$(uname -s) - +# Strip the clang-format version output down to the major version. Examples: +# clang-format version 7.0.0 (tags/google/stable/2018-01-11) +# clang-format version google3-trunk (trunk r333779) version=$(clang-format --version) + +# Remove leading "clang-format version" version="${version/*version /}" +# Remove trailing parenthetical version details +version="${version/ (*)/}" +# Remove everything after the first dot (note this is a glob, not a regex) version="${version/.*/}" -if [[ "$version" != 6 && "$version" != 7 ]]; then - # Allow an older clang-format to accommodate Travis version skew. - echo "Please upgrade to clang-format version 7." - echo "If it's installed via homebrew you can run: brew upgrade clang-format" - exit 1 -fi +case "$version" in + 6 | 7) + # Allow an older clang-format to accommodate Travis version skew. + ;; + google3-trunk) + echo "Please use a publicly released clang-format; a recent LLVM release" + echo "from http://releases.llvm.org/download.html will work." + echo "Prepend to PATH when running style.sh." + exit 1 + ;; + *) + echo "Please upgrade to clang-format version 7." + echo "If it's installed via homebrew you can run: brew upgrade clang-format" + exit 1 + ;; +esac + +system=$(uname -s) if [[ "$system" == "Darwin" ]]; then version=$(swiftformat --version) version="${version/*version /}" -- cgit v1.2.3 From bedeb01fd81dc628802de407b3f11a72dde00c00 Mon Sep 17 00:00:00 2001 From: Gil Date: Wed, 6 Jun 2018 15:56:54 -0700 Subject: Update changelog for 0.12.1 and 0.12.3 (#1387) These releases had no user visible changes --- Firestore/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'Firestore') diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index eac5c13..85223e4 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,10 +1,16 @@ # Unreleased +# v0.12.3 +- [changed] Internal improvements. + # v0.12.2 - [fixed] Fixed an issue where `FirestoreSettings` would accept a concurrent dispatch queue, but this configuration would trigger an assertion failure. Passing a concurrent dispatch queue should now work correctly (#988). +# v0.12.1 +- [changed] Internal improvements. + # v0.12.0 - [changed] Replaced the `DocumentListenOptions` object with a simple boolean. Instead of calling -- cgit v1.2.3 From 83fc5714b216fc867ac02875a3e2c6b6a013afce Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 7 Jun 2018 08:56:37 -0700 Subject: Adding AppCode schema for Fuzz tests (#1388) --- .../xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'Firestore') diff --git a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme index 039273b..051b1a4 100644 --- a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme +++ b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme @@ -5,6 +5,19 @@ + + + + + + Date: Thu, 7 Jun 2018 12:29:16 -0400 Subject: Force refresh token if RPC fails with "Unauthenticated" error (#1373) "Unauthenticated" is presumed to mean that token is expired (which might happen if local clock is wrong) and retried, subject to the usual backoff logic. --- .../Example/Tests/Integration/FSTStreamTests.mm | 73 ++++++++++++++++++++++ .../Example/Tests/Remote/FSTDatastoreTests.mm | 6 ++ Firestore/Source/Remote/FSTDatastore.mm | 46 +++++++------- Firestore/Source/Remote/FSTStream.mm | 17 ++--- .../firebase/firestore/auth/credentials_provider.h | 9 ++- .../firestore/auth/empty_credentials_provider.cc | 9 ++- .../firestore/auth/empty_credentials_provider.h | 3 +- .../auth/firebase_credentials_provider_apple.h | 6 +- .../auth/firebase_credentials_provider_apple.mm | 10 ++- .../auth/empty_credentials_provider_test.cc | 24 ++++--- .../auth/firebase_credentials_provider_test.mm | 62 ++++++++++++------ 11 files changed, 195 insertions(+), 70 deletions(-) (limited to 'Firestore') diff --git a/Firestore/Example/Tests/Integration/FSTStreamTests.mm b/Firestore/Example/Tests/Integration/FSTStreamTests.mm index b944a93..3ad6868 100644 --- a/Firestore/Example/Tests/Integration/FSTStreamTests.mm +++ b/Firestore/Example/Tests/Integration/FSTStreamTests.mm @@ -18,8 +18,11 @@ #import +#import #import +#include + #import "Firestore/Example/Tests/Util/FSTHelpers.h" #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h" #import "Firestore/Source/Remote/FSTDatastore.h" @@ -327,4 +330,74 @@ using firebase::firestore::model::SnapshotVersion; ]]; } +class MockCredentialsProvider : public firebase::firestore::auth::EmptyCredentialsProvider { + public: + MockCredentialsProvider() { + observed_states_ = [NSMutableArray new]; + } + + void GetToken(firebase::firestore::auth::TokenListener completion) override { + [observed_states_ addObject:@"GetToken"]; + EmptyCredentialsProvider::GetToken(std::move(completion)); + } + + void InvalidateToken() override { + [observed_states_ addObject:@"InvalidateToken"]; + EmptyCredentialsProvider::InvalidateToken(); + } + + NSMutableArray *observed_states() const { + return observed_states_; + } + + private: + NSMutableArray *observed_states_; +}; + +- (void)testStreamRefreshesTokenUponExpiration { + MockCredentialsProvider credentials; + FSTDatastore *datastore = [[FSTDatastore alloc] initWithDatabaseInfo:&_databaseInfo + workerDispatchQueue:_workerDispatchQueue + credentials:&credentials]; + FSTWatchStream *watchStream = [datastore createWatchStream]; + + [_delegate awaitNotificationFromBlock:^{ + [watchStream startWithDelegate:_delegate]; + }]; + + // Simulate callback from GRPC with an unauthenticated error -- this should invalidate the token. + NSError *unauthenticatedError = [NSError errorWithDomain:FIRFirestoreErrorDomain + code:FIRFirestoreErrorCodeUnauthenticated + userInfo:nil]; + dispatch_async(_testQueue, ^{ + [watchStream.callbackFilter writesFinishedWithError:unauthenticatedError]; + }); + // Drain the queue. + dispatch_sync(_testQueue, ^{ + }); + + // Try reconnecting. + [_delegate awaitNotificationFromBlock:^{ + [watchStream startWithDelegate:_delegate]; + }]; + // Simulate a different error -- token should not be invalidated this time. + NSError *unavailableError = [NSError errorWithDomain:FIRFirestoreErrorDomain + code:FIRFirestoreErrorCodeUnavailable + userInfo:nil]; + dispatch_async(_testQueue, ^{ + [watchStream.callbackFilter writesFinishedWithError:unavailableError]; + }); + dispatch_sync(_testQueue, ^{ + }); + + [_delegate awaitNotificationFromBlock:^{ + [watchStream startWithDelegate:_delegate]; + }]; + dispatch_sync(_testQueue, ^{ + }); + + NSArray *expected = @[ @"GetToken", @"InvalidateToken", @"GetToken", @"GetToken" ]; + XCTAssertEqualObjects(credentials.observed_states(), expected); +} + @end diff --git a/Firestore/Example/Tests/Remote/FSTDatastoreTests.mm b/Firestore/Example/Tests/Remote/FSTDatastoreTests.mm index 6d6e912..9783e37 100644 --- a/Firestore/Example/Tests/Remote/FSTDatastoreTests.mm +++ b/Firestore/Example/Tests/Remote/FSTDatastoreTests.mm @@ -53,6 +53,12 @@ code:FIRFirestoreErrorCodeUnavailable userInfo:nil]; XCTAssertFalse([FSTDatastore isPermanentWriteError:error]); + + // "unauthenticated" is considered a recoverable error due to expired token. + error = [NSError errorWithDomain:FIRFirestoreErrorDomain + code:FIRFirestoreErrorCodeUnauthenticated + userInfo:nil]; + XCTAssertFalse([FSTDatastore isPermanentWriteError:error]); } @end diff --git a/Firestore/Source/Remote/FSTDatastore.mm b/Firestore/Source/Remote/FSTDatastore.mm index 5f79122..fdbeea3 100644 --- a/Firestore/Source/Remote/FSTDatastore.mm +++ b/Firestore/Source/Remote/FSTDatastore.mm @@ -157,10 +157,9 @@ typedef GRPCProtoCall * (^RPCFactory)(void); case FIRFirestoreErrorCodeResourceExhausted: case FIRFirestoreErrorCodeInternal: case FIRFirestoreErrorCodeUnavailable: + // Unauthenticated means something went wrong with our token and we need to retry with new + // credentials which will happen automatically. case FIRFirestoreErrorCodeUnauthenticated: - // Unauthenticated means something went wrong with our token and we need - // to retry with new credentials which will happen automatically. - // TODO(b/37325376): Give up after second unauthenticated error. return NO; case FIRFirestoreErrorCodeInvalidArgument: case FIRFirestoreErrorCodeNotFound: @@ -174,6 +173,7 @@ typedef GRPCProtoCall * (^RPCFactory)(void); case FIRFirestoreErrorCodeOutOfRange: case FIRFirestoreErrorCodeUnimplemented: case FIRFirestoreErrorCodeDataLoss: + return YES; default: return YES; } @@ -239,6 +239,9 @@ typedef GRPCProtoCall * (^RPCFactory)(void); handler:^(GCFSCommitResponse *response, NSError *_Nullable error) { error = [FSTDatastore firestoreErrorForError:error]; [self.workerDispatchQueue dispatchAsync:^{ + if (error != nil && error.code == FIRFirestoreErrorCodeUnauthenticated) { + _credentials->InvalidateToken(); + } LOG_DEBUG("RPC CommitRequest completed. Error: %s", error); [FSTDatastore logHeadersForRPC:rpc RPCName:@"CommitRequest"]; completion(error); @@ -273,6 +276,9 @@ typedef GRPCProtoCall * (^RPCFactory)(void); [self.workerDispatchQueue dispatchAsync:^{ if (error) { LOG_DEBUG("RPC BatchGetDocuments completed. Error: %s", error); + if (error.code == FIRFirestoreErrorCodeUnauthenticated) { + _credentials->InvalidateToken(); + } [FSTDatastore logHeadersForRPC:rpc RPCName:@"BatchGetDocuments"]; completion(nil, error); return; @@ -310,25 +316,21 @@ typedef GRPCProtoCall * (^RPCFactory)(void); - (void)invokeRPCWithFactory:(GRPCProtoCall * (^)(void))rpcFactory errorHandler:(FSTVoidErrorBlock)errorHandler { - // TODO(mikelehen): We should force a refresh if the previous RPC failed due to an expired token, - // but I'm not sure how to detect that right now. http://b/32762461 - _credentials->GetToken( - /*force_refresh=*/false, [self, rpcFactory, errorHandler](util::StatusOr result) { - [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{ - if (!result.ok()) { - errorHandler(util::MakeNSError(result.status())); - } else { - GRPCProtoCall *rpc = rpcFactory(); - const Token &token = result.ValueOrDie(); - [FSTDatastore - prepareHeadersForRPC:rpc - databaseID:&self.databaseInfo->database_id() - token:(token.user().is_authenticated() ? token.token() - : absl::string_view())]; - [rpc start]; - } - }]; - }); + _credentials->GetToken([self, rpcFactory, errorHandler](util::StatusOr result) { + [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{ + if (!result.ok()) { + errorHandler(util::MakeNSError(result.status())); + } else { + GRPCProtoCall *rpc = rpcFactory(); + const Token &token = result.ValueOrDie(); + [FSTDatastore prepareHeadersForRPC:rpc + databaseID:&self.databaseInfo->database_id() + token:(token.user().is_authenticated() ? token.token() + : absl::string_view())]; + [rpc start]; + } + }]; + }); } - (FSTWatchStream *)createWatchStream { diff --git a/Firestore/Source/Remote/FSTStream.mm b/Firestore/Source/Remote/FSTStream.mm index 3a6c035..2c9c8d9 100644 --- a/Firestore/Source/Remote/FSTStream.mm +++ b/Firestore/Source/Remote/FSTStream.mm @@ -265,12 +265,11 @@ static const NSTimeInterval kIdleTimeout = 60.0; HARD_ASSERT(_delegate == nil, "Delegate must be nil"); _delegate = delegate; - _credentials->GetToken( - /*force_refresh=*/false, [self](util::StatusOr result) { - [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{ - [self resumeStartWithToken:result]; - }]; - }); + _credentials->GetToken([self](util::StatusOr result) { + [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{ + [self resumeStartWithToken:result]; + }]; + }); } /** Add an access token to our RPC, after obtaining one from the credentials provider. */ @@ -283,8 +282,6 @@ static const NSTimeInterval kIdleTimeout = 60.0; } HARD_ASSERT(self.state == FSTStreamStateAuth, "State should still be auth (was %s)", self.state); - // TODO(mikelehen): We should force a refresh if the previous RPC failed due to an expired token, - // but I'm not sure how to detect that right now. http://b/32762461 if (!result.ok()) { // RPC has not been started yet, so just invoke higher-level close handler. [self handleStreamClose:util::MakeNSError(result.status())]; @@ -383,6 +380,10 @@ static const NSTimeInterval kIdleTimeout = 60.0; LOG_DEBUG("%s %s Using maximum backoff delay to prevent overloading the backend.", [self class], (__bridge void *)self); [self.backoff resetToMax]; + } else if (error != nil && error.code == FIRFirestoreErrorCodeUnauthenticated) { + // "unauthenticated" error means the token was rejected. Try force refreshing it in case it just + // expired. + _credentials->InvalidateToken(); } if (finalState != FSTStreamStateError) { diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h index 0a1930a..d6ed39a 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h @@ -46,11 +46,14 @@ class CredentialsProvider { virtual ~CredentialsProvider(); + /** Requests token for the current user. */ + virtual void GetToken(TokenListener completion) = 0; + /** - * Requests token for the current user, optionally forcing a refreshed token - * to be fetched. + * Marks the last retrieved token as invalid, making the next `GetToken` + * request force refresh the token. */ - virtual void GetToken(bool force_refresh, TokenListener completion) = 0; + virtual void InvalidateToken() = 0; /** * Sets the listener to be notified of user changes (sign-in / sign-out). It diff --git a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc index da4198d..77156cc 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc @@ -14,17 +14,13 @@ * limitations under the License. */ -#define UNUSED(x) (void)(x) - #include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h" namespace firebase { namespace firestore { namespace auth { -void EmptyCredentialsProvider::GetToken(bool force_refresh, - TokenListener completion) { - UNUSED(force_refresh); +void EmptyCredentialsProvider::GetToken(TokenListener completion) { if (completion) { // Unauthenticated token will force the GRPC fallback to use default // settings. @@ -39,6 +35,9 @@ void EmptyCredentialsProvider::SetUserChangeListener( } } +void EmptyCredentialsProvider::InvalidateToken() { +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h index 55b3cc6..3ea0cab 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h @@ -26,7 +26,8 @@ namespace auth { /** `EmptyCredentialsProvider` always yields an empty token. */ class EmptyCredentialsProvider : public CredentialsProvider { public: - void GetToken(bool force_refresh, TokenListener completion) override; + void GetToken(TokenListener completion) override; + void InvalidateToken() override; void SetUserChangeListener(UserChangeListener listener) override; }; diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h index 0e1da31..f54b72f 100644 --- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h @@ -65,10 +65,12 @@ class FirebaseCredentialsProvider : public CredentialsProvider { ~FirebaseCredentialsProvider() override; - void GetToken(bool force_refresh, TokenListener completion) override; + void GetToken(TokenListener completion) override; void SetUserChangeListener(UserChangeListener listener) override; + void InvalidateToken() override; + private: /** * Most contents of the FirebaseCredentialProvider are kept in this @@ -95,6 +97,8 @@ class FirebaseCredentialsProvider : public CredentialsProvider { int user_counter = 0; std::mutex mutex; + + bool force_refresh = false; }; /** diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm index 9d5b89e..74858c6 100644 --- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm @@ -78,8 +78,7 @@ FirebaseCredentialsProvider::~FirebaseCredentialsProvider() { } } -void FirebaseCredentialsProvider::GetToken(bool force_refresh, - TokenListener completion) { +void FirebaseCredentialsProvider::GetToken(TokenListener completion) { HARD_ASSERT(auth_listener_handle_, "GetToken cannot be called after listener removed."); @@ -121,8 +120,13 @@ void FirebaseCredentialsProvider::GetToken(bool force_refresh, } }; - [contents_->app getTokenForcingRefresh:force_refresh + [contents_->app getTokenForcingRefresh:contents_->force_refresh withCallback:get_token_callback]; + contents_->force_refresh = false; +} + +void FirebaseCredentialsProvider::InvalidateToken() { + contents_->force_refresh = true; } void FirebaseCredentialsProvider::SetUserChangeListener( diff --git a/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc index 60845e5..a2f5780 100644 --- a/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc @@ -25,15 +25,14 @@ namespace auth { TEST(EmptyCredentialsProvider, GetToken) { EmptyCredentialsProvider credentials_provider; - credentials_provider.GetToken( - /*force_refresh=*/true, [](util::StatusOr result) { - EXPECT_TRUE(result.ok()); - const Token& token = result.ValueOrDie(); - EXPECT_ANY_THROW(token.token()); - const User& user = token.user(); - EXPECT_EQ("", user.uid()); - EXPECT_FALSE(user.is_authenticated()); - }); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_ANY_THROW(token.token()); + const User& user = token.user(); + EXPECT_EQ("", user.uid()); + EXPECT_FALSE(user.is_authenticated()); + }); } TEST(EmptyCredentialsProvider, SetListener) { @@ -46,6 +45,13 @@ TEST(EmptyCredentialsProvider, SetListener) { credentials_provider.SetUserChangeListener(nullptr); } +TEST(EmptyCredentialsProvider, InvalidateToken) { + EmptyCredentialsProvider credentials_provider; + credentials_provider.InvalidateToken(); + credentials_provider.GetToken( + [](util::StatusOr result) { EXPECT_TRUE(result.ok()); }); +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm index 9d358b5..873f1b2 100644 --- a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm +++ b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm @@ -51,30 +51,28 @@ TEST(FirebaseCredentialsProviderTest, GetTokenUnauthenticated) { FIRApp* app = AppWithFakeUid(nil); FirebaseCredentialsProvider credentials_provider(app); - credentials_provider.GetToken( - /*force_refresh=*/true, [](util::StatusOr result) { - EXPECT_TRUE(result.ok()); - const Token& token = result.ValueOrDie(); - EXPECT_ANY_THROW(token.token()); - const User& user = token.user(); - EXPECT_EQ("", user.uid()); - EXPECT_FALSE(user.is_authenticated()); - }); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_ANY_THROW(token.token()); + const User& user = token.user(); + EXPECT_EQ("", user.uid()); + EXPECT_FALSE(user.is_authenticated()); + }); } TEST(FirebaseCredentialsProviderTest, GetToken) { FIRApp* app = AppWithFakeUidAndToken(@"fake uid", @"token for fake uid"); FirebaseCredentialsProvider credentials_provider(app); - credentials_provider.GetToken( - /*force_refresh=*/true, [](util::StatusOr result) { - EXPECT_TRUE(result.ok()); - const Token& token = result.ValueOrDie(); - EXPECT_EQ("token for fake uid", token.token()); - const User& user = token.user(); - EXPECT_EQ("fake uid", user.uid()); - EXPECT_TRUE(user.is_authenticated()); - }); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_EQ("token for fake uid", token.token()); + const User& user = token.user(); + EXPECT_EQ("fake uid", user.uid()); + EXPECT_TRUE(user.is_authenticated()); + }); } TEST(FirebaseCredentialsProviderTest, SetListener) { @@ -89,6 +87,34 @@ TEST(FirebaseCredentialsProviderTest, SetListener) { credentials_provider.SetUserChangeListener(nullptr); } +FIRApp* FakeAppExpectingForceRefreshToken(NSString* _Nullable uid, + NSString* _Nullable token) { + FIRApp* app = testutil::AppForUnitTesting(); + app.getUIDImplementation = ^NSString* { + return uid; + }; + app.getTokenImplementation = + ^(BOOL force_refresh, FIRTokenCallback callback) { + EXPECT_TRUE(force_refresh); + callback(token, nil); + }; + return app; +} + +TEST(FirebaseCredentialsProviderTest, InvalidateToken) { + FIRApp* app = + FakeAppExpectingForceRefreshToken(@"fake uid", @"token for fake uid"); + + FirebaseCredentialsProvider credentials_provider{app}; + credentials_provider.InvalidateToken(); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_EQ("token for fake uid", token.token()); + EXPECT_EQ("fake uid", token.user().uid()); + }); +} + } // namespace auth } // namespace firestore } // namespace firebase -- cgit v1.2.3 From d13a51abace9e1510ae953079f98e7b1390b128b Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 7 Jun 2018 13:57:01 -0700 Subject: Allowing field deletes using merge fields (#1389) --- .../Tests/Integration/API/FIRDatabaseTests.mm | 82 +++++++++++++++++++++- Firestore/Source/API/FSTUserDataConverter.mm | 22 +++++- 2 files changed, 101 insertions(+), 3 deletions(-) (limited to 'Firestore') diff --git a/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm b/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm index f8091c0..e048a40 100644 --- a/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm @@ -162,8 +162,10 @@ NSDictionary *initialData = @{ @"updated" : @NO, }; - NSDictionary *mergeData = - @{@"time" : [FIRFieldValue fieldValueForServerTimestamp]}; + NSDictionary *mergeData = @{ + @"time" : [FIRFieldValue fieldValueForServerTimestamp], + @"nested" : @{@"time" : [FIRFieldValue fieldValueForServerTimestamp]} + }; [self writeDocumentRef:doc data:initialData]; @@ -182,6 +184,7 @@ FIRDocumentSnapshot *document = [self readDocumentForRef:doc]; XCTAssertEqual(document[@"updated"], @NO); XCTAssertTrue([document[@"time"] isKindOfClass:[FIRTimestamp class]]); + XCTAssertTrue([document[@"nested.time"] isKindOfClass:[FIRTimestamp class]]); } - (void)testCanDeleteFieldUsingMerge { @@ -218,6 +221,81 @@ XCTAssertNil(document[@"nested.foo"]); } +- (void)testCanDeleteFieldUsingMergeFields { + FIRDocumentReference *doc = [[self.db collectionWithPath:@"rooms"] documentWithAutoID]; + + NSDictionary *initialData = @{ + @"untouched" : @YES, + @"foo" : @"bar", + @"inner" : @{@"removed" : @YES, @"foo" : @"bar"}, + @"nested" : @{@"untouched" : @YES, @"foo" : @"bar"} + }; + NSDictionary *mergeData = @{ + @"foo" : [FIRFieldValue fieldValueForDelete], + @"inner" : @{@"foo" : [FIRFieldValue fieldValueForDelete]}, + @"nested" : @{ + @"untouched" : [FIRFieldValue fieldValueForDelete], + @"foo" : [FIRFieldValue fieldValueForDelete] + } + }; + NSDictionary *finalData = + @{ @"untouched" : @YES, + @"inner" : @{}, + @"nested" : @{@"untouched" : @YES} }; + + [self writeDocumentRef:doc data:initialData]; + + XCTestExpectation *completed = + [self expectationWithDescription:@"testCanMergeDataWithAnExistingDocumentUsingSet"]; + + [doc setData:mergeData + mergeFields:@[ @"foo", @"inner", @"nested.foo" ] + completion:^(NSError *error) { + XCTAssertNil(error); + [completed fulfill]; + }]; + + [self awaitExpectations]; + + FIRDocumentSnapshot *document = [self readDocumentForRef:doc]; + XCTAssertEqualObjects([document data], finalData); +} + +- (void)testCanSetServerTimestampsUsingMergeFields { + FIRDocumentReference *doc = [[self.db collectionWithPath:@"rooms"] documentWithAutoID]; + + NSDictionary *initialData = @{ + @"untouched" : @YES, + @"foo" : @"bar", + @"nested" : @{@"untouched" : @YES, @"foo" : @"bar"} + }; + NSDictionary *mergeData = @{ + @"foo" : [FIRFieldValue fieldValueForServerTimestamp], + @"inner" : @{@"foo" : [FIRFieldValue fieldValueForServerTimestamp]}, + @"nested" : @{@"foo" : [FIRFieldValue fieldValueForServerTimestamp]} + }; + + [self writeDocumentRef:doc data:initialData]; + + XCTestExpectation *completed = + [self expectationWithDescription:@"testCanMergeDataWithAnExistingDocumentUsingSet"]; + + [doc setData:mergeData + mergeFields:@[ @"foo", @"inner", @"nested.foo" ] + completion:^(NSError *error) { + XCTAssertNil(error); + [completed fulfill]; + }]; + + [self awaitExpectations]; + + FIRDocumentSnapshot *document = [self readDocumentForRef:doc]; + XCTAssertTrue([document exists]); + XCTAssertTrue([document[@"foo"] isKindOfClass:[FIRTimestamp class]]); + XCTAssertTrue([document[@"inner.foo"] isKindOfClass:[FIRTimestamp class]]); + XCTAssertTrue([document[@"nested.foo"] isKindOfClass:[FIRTimestamp class]]); +} + - (void)testMergeReplacesArrays { FIRDocumentReference *doc = [[self.db collectionWithPath:@"rooms"] documentWithAutoID]; diff --git a/Firestore/Source/API/FSTUserDataConverter.mm b/Firestore/Source/API/FSTUserDataConverter.mm index d73a870..d3e339d 100644 --- a/Firestore/Source/API/FSTUserDataConverter.mm +++ b/Firestore/Source/API/FSTUserDataConverter.mm @@ -218,6 +218,9 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { /** Returns true for the non-query parse contexts (Set, MergeSet and Update) */ - (BOOL)isWrite; +/** Returns 'YES' if 'fieldPath' was traversed when creating this context. */ +- (BOOL)containsFieldPath:(const FieldPath &)fieldPath; + - (const FieldPath *)path; - (const std::vector *)fieldMask; @@ -331,6 +334,22 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { } } +- (BOOL)containsFieldPath:(const FieldPath &)fieldPath { + for (const FieldPath &field : *_fieldMask) { + if (fieldPath.IsPrefixOf(field)) { + return YES; + } + } + + for (const FieldTransform &fieldTransform : *_fieldTransforms) { + if (fieldPath.IsPrefixOf(fieldTransform.path())) { + return YES; + } + } + + return NO; +} + - (void)validatePath { // TODO(b/34871131): Remove nil check once we have proper paths for fields within arrays. if (_path == nullptr) { @@ -444,7 +463,8 @@ typedef NS_ENUM(NSInteger, FSTUserDataSource) { @"All elements in mergeFields: must be NSStrings or FIRFieldPaths."); } - if ([updateData valueForPath:path] == nil) { + // Verify that all elements specified in the field mask are part of the parsed context. + if (![context containsFieldPath:path]) { FSTThrowInvalidArgument( @"Field '%s' is specified in your field mask but missing from your input data.", path.CanonicalString().c_str()); -- cgit v1.2.3 From f5bf0a37a7dd40e7538a1aed77af05471b7fe713 Mon Sep 17 00:00:00 2001 From: Mina Farid Date: Mon, 11 Jun 2018 17:23:07 -0400 Subject: Fuzz testing Header Search Paths settings (#1395) * Modified `HEADER_SEARCH_PATHS` in the project file. --- Firestore/Example/Firestore.xcodeproj/project.pbxproj | 12 ++++++++++++ .../xcschemes/Firestore_FuzzTests_iOS.xcscheme | 13 ------------- Firestore/Example/FuzzTests/FSTFuzzTestsPrincipal.mm | 14 ++++++++++++-- 3 files changed, 24 insertions(+), 15 deletions(-) (limited to 'Firestore') diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 0ce2716..11cabc4 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -2093,6 +2093,12 @@ "COCOAPODS=1", "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", ); + HEADER_SEARCH_PATHS = ( + "$(inherited)", + "\"${PODS_ROOT}/../../..\"", + "\"${PODS_ROOT}/../../../Firestore/third_party/abseil-cpp\"", + "\"${PODS_ROOT}/nanopb\"", + ); INFOPLIST_FILE = "FuzzTests/Firestore_FuzzTests_iOS-Info.plist"; OTHER_CFLAGS = ( "$(inherited)", @@ -2123,6 +2129,12 @@ "COCOAPODS=1", "GPB_USE_PROTOBUF_FRAMEWORK_IMPORTS=1", ); + HEADER_SEARCH_PATHS = ( + "$(inherited)", + "\"${PODS_ROOT}/../../..\"", + "\"${PODS_ROOT}/../../../Firestore/third_party/abseil-cpp\"", + "\"${PODS_ROOT}/nanopb\"", + ); INFOPLIST_FILE = "FuzzTests/Firestore_FuzzTests_iOS-Info.plist"; OTHER_CFLAGS = ( "$(inherited)", diff --git a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme index 051b1a4..039273b 100644 --- a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme +++ b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_FuzzTests_iOS.xcscheme @@ -5,19 +5,6 @@ - - - - - - ("RunFuzzTestingMain") // First argument is program name. + const_cast("RunFuzzTestingMain") // First arg is program name. }; char **argv = program_args; int argc = sizeof(program_args) / sizeof(program_args[0]); -- cgit v1.2.3