IID: version dependent on FirebaseInstallations #4533

maksymmalyhin · 2019-12-19T21:15:23Z

The contains:

InstanceID version that uses FIS iOS SDK to generate store and retrieve IID
related FCM tests changes

The changes in the branch have been already reviewed in the PRs into the branch, but a brief review is appreciated anyway.

…ase-ios-sdk into mm/fis-integration-master

* Fix IID FCM token check. * Unused constant deleted. * FCM tests: fix bad merge.

…ase-ios-sdk into mm/fis-integration-master

#4482) * IID: use a cached value of FID for -[FIRInstanceID appInstanceID:] * TODO comment * TODO updated

…4522) * Bump FIrebaseInstallations to 1.0.0 * Travis: remove mm/fis-integration-master branch * Travis: if_changed script updated to reflect IID dependency on FIS. * Revert "Travis: remove mm/fis-integration-master branch" This reverts commit 534dff9.

charlotteliang

This is awesome and thank you for cleaning up the keychain!

charlotteliang · 2019-12-19T21:27:24Z

Firebase/InstanceID/FIRInstanceIDKeychain.m

@@ -16,14 +16,10 @@

 #import "FIRInstanceIDKeychain.h"


This might be some thing I can help in another PR but I wonder if FIRInstanceIDKeychain is needed at all after FIS is in.

As far as I can see it is still used by FIRInstanceIDAuthKeyChain which is used in the FIRInstanceIDTokenManager, FIRInstanceIDCheckinStore and couple more classes.

I see, so it's used to store token and checkin, not just IID. We can deal with that later then. Thanks!

paulb777 · 2019-12-19T21:58:44Z

Example/Firebase.xcodeproj/project.pbxproj

@@ -2059,6 +2050,13 @@
 			name = "Podspec Metadata";
 			sourceTree = "<group>";
 		};
+		85A00DB227791D3B865D0CE2 /* Pods */ = {


paulb777 · 2019-12-19T21:59:21Z

Example/InstanceID/Tests/FIRInstanceIDTokenInfoTest.m

@@ -194,4 +194,5 @@ - (void)testTokenInconsistentWithIID {
                 firebaseAppID:FIRInstanceIDFirebaseAppID()];
  XCTAssertFalse([self.validTokenInfo isFreshWithIID:kIID]);
 }
+


Delete unless this is a style requirement.

paulb777 · 2019-12-19T22:00:28Z

Example/InstanceID/Tests/FIRInstanceIDTokenManagerTest.m

