Skip to content

gRPC: replace Objective-C implementation with the new C++ implementation #1968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
Oct 26, 2018
Merged

gRPC: replace Objective-C implementation with the new C++ implementation #1968

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
805b673
gRPC: add support for SSL disabled to `GrpcConnection`.
var-const Oct 18, 2018
839f408
gRPC: make gRPC calls using the C++ implementation:
var-const Oct 18, 2018
a93b783
C++: port `MockWatchStream` and `MockWriteStream` to C++.
var-const Oct 18, 2018
4fe32a1
No longer generate Objective-C gRPC service definitions from protos.
var-const Oct 18, 2018
0ea6fcc
Remove all references to Objective-C gRPC client.
var-const Oct 18, 2018
21d30b6
Use local certificate for now
var-const Oct 22, 2018
7f60ad0
Fix/improve ASCII conversion
var-const Oct 22, 2018
feb0285
Review feedback
var-const Oct 22, 2018
8332a5f
style.sh
var-const Oct 22, 2018
ab6df2e
Appease linter
var-const Oct 22, 2018
9f9d94d
Undo accidental change
var-const Oct 22, 2018
555e1d8
missing word
var-const Oct 22, 2018
6f0d4e6
add comment
var-const Oct 22, 2018
84e454b
Remove leftover reference to Objective-C gRPC
var-const Oct 22, 2018
c96f670
More using declarations
var-const Oct 22, 2018
e2e53b0
typo
var-const Oct 22, 2018
1bcfc0f
Use Path instead of plain string
var-const Oct 22, 2018
2d68f12
Missed include
var-const Oct 22, 2018
80a938e
Remove redundant word
var-const Oct 22, 2018
9c1dec6
Remove ASCII conversion when loading certificates, it is not necessary
var-const Oct 23, 2018
d638062
Compilation fix
var-const Oct 23, 2018
abceb14
Review feedback
var-const Oct 23, 2018
dc872a7
Tweak comment
var-const Oct 23, 2018
c4d99d7
Move certificate to Firestore/etc/roots.pem
var-const Oct 23, 2018
90c3b43
Forgotten file
var-const Oct 24, 2018
cf856e6
Review feedback
var-const Oct 24, 2018
33555c9
Forgotten comment
var-const Oct 24, 2018
f1c8f5d
Fix test
var-const Oct 24, 2018
1a5acc7
Review feedback
var-const Oct 25, 2018
101b611
Fix flaky test
var-const Oct 25, 2018
1f6ee21
Merge branch 'master' into varconst/grpc-integr82
var-const Oct 25, 2018
348a924
Fix bad port
var-const Oct 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion FirebaseFirestore.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,12 @@ Google Cloud Firestore is a NoSQL document database built for automatic scaling,
]
s.public_header_files = 'Firestore/Source/Public/*.h'

# TODO(varconst): remove once https://github.com/grpc/grpc/pull/16962 makes it
# into a release.
s.resource_bundles = { 'gRPCCertificates' => ['Firestore/etc/roots.pem'] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you've deleted the old location, but this commit doesn't seem to have added the new location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fixed.


s.dependency 'FirebaseAuthInterop', '~> 1.0'
s.dependency 'FirebaseCore', '~> 5.1'
s.dependency 'gRPC-ProtoRPC', '~> 1.0'
s.dependency 'gRPC-C++', '~> 0.0.3'
s.dependency 'leveldb-library', '~> 1.20'
s.dependency 'Protobuf', '~> 3.1'
Expand Down
86 changes: 38 additions & 48 deletions Firestore/Example/Firestore.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
5492E07A202154D600B64F25 /* FIRTypeTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E071202154D600B64F25 /* FIRTypeTests.mm */; };
5492E07F202154EC00B64F25 /* FSTTransactionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E07B202154EB00B64F25 /* FSTTransactionTests.mm */; };
5492E080202154EC00B64F25 /* FSTSmokeTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E07C202154EB00B64F25 /* FSTSmokeTests.mm */; };
5492E081202154EC00B64F25 /* FSTStreamTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E07D202154EB00B64F25 /* FSTStreamTests.mm */; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of unexpected changes to this file. Is that on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are on purpose:

  • FSTStreamTests was deleted. Most of the tests it contained are covered by unit tests, and integration tests are more effort to set up, so my intuition is not to create a C++ version of this file.
  • this PR intends to delete all references to gRPC Objective-C client, so it removes the dependency on related pods (GRPCClient is the Objective-C client, ProtoRPC and RxLibrary are its dependencies).
  • As far as I can tell, directly depending upon gRPC-Core pod is unnecessary, because gRPC-C++ has a dependency on it.

Let me know if I overlooked any changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually FSTStreamTests is cross platform i.e. the same test cases are also in Android native client code. However, they are not based on public API. I guess it is OK to remove them for now. But my 1 cent is that integration tests and unit tests are compliment each other (we may want to add one one day).

