Skip to content

Reuse existing Iid as Fid #924

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 10 commits into from
Oct 25, 2019
Merged

Reuse existing Iid as Fid #924

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2382cc1
Reusing existing Iid as Fid. Reading iid from "com.google.android.gms…
ankitaj224 Oct 16, 2019
7d7beed
Reusing existing Iid as Fid. Reading iid from "com.google.android.gms…
ankitaj224 Oct 16, 2019
e6ba1d7
Merge branch 'iid-migration' of github.com:firebase/firebase-android-…
ankitaj224 Oct 16, 2019
8446e9e
Addressing ciaran's comments.
ankitaj224 Oct 16, 2019
5b86dca
Fixing NPE.
ankitaj224 Oct 16, 2019
bec50b1
Addressing Rayo's comments.
ankitaj224 Oct 22, 2019
bce4059
Fix verifyformat`
ankitaj224 Oct 22, 2019
a5ffdd3
Reading iid store only for default firebase app
ankitaj224 Oct 23, 2019
ee514e8
Addressed Rayo's comments.
ankitaj224 Oct 23, 2019
d209690
Add FisError to the persisted installation entry. (#931)
ankitaj224 Oct 25, 2019
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: 5 additions & 0 deletions firebase-installations/api.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ package com.google.firebase.installations {

package com.google.firebase.installations.local {

public class IidStore {
ctor public IidStore(@NonNull FirebaseApp);
method @Nullable public String readIid();
}

public class PersistedInstallation {
ctor public PersistedInstallation(@NonNull FirebaseApp);
method @NonNull public boolean clear();
Expand Down
142 changes: 94 additions & 48 deletions ...oidTest/java/com/google/firebase/installations/FirebaseInstallationsInstrumentedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_CREATION_TIMESTAMP_2;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_FID_1;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_INSTALLATION_RESPONSE;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_INSTALLATION_RESPONSE_WITH_IID;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_INSTALLATION_TOKEN_RESULT;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_INSTANCE_ID_1;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_PROJECT_ID;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_REFRESH_TOKEN;
import static com.google.firebase.installations.FisAndroidTestConstants.TEST_TOKEN_EXPIRATION_TIMESTAMP;
Expand All @@ -52,6 +54,7 @@
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseException;
import com.google.firebase.FirebaseOptions;
import com.google.firebase.installations.local.IidStore;
import com.google.firebase.installations.local.PersistedInstallation;
import com.google.firebase.installations.local.PersistedInstallation.RegistrationStatus;
import com.google.firebase.installations.local.PersistedInstallationEntry;
Expand Down Expand Up @@ -88,6 +91,7 @@ public class FirebaseInstallationsInstrumentedTest {
@Mock private Utils mockUtils;
@Mock private PersistedInstallation mockPersistedInstallation;
@Mock private FirebaseInstallationServiceClient mockClient;
@Mock private IidStore mockIidStore;

private static final PersistedInstallationEntry REGISTERED_INSTALLATION_ENTRY =
PersistedInstallationEntry.builder()
Expand All @@ -99,6 +103,16 @@ public class FirebaseInstallationsInstrumentedTest {
.setRegistrationStatus(PersistedInstallation.RegistrationStatus.REGISTERED)
.build();

private static final PersistedInstallationEntry REGISTERED_IID_ENTRY =
PersistedInstallationEntry.builder()
.setFirebaseInstallationId(TEST_INSTANCE_ID_1)
.setAuthToken(TEST_AUTH_TOKEN)
.setRefreshToken(TEST_REFRESH_TOKEN)
.setTokenCreationEpochInSecs(TEST_CREATION_TIMESTAMP_2)
.setExpiresInSecs(TEST_TOKEN_EXPIRATION_TIMESTAMP)
.setRegistrationStatus(PersistedInstallation.RegistrationStatus.REGISTERED)
.build();

private static final PersistedInstallationEntry EXPIRED_AUTH_TOKEN_ENTRY =
PersistedInstallationEntry.builder()
.setFirebaseInstallationId(TEST_FID_1)
Expand Down Expand Up @@ -189,12 +203,20 @@ public void cleanUp() throws Exception {
persistedInstallation.clear();
}

private FirebaseInstallations getFirebaseInstallations() {
return new FirebaseInstallations(
executor,
firebaseApp,
backendClientReturnsOk,
persistedInstallation,
mockUtils,
mockIidStore);
}

@Test
public void testGetId_PersistedInstallationOk_BackendOk() throws Exception {
when(mockUtils.isAuthTokenExpired(REGISTERED_INSTALLATION_ENTRY)).thenReturn(false);
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
when(mockUtils.isAuthTokenExpired(REGISTERED_IID_ENTRY)).thenReturn(false);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

// No exception, means success.
assertWithMessage("getId Task failed.")
Expand All @@ -213,15 +235,39 @@ public void testGetId_PersistedInstallationOk_BackendOk() throws Exception {
assertThat(updatedInstallationEntry).hasRegistrationStatus(RegistrationStatus.REGISTERED);
}

@Test
public void testGetId_migrateIid_successful() throws Exception {
when(mockIidStore.readIid()).thenReturn(TEST_INSTANCE_ID_1);
when(mockUtils.isAuthTokenExpired(REGISTERED_INSTALLATION_ENTRY)).thenReturn(false);
when(backendClientReturnsOk.createFirebaseInstallation(
anyString(), anyString(), anyString(), anyString()))
.thenReturn(TEST_INSTALLATION_RESPONSE_WITH_IID);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

// No exception, means success.
assertWithMessage("getId Task failed.")
.that(Tasks.await(firebaseInstallations.getId()))
.isNotEmpty();
PersistedInstallationEntry entryValue =
persistedInstallation.readPersistedInstallationEntryValue();
assertThat(entryValue).hasFid(TEST_INSTANCE_ID_1);

// Waiting for Task that registers FID on the FIS Servers
executor.awaitTermination(500, TimeUnit.MILLISECONDS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this have to wait half a second?
Are we actually sending a request to FIS here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is for getId(). As you know, getId() return fid immediately but registers fid asynchronously. Tests have to wait for half a second while we mock registration. We dont send an actual request to FIS in tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend to add this explanation to the comment in the code.
Also, 500ms sounds excessive, but can't hurt maybe... :)


PersistedInstallationEntry updatedInstallationEntry =
persistedInstallation.readPersistedInstallationEntryValue();
assertThat(updatedInstallationEntry).hasFid(TEST_INSTANCE_ID_1);
assertThat(updatedInstallationEntry).hasRegistrationStatus(RegistrationStatus.REGISTERED);
}

@Test
public void testGetId_multipleCalls_sameFIDReturned() throws Exception {
when(mockUtils.isAuthTokenExpired(REGISTERED_INSTALLATION_ENTRY)).thenReturn(false);
when(backendClientReturnsOk.createFirebaseInstallation(
anyString(), anyString(), anyString(), anyString()))
.thenReturn(TEST_INSTALLATION_RESPONSE);
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

// Call getId multiple times
Task<String> task1 = firebaseInstallations.getId();
Expand Down Expand Up @@ -249,9 +295,7 @@ public void testGetId_invalidFid_storesValidFidFromResponse() throws Exception {
// Update local storage with installation entry that has invalid fid.
persistedInstallation.insertOrUpdatePersistedInstallationEntry(INVALID_INSTALLATION_ENTRY);
when(mockUtils.isAuthTokenExpired(REGISTERED_INSTALLATION_ENTRY)).thenReturn(false);
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

// No exception, means success.
assertWithMessage("getId Task failed.")
Expand All @@ -275,7 +319,12 @@ public void testGetId_invalidFid_storesValidFidFromResponse() throws Exception {
public void testGetId_PersistedInstallationOk_BackendError() throws Exception {
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsError, persistedInstallation, mockUtils);
executor,
firebaseApp,
backendClientReturnsError,
persistedInstallation,
mockUtils,
mockIidStore);

Tasks.await(firebaseInstallations.getId());

Expand All @@ -299,9 +348,7 @@ public void testGetId_ServerError_UnregisteredFID() throws Exception {
anyString(), anyString(), anyString(), anyString()))
.thenReturn(SERVER_ERROR_INSTALLATION_RESPONSE);

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

Tasks.await(firebaseInstallations.getId());

Expand All @@ -326,7 +373,8 @@ public void testGetId_PersistedInstallationError_BackendOk() throws InterruptedE
firebaseApp,
backendClientReturnsOk,
persistedInstallationReturnsError,
mockUtils);
mockUtils,
mockIidStore);

// Expect exception
try {
Expand Down Expand Up @@ -355,7 +403,7 @@ public void testGetId_fidRegistrationUncheckedException_statusUpdated() throws E

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, mockClient, persistedInstallation, mockUtils);
executor, firebaseApp, mockClient, persistedInstallation, mockUtils, mockIidStore);

Tasks.await(firebaseInstallations.getId());

Expand Down Expand Up @@ -387,7 +435,7 @@ public void testGetId_expiredAuthTokenUncheckedException_statusUpdated() throws

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, mockClient, persistedInstallation, mockUtils);
executor, firebaseApp, mockClient, persistedInstallation, mockUtils, mockIidStore);

assertWithMessage("getId Task failed")
.that(Tasks.await(firebaseInstallations.getId()))
Expand All @@ -412,9 +460,7 @@ public void testGetId_expiredAuthToken_refreshesAuthToken() throws Exception {
persistedInstallation.insertOrUpdatePersistedInstallationEntry(EXPIRED_AUTH_TOKEN_ENTRY);
when(mockUtils.isAuthTokenExpired(EXPIRED_AUTH_TOKEN_ENTRY)).thenReturn(true);

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

assertWithMessage("getId Task failed")
.that(Tasks.await(firebaseInstallations.getId()))
Expand All @@ -439,9 +485,7 @@ public void testGetId_expiredAuthToken_refreshesAuthToken() throws Exception {
@Test
public void testGetAuthToken_fidDoesNotExist_successful() throws Exception {
when(mockUtils.isAuthTokenExpired(REGISTERED_INSTALLATION_ENTRY)).thenReturn(false);
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

Tasks.await(firebaseInstallations.getAuthToken(FirebaseInstallationsApi.DO_NOT_FORCE_REFRESH));

Expand All @@ -459,7 +503,8 @@ public void testGetAuthToken_PersistedInstallationError_failure() throws Excepti
firebaseApp,
backendClientReturnsOk,
persistedInstallationReturnsError,
mockUtils);
mockUtils,
mockIidStore);

// Expect exception
try {
Expand All @@ -485,7 +530,12 @@ public void testGetAuthToken_fidExists_successful() throws Exception {

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, mockPersistedInstallation, mockUtils);
executor,
firebaseApp,
backendClientReturnsOk,
mockPersistedInstallation,
mockUtils,
mockIidStore);

InstallationTokenResult installationTokenResult =
Tasks.await(
Expand All @@ -504,9 +554,7 @@ public void testGetAuthToken_expiredAuthToken_fetchedNewTokenFromFIS() throws Ex
when(mockUtils.isAuthTokenExpired(EXPIRED_AUTH_TOKEN_ENTRY)).thenReturn(true);
when(mockUtils.isAuthTokenExpired(UPDATED_AUTH_TOKEN_ENTRY)).thenReturn(false);

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

InstallationTokenResult installationTokenResult =
Tasks.await(
Expand All @@ -526,9 +574,7 @@ public void testGetAuthToken_unregisteredFid_fetchedNewTokenFromFIS() throws Exc
persistedInstallation.insertOrUpdatePersistedInstallationEntry(UNREGISTERED_INSTALLATION_ENTRY);
when(mockUtils.isAuthTokenExpired(REGISTERED_INSTALLATION_ENTRY)).thenReturn(false);

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

InstallationTokenResult installationTokenResult =
Tasks.await(
Expand All @@ -552,7 +598,12 @@ public void testGetAuthToken_serverError_failure() throws Exception {

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsError, mockPersistedInstallation, mockUtils);
executor,
firebaseApp,
backendClientReturnsError,
mockPersistedInstallation,
mockUtils,
mockIidStore);

// Expect exception
try {
Expand All @@ -579,9 +630,7 @@ public void testGetAuthToken_multipleCallsDoNotForceRefresh_fetchedNewTokenOnce(
when(mockUtils.isAuthTokenExpired(EXPIRED_AUTH_TOKEN_ENTRY)).thenReturn(true);
when(mockUtils.isAuthTokenExpired(UPDATED_AUTH_TOKEN_ENTRY)).thenReturn(false);

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

// Call getAuthToken multiple times with DO_NOT_FORCE_REFRESH option
Task<InstallationTokenResult> task1 =
Expand Down Expand Up @@ -630,9 +679,7 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th
.generateAuthToken(anyString(), anyString(), anyString(), anyString());
when(mockUtils.isAuthTokenExpired(any())).thenReturn(false);

FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

// Call getAuthToken multiple times with FORCE_REFRESH option.
Task<InstallationTokenResult> task1 =
Expand All @@ -659,9 +706,7 @@ public void testGetAuthToken_multipleCallsForceRefresh_fetchedNewTokenTwice() th
public void testDelete_registeredFID_successful() throws Exception {
// Update local storage with a registered installation entry
persistedInstallation.insertOrUpdatePersistedInstallationEntry(REGISTERED_INSTALLATION_ENTRY);
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

Tasks.await(firebaseInstallations.delete());

Expand All @@ -676,9 +721,7 @@ public void testDelete_registeredFID_successful() throws Exception {
public void testDelete_unregisteredFID_successful() throws Exception {
// Update local storage with a unregistered installation entry
persistedInstallation.insertOrUpdatePersistedInstallationEntry(UNREGISTERED_INSTALLATION_ENTRY);
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

Tasks.await(firebaseInstallations.delete());

Expand All @@ -691,9 +734,7 @@ public void testDelete_unregisteredFID_successful() throws Exception {

@Test
public void testDelete_emptyPersistedFidEntry_successful() throws Exception {
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsOk, persistedInstallation, mockUtils);
FirebaseInstallations firebaseInstallations = getFirebaseInstallations();

Tasks.await(firebaseInstallations.delete());

Expand All @@ -710,7 +751,12 @@ public void testDelete_serverError_failure() throws Exception {
persistedInstallation.insertOrUpdatePersistedInstallationEntry(REGISTERED_INSTALLATION_ENTRY);
FirebaseInstallations firebaseInstallations =
new FirebaseInstallations(
executor, firebaseApp, backendClientReturnsError, persistedInstallation, mockUtils);
executor,
firebaseApp,
backendClientReturnsError,
persistedInstallation,
mockUtils,
mockIidStore);

// Expect exception
try {
Expand Down
16 changes: 16 additions & 0 deletions ...tions/src/androidTest/java/com/google/firebase/installations/FisAndroidTestConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public final class FisAndroidTestConstants {
public static final long TEST_CREATION_TIMESTAMP_1 = 2000L;
public static final long TEST_CREATION_TIMESTAMP_2 = 2L;

public static final String TEST_INSTANCE_ID_1 = "ccccccccccc";

public static final PersistedInstallationEntry DEFAULT_PERSISTED_INSTALLATION_ENTRY =
PersistedInstallationEntry.builder().build();
public static final InstallationResponse TEST_INSTALLATION_RESPONSE =
Expand All @@ -59,6 +61,20 @@ public final class FisAndroidTestConstants {
.setResponseCode(ResponseCode.OK)
.build();

public static final InstallationResponse TEST_INSTALLATION_RESPONSE_WITH_IID =
InstallationResponse.builder()
.setUri("/projects/" + TEST_PROJECT_ID + "/installations/" + TEST_INSTANCE_ID_1)
Copy link
Contributor

Choose a reason for hiding this comment

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

you could use a constant for this as well:
FIS_URI = "/projects/%s/installations/%s";

but not important.

.setFid(TEST_INSTANCE_ID_1)
.setRefreshToken(TEST_REFRESH_TOKEN)
.setAuthToken(
InstallationTokenResult.builder()
.setToken(TEST_AUTH_TOKEN)
.setTokenExpirationTimestamp(TEST_TOKEN_EXPIRATION_TIMESTAMP)
.setTokenCreationTimestamp(TEST_CREATION_TIMESTAMP_1)
.build())
.setResponseCode(ResponseCode.OK)
.build();

public static final InstallationTokenResult TEST_INSTALLATION_TOKEN_RESULT =
InstallationTokenResult.builder()
.setToken(TEST_AUTH_TOKEN_2)
Expand Down
Loading