@@ -56,23 +59,23 @@ - (BOOL)checkTokenRefreshPolicyForIID:(NSString *)IID;
 - (void)updateToAPNSDeviceToken:(NSData *)deviceToken isSandbox:(BOOL)isSandbox;
 /**
 *  Create a fetch operation. This method can be stubbed to return a particular operation instance,
- *  which makes it easier to unit test different behaviors.
+ *  which makes it easier to unit test different behaviours.


Hmm, an American to Canadian change. @ryanwilson Do we have any standard guidance about English dialects in comments?

I can revert it. I just followed Xcode suggestions

paulb777 · 2019-12-19T22:02:14Z

Firebase/InstanceID/FIRInstanceIDTokenManager.h

@@ -46,7 +45,7 @@ typedef NS_OPTIONS(NSUInteger, FIRInstanceIDInvalidTokenReason) {
 *
 *  @param authorizedEntity The authorized entity for the token, should not be nil.
 *  @param scope            The scope for the token, should not be nil.
- *  @param keyPair          The keyPair that represents the app identity.
+ *  @param instanceID   The unique string identifying the app instance.


inconsistent indent

paulb777 · 2019-12-19T22:02:23Z

Firebase/InstanceID/FIRInstanceIDTokenManager.h

@@ -77,7 +76,7 @@ typedef NS_OPTIONS(NSUInteger, FIRInstanceIDInvalidTokenReason) {
 *
 *  @param authorizedEntity The authorized entity for the token, should not be nil.
 *  @param scope            The scope for the token, should not be nil.
- *  @param keyPair          The keyPair that represents the app identity.
+ *  @param instanceID   The unique string identifying the app instance.


maksymmalyhin · 2019-12-19T22:26:39Z

@paulb777 Thank you for the review! I pushed the fixes.

paulb777 · 2019-12-19T22:44:55Z

Note that if you want to keep the individual commits in the git history, do a command line merge to master.

maksymmalyhin · 2019-12-19T22:50:44Z

@paulb777 Initially I though not to keep the separate commits, though it is probably a good idea if we are fine with several extra commit pushed to the branch without PRs (the recent ones and conflict resolutions from the past). WDYT?

paulb777 · 2019-12-20T15:17:50Z

I'm ok either way. Even if you squash, you can still include the PR's from the commit message that GitHub generates automatically.

maksymmalyhin · 2019-12-20T15:54:06Z

I'll squash and merge including the PR's in the commit message.

maksymmalyhin and others added 30 commits July 26, 2019 12:52

Firebase Installations: Instance ID now uses FIS under the hood. (#3398)

8dbd5da

Merge branch 'master' into mm/fis-integration-master

fb6c94a

Fix merge errors

5b25ae2

Use modular import for FIRInstanceID_Private.h

46d7a30

Use modular imports for FirebaseInstanceID.h (#3509)

e2783bc

Merge branch 'master' into mm/fis-integration-master

94d6fba

Merge branch 'master' into mm/fis-integration-master

4ada515

Merge branch 'master' into mm/fis-integration-master

99aa832

Changes missing during the fix.

f720ba0

Changes missing during merge.

44d9a20

Merge branch 'mm/fis-integration-master' of github.com:firebase/fireb…

f8b5c01

…ase-ios-sdk into mm/fis-integration-master

Merge branch 'master' into mm/fis-integration-master

f6e938a

Merge branch 'master' into mm/fis-integration-master

d78b2b7

Fix header import.

2acf4eb

./scripts/style.sh

b27aee6

Merge branch 'master' into mm/fis-integration-master

60d781d

Merge branch 'master' into mm/fis-integration-master

afd2d79

Merge branch 'master' into mm/fis-integration-master

058e68f

Merge branch 'mm/fis-integration-master' of github.com:firebase/fireb…

356c7ee

…ase-ios-sdk into mm/fis-integration-master

Fix FCM tests. (#3888)

ecd4bac

Merge branch 'master' into mm/fis-integration-master

ccbcf70

Merge branch 'master' into mm/fis-integration-master

8bde188

Fix IID FCM token check.

d798eb1

FirebaseInstallation: merge master into FIS integration branch (#4076)

f8894d0

* Fix IID FCM token check. * Unused constant deleted. * FCM tests: fix bad merge.

Merge branch 'master' into mm/fis-integration-master

6a12c65

Merge branch 'mm/fis-integration-master' of github.com:firebase/fireb…

e059561

…ase-ios-sdk into mm/fis-integration-master

Remote Config test app: use local IID and FIS. (#4089)

58df881

token request should pass through if token is not fresh (#4104)

abe3709

Merge branch 'master' into mm/fis-integration-master

f62f082

Merge errors fix.

a159792

maksymmalyhin added 7 commits November 27, 2019 12:28

Merge branch 'master' into mm/fis-integration-master

980f1df

Merge branch 'master' into mm/fis-integration-master

faae192

Make -[FIRInstanceID appInstanceID:] safe to be called from any thread (

46a7934

#4482) * IID: use a cached value of FID for -[FIRInstanceID appInstanceID:] * TODO comment * TODO updated

Merge branch 'master' into mm/fis-integration-master

60318e4

Merge branch 'master' into mm/fis-integration-master

759b9f4

Travis: remove mm/fis-integration-master branch (#4526)

c159e38

maksymmalyhin added this to the M62 milestone Dec 19, 2019

maksymmalyhin requested review from charlotteliang and paulb777 December 19, 2019 21:15

googlebot added the cla: yes label Dec 19, 2019

charlotteliang approved these changes Dec 19, 2019

View reviewed changes

google-oss-bot added api: inappmessaging Firebase In App Messaging api: instanceid api: remoteconfig labels Dec 19, 2019

paulb777 reviewed Dec 19, 2019

View reviewed changes

maksymmalyhin added 2 commits December 19, 2019 17:23

Formatting and spelling.

9ee220b

Firebase example proj: pod deintegrate

f34038c

maksymmalyhin requested a review from paulb777 December 19, 2019 22:26

paulb777 approved these changes Dec 19, 2019

View reviewed changes

maksymmalyhin merged commit f62c7dd into master Dec 20, 2019

maksymmalyhin deleted the mm/fis-integration-master branch December 20, 2019 15:54

maksymmalyhin restored the mm/fis-integration-master branch January 2, 2020 16:26

firebase locked and limited conversation to collaborators Jan 20, 2020

paulb777 deleted the mm/fis-integration-master branch April 2, 2020 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IID: version dependent on FirebaseInstallations #4533

IID: version dependent on FirebaseInstallations #4533

maksymmalyhin commented Dec 19, 2019

charlotteliang left a comment

charlotteliang Dec 19, 2019

maksymmalyhin Dec 19, 2019

charlotteliang Dec 19, 2019

paulb777 Dec 19, 2019

paulb777 Dec 19, 2019

paulb777 Dec 19, 2019

maksymmalyhin Dec 19, 2019

paulb777 Dec 19, 2019

paulb777 Dec 19, 2019

maksymmalyhin commented Dec 19, 2019

paulb777 commented Dec 19, 2019

maksymmalyhin commented Dec 19, 2019 •

edited

Loading

paulb777 commented Dec 20, 2019

maksymmalyhin commented Dec 20, 2019

IID: version dependent on FirebaseInstallations #4533

IID: version dependent on FirebaseInstallations #4533

Conversation

maksymmalyhin commented Dec 19, 2019

charlotteliang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maksymmalyhin commented Dec 19, 2019

paulb777 commented Dec 19, 2019

maksymmalyhin commented Dec 19, 2019 • edited Loading

paulb777 commented Dec 20, 2019

maksymmalyhin commented Dec 20, 2019

maksymmalyhin commented Dec 19, 2019 •

edited

Loading