5492E082202154EC00B64F25 /* FSTDatastoreTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E07E202154EC00B64F25 /* FSTDatastoreTests.mm */; };
5492E09D2021552D00B64F25 /* FSTLocalStoreTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0832021552A00B64F25 /* FSTLocalStoreTests.mm */; };
5492E09F2021552D00B64F25 /* FSTLevelDBMigrationsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E0862021552A00B64F25 /* FSTLevelDBMigrationsTests.mm */; };
Expand Down Expand Up @@ -360,7 +359,6 @@
5492E071202154D600B64F25 /* FIRTypeTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FIRTypeTests.mm; sourceTree = "<group>"; };
5492E07B202154EB00B64F25 /* FSTTransactionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTTransactionTests.mm; sourceTree = "<group>"; };
5492E07C202154EB00B64F25 /* FSTSmokeTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTSmokeTests.mm; sourceTree = "<group>"; };
5492E07D202154EB00B64F25 /* FSTStreamTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTStreamTests.mm; sourceTree = "<group>"; };
5492E07E202154EC00B64F25 /* FSTDatastoreTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTDatastoreTests.mm; sourceTree = "<group>"; };
5492E0832021552A00B64F25 /* FSTLocalStoreTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTLocalStoreTests.mm; sourceTree = "<group>"; };
5492E0852021552A00B64F25 /* FSTRemoteDocumentCacheTests.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FSTRemoteDocumentCacheTests.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1231,7 +1229,6 @@
DE03B3621F215E1600A30B9C /* CAcert.pem */,
5492E07E202154EC00B64F25 /* FSTDatastoreTests.mm */,
5492E07C202154EB00B64F25 /* FSTSmokeTests.mm */,
5492E07D202154EB00B64F25 /* FSTStreamTests.mm */,
5492E07B202154EB00B64F25 /* FSTTransactionTests.mm */,
);
path = Integration;
Expand Down Expand Up @@ -1267,6 +1264,7 @@
54C9EDEE2040E16300A969CD /* Frameworks */,
54C9EDEF2040E16300A969CD /* Resources */,
EA424838F4A5DD7B337F57AB /* [CP] Embed Pods Frameworks */,
DE5C36328822481F6EB6EF16 /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand Down Expand Up @@ -1307,6 +1305,7 @@
6003F587195388D20070C39A /* Frameworks */,
6003F588195388D20070C39A /* Resources */,
1EE692C7509A98D7EB03CA51 /* [CP] Embed Pods Frameworks */,
1E7DAED3207D01F1744EA227 /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand Down Expand Up @@ -1507,6 +1506,24 @@
/* End PBXResourcesBuildPhase section */

