From 9cd31e8875e8f643f5411be465759bd85d9c187e Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 29 Jan 2018 16:13:46 -0500 Subject: [PATCH 01/18] Implement firestore/auth/user --- Firestore/core/CMakeLists.txt | 2 + .../firebase/firestore/auth/CMakeLists.txt | 23 +++++++ .../core/src/firebase/firestore/auth/user.cc | 34 ++++++++++ .../core/src/firebase/firestore/auth/user.h | 65 +++++++++++++++++++ .../firebase/firestore/auth/CMakeLists.txt | 21 ++++++ .../test/firebase/firestore/auth/user_test.cc | 44 +++++++++++++ 6 files changed, 189 insertions(+) create mode 100644 Firestore/core/src/firebase/firestore/auth/CMakeLists.txt create mode 100644 Firestore/core/src/firebase/firestore/auth/user.cc create mode 100644 Firestore/core/src/firebase/firestore/auth/user.h create mode 100644 Firestore/core/test/firebase/firestore/auth/CMakeLists.txt create mode 100644 Firestore/core/test/firebase/firestore/auth/user_test.cc diff --git a/Firestore/core/CMakeLists.txt b/Firestore/core/CMakeLists.txt index f143a71c1c7..9b3886aedcc 100644 --- a/Firestore/core/CMakeLists.txt +++ b/Firestore/core/CMakeLists.txt @@ -13,12 +13,14 @@ # limitations under the License. add_subdirectory(src/firebase/firestore) +add_subdirectory(src/firebase/firestore/auth) add_subdirectory(src/firebase/firestore/core) add_subdirectory(src/firebase/firestore/model) add_subdirectory(src/firebase/firestore/remote) add_subdirectory(src/firebase/firestore/util) add_subdirectory(test/firebase/firestore) +add_subdirectory(test/firebase/firestore/auth) add_subdirectory(test/firebase/firestore/core) add_subdirectory(test/firebase/firestore/model) add_subdirectory(test/firebase/firestore/remote) diff --git a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt new file mode 100644 index 00000000000..feb7b2c7664 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt @@ -0,0 +1,23 @@ +# 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. + +cc_library( + firebase_firestore_auth + SOURCES + user.cc + user.h + DEPENDS + absl_strings + firebase_firestore_util +) diff --git a/Firestore/core/src/firebase/firestore/auth/user.cc b/Firestore/core/src/firebase/firestore/auth/user.cc new file mode 100644 index 00000000000..eab8e6719c1 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/user.cc @@ -0,0 +1,34 @@ +/* + * 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/auth/user.h" + +#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" + +namespace firebase { +namespace firestore { +namespace model { + +User::User() : is_authenticated_(false) { +} + +User::User(const absl::string_view uid) : uid_(uid), is_authenticated_(true) { + FIREBASE_ASSERT(!uid.empty()); +} + +} // namespace model +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/user.h b/Firestore/core/src/firebase/firestore/auth/user.h new file mode 100644 index 00000000000..1594479cd6c --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/user.h @@ -0,0 +1,65 @@ +/* + * 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_AUTH_USER_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_USER_H_ + +#include + +#include "absl/strings/string_view.h" + +namespace firebase { +namespace firestore { +namespace auth { + +/** + * Simple wrapper around a nullable UID. Mostly exists to make code more readable and for use as + * a key in dictionaries (since keys cannot be nil). + */ +class User { + public: + /** Construct an unauthenticated user. */ + User(); + + /** Construct an authenticated user with the given UID. */ + User(const absl::string_view uid); + + const std::string& uid(); + + // PORTING NOTE: Here use more clear naming is_authenticated() instead of is_unauthenticated(). + bool is_authenticated(); + + friend bool operator==(const User& lhs, const User& rhs); + + private: + const std::string uid_; + const bool is_authenticated_; +}; + +inline bool operator==(const FieldValue& lhs, const FieldValue& rhs) { + return lhs.is_authenticated_ == rhs.is_authenticated_ && + (!lhs.is_authenicated_ || lhs.uid_ == rhs.uid_); +} + +inline bool operator!=(const User& lhs, const User& rhs) { + return !(lhs == rhs); +} + +} // namespace auth +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_USER_H_ diff --git a/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt new file mode 100644 index 00000000000..b832610f52f --- /dev/null +++ b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt @@ -0,0 +1,21 @@ +# 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. + +cc_test( + firebase_firestore_auth_test + SOURCES + user_test.cc + DEPENDS + firebase_firestore_auth +) diff --git a/Firestore/core/test/firebase/firestore/auth/user_test.cc b/Firestore/core/test/firebase/firestore/auth/user_test.cc new file mode 100644 index 00000000000..cddf4517195 --- /dev/null +++ b/Firestore/core/test/firebase/firestore/auth/user_test.cc @@ -0,0 +1,44 @@ +/* + * 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/auth/user.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace auth { + +TEST(User, Getter) { + User anonymous; + EXPECT_EQ("", anonymous.uid()); + EXPECT_FALSE(anonymous.is_authenticated()); + + User signin("abc"); + EXPECT_EQ("abc", signin.uid()); + EXPECT_TRUE(signin.is_authenticated()); +} + +TEST(Timestamp, Comparison) { + EXPECT_EQ(User(), User()); + EXPECT_EQ(User("abc"), User("abc")); + EXPECT_NE(User(), User("abc")); + EXPECT_NE(User("abc"), User("xyz")); +} + +} // namespace auth +} // namespace firestore +} // namespace firebase From f94e4e83e4aa1bd69a84040b7f126563cedff5d1 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 29 Jan 2018 16:47:36 -0500 Subject: [PATCH 02/18] add user to project and some fixes --- .../Example/Firestore.xcodeproj/project.pbxproj | 13 +++++++++++++ Firestore/core/src/firebase/firestore/auth/user.cc | 4 ++-- Firestore/core/src/firebase/firestore/auth/user.h | 12 ++++++++---- .../core/test/firebase/firestore/auth/user_test.cc | 2 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 6a5b7957207..55d7266b63a 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -78,6 +78,7 @@ AB99452C1FE3018D00DFC1E6 /* FIRQuerySnapshotTests.m in Sources */ = {isa = PBXBuildFile; fileRef = AB99452B1FE3018D00DFC1E6 /* FIRQuerySnapshotTests.m */; }; AB99452E1FE30AC800DFC1E6 /* FIRFieldValueTests.m in Sources */ = {isa = PBXBuildFile; fileRef = AB99452D1FE30AC800DFC1E6 /* FIRFieldValueTests.m */; }; ABAEEF4F1FD5F8B100C966CB /* FIRQueryTests.m in Sources */ = {isa = PBXBuildFile; fileRef = ABAEEF4E1FD5F8B100C966CB /* FIRQueryTests.m */; }; + ABE6637F201FCCC200ED349A /* user_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABE6637D201FCC9E00ED349A /* user_test.cc */; }; ABF341051FE860CA00C48322 /* FSTAPIHelpers.m in Sources */ = {isa = PBXBuildFile; fileRef = ABF341021FE8593500C48322 /* FSTAPIHelpers.m */; }; ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; }; AFE6114F0D4DAECBA7B7C089 /* Pods_Firestore_IntegrationTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B2FA635DF5D116A67A7441CD /* Pods_Firestore_IntegrationTests.framework */; }; @@ -264,6 +265,7 @@ AB99452B1FE3018D00DFC1E6 /* FIRQuerySnapshotTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FIRQuerySnapshotTests.m; sourceTree = ""; }; AB99452D1FE30AC800DFC1E6 /* FIRFieldValueTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FIRFieldValueTests.m; sourceTree = ""; }; ABAEEF4E1FD5F8B100C966CB /* FIRQueryTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FIRQueryTests.m; sourceTree = ""; }; + ABE6637D201FCC9E00ED349A /* user_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = user_test.cc; sourceTree = ""; }; ABF341011FE858B500C48322 /* FSTAPIHelpers.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = FSTAPIHelpers.h; sourceTree = ""; }; ABF341021FE8593500C48322 /* FSTAPIHelpers.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FSTAPIHelpers.m; sourceTree = ""; }; ABF6506B201131F8005F2C74 /* timestamp_test.cc */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = timestamp_test.cc; sourceTree = ""; }; @@ -422,6 +424,7 @@ 54764FAC1FAA0C390085E60A /* GoogleTests */ = { isa = PBXGroup; children = ( + ABE6637C201FCC6700ED349A /* auth */, AB380CF7201937B800D97691 /* core */, AB356EF5200E9D1A0089B766 /* model */, 54740A561FC913EB00713A1A /* util */, @@ -567,6 +570,15 @@ path = ../../core/test/firebase/firestore/core; sourceTree = ""; }; + ABE6637C201FCC6700ED349A /* auth */ = { + isa = PBXGroup; + children = ( + ABE6637D201FCC9E00ED349A /* user_test.cc */, + ); + name = auth; + path = ../../core/test/firebase/firestore/auth; + sourceTree = ""; + }; DE0761E51F2FE611003233AF /* SwiftBuildTest */ = { isa = PBXGroup; children = ( @@ -1291,6 +1303,7 @@ DE51B1E41F0D490D0013853F /* FSTQueryCacheTests.m in Sources */, AB380D02201BC69F00D97691 /* bits_test.cc in Sources */, DE51B1CD1F0D48CD0013853F /* FSTDatabaseInfoTests.m in Sources */, + ABE6637F201FCCC200ED349A /* user_test.cc in Sources */, AB382F7E1FE03059007CA955 /* FIRFieldPathTests.m in Sources */, 548DB929200D59F600E00ABC /* comparison_test.cc in Sources */, DE51B1F21F0D49140013853F /* FSTPathTests.m in Sources */, diff --git a/Firestore/core/src/firebase/firestore/auth/user.cc b/Firestore/core/src/firebase/firestore/auth/user.cc index eab8e6719c1..899a1896af0 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.cc +++ b/Firestore/core/src/firebase/firestore/auth/user.cc @@ -20,7 +20,7 @@ namespace firebase { namespace firestore { -namespace model { +namespace auth { User::User() : is_authenticated_(false) { } @@ -29,6 +29,6 @@ User::User(const absl::string_view uid) : uid_(uid), is_authenticated_(true) { FIREBASE_ASSERT(!uid.empty()); } -} // namespace model +} // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/user.h b/Firestore/core/src/firebase/firestore/auth/user.h index 1594479cd6c..7ead0e0f2ca 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.h +++ b/Firestore/core/src/firebase/firestore/auth/user.h @@ -37,10 +37,14 @@ class User { /** Construct an authenticated user with the given UID. */ User(const absl::string_view uid); - const std::string& uid(); + const std::string& uid() { + return uid_; + } // PORTING NOTE: Here use more clear naming is_authenticated() instead of is_unauthenticated(). - bool is_authenticated(); + bool is_authenticated() { + return is_authenticated_; + } friend bool operator==(const User& lhs, const User& rhs); @@ -49,9 +53,9 @@ class User { const bool is_authenticated_; }; -inline bool operator==(const FieldValue& lhs, const FieldValue& rhs) { +inline bool operator==(const User& lhs, const User& rhs) { return lhs.is_authenticated_ == rhs.is_authenticated_ && - (!lhs.is_authenicated_ || lhs.uid_ == rhs.uid_); + (!lhs.is_authenticated_ || lhs.uid_ == rhs.uid_); } inline bool operator!=(const User& lhs, const User& rhs) { diff --git a/Firestore/core/test/firebase/firestore/auth/user_test.cc b/Firestore/core/test/firebase/firestore/auth/user_test.cc index cddf4517195..d8ad729d4f0 100644 --- a/Firestore/core/test/firebase/firestore/auth/user_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/user_test.cc @@ -32,7 +32,7 @@ TEST(User, Getter) { EXPECT_TRUE(signin.is_authenticated()); } -TEST(Timestamp, Comparison) { +TEST(User, Comparison) { EXPECT_EQ(User(), User()); EXPECT_EQ(User("abc"), User("abc")); EXPECT_NE(User(), User("abc")); From ad6f9814a199cfdd2ce21a2a6d8e14fb82978ba4 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 30 Jan 2018 15:42:43 -0500 Subject: [PATCH 03/18] implement firestore/auth/{credentials_provider,empty_credentials_provider} --- .../Firestore.xcodeproj/project.pbxproj | 8 ++ .../firebase/firestore/auth/CMakeLists.txt | 28 ++++- .../firestore/auth/credentials_provider.cc | 29 +++++ .../firestore/auth/credentials_provider.h | 105 ++++++++++++++++++ .../auth/empty_credentials_provider.cc | 41 +++++++ .../auth/empty_credentials_provider.h | 37 ++++++ .../core/src/firebase/firestore/auth/user.h | 13 ++- .../firebase/firestore/auth/CMakeLists.txt | 2 + .../auth/credentials_provider_test.cc | 47 ++++++++ .../auth/empty_credentials_provider_test.cc | 47 ++++++++ 10 files changed, 350 insertions(+), 7 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/auth/credentials_provider.cc create mode 100644 Firestore/core/src/firebase/firestore/auth/credentials_provider.h create mode 100644 Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc create mode 100644 Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h create mode 100644 Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc create mode 100644 Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 55d7266b63a..ab314a217ea 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -71,6 +71,8 @@ AB380D04201BC6E400D97691 /* ordered_code_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380D03201BC6E400D97691 /* ordered_code_test.cc */; }; AB382F7C1FE02A1F007CA955 /* FIRDocumentReferenceTests.m in Sources */ = {isa = PBXBuildFile; fileRef = AB382F7B1FE02A1F007CA955 /* FIRDocumentReferenceTests.m */; }; AB382F7E1FE03059007CA955 /* FIRFieldPathTests.m in Sources */ = {isa = PBXBuildFile; fileRef = AB382F7D1FE03059007CA955 /* FIRFieldPathTests.m */; }; + AB414A4A20210E7D0006939E /* empty_credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB414A4820210E6F0006939E /* empty_credentials_provider_test.cc */; }; + AB414A4D20210E970006939E /* credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB414A4B20210E8B0006939E /* credentials_provider_test.cc */; }; AB7BAB342012B519001E0872 /* geo_point_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB7BAB332012B519001E0872 /* geo_point_test.cc */; }; AB9945261FE2D71100DFC1E6 /* FIRCollectionReferenceTests.m in Sources */ = {isa = PBXBuildFile; fileRef = AB9945251FE2D71100DFC1E6 /* FIRCollectionReferenceTests.m */; }; AB9945281FE2DE0C00DFC1E6 /* FIRSnapshotMetadataTests.m in Sources */ = {isa = PBXBuildFile; fileRef = AB9945271FE2DE0C00DFC1E6 /* FIRSnapshotMetadataTests.m */; }; @@ -258,6 +260,8 @@ AB380D03201BC6E400D97691 /* ordered_code_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ordered_code_test.cc; path = ../../core/test/firebase/firestore/util/ordered_code_test.cc; sourceTree = ""; }; AB382F7B1FE02A1F007CA955 /* FIRDocumentReferenceTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FIRDocumentReferenceTests.m; sourceTree = ""; }; AB382F7D1FE03059007CA955 /* FIRFieldPathTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FIRFieldPathTests.m; sourceTree = ""; }; + AB414A4820210E6F0006939E /* empty_credentials_provider_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = empty_credentials_provider_test.cc; sourceTree = ""; }; + AB414A4B20210E8B0006939E /* credentials_provider_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = credentials_provider_test.cc; sourceTree = ""; }; AB7BAB332012B519001E0872 /* geo_point_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = geo_point_test.cc; path = ../../core/test/firebase/firestore/geo_point_test.cc; sourceTree = ""; }; AB9945251FE2D71100DFC1E6 /* FIRCollectionReferenceTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FIRCollectionReferenceTests.m; sourceTree = ""; }; AB9945271FE2DE0C00DFC1E6 /* FIRSnapshotMetadataTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = FIRSnapshotMetadataTests.m; sourceTree = ""; }; @@ -574,6 +578,8 @@ isa = PBXGroup; children = ( ABE6637D201FCC9E00ED349A /* user_test.cc */, + AB414A4820210E6F0006939E /* empty_credentials_provider_test.cc */, + AB414A4B20210E8B0006939E /* credentials_provider_test.cc */, ); name = auth; path = ../../core/test/firebase/firestore/auth; @@ -1239,10 +1245,12 @@ DE51B1CC1F0D48C00013853F /* FIRGeoPointTests.m in Sources */, DE51B1E11F0D490D0013853F /* FSTMemoryRemoteDocumentCacheTests.m in Sources */, DE51B1FF1F0D493A0013853F /* FSTAssertTests.m in Sources */, + AB414A4A20210E7D0006939E /* empty_credentials_provider_test.cc in Sources */, DE51B1D31F0D48CD0013853F /* FSTViewSnapshotTest.m in Sources */, AB380CFE201A2F4500D97691 /* string_util_test.cc in Sources */, DE51B1F91F0D491F0013853F /* FSTWatchChangeTests.m in Sources */, DE51B1F81F0D491F0013853F /* FSTWatchChange+Testing.m in Sources */, + AB414A4D20210E970006939E /* credentials_provider_test.cc in Sources */, DE51B1EB1F0D490D0013853F /* FSTWriteGroupTests.mm in Sources */, 54C2294F1FECABAE007D065B /* log_test.cc in Sources */, DE51B2011F0D493E0013853F /* FSTHelpers.m in Sources */, diff --git a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt index feb7b2c7664..796a11e0404 100644 --- a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt @@ -13,11 +13,37 @@ # limitations under the License. cc_library( - firebase_firestore_auth + firebase_firestore_auth_base SOURCES + credentials_provider.cc + credentials_provider.h user.cc user.h DEPENDS absl_strings firebase_firestore_util ) + +#cc_library( +# firebase_firestore_auth_apple +# SOURCES +# firebase_credentials_provider.h +# firebase_credentials_provider.mm +# DEPENDS +# firebase_firestore_auth_base +# EXCLUDE_FROM_ALL +#) + +#if(APPLE) +# list(APPEND AUTH_DEPENDS firebase_firestore_auth_apple) +#endif() + +cc_library( + firebase_firestore_auth + SOURCES + empty_credentials_provider.cc + empty_credentials_provider.h + DEPENDS + ${AUTH_DEPENDS} + firebase_firestore_auth_base +) diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc new file mode 100644 index 00000000000..be0055398d2 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc @@ -0,0 +1,29 @@ +/* + * 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/auth/credentials_provider.h" + +namespace firebase { +namespace firestore { +namespace auth { + +Token::Token(const absl::string_view token, const User& user) + : token_(token), user_(user) { +} + +} // namespace auth +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h new file mode 100644 index 00000000000..fc194e3c7d9 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h @@ -0,0 +1,105 @@ +/* + * 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_AUTH_CREDENTIALS_PROVIDER_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ + +#include + +#include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "absl/strings/string_view.h" + +namespace firebase { +namespace firestore { +namespace auth { + +/** + * The current FSTUser and the authentication token provided by the underlying + * authentication mechanism. This is the result of calling + * -[FSTCredentialsProvider getTokenForcingRefresh]. + * + * ## Portability notes: no TokenType on iOS + * + * The TypeScript client supports 1st party Oauth tokens (for the Firebase + * Console to auth as the developer) and OAuth2 tokens for the node.js sdk to + * auth with a service account. We don't have plans to support either case on + * mobile so there's no TokenType here. + */ +// TODO(zxu123): Make this support token-type for desktop workflow. +class Token { + public: + Token(const absl::string_view token, const User& user); + + /** The actual raw token. */ + const std::string& token() const { + return token_; + } + + /** The user with which the token is associated (used for persisting user + * state on disk, etc.). */ + const User& user() const { + return user_; + } + + private: + const std::string token_; + const User user_; +}; + +// `TokenErrorListener` is a listener that gets a token or an error. +// token: An auth token as a string, or nullptr if error occurred. +// error: The error if one occurred, or else nullptr. +typedef void (*TokenListener)(const Token& token, + const absl::string_view error); + +// Listener notified with a User. +typedef void (*UserListener)(const User& user); + +/** Provides methods for getting the uid and token for the current user and + * listen for changes. */ +class CredentialsProvider { + public: + /** Requests token for the current user, optionally forcing a refreshed token + * to be fetched. */ + virtual void GetToken(bool force_refresh, TokenListener completion) = 0; + + /** + * Sets the listener to be notified of user changes (sign-in / sign-out). It + * is immediately called once with the initial user. + */ + virtual void set_user_change_listener(UserListener listener) = 0; + + /** Removes the listener set with {@link #setUserChangeListener}. */ + virtual void RemoveUserChangeListener() { + set_user_change_listener(nullptr); + } + + protected: + /** + * A listener to be notified of user changes (sign-in / sign-out). It is + * immediately called once with the initial user. + * + * Note that this block will be called back on an arbitrary thread that is not + * the normal Firestore worker thread. + */ + UserListener user_change_listener_; +}; + +} // namespace auth +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ diff --git a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc new file mode 100644 index 00000000000..3affb16e70e --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc @@ -0,0 +1,41 @@ +/* + * 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. + */ + +#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); + if (completion) { + completion({"", User()}, ""); + } +} + +void EmptyCredentialsProvider::set_user_change_listener(UserListener listener) { + if (listener) { + listener({}); + } +} + +} // 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 new file mode 100644 index 00000000000..88bdb4091d5 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h @@ -0,0 +1,37 @@ +/* + * 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_AUTH_EMPTY_CREDENTIALS_PROVIDER_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_EMPTY_CREDENTIALS_PROVIDER_H_ + +#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" + +namespace firebase { +namespace firestore { +namespace auth { + +/** `EmptyCredentialsProvider` always yields an empty token. */ +class EmptyCredentialsProvider : CredentialsProvider { + public: + void GetToken(bool force_refresh, TokenListener completion) override; + void set_user_change_listener(UserListener listener) override; +}; + +} // namespace auth +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_EMPTY_CREDENTIALS_PROVIDER_H_ diff --git a/Firestore/core/src/firebase/firestore/auth/user.h b/Firestore/core/src/firebase/firestore/auth/user.h index 7ead0e0f2ca..126f31be196 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.h +++ b/Firestore/core/src/firebase/firestore/auth/user.h @@ -26,8 +26,8 @@ namespace firestore { namespace auth { /** - * Simple wrapper around a nullable UID. Mostly exists to make code more readable and for use as - * a key in dictionaries (since keys cannot be nil). + * Simple wrapper around a nullable UID. Mostly exists to make code more + * readable and for use as a key in dictionaries (since keys cannot be nil). */ class User { public: @@ -37,12 +37,13 @@ class User { /** Construct an authenticated user with the given UID. */ User(const absl::string_view uid); - const std::string& uid() { + const std::string& uid() const { return uid_; } - // PORTING NOTE: Here use more clear naming is_authenticated() instead of is_unauthenticated(). - bool is_authenticated() { + // PORTING NOTE: Here use more clear naming is_authenticated() instead of + // is_unauthenticated(). + bool is_authenticated() const { return is_authenticated_; } @@ -55,7 +56,7 @@ class User { inline bool operator==(const User& lhs, const User& rhs) { return lhs.is_authenticated_ == rhs.is_authenticated_ && - (!lhs.is_authenticated_ || lhs.uid_ == rhs.uid_); + (!lhs.is_authenticated_ || lhs.uid_ == rhs.uid_); } inline bool operator!=(const User& lhs, const User& rhs) { diff --git a/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt index b832610f52f..a77c4b533a5 100644 --- a/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt @@ -15,6 +15,8 @@ cc_test( firebase_firestore_auth_test SOURCES + credentials_provider_test.cc + empty_credentials_provider_test.cc user_test.cc DEPENDS firebase_firestore_auth diff --git a/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc new file mode 100644 index 00000000000..5b8ea656ef0 --- /dev/null +++ b/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc @@ -0,0 +1,47 @@ +/* + * 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/auth/credentials_provider.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace auth { + +TEST(Token, Getter) { + Token token("token", User("abc")); + EXPECT_EQ("token", token.token()); + EXPECT_EQ(User("abc"), token.user()); +} + +#define UNUSED(x) (void)(x) + +TEST(CredentialsProvider, Typedef) { + TokenListener token_listener = [](const Token& token, + const absl::string_view error) { + UNUSED(token); + UNUSED(error); + }; + EXPECT_NE(nullptr, token_listener); + + UserListener user_listener = [](const User& user) { UNUSED(user); }; + EXPECT_NE(nullptr, user_listener); +} + +} // namespace auth +} // namespace firestore +} // namespace firebase 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 new file mode 100644 index 00000000000..38b5bc07d5f --- /dev/null +++ b/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc @@ -0,0 +1,47 @@ +/* + * 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/auth/empty_credentials_provider.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace auth { + +TEST(EmptyCredentialsProvider, GetToken) { + EmptyCredentialsProvider credentials_provider; + credentials_provider.GetToken( + true, [](const Token& token, const absl::string_view error) { + EXPECT_EQ("", token.token()); + const User& user = token.user(); + EXPECT_EQ("", user.uid()); + EXPECT_FALSE(user.is_authenticated()); + EXPECT_EQ("", error); + }); +} + +TEST(EmptyCredentialsProvider, SetListener) { + EmptyCredentialsProvider credentials_provider; + credentials_provider.set_user_change_listener([](const User& user) { + EXPECT_EQ("", user.uid()); + EXPECT_FALSE(user.is_authenticated()); + }); +} + +} // namespace auth +} // namespace firestore +} // namespace firebase From 31e0a2e29b6433fdd464d628acd8cfa63bd2ec6f Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 30 Jan 2018 16:42:25 -0500 Subject: [PATCH 04/18] implement firestore/auth/firebase_credentials_provider --- .../firebase/firestore/auth/CMakeLists.txt | 25 ++-- .../auth/firebase_credentials_provider.h | 90 +++++++++++++ .../firebase_credentials_provider_apple.mm | 121 ++++++++++++++++++ .../core/src/firebase/firestore/auth/user.h | 6 +- .../firebase_credentials_provider_test.cc | 33 +++++ .../test/firebase/firestore/auth/user_test.cc | 4 + 6 files changed, 265 insertions(+), 14 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h create mode 100644 Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm create mode 100644 Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc diff --git a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt index 796a11e0404..535ec3f61fb 100644 --- a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt @@ -24,19 +24,20 @@ cc_library( firebase_firestore_util ) -#cc_library( -# firebase_firestore_auth_apple -# SOURCES -# firebase_credentials_provider.h -# firebase_credentials_provider.mm -# DEPENDS -# firebase_firestore_auth_base -# EXCLUDE_FROM_ALL -#) +cc_library( + firebase_firestore_auth_apple + SOURCES + firebase_credentials_provider.h + firebase_credentials_provider_apple.mm + DEPENDS + FirebaseCore + firebase_firestore_auth_base + EXCLUDE_FROM_ALL +) -#if(APPLE) -# list(APPEND AUTH_DEPENDS firebase_firestore_auth_apple) -#endif() +if(APPLE) + list(APPEND AUTH_DEPENDS firebase_firestore_auth_apple) +endif() cc_library( firebase_firestore_auth diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h new file mode 100644 index 00000000000..e7624009120 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h @@ -0,0 +1,90 @@ +/* + * 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. + */ + +// Right now, FirebaseCredentialsProvider only support APPLE build. +// TODO(zxu123): Make it for desktop workflow as well. + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ + +#import + +#include +#include + +#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" +#include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "absl/strings/string_view.h" + +@class FIRApp; + +namespace firebase { +namespace firestore { +namespace auth { + +/** + * `FirebaseCredentialsProvider` uses Firebase Auth via `FIRApp` to get an auth + * token. + * + * NOTE: To simplify the implementation, it requires that you set + * `userChangeListener` with a non-`nil` value no more than once and don't call + * `getTokenForcingRefresh:` after setting it to `nil`. + * + * This class must be implemented in a thread-safe manner since it is accessed + * from the thread backing our internal worker queue and the callbacks from + * FIRAuth will be executed on an arbitrary different thread. + * + * For non-Apple desktop build, this is right now just a stub. + */ +class FirebaseCredentialsProvider : CredentialsProvider { + public: + // TODO(zxu123): Provide a ctor to accept the C++ Firebase Games App, which + // deals all platforms. + /** + * Initializes a new FirebaseCredentialsProvider. + * + * @param app The Firebase app from which to get credentials. + */ + FirebaseCredentialsProvider(FIRApp* app); + + void GetToken(bool force_refresh, TokenListener completion) override; + + void set_user_change_listener(UserListener listener) override; + + private: + const FIRApp* app_; + + /** Handle used to stop receiving auth changes once userChangeListener is + * removed. */ + id auth_listener_handle_; + + /** The current user as reported to us via our AuthStateDidChangeListener. */ + User current_user_; + + /** + * Counter used to detect if the user changed while a -getTokenForcingRefresh: + * request was outstanding. + */ + int user_counter_; + + std::mutex mutex_; +}; + +} // namespace auth +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ 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 new file mode 100644 index 00000000000..1835de752b7 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm @@ -0,0 +1,121 @@ +/* + * 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/auth/firebase_credentials_provider.h" + +#import +#import + +#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "Firestore/core/src/firebase/firestore/util/string_apple.h" + +#define UNUSED(x) (void)(x) + +namespace firebase { +namespace firestore { +namespace auth { + +FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app) + : app_(app), + current_user_(firebase::firestore::util::MakeStringView([app getUID])), + user_counter_(0) { + auth_listener_handle_ = [[NSNotificationCenter defaultCenter] + addObserverForName:FIRAuthStateDidChangeInternalNotification + object:nil + queue:nil + usingBlock:^(NSNotification* notification) { + std::unique_lock lock(this->mutex_); + NSDictionary* user_info = notification.userInfo; + + // ensure we're only notifiying for the current app. + FIRApp* notified_app = + user_info[FIRAuthStateDidChangeInternalNotificationAppKey]; + if (![this->app_ isEqual:notified_app]) { + return; + } + + NSString* user_id = + user_info[FIRAuthStateDidChangeInternalNotificationUIDKey]; + User new_user( + firebase::firestore::util::MakeStringView(user_id)); + if (new_user != this->current_user_) { + this->current_user_ = new_user; + this->user_counter_++; + UserListener listener = this->user_change_listener_; + if (listener) { + listener(this->current_user_); + } + } + }]; +} + +void FirebaseCredentialsProvider::GetToken(bool force_refresh, + TokenListener completion) { + FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( + this->auth_listener_handle_, this->auth_listener_handle_, + "GetToken cannot be called after listener removed."); + + // Take note of the current value of the userCounter so that this method can + // fail if there is a user change while the request is outstanding. + int initial_user_counter = this->user_counter_; + + void (^get_token_callback)(NSString*, NSError*) = + ^(NSString* _Nullable token, NSError* _Nullable error) { + std::unique_lock lock(this->mutex_); + if (initial_user_counter != this->user_counter_) { + // Cancel the request since the user changed while the request was + // outstanding so the response is likely for a previous user (which + // user, we can't be sure). + completion({"", User()}, "getToken aborted due to user change."); + } else { + completion({firebase::firestore::util::MakeStringView(token), + this->current_user_}, + error == nil ? "" + : firebase::firestore::util::MakeStringView( + (NSString*)error)); + } + }; + + [this->app_ getTokenForcingRefresh:force_refresh + withCallback:get_token_callback]; +} + +void FirebaseCredentialsProvider::set_user_change_listener( + UserListener listener) { + std::unique_lock lock(this->mutex_); + if (listener) { + FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION(!this->user_change_listener_, + !this->user_change_listener_, + "set user_change_listener twice!"); + // Fire initial event. + listener(this->current_user_); + } else { + FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( + this->auth_listener_handle_, this->auth_listener_handle_, + "removed user_change_listener twice!"); + FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( + this->user_change_listener_, this->user_change_listener_, + "user_change_listener removed without being set!"); + [[NSNotificationCenter defaultCenter] + removeObserver:this->auth_listener_handle_]; + this->auth_listener_handle_ = nullptr; + } + this->user_change_listener_ = listener; +} + +} // namespace auth +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/user.h b/Firestore/core/src/firebase/firestore/auth/user.h index 126f31be196..cbda7cbf284 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.h +++ b/Firestore/core/src/firebase/firestore/auth/user.h @@ -47,11 +47,13 @@ class User { return is_authenticated_; } + User& operator=(const User& other) = default; + friend bool operator==(const User& lhs, const User& rhs); private: - const std::string uid_; - const bool is_authenticated_; + std::string uid_; + bool is_authenticated_; }; inline bool operator==(const User& lhs, const User& rhs) { diff --git a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc new file mode 100644 index 00000000000..bfbc5f6df3d --- /dev/null +++ b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc @@ -0,0 +1,33 @@ +/* + * 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/auth/credentials_provider.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace auth { + +TEST(Token, Getter) { + Token token("token", User("abc")); + EXPECT_EQ("token", token.token()); + EXPECT_EQ(User("abc"), token.user()); +} + +} // namespace auth +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/test/firebase/firestore/auth/user_test.cc b/Firestore/core/test/firebase/firestore/auth/user_test.cc index d8ad729d4f0..da26d8df64c 100644 --- a/Firestore/core/test/firebase/firestore/auth/user_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/user_test.cc @@ -30,6 +30,10 @@ TEST(User, Getter) { User signin("abc"); EXPECT_EQ("abc", signin.uid()); EXPECT_TRUE(signin.is_authenticated()); + + User copy; + copy = signin; + EXPECT_EQ(signin, copy); } TEST(User, Comparison) { From 6f203560975663b306839a098b82fe91732fc66c Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 31 Jan 2018 12:13:30 -0500 Subject: [PATCH 05/18] refactoring firebase_credentials_provider and add (disabled but working) unit test --- .../firebase/firestore/auth/CMakeLists.txt | 1 + .../firestore/auth/credentials_provider.cc | 3 + .../firestore/auth/credentials_provider.h | 2 + .../auth/empty_credentials_provider.h | 2 +- .../auth/firebase_credentials_provider.h | 30 ++++++---- .../firebase_credentials_provider_apple.h | 56 +++++++++++++++++++ .../firebase_credentials_provider_apple.mm | 49 ++++++++++++---- .../firebase/firestore/auth/CMakeLists.txt | 10 ++++ .../auth/empty_credentials_provider_test.cc | 1 + .../firebase_credentials_provider_test.cc | 44 +++++++++++++-- 10 files changed, 168 insertions(+), 30 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h diff --git a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt index 535ec3f61fb..612ed4beb66 100644 --- a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt @@ -28,6 +28,7 @@ cc_library( firebase_firestore_auth_apple SOURCES firebase_credentials_provider.h + firebase_credentials_provider_apple.h firebase_credentials_provider_apple.mm DEPENDS FirebaseCore diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc index be0055398d2..a6a2bc11f6c 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc @@ -24,6 +24,9 @@ Token::Token(const absl::string_view token, const User& user) : token_(token), user_(user) { } +CredentialsProvider::CredentialsProvider() : user_change_listener_(nullptr) { +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h index fc194e3c7d9..fe3961811b3 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h @@ -72,6 +72,8 @@ typedef void (*UserListener)(const User& user); * listen for changes. */ class CredentialsProvider { public: + CredentialsProvider(); + /** Requests token for the current user, optionally forcing a refreshed token * to be fetched. */ virtual void GetToken(bool force_refresh, TokenListener completion) = 0; 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 88bdb4091d5..501b4dacae7 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h @@ -24,7 +24,7 @@ namespace firestore { namespace auth { /** `EmptyCredentialsProvider` always yields an empty token. */ -class EmptyCredentialsProvider : CredentialsProvider { +class EmptyCredentialsProvider : public CredentialsProvider { public: void GetToken(bool force_refresh, TokenListener completion) override; void set_user_change_listener(UserListener listener) override; diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h index e7624009120..9ad79d35aa7 100644 --- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h @@ -20,21 +20,20 @@ #ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ -#import - +#include #include -#include #include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" #include "absl/strings/string_view.h" -@class FIRApp; - namespace firebase { namespace firestore { namespace auth { +class AppImpl; +struct AuthImpl; + /** * `FirebaseCredentialsProvider` uses Firebase Auth via `FIRApp` to get an auth * token. @@ -49,27 +48,34 @@ namespace auth { * * For non-Apple desktop build, this is right now just a stub. */ -class FirebaseCredentialsProvider : CredentialsProvider { +class FirebaseCredentialsProvider : public CredentialsProvider { public: // TODO(zxu123): Provide a ctor to accept the C++ Firebase Games App, which - // deals all platforms. + // deals all platforms. Right now, AppImpl is only a wrapper for FIRApp*. /** * Initializes a new FirebaseCredentialsProvider. * * @param app The Firebase app from which to get credentials. */ - FirebaseCredentialsProvider(FIRApp* app); + FirebaseCredentialsProvider(const AppImpl& app); + + ~FirebaseCredentialsProvider(); void GetToken(bool force_refresh, TokenListener completion) override; void set_user_change_listener(UserListener listener) override; + friend class FirebaseCredentialsProvider_GetToken_Test; + friend class FirebaseCredentialsProvider_SetListener_Test; + private: - const FIRApp* app_; + /** Initialize with default app for internal usage such as test. */ + FirebaseCredentialsProvider(); + + static void PlatformDependentTestSetup(const absl::string_view config_path); - /** Handle used to stop receiving auth changes once userChangeListener is - * removed. */ - id auth_listener_handle_; + /** Platform-dependent members defined inside. */ + std::unique_ptr auth_; /** The current user as reported to us via our AuthStateDidChangeListener. */ User current_user_; 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 new file mode 100644 index 00000000000..695ef9d9e74 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h @@ -0,0 +1,56 @@ +/* + * 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. + */ + +// Right now, FirebaseCredentialsProvider only support APPLE build. +// TODO(zxu123): Make it for desktop workflow as well. + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_APPLE_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_APPLE_H_ + +#import + +@class FIRApp; + +namespace firebase { +namespace firestore { +namespace auth { + +class AppImpl { + public: + AppImpl(FIRApp* app) : app_(app) { + } + + operator FIRApp*() const { + return app_; + } + + private: + FIRApp* app_; +}; + +struct AuthImpl { + const FIRApp* app; + + /** Handle used to stop receiving auth changes once userChangeListener is + * removed. */ + id auth_listener_handle; +}; + +} // namespace auth +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_APPLE_H_ 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 1835de752b7..3d36a29a873 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 @@ -14,25 +14,29 @@ * limitations under the License. */ +#include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h" #include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h" #import #import +#import #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" -#define UNUSED(x) (void)(x) - namespace firebase { namespace firestore { namespace auth { -FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app) - : app_(app), +FirebaseCredentialsProvider::FirebaseCredentialsProvider() + : FirebaseCredentialsProvider([FIRApp defaultApp]) { +} + +FirebaseCredentialsProvider::FirebaseCredentialsProvider(const AppImpl& app) + : auth_(new AuthImpl{app, nullptr}), current_user_(firebase::firestore::util::MakeStringView([app getUID])), user_counter_(0) { - auth_listener_handle_ = [[NSNotificationCenter defaultCenter] + auth_->auth_listener_handle = [[NSNotificationCenter defaultCenter] addObserverForName:FIRAuthStateDidChangeInternalNotification object:nil queue:nil @@ -43,7 +47,7 @@ // ensure we're only notifiying for the current app. FIRApp* notified_app = user_info[FIRAuthStateDidChangeInternalNotificationAppKey]; - if (![this->app_ isEqual:notified_app]) { + if (![this->auth_->app isEqual:notified_app]) { return; } @@ -62,10 +66,14 @@ User new_user( }]; } +FirebaseCredentialsProvider::~FirebaseCredentialsProvider() { + auth_.reset(nullptr); +} + void FirebaseCredentialsProvider::GetToken(bool force_refresh, TokenListener completion) { FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( - this->auth_listener_handle_, this->auth_listener_handle_, + this->auth_->auth_listener_handle, this->auth_->auth_listener_handle, "GetToken cannot be called after listener removed."); // Take note of the current value of the userCounter so that this method can @@ -89,8 +97,8 @@ User new_user( } }; - [this->app_ getTokenForcingRefresh:force_refresh - withCallback:get_token_callback]; + [this->auth_->app getTokenForcingRefresh:force_refresh + withCallback:get_token_callback]; } void FirebaseCredentialsProvider::set_user_change_listener( @@ -104,18 +112,35 @@ User new_user( listener(this->current_user_); } else { FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( - this->auth_listener_handle_, this->auth_listener_handle_, + this->auth_->auth_listener_handle, this->auth_->auth_listener_handle, "removed user_change_listener twice!"); FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( this->user_change_listener_, this->user_change_listener_, "user_change_listener removed without being set!"); [[NSNotificationCenter defaultCenter] - removeObserver:this->auth_listener_handle_]; - this->auth_listener_handle_ = nullptr; + removeObserver:this->auth_->auth_listener_handle]; + this->auth_->auth_listener_handle = nullptr; } this->user_change_listener_ = listener; } +void FirebaseCredentialsProvider::PlatformDependentTestSetup( + const absl::string_view config_path) { + static dispatch_once_t once_token; + dispatch_once(&once_token, ^{ + NSString* file_path = + firebase::firestore::util::WrapNSStringNoCopy(config_path.data()); + FIROptions* options = [[FIROptions alloc] initWithContentsOfFile:file_path]; + [FIRApp configureWithOptions:options]; + }); + + // Set getUID implementation. + FIRApp* default_app = [FIRApp defaultApp]; + default_app.getUIDImplementation = ^NSString* { + return @"I'm a fake uid."; + }; +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt index a77c4b533a5..d3f593a9efa 100644 --- a/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt @@ -21,3 +21,13 @@ cc_test( DEPENDS firebase_firestore_auth ) + +if(APPLE) + cc_test( + firebase_firestore_auth_apple_test + SOURCES + firebase_credentials_provider_test.cc + DEPENDS + firebase_firestore_auth_apple + ) +endif(APPLE) 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 38b5bc07d5f..70780c28744 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 @@ -40,6 +40,7 @@ TEST(EmptyCredentialsProvider, SetListener) { EXPECT_EQ("", user.uid()); EXPECT_FALSE(user.is_authenticated()); }); + credentials_provider.RemoveUserChangeListener(); } } // namespace auth diff --git a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc index bfbc5f6df3d..9bd619b20bb 100644 --- a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" +#include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h" #include "gtest/gtest.h" @@ -22,10 +22,44 @@ namespace firebase { namespace firestore { namespace auth { -TEST(Token, Getter) { - Token token("token", User("abc")); - EXPECT_EQ("token", token.token()); - EXPECT_EQ(User("abc"), token.user()); +// Set a .plist file here to enable the test-case. +static const char* kPlist = ""; + +// Set kPlist above before enable. +TEST(DISABLED_FirebaseCredentialsProvider, GetToken) { + absl::string_view plist(kPlist); + if (plist.substr(plist.length() - 6) != ".plist") { + return; + } + + FirebaseCredentialsProvider::PlatformDependentTestSetup( + "/Users/zxu/Downloads/GoogleService-Info.plist"); + FirebaseCredentialsProvider credentials_provider; + credentials_provider.GetToken( + true, [](const Token& token, const absl::string_view error) { + EXPECT_EQ("", token.token()); + const User& user = token.user(); + EXPECT_EQ("I'm a fake uid.", user.uid()); + EXPECT_TRUE(user.is_authenticated()); + EXPECT_EQ("", error) << error; + }); +} + +// Set kPlist above before enable. +TEST(DISABLED_FirebaseCredentialsProvider, SetListener) { + absl::string_view plist(kPlist); + if (plist.substr(plist.length() - 6) != ".plist") { + return; + } + + FirebaseCredentialsProvider::PlatformDependentTestSetup( + "/Users/zxu/Downloads/GoogleService-Info.plist"); + FirebaseCredentialsProvider credentials_provider; + credentials_provider.set_user_change_listener([](const User& user) { + EXPECT_EQ("I'm a fake uid.", user.uid()); + EXPECT_TRUE(user.is_authenticated()); + }); + credentials_provider.RemoveUserChangeListener(); } } // namespace auth From b53345fde636393c0e97deaae377f225fe64a852 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Thu, 1 Feb 2018 14:22:44 -0500 Subject: [PATCH 06/18] add auth test to project --- .../Example/Firestore.xcodeproj/project.pbxproj | 12 ++++++------ .../firestore/auth/firebase_credentials_provider.h | 2 ++ .../firestore/auth/credentials_provider_test.cpp | 9 --------- .../auth/empty_credentials_provider_test.cpp | 9 --------- .../core/test/firebase/firestore/auth/user_test.cpp | 9 --------- 5 files changed, 8 insertions(+), 33 deletions(-) delete mode 100644 Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cpp delete mode 100644 Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cpp delete mode 100644 Firestore/core/test/firebase/firestore/auth/user_test.cpp diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 572d4e97976..4258bf7d953 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -141,10 +141,10 @@ AB380CFE201A2F4500D97691 /* string_util_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380CFC201A2EE200D97691 /* string_util_test.cc */; }; AB380D02201BC69F00D97691 /* bits_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380D01201BC69F00D97691 /* bits_test.cc */; }; AB380D04201BC6E400D97691 /* ordered_code_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380D03201BC6E400D97691 /* ordered_code_test.cc */; }; - AB38D93320239654000A432D /* user_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93220239654000A432D /* user_test.cc */; }; - AB38D9352023966E000A432D /* credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D9342023966E000A432D /* credentials_provider_test.cc */; }; - AB38D93720239689000A432D /* empty_credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93620239689000A432D /* empty_credentials_provider_test.cc */; }; AB7BAB342012B519001E0872 /* geo_point_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB7BAB332012B519001E0872 /* geo_point_test.cc */; }; + ABC1D7DC2023A04B00BA84F0 /* credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D9342023966E000A432D /* credentials_provider_test.cc */; }; + ABC1D7DD2023A04F00BA84F0 /* empty_credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93620239689000A432D /* empty_credentials_provider_test.cc */; }; + ABC1D7DE2023A05300BA84F0 /* user_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93220239654000A432D /* user_test.cc */; }; ABE6637A201FA81900ED349A /* database_id_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB71064B201FA60300344F18 /* database_id_test.cc */; }; ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; }; AFE6114F0D4DAECBA7B7C089 /* Pods_Firestore_IntegrationTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B2FA635DF5D116A67A7441CD /* Pods_Firestore_IntegrationTests.framework */; }; @@ -1238,10 +1238,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - AB38D9352023966E000A432D /* credentials_provider_test.cc in Sources */, - AB38D93320239654000A432D /* user_test.cc in Sources */, 6003F59E195388D20070C39A /* FIRAppDelegate.m in Sources */, - AB38D93720239689000A432D /* empty_credentials_provider_test.cc in Sources */, 6003F5A7195388D20070C39A /* FIRViewController.m in Sources */, 6003F59A195388D20070C39A /* main.m in Sources */, ); @@ -1269,7 +1266,9 @@ 5492E058202154AB00B64F25 /* FSTAPIHelpers.mm in Sources */, AB380CFB2019388600D97691 /* target_id_generator_test.cc in Sources */, 5492E0A82021552D00B64F25 /* FSTLevelDBLocalStoreTests.mm in Sources */, + ABC1D7DE2023A05300BA84F0 /* user_test.cc in Sources */, 5491BC721FB44593008B3588 /* FSTIntegrationTestCase.mm in Sources */, + ABC1D7DD2023A04F00BA84F0 /* empty_credentials_provider_test.cc in Sources */, DE2EF0861F3D0B6E003D0CDC /* FSTImmutableSortedDictionary+Testing.m in Sources */, 5492E03120213FFC00B64F25 /* FSTLevelDBSpecTests.mm in Sources */, 5492E0B12021552D00B64F25 /* FSTRemoteDocumentCacheTests.mm in Sources */, @@ -1316,6 +1315,7 @@ 5492E0C92021557E00B64F25 /* FSTRemoteEventTests.mm in Sources */, ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */, 5492E0AE2021552D00B64F25 /* FSTLevelDBQueryCacheTests.mm in Sources */, + ABC1D7DC2023A04B00BA84F0 /* credentials_provider_test.cc in Sources */, 5492E059202154AB00B64F25 /* FIRQuerySnapshotTests.mm in Sources */, 5492E050202154AA00B64F25 /* FIRCollectionReferenceTests.mm in Sources */, 5492E0A02021552D00B64F25 /* FSTLevelDBMutationQueueTests.mm in Sources */, diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h index 9ad79d35aa7..c872dda9df1 100644 --- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h @@ -67,6 +67,8 @@ class FirebaseCredentialsProvider : public CredentialsProvider { friend class FirebaseCredentialsProvider_GetToken_Test; friend class FirebaseCredentialsProvider_SetListener_Test; + friend class DISABLED_FirebaseCredentialsProvider_GetToken_Test; + friend class DISABLED_FirebaseCredentialsProvider_SetListener_Test; private: /** Initialize with default app for internal usage such as test. */ diff --git a/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cpp b/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cpp deleted file mode 100644 index e9caaf7b60d..00000000000 --- a/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cpp +++ /dev/null @@ -1,9 +0,0 @@ -// -// credentials_provider_test.cpp -// Firestore -// -// Created by Zhi Xu on 2/1/18. -// Copyright © 2018 Google. All rights reserved. -// - -#include diff --git a/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cpp b/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cpp deleted file mode 100644 index 27ba3a222ea..00000000000 --- a/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cpp +++ /dev/null @@ -1,9 +0,0 @@ -// -// empty_credentials_provider_test.cpp -// Firestore -// -// Created by Zhi Xu on 2/1/18. -// Copyright © 2018 Google. All rights reserved. -// - -#include diff --git a/Firestore/core/test/firebase/firestore/auth/user_test.cpp b/Firestore/core/test/firebase/firestore/auth/user_test.cpp deleted file mode 100644 index b8d6de9aff9..00000000000 --- a/Firestore/core/test/firebase/firestore/auth/user_test.cpp +++ /dev/null @@ -1,9 +0,0 @@ -// -// user_test.cpp -// Firestore -// -// Created by Zhi Xu on 2/1/18. -// Copyright © 2018 Google. All rights reserved. -// - -#include From 7a06f1ca39105d30c4eef7fafc5de422929d9e67 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Thu, 1 Feb 2018 17:37:04 -0500 Subject: [PATCH 07/18] address changes --- .../Firestore.xcodeproj/project.pbxproj | 8 ++ .../firebase/firestore/auth/CMakeLists.txt | 3 +- .../firestore/auth/credentials_provider.cc | 4 - .../firestore/auth/credentials_provider.h | 66 ++++--------- .../auth/empty_credentials_provider.cc | 7 +- .../auth/empty_credentials_provider.h | 2 +- .../auth/firebase_credentials_provider.h | 98 ------------------- .../firebase_credentials_provider_apple.h | 82 ++++++++++++---- .../firebase_credentials_provider_apple.mm | 96 +++++++----------- .../core/src/firebase/firestore/auth/token.cc | 29 ++++++ .../core/src/firebase/firestore/auth/token.h | 66 +++++++++++++ .../core/src/firebase/firestore/auth/user.cc | 5 + .../core/src/firebase/firestore/auth/user.h | 6 +- .../firebase/firestore/util/firebase_assert.h | 11 +++ .../firebase/firestore/auth/CMakeLists.txt | 3 +- .../auth/credentials_provider_test.cc | 22 +++-- .../auth/empty_credentials_provider_test.cc | 8 +- ... => firebase_credentials_provider_test.mm} | 45 +++++++-- .../firebase/firestore/auth/token_test.cc | 33 +++++++ .../test/firebase/firestore/auth/user_test.cc | 6 ++ 20 files changed, 343 insertions(+), 257 deletions(-) delete mode 100644 Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h create mode 100644 Firestore/core/src/firebase/firestore/auth/token.cc create mode 100644 Firestore/core/src/firebase/firestore/auth/token.h rename Firestore/core/test/firebase/firestore/auth/{firebase_credentials_provider_test.cc => firebase_credentials_provider_test.mm} (57%) create mode 100644 Firestore/core/test/firebase/firestore/auth/token_test.cc diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 4258bf7d953..20e541252ef 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -145,6 +145,8 @@ ABC1D7DC2023A04B00BA84F0 /* credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D9342023966E000A432D /* credentials_provider_test.cc */; }; ABC1D7DD2023A04F00BA84F0 /* empty_credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93620239689000A432D /* empty_credentials_provider_test.cc */; }; ABC1D7DE2023A05300BA84F0 /* user_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93220239654000A432D /* user_test.cc */; }; + ABC1D7E12023A40C00BA84F0 /* token_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABC1D7DF2023A3EF00BA84F0 /* token_test.cc */; }; + ABC1D7E32023CDC500BA84F0 /* firebase_credentials_provider_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = ABC1D7E22023CDC500BA84F0 /* firebase_credentials_provider_test.mm */; }; ABE6637A201FA81900ED349A /* database_id_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB71064B201FA60300344F18 /* database_id_test.cc */; }; ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; }; AFE6114F0D4DAECBA7B7C089 /* Pods_Firestore_IntegrationTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B2FA635DF5D116A67A7441CD /* Pods_Firestore_IntegrationTests.framework */; }; @@ -344,6 +346,8 @@ AB38D93620239689000A432D /* empty_credentials_provider_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = empty_credentials_provider_test.cc; sourceTree = ""; }; AB71064B201FA60300344F18 /* database_id_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = database_id_test.cc; sourceTree = ""; }; AB7BAB332012B519001E0872 /* geo_point_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = geo_point_test.cc; path = ../../core/test/firebase/firestore/geo_point_test.cc; sourceTree = ""; }; + ABC1D7DF2023A3EF00BA84F0 /* token_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = token_test.cc; sourceTree = ""; }; + ABC1D7E22023CDC500BA84F0 /* firebase_credentials_provider_test.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = firebase_credentials_provider_test.mm; sourceTree = ""; }; ABF6506B201131F8005F2C74 /* timestamp_test.cc */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = timestamp_test.cc; sourceTree = ""; }; B2FA635DF5D116A67A7441CD /* Pods_Firestore_IntegrationTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_IntegrationTests.framework; sourceTree = BUILT_PRODUCTS_DIR; }; CE00BABB5A3AAB44A4C209E2 /* Pods-Firestore_Tests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Tests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Tests/Pods-Firestore_Tests.debug.xcconfig"; sourceTree = ""; }; @@ -596,6 +600,8 @@ children = ( AB38D9342023966E000A432D /* credentials_provider_test.cc */, AB38D93620239689000A432D /* empty_credentials_provider_test.cc */, + ABC1D7E22023CDC500BA84F0 /* firebase_credentials_provider_test.mm */, + ABC1D7DF2023A3EF00BA84F0 /* token_test.cc */, AB38D93220239654000A432D /* user_test.cc */, ); name = auth; @@ -1240,6 +1246,7 @@ files = ( 6003F59E195388D20070C39A /* FIRAppDelegate.m in Sources */, 6003F5A7195388D20070C39A /* FIRViewController.m in Sources */, + ABC1D7E32023CDC500BA84F0 /* firebase_credentials_provider_test.mm in Sources */, 6003F59A195388D20070C39A /* main.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -1288,6 +1295,7 @@ 5492E0AC2021552D00B64F25 /* FSTMutationQueueTests.mm in Sources */, 5492E056202154AB00B64F25 /* FIRFieldPathTests.mm in Sources */, 5492E03220213FFC00B64F25 /* FSTMockDatastore.mm in Sources */, + ABC1D7E12023A40C00BA84F0 /* token_test.cc in Sources */, AB356EF7200EA5EB0089B766 /* field_value_test.cc in Sources */, AB7BAB342012B519001E0872 /* geo_point_test.cc in Sources */, 5492E0AD2021552D00B64F25 /* FSTMemoryMutationQueueTests.mm in Sources */, diff --git a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt index 612ed4beb66..2241fae2a8c 100644 --- a/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/auth/CMakeLists.txt @@ -17,6 +17,8 @@ cc_library( SOURCES credentials_provider.cc credentials_provider.h + token.cc + token.h user.cc user.h DEPENDS @@ -27,7 +29,6 @@ cc_library( cc_library( firebase_firestore_auth_apple SOURCES - firebase_credentials_provider.h firebase_credentials_provider_apple.h firebase_credentials_provider_apple.mm DEPENDS diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc index a6a2bc11f6c..786f6d26d24 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc @@ -20,10 +20,6 @@ namespace firebase { namespace firestore { namespace auth { -Token::Token(const absl::string_view token, const User& user) - : token_(token), user_(user) { -} - CredentialsProvider::CredentialsProvider() : user_change_listener_(nullptr) { } diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h index fe3961811b3..c9419e24539 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h @@ -17,8 +17,10 @@ #ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ +#include #include +#include "Firestore/core/src/firebase/firestore/auth/token.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" #include "absl/strings/string_view.h" @@ -26,68 +28,36 @@ namespace firebase { namespace firestore { namespace auth { -/** - * The current FSTUser and the authentication token provided by the underlying - * authentication mechanism. This is the result of calling - * -[FSTCredentialsProvider getTokenForcingRefresh]. - * - * ## Portability notes: no TokenType on iOS - * - * The TypeScript client supports 1st party Oauth tokens (for the Firebase - * Console to auth as the developer) and OAuth2 tokens for the node.js sdk to - * auth with a service account. We don't have plans to support either case on - * mobile so there's no TokenType here. - */ -// TODO(zxu123): Make this support token-type for desktop workflow. -class Token { - public: - Token(const absl::string_view token, const User& user); - - /** The actual raw token. */ - const std::string& token() const { - return token_; - } - - /** The user with which the token is associated (used for persisting user - * state on disk, etc.). */ - const User& user() const { - return user_; - } - - private: - const std::string token_; - const User user_; -}; - // `TokenErrorListener` is a listener that gets a token or an error. // token: An auth token as a string, or nullptr if error occurred. // error: The error if one occurred, or else nullptr. -typedef void (*TokenListener)(const Token& token, - const absl::string_view error); +typedef std::function + TokenListener; -// Listener notified with a User. -typedef void (*UserListener)(const User& user); +// Listener notified with a User change. +typedef std::function UserChangeListener; -/** Provides methods for getting the uid and token for the current user and - * listen for changes. */ +/** + * Provides methods for getting the uid and token for the current user and + * listen for changes. + */ class CredentialsProvider { public: CredentialsProvider(); - /** Requests token for the current user, optionally forcing a refreshed token - * to be fetched. */ + /** + * Requests token for the current user, optionally forcing a refreshed token + * to be fetched. + */ virtual void GetToken(bool force_refresh, TokenListener completion) = 0; /** * Sets the listener to be notified of user changes (sign-in / sign-out). It * is immediately called once with the initial user. + * + * Call with nullptr to remove previous listener. */ - virtual void set_user_change_listener(UserListener listener) = 0; - - /** Removes the listener set with {@link #setUserChangeListener}. */ - virtual void RemoveUserChangeListener() { - set_user_change_listener(nullptr); - } + virtual void SetUserChangeListener(UserChangeListener listener) = 0; protected: /** @@ -97,7 +67,7 @@ class CredentialsProvider { * Note that this block will be called back on an arbitrary thread that is not * the normal Firestore worker thread. */ - UserListener user_change_listener_; + UserChangeListener user_change_listener_; }; } // namespace auth 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 3affb16e70e..6ee7f61853c 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc @@ -26,13 +26,14 @@ void EmptyCredentialsProvider::GetToken(bool force_refresh, TokenListener completion) { UNUSED(force_refresh); if (completion) { - completion({"", User()}, ""); + completion({"", User::Unauthenticated()}, ""); } } -void EmptyCredentialsProvider::set_user_change_listener(UserListener listener) { +void EmptyCredentialsProvider::SetUserChangeListener( + UserChangeListener listener) { if (listener) { - listener({}); + listener(User::Unauthenticated()); } } 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 501b4dacae7..55b3cc62c09 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h @@ -27,7 +27,7 @@ namespace auth { class EmptyCredentialsProvider : public CredentialsProvider { public: void GetToken(bool force_refresh, TokenListener completion) override; - void set_user_change_listener(UserListener listener) override; + void SetUserChangeListener(UserChangeListener listener) override; }; } // namespace auth diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h deleted file mode 100644 index c872dda9df1..00000000000 --- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h +++ /dev/null @@ -1,98 +0,0 @@ -/* - * 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. - */ - -// Right now, FirebaseCredentialsProvider only support APPLE build. -// TODO(zxu123): Make it for desktop workflow as well. - -#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ -#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ - -#include -#include - -#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" -#include "Firestore/core/src/firebase/firestore/auth/user.h" -#include "absl/strings/string_view.h" - -namespace firebase { -namespace firestore { -namespace auth { - -class AppImpl; -struct AuthImpl; - -/** - * `FirebaseCredentialsProvider` uses Firebase Auth via `FIRApp` to get an auth - * token. - * - * NOTE: To simplify the implementation, it requires that you set - * `userChangeListener` with a non-`nil` value no more than once and don't call - * `getTokenForcingRefresh:` after setting it to `nil`. - * - * This class must be implemented in a thread-safe manner since it is accessed - * from the thread backing our internal worker queue and the callbacks from - * FIRAuth will be executed on an arbitrary different thread. - * - * For non-Apple desktop build, this is right now just a stub. - */ -class FirebaseCredentialsProvider : public CredentialsProvider { - public: - // TODO(zxu123): Provide a ctor to accept the C++ Firebase Games App, which - // deals all platforms. Right now, AppImpl is only a wrapper for FIRApp*. - /** - * Initializes a new FirebaseCredentialsProvider. - * - * @param app The Firebase app from which to get credentials. - */ - FirebaseCredentialsProvider(const AppImpl& app); - - ~FirebaseCredentialsProvider(); - - void GetToken(bool force_refresh, TokenListener completion) override; - - void set_user_change_listener(UserListener listener) override; - - friend class FirebaseCredentialsProvider_GetToken_Test; - friend class FirebaseCredentialsProvider_SetListener_Test; - friend class DISABLED_FirebaseCredentialsProvider_GetToken_Test; - friend class DISABLED_FirebaseCredentialsProvider_SetListener_Test; - - private: - /** Initialize with default app for internal usage such as test. */ - FirebaseCredentialsProvider(); - - static void PlatformDependentTestSetup(const absl::string_view config_path); - - /** Platform-dependent members defined inside. */ - std::unique_ptr auth_; - - /** The current user as reported to us via our AuthStateDidChangeListener. */ - User current_user_; - - /** - * Counter used to detect if the user changed while a -getTokenForcingRefresh: - * request was outstanding. - */ - int user_counter_; - - std::mutex mutex_; -}; - -} // namespace auth -} // namespace firestore -} // namespace firebase - -#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ 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 695ef9d9e74..7760946c507 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 @@ -15,42 +15,90 @@ */ // Right now, FirebaseCredentialsProvider only support APPLE build. -// TODO(zxu123): Make it for desktop workflow as well. +#if !defined(__OBJC__) +#error "This .header can only be used in .mm file for iOS build." +#endif // !defined(__OBJC__) -#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_APPLE_H_ -#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_APPLE_H_ +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ #import +#include +#include + +#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" +#include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "absl/strings/string_view.h" + @class FIRApp; namespace firebase { namespace firestore { namespace auth { -class AppImpl { +/** + * `FirebaseCredentialsProvider` uses Firebase Auth via `FIRApp` to get an auth + * token. + * + * NOTE: To simplify the implementation, it requires that you set + * `userChangeListener` with a non-`nil` value no more than once and don't call + * `getTokenForcingRefresh:` after setting it to `nil`. + * + * This class must be implemented in a thread-safe manner since it is accessed + * from the thread backing our internal worker queue and the callbacks from + * FIRAuth will be executed on an arbitrary different thread. + * + * For non-Apple desktop build, this is right now just a stub. + */ +class FirebaseCredentialsProvider : public CredentialsProvider { public: - AppImpl(FIRApp* app) : app_(app) { - } + // TODO(zxu123): Provide a ctor to accept the C++ Firebase Games App, which + // deals all platforms. Right now, only works for FIRApp*. + /** + * Initializes a new FirebaseCredentialsProvider. + * + * @param app The Firebase app from which to get credentials. + */ + FirebaseCredentialsProvider(FIRApp* app); + + void GetToken(bool force_refresh, TokenListener completion) override; - operator FIRApp*() const { - return app_; - } + void SetUserChangeListener(UserChangeListener listener) override; + + friend class FirebaseCredentialsProviderTest_GetToken_Test; + friend class FirebaseCredentialsProviderTest_SetListener_Test; + friend class DISABLED_FirebaseCredentialsProviderTest_GetToken_Test; + friend class DISABLED_FirebaseCredentialsProviderTest_SetListener_Test; private: - FIRApp* app_; -}; + /** Initialize with default app for internal usage such as test. */ + FirebaseCredentialsProvider(); + + static void PlatformDependentTestSetup(const absl::string_view config_path); + + const FIRApp* app_; + + /** + * Handle used to stop receiving auth changes once userChangeListener is + * removed. + */ + id auth_listener_handle_; + + /** The current user as reported to us via our AuthStateDidChangeListener. */ + User current_user_; -struct AuthImpl { - const FIRApp* app; + /** + * Counter used to detect if the user changed while a -getTokenForcingRefresh: + * request was outstanding. + */ + int user_counter_; - /** Handle used to stop receiving auth changes once userChangeListener is - * removed. */ - id auth_listener_handle; + std::mutex mutex_; }; } // namespace auth } // namespace firestore } // namespace firebase -#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_APPLE_H_ +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ 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 3d36a29a873..27a55eaa43d 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 @@ -15,7 +15,6 @@ */ #include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h" -#include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h" #import #import @@ -32,22 +31,22 @@ : FirebaseCredentialsProvider([FIRApp defaultApp]) { } -FirebaseCredentialsProvider::FirebaseCredentialsProvider(const AppImpl& app) - : auth_(new AuthImpl{app, nullptr}), +FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app) + : app_(app), current_user_(firebase::firestore::util::MakeStringView([app getUID])), user_counter_(0) { - auth_->auth_listener_handle = [[NSNotificationCenter defaultCenter] + auth_listener_handle_ = [[NSNotificationCenter defaultCenter] addObserverForName:FIRAuthStateDidChangeInternalNotification object:nil queue:nil usingBlock:^(NSNotification* notification) { - std::unique_lock lock(this->mutex_); + std::unique_lock lock(mutex_); NSDictionary* user_info = notification.userInfo; // ensure we're only notifiying for the current app. FIRApp* notified_app = user_info[FIRAuthStateDidChangeInternalNotificationAppKey]; - if (![this->auth_->app isEqual:notified_app]) { + if (![app_ isEqual:notified_app]) { return; } @@ -55,90 +54,63 @@ user_info[FIRAuthStateDidChangeInternalNotificationUIDKey]; User new_user( firebase::firestore::util::MakeStringView(user_id)); - if (new_user != this->current_user_) { - this->current_user_ = new_user; - this->user_counter_++; - UserListener listener = this->user_change_listener_; + if (new_user != current_user_) { + current_user_ = new_user; + user_counter_++; + UserChangeListener listener = user_change_listener_; if (listener) { - listener(this->current_user_); + listener(current_user_); } } }]; } -FirebaseCredentialsProvider::~FirebaseCredentialsProvider() { - auth_.reset(nullptr); -} - void FirebaseCredentialsProvider::GetToken(bool force_refresh, TokenListener completion) { - FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( - this->auth_->auth_listener_handle, this->auth_->auth_listener_handle, - "GetToken cannot be called after listener removed."); + FIREBASE_ASSERT_MESSAGE(auth_listener_handle_, + "GetToken cannot be called after listener removed."); // Take note of the current value of the userCounter so that this method can // fail if there is a user change while the request is outstanding. - int initial_user_counter = this->user_counter_; + int initial_user_counter = user_counter_; void (^get_token_callback)(NSString*, NSError*) = ^(NSString* _Nullable token, NSError* _Nullable error) { - std::unique_lock lock(this->mutex_); - if (initial_user_counter != this->user_counter_) { + std::unique_lock lock(mutex_); + if (initial_user_counter != user_counter_) { // Cancel the request since the user changed while the request was // outstanding so the response is likely for a previous user (which // user, we can't be sure). completion({"", User()}, "getToken aborted due to user change."); } else { - completion({firebase::firestore::util::MakeStringView(token), - this->current_user_}, - error == nil ? "" - : firebase::firestore::util::MakeStringView( - (NSString*)error)); + completion( + {firebase::firestore::util::MakeStringView(token), current_user_}, + error == nil ? "" + : firebase::firestore::util::MakeStringView( + error.localizedDescription)); } }; - [this->auth_->app getTokenForcingRefresh:force_refresh - withCallback:get_token_callback]; + [app_ getTokenForcingRefresh:force_refresh withCallback:get_token_callback]; } -void FirebaseCredentialsProvider::set_user_change_listener( - UserListener listener) { - std::unique_lock lock(this->mutex_); +void FirebaseCredentialsProvider::SetUserChangeListener( + UserChangeListener listener) { + std::unique_lock lock(mutex_); if (listener) { - FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION(!this->user_change_listener_, - !this->user_change_listener_, - "set user_change_listener twice!"); + FIREBASE_ASSERT_MESSAGE(!user_change_listener_, + "set user_change_listener twice!"); // Fire initial event. - listener(this->current_user_); + listener(current_user_); } else { - FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( - this->auth_->auth_listener_handle, this->auth_->auth_listener_handle, - "removed user_change_listener twice!"); - FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION( - this->user_change_listener_, this->user_change_listener_, - "user_change_listener removed without being set!"); - [[NSNotificationCenter defaultCenter] - removeObserver:this->auth_->auth_listener_handle]; - this->auth_->auth_listener_handle = nullptr; + FIREBASE_ASSERT_MESSAGE(auth_listener_handle_, + "removed user_change_listener twice!"); + FIREBASE_ASSERT_MESSAGE(user_change_listener_, + "user_change_listener removed without being set!"); + [[NSNotificationCenter defaultCenter] removeObserver:auth_listener_handle_]; + auth_listener_handle_ = nullptr; } - this->user_change_listener_ = listener; -} - -void FirebaseCredentialsProvider::PlatformDependentTestSetup( - const absl::string_view config_path) { - static dispatch_once_t once_token; - dispatch_once(&once_token, ^{ - NSString* file_path = - firebase::firestore::util::WrapNSStringNoCopy(config_path.data()); - FIROptions* options = [[FIROptions alloc] initWithContentsOfFile:file_path]; - [FIRApp configureWithOptions:options]; - }); - - // Set getUID implementation. - FIRApp* default_app = [FIRApp defaultApp]; - default_app.getUIDImplementation = ^NSString* { - return @"I'm a fake uid."; - }; + user_change_listener_ = listener; } } // namespace auth diff --git a/Firestore/core/src/firebase/firestore/auth/token.cc b/Firestore/core/src/firebase/firestore/auth/token.cc new file mode 100644 index 00000000000..be0055398d2 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/token.cc @@ -0,0 +1,29 @@ +/* + * 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/auth/credentials_provider.h" + +namespace firebase { +namespace firestore { +namespace auth { + +Token::Token(const absl::string_view token, const User& user) + : token_(token), user_(user) { +} + +} // namespace auth +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/token.h b/Firestore/core/src/firebase/firestore/auth/token.h new file mode 100644 index 00000000000..dd2d84d9b82 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/auth/token.h @@ -0,0 +1,66 @@ +/* + * 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_AUTH_TOKEN_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_TOKEN_H_ + +#include + +#include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "absl/strings/string_view.h" + +namespace firebase { +namespace firestore { +namespace auth { + +/** + * The current User and the authentication token provided by the underlying + * authentication mechanism. This is the result of calling + * CredentialsProvider::GetToken(). + * + * ## Portability notes: no TokenType on iOS + * + * The TypeScript client supports 1st party Oauth tokens (for the Firebase + * Console to auth as the developer) and OAuth2 tokens for the node.js sdk to + * auth with a service account. We don't have plans to support either case on + * mobile so there's no TokenType here. + */ +// TODO(zxu123): Make this support token-type for desktop workflow. +class Token { + public: + Token(const absl::string_view token, const User& user); + + /** The actual raw token. */ + const std::string& token() const { + return token_; + } + + /** The user with which the token is associated (used for persisting user + * state on disk, etc.). */ + const User& user() const { + return user_; + } + + private: + const std::string token_; + const User user_; +}; + +} // namespace auth +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_TOKEN_H_ diff --git a/Firestore/core/src/firebase/firestore/auth/user.cc b/Firestore/core/src/firebase/firestore/auth/user.cc index 899a1896af0..4793fed100d 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.cc +++ b/Firestore/core/src/firebase/firestore/auth/user.cc @@ -29,6 +29,11 @@ User::User(const absl::string_view uid) : uid_(uid), is_authenticated_(true) { FIREBASE_ASSERT(!uid.empty()); } +const User& User::Unauthenticated() { + static const User kUnauthenticated; + return kUnauthenticated; +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/user.h b/Firestore/core/src/firebase/firestore/auth/user.h index cbda7cbf284..e74f791f8c8 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.h +++ b/Firestore/core/src/firebase/firestore/auth/user.h @@ -27,7 +27,8 @@ namespace auth { /** * Simple wrapper around a nullable UID. Mostly exists to make code more - * readable and for use as a key in dictionaries (since keys cannot be nil). + * readable and for compatibility with other clients where map keys cannot be + * null. */ class User { public: @@ -47,6 +48,9 @@ class User { return is_authenticated_; } + /** Returns an unauthenticated instance. */ + static const User& Unauthenticated(); + User& operator=(const User& other) = default; friend bool operator==(const User& lhs, const User& rhs); diff --git a/Firestore/core/src/firebase/firestore/util/firebase_assert.h b/Firestore/core/src/firebase/firestore/util/firebase_assert.h index cff550abf2d..172a126cd9f 100644 --- a/Firestore/core/src/firebase/firestore/util/firebase_assert.h +++ b/Firestore/core/src/firebase/firestore/util/firebase_assert.h @@ -87,6 +87,17 @@ FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION(condition, expression, __VA_ARGS__) #endif // defined(NDEBUG) +// Assert expression is true otherwise display the specified message and +// abort. +#define FIREBASE_ASSERT_MESSAGE(expression, ...) \ + FIREBASE_ASSERT_MESSAGE_WITH_EXPRESSION(expression, expression, __VA_ARGS__) + +// Assert expression is true otherwise display the specified message and +// abort. Compiled out of release builds. +#define FIREBASE_DEV_ASSERT_MESSAGE(expression, ...) \ + FIREBASE_DEV_ASSERT_MESSAGE_WITH_EXPRESSION(expression, expression, \ + __VA_ARGS__) + namespace firebase { namespace firestore { namespace util { diff --git a/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt index d3f593a9efa..f470bd76591 100644 --- a/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/auth/CMakeLists.txt @@ -17,6 +17,7 @@ cc_test( SOURCES credentials_provider_test.cc empty_credentials_provider_test.cc + token_test.cc user_test.cc DEPENDS firebase_firestore_auth @@ -26,7 +27,7 @@ if(APPLE) cc_test( firebase_firestore_auth_apple_test SOURCES - firebase_credentials_provider_test.cc + firebase_credentials_provider_test.mm DEPENDS firebase_firestore_auth_apple ) diff --git a/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc index 5b8ea656ef0..17484221010 100644 --- a/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc @@ -22,12 +22,6 @@ namespace firebase { namespace firestore { namespace auth { -TEST(Token, Getter) { - Token token("token", User("abc")); - EXPECT_EQ("token", token.token()); - EXPECT_EQ(User("abc"), token.user()); -} - #define UNUSED(x) (void)(x) TEST(CredentialsProvider, Typedef) { @@ -37,9 +31,21 @@ TEST(CredentialsProvider, Typedef) { UNUSED(error); }; EXPECT_NE(nullptr, token_listener); + EXPECT_TRUE(token_listener); + + token_listener = nullptr; + EXPECT_EQ(nullptr, token_listener); + EXPECT_FALSE(token_listener); + + UserChangeListener user_change_listener = [](const User& user) { + UNUSED(user); + }; + EXPECT_NE(nullptr, user_change_listener); + EXPECT_TRUE(user_change_listener); - UserListener user_listener = [](const User& user) { UNUSED(user); }; - EXPECT_NE(nullptr, user_listener); + user_change_listener = nullptr; + EXPECT_EQ(nullptr, user_change_listener); + EXPECT_FALSE(user_change_listener); } } // namespace auth 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 70780c28744..123f9527e3b 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,7 +25,8 @@ namespace auth { TEST(EmptyCredentialsProvider, GetToken) { EmptyCredentialsProvider credentials_provider; credentials_provider.GetToken( - true, [](const Token& token, const absl::string_view error) { + /*force_refresh=*/true, + [](const Token& token, const absl::string_view error) { EXPECT_EQ("", token.token()); const User& user = token.user(); EXPECT_EQ("", user.uid()); @@ -36,11 +37,12 @@ TEST(EmptyCredentialsProvider, GetToken) { TEST(EmptyCredentialsProvider, SetListener) { EmptyCredentialsProvider credentials_provider; - credentials_provider.set_user_change_listener([](const User& user) { + credentials_provider.SetUserChangeListener([](const User& user) { EXPECT_EQ("", user.uid()); EXPECT_FALSE(user.is_authenticated()); }); - credentials_provider.RemoveUserChangeListener(); + + credentials_provider.SetUserChangeListener(nullptr); } } // namespace auth diff --git a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm similarity index 57% rename from Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc rename to Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm index 9bd619b20bb..a979c74c7c4 100644 --- a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm @@ -14,7 +14,13 @@ * limitations under the License. */ -#include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider.h" +#include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h" + +#import +#import +#import + +#include "Firestore/core/src/firebase/firestore/util/string_apple.h" #include "gtest/gtest.h" @@ -23,17 +29,36 @@ namespace firestore { namespace auth { // Set a .plist file here to enable the test-case. -static const char* kPlist = ""; +static const char* kPlist = "/Users/zxu/Downloads/GoogleService-Info.plist"; + +class FirebaseCredentialsProviderTest : public ::testing::Test { + protected: + void SetUp() override { + static dispatch_once_t once_token; + dispatch_once(&once_token, ^{ + NSString* file_path = + firebase::firestore::util::WrapNSStringNoCopy(kPlist); + FIROptions* options = + [[FIROptions alloc] initWithContentsOfFile:file_path]; + [FIRApp configureWithOptions:options]; + }); + + // Set getUID implementation. + FIRApp* default_app = [FIRApp defaultApp]; + default_app.getUIDImplementation = ^NSString* { + return @"I'm a fake uid."; + }; + } +}; // Set kPlist above before enable. -TEST(DISABLED_FirebaseCredentialsProvider, GetToken) { +// TEST(DISABLED_FirebaseCredentialsProvider, GetToken) { +TEST_F(FirebaseCredentialsProviderTest, GetToken) { absl::string_view plist(kPlist); if (plist.substr(plist.length() - 6) != ".plist") { return; } - FirebaseCredentialsProvider::PlatformDependentTestSetup( - "/Users/zxu/Downloads/GoogleService-Info.plist"); FirebaseCredentialsProvider credentials_provider; credentials_provider.GetToken( true, [](const Token& token, const absl::string_view error) { @@ -46,20 +71,20 @@ TEST(DISABLED_FirebaseCredentialsProvider, GetToken) { } // Set kPlist above before enable. -TEST(DISABLED_FirebaseCredentialsProvider, SetListener) { +// TEST(DISABLED_FirebaseCredentialsProvider, SetListener) { +TEST_F(FirebaseCredentialsProviderTest, SetListener) { absl::string_view plist(kPlist); if (plist.substr(plist.length() - 6) != ".plist") { return; } - FirebaseCredentialsProvider::PlatformDependentTestSetup( - "/Users/zxu/Downloads/GoogleService-Info.plist"); FirebaseCredentialsProvider credentials_provider; - credentials_provider.set_user_change_listener([](const User& user) { + credentials_provider.SetUserChangeListener([](const User& user) { EXPECT_EQ("I'm a fake uid.", user.uid()); EXPECT_TRUE(user.is_authenticated()); }); - credentials_provider.RemoveUserChangeListener(); + + credentials_provider.SetUserChangeListener(nullptr); } } // namespace auth diff --git a/Firestore/core/test/firebase/firestore/auth/token_test.cc b/Firestore/core/test/firebase/firestore/auth/token_test.cc new file mode 100644 index 00000000000..a0f2c48abca --- /dev/null +++ b/Firestore/core/test/firebase/firestore/auth/token_test.cc @@ -0,0 +1,33 @@ +/* + * 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/auth/token.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace auth { + +TEST(Token, Getter) { + Token token("token", User("abc")); + EXPECT_EQ("token", token.token()); + EXPECT_EQ(User("abc"), token.user()); +} + +} // namespace auth +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/test/firebase/firestore/auth/user_test.cc b/Firestore/core/test/firebase/firestore/auth/user_test.cc index da26d8df64c..a9f764d6644 100644 --- a/Firestore/core/test/firebase/firestore/auth/user_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/user_test.cc @@ -36,6 +36,12 @@ TEST(User, Getter) { EXPECT_EQ(signin, copy); } +TEST(User, Unauthenticated) { + User unauthenticated = User::Unauthenticated(); + EXPECT_EQ("", unauthenticated.uid()); + EXPECT_FALSE(unauthenticated.is_authenticated()); +} + TEST(User, Comparison) { EXPECT_EQ(User(), User()); EXPECT_EQ(User("abc"), User("abc")); From 9744768617e77cb1993940db3b54efeee90f8244 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Fri, 2 Feb 2018 09:41:20 -0500 Subject: [PATCH 08/18] small fix to style and project --- Firestore/Example/Firestore.xcodeproj/project.pbxproj | 4 ++-- Firestore/core/src/firebase/firestore/auth/token.h | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 20e541252ef..616138d1c06 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -146,7 +146,7 @@ ABC1D7DD2023A04F00BA84F0 /* empty_credentials_provider_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93620239689000A432D /* empty_credentials_provider_test.cc */; }; ABC1D7DE2023A05300BA84F0 /* user_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB38D93220239654000A432D /* user_test.cc */; }; ABC1D7E12023A40C00BA84F0 /* token_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABC1D7DF2023A3EF00BA84F0 /* token_test.cc */; }; - ABC1D7E32023CDC500BA84F0 /* firebase_credentials_provider_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = ABC1D7E22023CDC500BA84F0 /* firebase_credentials_provider_test.mm */; }; + ABC1D7E42024AFDE00BA84F0 /* firebase_credentials_provider_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = ABC1D7E22023CDC500BA84F0 /* firebase_credentials_provider_test.mm */; }; ABE6637A201FA81900ED349A /* database_id_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB71064B201FA60300344F18 /* database_id_test.cc */; }; ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; }; AFE6114F0D4DAECBA7B7C089 /* Pods_Firestore_IntegrationTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B2FA635DF5D116A67A7441CD /* Pods_Firestore_IntegrationTests.framework */; }; @@ -1246,7 +1246,6 @@ files = ( 6003F59E195388D20070C39A /* FIRAppDelegate.m in Sources */, 6003F5A7195388D20070C39A /* FIRViewController.m in Sources */, - ABC1D7E32023CDC500BA84F0 /* firebase_credentials_provider_test.mm in Sources */, 6003F59A195388D20070C39A /* main.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -1321,6 +1320,7 @@ 5492E068202154B900B64F25 /* FSTQueryTests.mm in Sources */, 5492E0AB2021552D00B64F25 /* StringViewTests.mm in Sources */, 5492E0C92021557E00B64F25 /* FSTRemoteEventTests.mm in Sources */, + ABC1D7E42024AFDE00BA84F0 /* firebase_credentials_provider_test.mm in Sources */, ABF6506C201131F8005F2C74 /* timestamp_test.cc in Sources */, 5492E0AE2021552D00B64F25 /* FSTLevelDBQueryCacheTests.mm in Sources */, ABC1D7DC2023A04B00BA84F0 /* credentials_provider_test.cc in Sources */, diff --git a/Firestore/core/src/firebase/firestore/auth/token.h b/Firestore/core/src/firebase/firestore/auth/token.h index dd2d84d9b82..f3b73633a96 100644 --- a/Firestore/core/src/firebase/firestore/auth/token.h +++ b/Firestore/core/src/firebase/firestore/auth/token.h @@ -48,8 +48,10 @@ class Token { return token_; } - /** The user with which the token is associated (used for persisting user - * state on disk, etc.). */ + /** + * The user with which the token is associated (used for persisting user + * state on disk, etc.). + */ const User& user() const { return user_; } From ea661f36afabd3dc4fa679dd5b136962b1171ecb Mon Sep 17 00:00:00 2001 From: zxu123 Date: Fri, 2 Feb 2018 11:45:39 -0500 Subject: [PATCH 09/18] fix the firebase_credentials_provider_test --- .../firebase_credentials_provider_apple.h | 4 +++- .../firebase_credentials_provider_apple.mm | 7 +++++-- .../firebase_credentials_provider_test.mm | 21 ++++++++++++------- 3 files changed, 22 insertions(+), 10 deletions(-) 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 7760946c507..da6e7f2d949 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 @@ -94,7 +94,9 @@ class FirebaseCredentialsProvider : public CredentialsProvider { */ int user_counter_; - std::mutex mutex_; + // Make it static as as it is used in some of the callbacks. Otherwise, we saw + // mutex lock failed: Invalid argument. + static std::mutex mutex_; }; } // namespace auth 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 27a55eaa43d..c1fa77ba5a3 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 @@ -27,6 +27,8 @@ namespace firestore { namespace auth { +std::mutex FirebaseCredentialsProvider::mutex_; + FirebaseCredentialsProvider::FirebaseCredentialsProvider() : FirebaseCredentialsProvider([FIRApp defaultApp]) { } @@ -81,7 +83,8 @@ User new_user( // Cancel the request since the user changed while the request was // outstanding so the response is likely for a previous user (which // user, we can't be sure). - completion({"", User()}, "getToken aborted due to user change."); + completion({"", User::Unauthenticated()}, + "getToken aborted due to user change."); } else { completion( {firebase::firestore::util::MakeStringView(token), current_user_}, @@ -108,7 +111,7 @@ User new_user( FIREBASE_ASSERT_MESSAGE(user_change_listener_, "user_change_listener removed without being set!"); [[NSNotificationCenter defaultCenter] removeObserver:auth_listener_handle_]; - auth_listener_handle_ = nullptr; + auth_listener_handle_ = nil; } user_change_listener_ = listener; } 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 a979c74c7c4..dc4ab9319ec 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 @@ -29,11 +29,17 @@ namespace auth { // Set a .plist file here to enable the test-case. -static const char* kPlist = "/Users/zxu/Downloads/GoogleService-Info.plist"; +static const char* kPlist = ""; class FirebaseCredentialsProviderTest : public ::testing::Test { protected: void SetUp() override { + app_ready_ = false; + absl::string_view plist(kPlist); + if (plist.length() < 6 || plist.substr(plist.length() - 6) != ".plist") { + return; + } + static dispatch_once_t once_token; dispatch_once(&once_token, ^{ NSString* file_path = @@ -48,14 +54,16 @@ void SetUp() override { default_app.getUIDImplementation = ^NSString* { return @"I'm a fake uid."; }; + app_ready_ = true; } + + bool app_ready_; }; // Set kPlist above before enable. -// TEST(DISABLED_FirebaseCredentialsProvider, GetToken) { +// TEST_F(DISABLED_FirebaseCredentialsProviderTest, GetToken) { TEST_F(FirebaseCredentialsProviderTest, GetToken) { - absl::string_view plist(kPlist); - if (plist.substr(plist.length() - 6) != ".plist") { + if (!app_ready_) { return; } @@ -71,10 +79,9 @@ void SetUp() override { } // Set kPlist above before enable. -// TEST(DISABLED_FirebaseCredentialsProvider, SetListener) { +//TEST_F(DISABLED_FirebaseCredentialsProviderTest, SetListener) { TEST_F(FirebaseCredentialsProviderTest, SetListener) { - absl::string_view plist(kPlist); - if (plist.substr(plist.length() - 6) != ".plist") { + if (!app_ready_) { return; } From 9175715a01ea1f0e63d4ef8068745cb99a566075 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Fri, 2 Feb 2018 13:33:05 -0500 Subject: [PATCH 10/18] fix style --- .../firestore/auth/firebase_credentials_provider_test.mm | 2 -- 1 file changed, 2 deletions(-) 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 dc4ab9319ec..e258e92aae8 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 @@ -61,7 +61,6 @@ void SetUp() override { }; // Set kPlist above before enable. -// TEST_F(DISABLED_FirebaseCredentialsProviderTest, GetToken) { TEST_F(FirebaseCredentialsProviderTest, GetToken) { if (!app_ready_) { return; @@ -79,7 +78,6 @@ void SetUp() override { } // Set kPlist above before enable. -//TEST_F(DISABLED_FirebaseCredentialsProviderTest, SetListener) { TEST_F(FirebaseCredentialsProviderTest, SetListener) { if (!app_ready_) { return; From db6ca1694b91934b8a87a7babf0b8896de05013b Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 5 Feb 2018 14:15:27 -0500 Subject: [PATCH 11/18] address changes --- .../auth/firebase_credentials_provider_apple.h | 16 +++++++--------- .../auth/firebase_credentials_provider_apple.mm | 8 ++++---- .../core/src/firebase/firestore/auth/token.cc | 2 +- .../core/src/firebase/firestore/auth/user.h | 2 +- .../auth/firebase_credentials_provider_test.mm | 10 ++++------ 5 files changed, 17 insertions(+), 21 deletions(-) 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 da6e7f2d949..ebe0ac419d1 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 @@ -16,16 +16,16 @@ // Right now, FirebaseCredentialsProvider only support APPLE build. #if !defined(__OBJC__) -#error "This .header can only be used in .mm file for iOS build." +#error "This header only supports Objective-C++." #endif // !defined(__OBJC__) -#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ -#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_H_ +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_APPLE_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_APPLE_H_ #import #include -#include +#include // NOLINT(build/c++11) #include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" @@ -60,7 +60,7 @@ class FirebaseCredentialsProvider : public CredentialsProvider { * * @param app The Firebase app from which to get credentials. */ - FirebaseCredentialsProvider(FIRApp* app); + explicit FirebaseCredentialsProvider(FIRApp* app); void GetToken(bool force_refresh, TokenListener completion) override; @@ -94,13 +94,11 @@ class FirebaseCredentialsProvider : public CredentialsProvider { */ int user_counter_; - // Make it static as as it is used in some of the callbacks. Otherwise, we saw - // mutex lock failed: Invalid argument. - static std::mutex mutex_; + std::mutex mutex_; }; } // namespace auth } // namespace firestore } // namespace firebase -#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_CREDENTIALS_PROVIDER_H_ +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_AUTH_FIREBASE_CREDENTIALS_PROVIDER_APPLE_H_ 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 c1fa77ba5a3..6eb79669ba0 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 @@ -27,23 +27,23 @@ namespace firestore { namespace auth { -std::mutex FirebaseCredentialsProvider::mutex_; - FirebaseCredentialsProvider::FirebaseCredentialsProvider() : FirebaseCredentialsProvider([FIRApp defaultApp]) { } FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app) : app_(app), + auth_listener_handle_(nil), current_user_(firebase::firestore::util::MakeStringView([app getUID])), - user_counter_(0) { + user_counter_(0), + mutex_() { auth_listener_handle_ = [[NSNotificationCenter defaultCenter] addObserverForName:FIRAuthStateDidChangeInternalNotification object:nil queue:nil usingBlock:^(NSNotification* notification) { std::unique_lock lock(mutex_); - NSDictionary* user_info = notification.userInfo; + NSDictionary* user_info = notification.userInfo; // ensure we're only notifiying for the current app. FIRApp* notified_app = diff --git a/Firestore/core/src/firebase/firestore/auth/token.cc b/Firestore/core/src/firebase/firestore/auth/token.cc index be0055398d2..0618ddb532a 100644 --- a/Firestore/core/src/firebase/firestore/auth/token.cc +++ b/Firestore/core/src/firebase/firestore/auth/token.cc @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h" +#include "Firestore/core/src/firebase/firestore/auth/token.h" namespace firebase { namespace firestore { diff --git a/Firestore/core/src/firebase/firestore/auth/user.h b/Firestore/core/src/firebase/firestore/auth/user.h index e74f791f8c8..58b8b55bfe4 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.h +++ b/Firestore/core/src/firebase/firestore/auth/user.h @@ -36,7 +36,7 @@ class User { User(); /** Construct an authenticated user with the given UID. */ - User(const absl::string_view uid); + explicit User(const absl::string_view uid); const std::string& uid() const { return uid_; 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 e258e92aae8..e451563ff9d 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 @@ -28,24 +28,22 @@ namespace firestore { namespace auth { +// TODO(zxu123): Make this an integration test and get infos from environment. // Set a .plist file here to enable the test-case. -static const char* kPlist = ""; +static NSString *const kPlist = @"/Users/zxu/Downloads/GoogleService-Info.plist"; class FirebaseCredentialsProviderTest : public ::testing::Test { protected: void SetUp() override { app_ready_ = false; - absl::string_view plist(kPlist); - if (plist.length() < 6 || plist.substr(plist.length() - 6) != ".plist") { + if (![kPlist hasSuffix:@".plist"]) { return; } static dispatch_once_t once_token; dispatch_once(&once_token, ^{ - NSString* file_path = - firebase::firestore::util::WrapNSStringNoCopy(kPlist); FIROptions* options = - [[FIROptions alloc] initWithContentsOfFile:file_path]; + [[FIROptions alloc] initWithContentsOfFile:kPlist]; [FIRApp configureWithOptions:options]; }); From 6af9544128b6d78dd68b062e87e31c99b6251516 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 5 Feb 2018 14:25:47 -0500 Subject: [PATCH 12/18] revert the change to static mutex_ --- .../firestore/auth/firebase_credentials_provider_apple.h | 4 +++- .../firestore/auth/firebase_credentials_provider_apple.mm | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) 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 ebe0ac419d1..1874e722f52 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 @@ -94,7 +94,9 @@ class FirebaseCredentialsProvider : public CredentialsProvider { */ int user_counter_; - std::mutex mutex_; + // Make it static as as it is used in some of the callbacks. Otherwise, we saw + // mutex lock failed: Invalid argument. + static std::mutex mutex_; }; } // namespace auth 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 6eb79669ba0..09f9aee08f5 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 @@ -27,6 +27,8 @@ namespace firestore { namespace auth { +std::mutex FirebaseCredentialsProvider::mutex_; + FirebaseCredentialsProvider::FirebaseCredentialsProvider() : FirebaseCredentialsProvider([FIRApp defaultApp]) { } @@ -35,8 +37,7 @@ : app_(app), auth_listener_handle_(nil), current_user_(firebase::firestore::util::MakeStringView([app getUID])), - user_counter_(0), - mutex_() { + user_counter_(0) { auth_listener_handle_ = [[NSNotificationCenter defaultCenter] addObserverForName:FIRAuthStateDidChangeInternalNotification object:nil From 4d4c7a970f7954449749bafd68585288f554fb36 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 5 Feb 2018 14:27:02 -0500 Subject: [PATCH 13/18] remove my custom plist path --- .../firestore/auth/firebase_credentials_provider_test.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e451563ff9d..7e99f86ee2f 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 @@ -30,7 +30,7 @@ // TODO(zxu123): Make this an integration test and get infos from environment. // Set a .plist file here to enable the test-case. -static NSString *const kPlist = @"/Users/zxu/Downloads/GoogleService-Info.plist"; +static NSString *const kPlist = @""; class FirebaseCredentialsProviderTest : public ::testing::Test { protected: From 8bc8550f2d083c64f6b5945711e5c4048f115bbe Mon Sep 17 00:00:00 2001 From: zxu123 Date: Mon, 5 Feb 2018 14:30:05 -0500 Subject: [PATCH 14/18] fix style --- .../firestore/auth/firebase_credentials_provider_apple.mm | 2 +- .../firestore/auth/firebase_credentials_provider_test.mm | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) 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 09f9aee08f5..576c7e35f11 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 @@ -44,7 +44,7 @@ queue:nil usingBlock:^(NSNotification* notification) { std::unique_lock lock(mutex_); - NSDictionary* user_info = notification.userInfo; + NSDictionary* user_info = notification.userInfo; // ensure we're only notifiying for the current app. FIRApp* notified_app = 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 7e99f86ee2f..ba1967c8c97 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 @@ -30,7 +30,7 @@ // TODO(zxu123): Make this an integration test and get infos from environment. // Set a .plist file here to enable the test-case. -static NSString *const kPlist = @""; +static NSString* const kPlist = @""; class FirebaseCredentialsProviderTest : public ::testing::Test { protected: @@ -42,8 +42,7 @@ void SetUp() override { static dispatch_once_t once_token; dispatch_once(&once_token, ^{ - FIROptions* options = - [[FIROptions alloc] initWithContentsOfFile:kPlist]; + FIROptions* options = [[FIROptions alloc] initWithContentsOfFile:kPlist]; [FIRApp configureWithOptions:options]; }); From 9b222e2e4e2858c59e461088fb8886d6247cd82a Mon Sep 17 00:00:00 2001 From: zxu123 Date: Tue, 6 Feb 2018 10:07:32 -0500 Subject: [PATCH 15/18] address changes --- .../auth/firebase_credentials_provider_apple.h | 10 +++++----- .../auth/firebase_credentials_provider_apple.mm | 4 ---- .../auth/firebase_credentials_provider_test.mm | 4 ++-- 3 files changed, 7 insertions(+), 11 deletions(-) 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 1874e722f52..d082d6cef68 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 @@ -71,12 +71,12 @@ class FirebaseCredentialsProvider : public CredentialsProvider { friend class DISABLED_FirebaseCredentialsProviderTest_GetToken_Test; friend class DISABLED_FirebaseCredentialsProviderTest_SetListener_Test; - private: - /** Initialize with default app for internal usage such as test. */ - FirebaseCredentialsProvider(); - - static void PlatformDependentTestSetup(const absl::string_view config_path); + friend class FirebaseCredentialsProviderTest_GetToken_Test; + friend class FirebaseCredentialsProviderTest_SetListener_Test; + friend class DISABLED_FirebaseCredentialsProviderTest_GetToken_Test; + friend class DISABLED_FirebaseCredentialsProviderTest_SetListener_Test; + private: const FIRApp* app_; /** 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 576c7e35f11..eb617a7123d 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 @@ -29,10 +29,6 @@ std::mutex FirebaseCredentialsProvider::mutex_; -FirebaseCredentialsProvider::FirebaseCredentialsProvider() - : FirebaseCredentialsProvider([FIRApp defaultApp]) { -} - FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app) : app_(app), auth_listener_handle_(nil), 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 ba1967c8c97..8222811837d 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 @@ -63,7 +63,7 @@ void SetUp() override { return; } - FirebaseCredentialsProvider credentials_provider; + FirebaseCredentialsProvider credentials_provider([FIRApp defaultApp]); credentials_provider.GetToken( true, [](const Token& token, const absl::string_view error) { EXPECT_EQ("", token.token()); @@ -80,7 +80,7 @@ void SetUp() override { return; } - FirebaseCredentialsProvider credentials_provider; + FirebaseCredentialsProvider credentials_provider([FIRApp defaultApp]); credentials_provider.SetUserChangeListener([](const User& user) { EXPECT_EQ("I'm a fake uid.", user.uid()); EXPECT_TRUE(user.is_authenticated()); From 87175a4146267d403a774f138b85f8d3b532fa4b Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 7 Feb 2018 10:22:14 -0500 Subject: [PATCH 16/18] refactoring FirebaseCredentialsProvider to fix the issue w.r.t. auth global dispatch queue --- .../firestore/auth/credentials_provider.cc | 3 ++ .../firestore/auth/credentials_provider.h | 2 + .../firebase_credentials_provider_apple.h | 39 +++++++++++++------ .../firebase_credentials_provider_apple.mm | 14 +++++-- .../core/src/firebase/firestore/auth/user.cc | 2 +- .../firebase_credentials_provider_test.mm | 19 ++++++++- 6 files changed, 61 insertions(+), 18 deletions(-) diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc index 786f6d26d24..03019441341 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.cc @@ -23,6 +23,9 @@ namespace auth { CredentialsProvider::CredentialsProvider() : user_change_listener_(nullptr) { } +CredentialsProvider::~CredentialsProvider() { +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h index c9419e24539..2a52c99fcf1 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h @@ -45,6 +45,8 @@ class CredentialsProvider { public: CredentialsProvider(); + virtual ~CredentialsProvider(); + /** * Requests token for the current user, optionally forcing a refreshed token * to be fetched. 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 d082d6cef68..3fe1270c818 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 @@ -49,7 +49,30 @@ namespace auth { * from the thread backing our internal worker queue and the callbacks from * FIRAuth will be executed on an arbitrary different thread. * - * For non-Apple desktop build, this is right now just a stub. + * Any instance that has GetToken() calls has to be destructed in + * FIRAuthGlobalWorkQueue i.e through another call to GetToken. This prevents + * the object being destructed before the callback. For example, use the + * following pattern: + * + * class Bar { + * Bar(): provider_(new FirebaseCredentialsProvider([FIRApp defaultApp])) {} + * + * ~Bar() { + * credentials_provider->GetToken( + * false, [provider_](const Token& token, const absl::string_view error) + * { delete provider_; + * }); + * } + * + * Foo() { + * credentials_provider->GetToken( + * true, [](const Token& token, const absl::string_view error) { + * ... ... + * }); + * } + * + * FirebaseCredentialsProvider* provider_; + * }; */ class FirebaseCredentialsProvider : public CredentialsProvider { public: @@ -62,20 +85,12 @@ class FirebaseCredentialsProvider : public CredentialsProvider { */ explicit FirebaseCredentialsProvider(FIRApp* app); + ~FirebaseCredentialsProvider() override; + void GetToken(bool force_refresh, TokenListener completion) override; void SetUserChangeListener(UserChangeListener listener) override; - friend class FirebaseCredentialsProviderTest_GetToken_Test; - friend class FirebaseCredentialsProviderTest_SetListener_Test; - friend class DISABLED_FirebaseCredentialsProviderTest_GetToken_Test; - friend class DISABLED_FirebaseCredentialsProviderTest_SetListener_Test; - - friend class FirebaseCredentialsProviderTest_GetToken_Test; - friend class FirebaseCredentialsProviderTest_SetListener_Test; - friend class DISABLED_FirebaseCredentialsProviderTest_GetToken_Test; - friend class DISABLED_FirebaseCredentialsProviderTest_SetListener_Test; - private: const FIRApp* app_; @@ -96,7 +111,7 @@ class FirebaseCredentialsProvider : public CredentialsProvider { // Make it static as as it is used in some of the callbacks. Otherwise, we saw // mutex lock failed: Invalid argument. - static std::mutex mutex_; + std::mutex mutex_; }; } // namespace auth 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 eb617a7123d..980180b47ed 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 @@ -27,13 +27,12 @@ namespace firestore { namespace auth { -std::mutex FirebaseCredentialsProvider::mutex_; - FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app) : app_(app), auth_listener_handle_(nil), current_user_(firebase::firestore::util::MakeStringView([app getUID])), - user_counter_(0) { + user_counter_(0), + mutex_() { auth_listener_handle_ = [[NSNotificationCenter defaultCenter] addObserverForName:FIRAuthStateDidChangeInternalNotification object:nil @@ -64,6 +63,15 @@ User new_user( }]; } +FirebaseCredentialsProvider::~FirebaseCredentialsProvider() { + if (auth_listener_handle_) { + // For iOS 9.0 and later or macOS 10.11 and later, it is not required to + // unregister an observer in dealloc. Nothing is said for C++ destruction + // and thus we do it here just to be sure. + [[NSNotificationCenter defaultCenter] removeObserver:auth_listener_handle_]; + } +} + void FirebaseCredentialsProvider::GetToken(bool force_refresh, TokenListener completion) { FIREBASE_ASSERT_MESSAGE(auth_listener_handle_, diff --git a/Firestore/core/src/firebase/firestore/auth/user.cc b/Firestore/core/src/firebase/firestore/auth/user.cc index 4793fed100d..f442d7b3d3d 100644 --- a/Firestore/core/src/firebase/firestore/auth/user.cc +++ b/Firestore/core/src/firebase/firestore/auth/user.cc @@ -22,7 +22,7 @@ namespace firebase { namespace firestore { namespace auth { -User::User() : is_authenticated_(false) { +User::User() : uid_(), is_authenticated_(false) { } User::User(const absl::string_view uid) : uid_(uid), is_authenticated_(true) { 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 8222811837d..85a9cd7acb0 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 @@ -24,6 +24,8 @@ #include "gtest/gtest.h" +#define UNUSED(x) (void)(x) + namespace firebase { namespace firestore { namespace auth { @@ -63,8 +65,11 @@ void SetUp() override { return; } - FirebaseCredentialsProvider credentials_provider([FIRApp defaultApp]); - credentials_provider.GetToken( + // GetToken() registers callback and thus we do not allocate it in stack. + FirebaseCredentialsProvider* credentials_provider = + new FirebaseCredentialsProvider([FIRApp defaultApp]); + + credentials_provider->GetToken( true, [](const Token& token, const absl::string_view error) { EXPECT_EQ("", token.token()); const User& user = token.user(); @@ -72,6 +77,16 @@ void SetUp() override { EXPECT_TRUE(user.is_authenticated()); EXPECT_EQ("", error) << error; }); + + // Destruct credentials_provider via the FIRAuthGlobalWorkQueue, which is + // serial. + credentials_provider->GetToken( + false, [credentials_provider](const Token& token, + const absl::string_view error) { + UNUSED(token); + UNUSED(error); + delete credentials_provider; + }); } // Set kPlist above before enable. From 485f8373940d7114939614bcd4dcbc6567e4cbc3 Mon Sep 17 00:00:00 2001 From: zxu123 Date: Wed, 7 Feb 2018 10:41:55 -0500 Subject: [PATCH 17/18] add /*force_refresh=*/ tag to bool literal for style purpose --- .../firestore/auth/firebase_credentials_provider_test.mm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 85a9cd7acb0..c9ed030ac29 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 @@ -70,7 +70,8 @@ void SetUp() override { new FirebaseCredentialsProvider([FIRApp defaultApp]); credentials_provider->GetToken( - true, [](const Token& token, const absl::string_view error) { + /*force_refresh=*/true, + [](const Token& token, const absl::string_view error) { EXPECT_EQ("", token.token()); const User& user = token.user(); EXPECT_EQ("I'm a fake uid.", user.uid()); @@ -81,8 +82,9 @@ void SetUp() override { // Destruct credentials_provider via the FIRAuthGlobalWorkQueue, which is // serial. credentials_provider->GetToken( - false, [credentials_provider](const Token& token, - const absl::string_view error) { + /*force_refresh=*/false, + [credentials_provider](const Token& token, + const absl::string_view error) { UNUSED(token); UNUSED(error); delete credentials_provider; From c5ae8448442cbb213d2610379a3ae6377ee158ca Mon Sep 17 00:00:00 2001 From: Gil Date: Fri, 9 Feb 2018 11:00:27 -0800 Subject: [PATCH 18/18] Use a shared_ptr/weak_ptr handoff on FirebaseCredentialsProvider (#778) * Revert "refactoring FirebaseCredentialsProvider to fix the issue w.r.t. auth global dispatch queue" This reverts commit 87175a4146267d403a774f138b85f8d3b532fa4b. * Use a shared_ptr/weak_ptr handoff on FirebaseCredentialsProvider This avoids any problems with callsbacks retaining pointers to objects destroyed by a C++ destructor --- .../firebase_credentials_provider_apple.h | 65 ++++++-------- .../firebase_credentials_provider_apple.mm | 86 ++++++++++--------- .../firebase_credentials_provider_test.mm | 23 ++--- 3 files changed, 80 insertions(+), 94 deletions(-) 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 3fe1270c818..65c4c655ae3 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 @@ -49,30 +49,7 @@ namespace auth { * from the thread backing our internal worker queue and the callbacks from * FIRAuth will be executed on an arbitrary different thread. * - * Any instance that has GetToken() calls has to be destructed in - * FIRAuthGlobalWorkQueue i.e through another call to GetToken. This prevents - * the object being destructed before the callback. For example, use the - * following pattern: - * - * class Bar { - * Bar(): provider_(new FirebaseCredentialsProvider([FIRApp defaultApp])) {} - * - * ~Bar() { - * credentials_provider->GetToken( - * false, [provider_](const Token& token, const absl::string_view error) - * { delete provider_; - * }); - * } - * - * Foo() { - * credentials_provider->GetToken( - * true, [](const Token& token, const absl::string_view error) { - * ... ... - * }); - * } - * - * FirebaseCredentialsProvider* provider_; - * }; + * For non-Apple desktop build, this is right now just a stub. */ class FirebaseCredentialsProvider : public CredentialsProvider { public: @@ -92,26 +69,40 @@ class FirebaseCredentialsProvider : public CredentialsProvider { void SetUserChangeListener(UserChangeListener listener) override; private: - const FIRApp* app_; - /** - * Handle used to stop receiving auth changes once userChangeListener is - * removed. + * Most contents of the FirebaseCredentialProvider are kept in this + * Contents object and pointed to with a shared pointer. Callbacks + * registered with FirebaseAuth use weak pointers to the Contents to + * avoid races between notifications arriving and C++ object destruction. */ - id auth_listener_handle_; + struct Contents { + Contents(FIRApp* app, const absl::string_view uid) + : app(app), current_user(uid), mutex() { + } + + const FIRApp* app; + + /** + * The current user as reported to us via our AuthStateDidChangeListener. + */ + User current_user; + + /** + * Counter used to detect if the user changed while a + * -getTokenForcingRefresh: request was outstanding. + */ + int user_counter = 0; - /** The current user as reported to us via our AuthStateDidChangeListener. */ - User current_user_; + std::mutex mutex; + }; /** - * Counter used to detect if the user changed while a -getTokenForcingRefresh: - * request was outstanding. + * Handle used to stop receiving auth changes once userChangeListener is + * removed. */ - int user_counter_; + id auth_listener_handle_; - // Make it static as as it is used in some of the callbacks. Otherwise, we saw - // mutex lock failed: Invalid argument. - std::mutex mutex_; + std::shared_ptr contents_; }; } // namespace auth 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 980180b47ed..f463958a6f2 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 @@ -28,36 +28,39 @@ namespace auth { FirebaseCredentialsProvider::FirebaseCredentialsProvider(FIRApp* app) - : app_(app), - auth_listener_handle_(nil), - current_user_(firebase::firestore::util::MakeStringView([app getUID])), - user_counter_(0), - mutex_() { + : contents_( + std::make_shared(app, util::MakeStringView([app getUID]))) { + std::weak_ptr weak_contents = contents_; + auth_listener_handle_ = [[NSNotificationCenter defaultCenter] addObserverForName:FIRAuthStateDidChangeInternalNotification object:nil queue:nil usingBlock:^(NSNotification* notification) { - std::unique_lock lock(mutex_); + std::shared_ptr contents = weak_contents.lock(); + if (!contents) { + return; + } + + std::unique_lock lock(contents->mutex); NSDictionary* user_info = notification.userInfo; // ensure we're only notifiying for the current app. FIRApp* notified_app = user_info[FIRAuthStateDidChangeInternalNotificationAppKey]; - if (![app_ isEqual:notified_app]) { + if (![contents->app isEqual:notified_app]) { return; } NSString* user_id = user_info[FIRAuthStateDidChangeInternalNotificationUIDKey]; - User new_user( - firebase::firestore::util::MakeStringView(user_id)); - if (new_user != current_user_) { - current_user_ = new_user; - user_counter_++; + User new_user(util::MakeStringView(user_id)); + if (new_user != contents->current_user) { + contents->current_user = new_user; + contents->user_counter++; UserChangeListener listener = user_change_listener_; if (listener) { - listener(current_user_); + listener(contents->current_user); } } }]; @@ -65,9 +68,9 @@ User new_user( FirebaseCredentialsProvider::~FirebaseCredentialsProvider() { if (auth_listener_handle_) { - // For iOS 9.0 and later or macOS 10.11 and later, it is not required to - // unregister an observer in dealloc. Nothing is said for C++ destruction - // and thus we do it here just to be sure. + // Even though iOS 9 (and later) and macOS 10.11 (and later) keep a weak + // reference to the observer so we could avoid this removeObserver call, we + // still support iOS 8 which requires it. [[NSNotificationCenter defaultCenter] removeObserver:auth_listener_handle_]; } } @@ -79,37 +82,42 @@ User new_user( // Take note of the current value of the userCounter so that this method can // fail if there is a user change while the request is outstanding. - int initial_user_counter = user_counter_; - - void (^get_token_callback)(NSString*, NSError*) = - ^(NSString* _Nullable token, NSError* _Nullable error) { - std::unique_lock lock(mutex_); - if (initial_user_counter != user_counter_) { - // Cancel the request since the user changed while the request was - // outstanding so the response is likely for a previous user (which - // user, we can't be sure). - completion({"", User::Unauthenticated()}, - "getToken aborted due to user change."); - } else { - completion( - {firebase::firestore::util::MakeStringView(token), current_user_}, - error == nil ? "" - : firebase::firestore::util::MakeStringView( - error.localizedDescription)); - } - }; - - [app_ getTokenForcingRefresh:force_refresh withCallback:get_token_callback]; + int initial_user_counter = contents_->user_counter; + + std::weak_ptr weak_contents = contents_; + void (^get_token_callback)(NSString*, NSError*) = ^( + NSString* _Nullable token, NSError* _Nullable error) { + std::shared_ptr contents = weak_contents.lock(); + if (!contents) { + return; + } + + std::unique_lock lock(contents->mutex); + if (initial_user_counter != contents->user_counter) { + // Cancel the request since the user changed while the request was + // outstanding so the response is likely for a previous user (which + // user, we can't be sure). + completion({"", User::Unauthenticated()}, + "getToken aborted due to user change."); + } else { + completion( + {util::MakeStringView(token), contents->current_user}, + error == nil ? "" : util::MakeStringView(error.localizedDescription)); + } + }; + + [contents_->app getTokenForcingRefresh:force_refresh + withCallback:get_token_callback]; } void FirebaseCredentialsProvider::SetUserChangeListener( UserChangeListener listener) { - std::unique_lock lock(mutex_); + std::unique_lock lock(contents_->mutex); if (listener) { FIREBASE_ASSERT_MESSAGE(!user_change_listener_, "set user_change_listener twice!"); // Fire initial event. - listener(current_user_); + listener(contents_->current_user); } else { FIREBASE_ASSERT_MESSAGE(auth_listener_handle_, "removed user_change_listener twice!"); 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 c9ed030ac29..8d2b361b965 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 @@ -24,8 +24,6 @@ #include "gtest/gtest.h" -#define UNUSED(x) (void)(x) - namespace firebase { namespace firestore { namespace auth { @@ -65,11 +63,8 @@ void SetUp() override { return; } - // GetToken() registers callback and thus we do not allocate it in stack. - FirebaseCredentialsProvider* credentials_provider = - new FirebaseCredentialsProvider([FIRApp defaultApp]); - - credentials_provider->GetToken( + FirebaseCredentialsProvider credentials_provider([FIRApp defaultApp]); + credentials_provider.GetToken( /*force_refresh=*/true, [](const Token& token, const absl::string_view error) { EXPECT_EQ("", token.token()); @@ -78,17 +73,6 @@ void SetUp() override { EXPECT_TRUE(user.is_authenticated()); EXPECT_EQ("", error) << error; }); - - // Destruct credentials_provider via the FIRAuthGlobalWorkQueue, which is - // serial. - credentials_provider->GetToken( - /*force_refresh=*/false, - [credentials_provider](const Token& token, - const absl::string_view error) { - UNUSED(token); - UNUSED(error); - delete credentials_provider; - }); } // Set kPlist above before enable. @@ -103,6 +87,9 @@ void SetUp() override { EXPECT_TRUE(user.is_authenticated()); }); + // TODO(wilhuff): We should wait for the above expectations to actually happen + // before continuing. + credentials_provider.SetUserChangeListener(nullptr); }