/* Begin PBXShellScriptBuildPhase section */
1E7DAED3207D01F1744EA227 /* [CP] Copy Pods Resources */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
files = (
);
inputPaths = (
"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS-resources.sh",
"${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates.bundle",
);
name = "[CP] Copy Pods Resources";
outputPaths = (
"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates.bundle",
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS-resources.sh\"\n";
showEnvVarsInLog = 0;
};
1EE692C7509A98D7EB03CA51 /* [CP] Embed Pods Frameworks */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
Expand All @@ -1518,11 +1535,8 @@
"${BUILT_PRODUCTS_DIR}/GTMSessionFetcher/GTMSessionFetcher.framework",
"${BUILT_PRODUCTS_DIR}/GoogleUtilities/GoogleUtilities.framework",
"${BUILT_PRODUCTS_DIR}/Protobuf-iOS8.0/Protobuf.framework",
"${BUILT_PRODUCTS_DIR}/gRPC/GRPCClient.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-C++/grpcpp.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-Core/grpc.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-ProtoRPC/ProtoRPC.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-RxLibrary/RxLibrary.framework",
"${BUILT_PRODUCTS_DIR}/leveldb-library/leveldb.framework",
"${BUILT_PRODUCTS_DIR}/nanopb/nanopb.framework",
);
Expand All @@ -1532,11 +1546,8 @@
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/GTMSessionFetcher.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/GoogleUtilities.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/Protobuf.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/GRPCClient.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/grpcpp.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/grpc.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/ProtoRPC.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/RxLibrary.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/leveldb.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/nanopb.framework",
);
Expand Down Expand Up @@ -1754,6 +1765,24 @@
shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n # print error to STDERR\n echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n exit 1\nfi\n# This output is used by Xcode 'outputs' to avoid re-running this script phase.\necho \"SUCCESS\" > \"${SCRIPT_OUTPUT_FILE_0}\"\n";
showEnvVarsInLog = 0;
};
DE5C36328822481F6EB6EF16 /* [CP] Copy Pods Resources */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
files = (
);
inputPaths = (
"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS-resources.sh",
"${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates.bundle",
);
name = "[CP] Copy Pods Resources";
outputPaths = (
"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates.bundle",
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS-resources.sh\"\n";
showEnvVarsInLog = 0;
};
EA424838F4A5DD7B337F57AB /* [CP] Embed Pods Frameworks */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
Expand All @@ -1765,11 +1794,8 @@
"${BUILT_PRODUCTS_DIR}/GTMSessionFetcher/GTMSessionFetcher.framework",
"${BUILT_PRODUCTS_DIR}/GoogleUtilities/GoogleUtilities.framework",
"${BUILT_PRODUCTS_DIR}/Protobuf-iOS8.0/Protobuf.framework",
"${BUILT_PRODUCTS_DIR}/gRPC/GRPCClient.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-C++/grpcpp.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-Core/grpc.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-ProtoRPC/ProtoRPC.framework",
"${BUILT_PRODUCTS_DIR}/gRPC-RxLibrary/RxLibrary.framework",
"${BUILT_PRODUCTS_DIR}/leveldb-library/leveldb.framework",
"${BUILT_PRODUCTS_DIR}/nanopb/nanopb.framework",
);
Expand All @@ -1779,11 +1805,8 @@
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/GTMSessionFetcher.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/GoogleUtilities.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/Protobuf.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/GRPCClient.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/grpcpp.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/grpc.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/ProtoRPC.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/RxLibrary.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/leveldb.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/nanopb.framework",
);
Expand Down Expand Up @@ -2002,7 +2025,6 @@
5492E0422021440500B64F25 /* FSTHelpers.mm in Sources */,
5491BC731FB44593008B3588 /* FSTIntegrationTestCase.mm in Sources */,
5492E080202154EC00B64F25 /* FSTSmokeTests.mm in Sources */,
5492E081202154EC00B64F25 /* FSTStreamTests.mm in Sources */,
5492E07F202154EC00B64F25 /* FSTTransactionTests.mm in Sources */,
5492E0442021457E00B64F25 /* XCTestCase+Await.mm in Sources */,
B67BF44A216EB43000CA9097 /* create_noop_connectivity_monitor.cc in Sources */,
Expand Down Expand Up @@ -2217,14 +2239,6 @@
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/Protobuf/Protobuf.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-Core/grpc.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-ProtoRPC/ProtoRPC.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-RxLibrary/RxLibrary.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC/GRPCClient.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/leveldb-library/leveldb.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/nanopb/nanopb.framework/Headers\"",
Expand Down Expand Up @@ -2301,14 +2315,6 @@
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/Protobuf/Protobuf.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-Core/grpc.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-ProtoRPC/ProtoRPC.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-RxLibrary/RxLibrary.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC/GRPCClient.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/leveldb-library/leveldb.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/nanopb/nanopb.framework/Headers\"",
Expand Down Expand Up @@ -2510,14 +2516,6 @@
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/Protobuf/Protobuf.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-Core/grpc.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-ProtoRPC/ProtoRPC.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-RxLibrary/RxLibrary.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC/GRPCClient.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/leveldb-library/leveldb.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/nanopb/nanopb.framework/Headers\"",
Expand Down Expand Up @@ -2595,14 +2593,6 @@
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/Protobuf/Protobuf.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-Core/grpc.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-ProtoRPC/ProtoRPC.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC-RxLibrary/RxLibrary.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/gRPC/GRPCClient.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/leveldb-library/leveldb.framework/Headers\"",
"-iquote",
"\"${PODS_CONFIGURATION_BUILD_DIR}/nanopb/nanopb.framework/Headers\"",
Expand Down
5 changes: 2 additions & 3 deletions Firestore/Example/Tests/Integration/FSTDatastoreTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#import <FirebaseFirestore/FirebaseFirestore.h>

#import <FirebaseFirestore/FIRTimestamp.h>
#import <GRPCClient/GRPCCall+ChannelCredentials.h>
#import <GRPCClient/GRPCCall+Tests.h>
#import <XCTest/XCTest.h>

#import "Firestore/Source/API/FIRDocumentReference+Internal.h"
Expand Down Expand Up @@ -52,6 +50,7 @@
using firebase::firestore::model::DocumentKeySet;
using firebase::firestore::model::Precondition;
using firebase::firestore::model::TargetId;
using firebase::firestore::remote::GrpcConnection;

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -162,7 +161,7 @@ - (void)setUp {
NSString *projectID = [FSTIntegrationTestCase projectID];
FIRFirestoreSettings *settings = [FSTIntegrationTestCase settings];
if (!settings.sslEnabled) {
[GRPCCall useInsecureConnectionsForHost:settings.host];
GrpcConnection::UseInsecureChannel(util::MakeString(settings.host));
}

DatabaseId database_id(util::MakeString(projectID), DatabaseId::kDefault);
Expand Down
Loading