Skip to content

Commit 8c9afbc

Browse files
mroberDavid Motsonashvili
authored andcommitted
Add fid to Crashlytics report (#5052)
* Add fid to Crashlytics report * Format
1 parent 64a0bf0 commit 8c9afbc

File tree

13 files changed

+149
-65
lines changed

13 files changed

+149
-65
lines changed

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/IdManagerTest.java

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ private IdManager createIdManager(String instanceId, DataCollectionArbiter arbit
6969
public void testCreateUUID() {
7070
final String fid = "test_fid";
7171
final IdManager idManager = createIdManager(fid, MOCK_ARBITER_ENABLED);
72-
final String installId = idManager.getCrashlyticsInstallId();
72+
final String installId = idManager.getInstallIds().getCrashlyticsInstallId();
7373
assertNotNull(installId);
7474

7575
assertEquals(installId, prefs.getString(IdManager.PREFKEY_INSTALLATION_UUID, null));
7676
assertNull(prefs.getString(IdManager.PREFKEY_ADVERTISING_ID, null));
7777
assertEquals(fid, prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null));
7878

7979
// subsequent calls should return the same id
80-
assertEquals(installId, idManager.getCrashlyticsInstallId());
80+
assertEquals(installId, idManager.getInstallIds().getCrashlyticsInstallId());
8181
}
8282

8383
public void testGetIdExceptionalCase_doesNotRotateInstallId() {
@@ -93,7 +93,7 @@ public void testGetIdExceptionalCase_doesNotRotateInstallId() {
9393

9494
final IdManager idManager =
9595
new IdManager(getContext(), getContext().getPackageName(), fis, MOCK_ARBITER_ENABLED);
96-
final String actualInstallId = idManager.getCrashlyticsInstallId();
96+
final String actualInstallId = idManager.getInstallIds().getCrashlyticsInstallId();
9797
assertNotNull(actualInstallId);
9898
assertEquals(expectedInstallId, actualInstallId);
9999
}
@@ -111,15 +111,15 @@ public void testInstanceIdChanges_dataCollectionEnabled() {
111111
// Initialize the manager with a different FID.
112112
IdManager idManager = createIdManager(newFid, MOCK_ARBITER_ENABLED);
113113

114-
String installId = idManager.getCrashlyticsInstallId();
114+
String installId = idManager.getInstallIds().getCrashlyticsInstallId();
115115
assertNotNull(installId);
116116
assertFalse(installId.equals(oldUuid));
117117

118118
assertEquals(installId, prefs.getString(IdManager.PREFKEY_INSTALLATION_UUID, null));
119119
assertEquals(newFid, prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null));
120120

121121
// subsequent calls should return the same id
122-
assertEquals(installId, idManager.getCrashlyticsInstallId());
122+
assertEquals(installId, idManager.getInstallIds().getCrashlyticsInstallId());
123123
}
124124

125125
void validateInstanceIdDoesntChange(boolean dataCollectionEnabled) {
@@ -136,7 +136,7 @@ void validateInstanceIdDoesntChange(boolean dataCollectionEnabled) {
136136
IdManager idManager =
137137
createIdManager(fid, dataCollectionEnabled ? MOCK_ARBITER_ENABLED : MOCK_ARBITER_DISABLED);
138138

139-
String installId = idManager.getCrashlyticsInstallId();
139+
String installId = idManager.getInstallIds().getCrashlyticsInstallId();
140140
assertNotNull(installId);
141141

142142
// Test that the UUID didn't change.
@@ -146,15 +146,15 @@ void validateInstanceIdDoesntChange(boolean dataCollectionEnabled) {
146146
assertEquals(fid, prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null));
147147

148148
// subsequent calls should return the same id
149-
assertEquals(oldUuid, idManager.getCrashlyticsInstallId());
149+
assertEquals(oldUuid, idManager.getInstallIds().getCrashlyticsInstallId());
150150
}
151151

152152
public void testInstanceIdDoesntChange_dataCollectionEnabled() {
153-
validateInstanceIdDoesntChange(/*dataCollectionEnabled=*/ true);
153+
validateInstanceIdDoesntChange(/* dataCollectionEnabled= */ true);
154154
}
155155

156156
public void testInstanceIdDoesntChange_dataCollectionDisabled() {
157-
validateInstanceIdDoesntChange(/*dataCollectionEnabled=*/ false);
157+
validateInstanceIdDoesntChange(/* dataCollectionEnabled= */ false);
158158
}
159159

160160
public void testInstanceIdRotatesWithDataCollectionFlag() {
@@ -169,35 +169,42 @@ public void testInstanceIdRotatesWithDataCollectionFlag() {
169169

170170
// Initialize the manager with the same FID.
171171
IdManager idManager = createIdManager(originalFid, MOCK_ARBITER_ENABLED);
172-
String firstUuid = idManager.getCrashlyticsInstallId();
172+
String firstUuid = idManager.getInstallIds().getCrashlyticsInstallId();
173173
assertNotNull(firstUuid);
174174
assertEquals(originalUuid, firstUuid);
175175

176176
// subsequent calls should return the same id
177-
assertEquals(firstUuid, idManager.getCrashlyticsInstallId());
177+
assertEquals(firstUuid, idManager.getInstallIds().getCrashlyticsInstallId());
178178
assertEquals(
179-
firstUuid, createIdManager(originalFid, MOCK_ARBITER_ENABLED).getCrashlyticsInstallId());
179+
firstUuid,
180+
createIdManager(originalFid, MOCK_ARBITER_ENABLED)
181+
.getInstallIds()
182+
.getCrashlyticsInstallId());
180183

181184
// Disable data collection manager and confirm we get a different id
182185
idManager = createIdManager(originalFid, MOCK_ARBITER_DISABLED);
183-
String secondUuid = idManager.getCrashlyticsInstallId();
186+
String secondUuid = idManager.getInstallIds().getCrashlyticsInstallId();
184187
assertNotSame(secondUuid, firstUuid);
185-
assertEquals(secondUuid, idManager.getCrashlyticsInstallId());
188+
assertEquals(secondUuid, idManager.getInstallIds().getCrashlyticsInstallId());
186189
assertEquals(
187-
secondUuid, createIdManager(null, MOCK_ARBITER_DISABLED).getCrashlyticsInstallId());
190+
secondUuid,
191+
createIdManager(null, MOCK_ARBITER_DISABLED).getInstallIds().getCrashlyticsInstallId());
188192
// Check that we cached an synthetic FID
189193
final SharedPreferences prefs = CommonUtils.getSharedPrefs(getContext());
190194
String cachedFid = prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null);
191195
assertTrue(IdManager.isSyntheticFid(cachedFid));
192196

193197
// re-enable data collection
194198
idManager = createIdManager(originalFid, MOCK_ARBITER_ENABLED);
195-
String thirdUuid = idManager.getCrashlyticsInstallId();
199+
String thirdUuid = idManager.getInstallIds().getCrashlyticsInstallId();
196200
assertNotSame(thirdUuid, firstUuid);
197201
assertNotSame(thirdUuid, secondUuid);
198-
assertEquals(thirdUuid, idManager.getCrashlyticsInstallId());
202+
assertEquals(thirdUuid, idManager.getInstallIds().getCrashlyticsInstallId());
199203
assertEquals(
200-
thirdUuid, createIdManager(originalFid, MOCK_ARBITER_ENABLED).getCrashlyticsInstallId());
204+
thirdUuid,
205+
createIdManager(originalFid, MOCK_ARBITER_ENABLED)
206+
.getInstallIds()
207+
.getCrashlyticsInstallId());
201208
// The cached ID should be back to the original
202209
cachedFid = prefs.getString(IdManager.PREFKEY_FIREBASE_IID, null);
203210
assertEquals(cachedFid, originalFid);

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinatorTest.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public class SessionReportingCoordinatorTest {
5858
@Mock private DataTransportCrashlyticsReportSender reportSender;
5959
@Mock private LogFileManager logFileManager;
6060
@Mock private UserMetadata reportMetadata;
61+
@Mock private IdManager idManager;
6162
@Mock private CrashlyticsReport mockReport;
6263
@Mock private CrashlyticsReport.Session.Event mockEvent;
6364
@Mock private CrashlyticsReport.Session.Event.Builder mockEventBuilder;
@@ -74,7 +75,12 @@ public void setUp() {
7475

7576
reportingCoordinator =
7677
new SessionReportingCoordinator(
77-
dataCapture, reportPersistence, reportSender, logFileManager, reportMetadata);
78+
dataCapture,
79+
reportPersistence,
80+
reportSender,
81+
logFileManager,
82+
reportMetadata,
83+
idManager);
7884
}
7985

8086
@Test
@@ -442,6 +448,7 @@ public void onReportSend_successfulReportsAreDeleted() {
442448
when(reportSender.enqueueReport(mockReport1, false)).thenReturn(successfulTask);
443449
when(reportSender.enqueueReport(mockReport2, false)).thenReturn(failedTask);
444450

451+
when(idManager.fetchTrueFid()).thenReturn("fid");
445452
reportingCoordinator.sendReports(Runnable::run);
446453

447454
verify(reportSender).enqueueReport(mockReport1, false);
@@ -491,6 +498,7 @@ private static CrashlyticsReport mockReport(String sessionId) {
491498
final CrashlyticsReport.Session mockSession = mock(CrashlyticsReport.Session.class);
492499
when(mockSession.getIdentifier()).thenReturn(sessionId);
493500
when(mockReport.getSession()).thenReturn(mockSession);
501+
when(mockReport.withFirebaseInstallationId(anyString())).thenReturn(mockReport);
494502
return mockReport;
495503
}
496504

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsControllerTest.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.google.firebase.crashlytics.internal.common.DeliveryMechanism;
3232
import com.google.firebase.crashlytics.internal.common.ExecutorUtils;
3333
import com.google.firebase.crashlytics.internal.common.InstallIdProvider;
34+
import com.google.firebase.crashlytics.internal.common.InstallIdProvider.InstallIds;
3435
import java.util.concurrent.Executor;
3536
import java.util.concurrent.TimeUnit;
3637
import org.json.JSONObject;
@@ -380,13 +381,7 @@ public void testNoAvailableSettingsLoad() throws Exception {
380381
}
381382

382383
private SettingsRequest buildSettingsRequest() {
383-
InstallIdProvider installIdProvider =
384-
new InstallIdProvider() {
385-
@Override
386-
public String getCrashlyticsInstallId() {
387-
return installationId;
388-
}
389-
};
384+
InstallIdProvider installIdProvider = () -> InstallIds.createWithoutFid(installationId);
390385

391386
return new SettingsRequest(
392387
googleAppId,

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/settings/DefaultSettingsSpiCallTest.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.firebase.crashlytics.internal.Logger;
2121
import com.google.firebase.crashlytics.internal.common.CommonUtils;
2222
import com.google.firebase.crashlytics.internal.common.InstallIdProvider;
23+
import com.google.firebase.crashlytics.internal.common.InstallIdProvider.InstallIds;
2324
import com.google.firebase.crashlytics.internal.network.HttpGetRequest;
2425
import com.google.firebase.crashlytics.internal.network.HttpRequestFactory;
2526
import com.google.firebase.crashlytics.internal.network.HttpResponse;
@@ -230,13 +231,7 @@ public void testRequestWasSuccessful_unsuccessfulStatusCodes() {
230231
}
231232

232233
private SettingsRequest buildSettingsRequest(String instanceId) {
233-
final InstallIdProvider installIdProvider =
234-
new InstallIdProvider() {
235-
@Override
236-
public String getCrashlyticsInstallId() {
237-
return INSTALLATION_ID;
238-
}
239-
};
234+
final InstallIdProvider installIdProvider = () -> InstallIds.createWithoutFid(INSTALLATION_ID);
240235
return new SettingsRequest(
241236
GOOGLE_APP_ID,
242237
DEVICE_MODEL,

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CLSUUID.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,9 @@ class CLSUUID {
4040
this.populateSequenceNumber(bytes);
4141
this.populatePID(bytes);
4242

43-
// appInstallationUUID may be empty but is guaranteed not to be null.
4443
// sha1 it to ensure the string is long enough, has only valid hex characters, and to
4544
// increase entropy.
46-
final String idSha = CommonUtils.sha1(idManager.getCrashlyticsInstallId());
45+
final String idSha = CommonUtils.sha1(idManager.getInstallIds().getCrashlyticsInstallId());
4746
final String timeSeqPid = CommonUtils.hexify(bytes);
4847

4948
_clsId =

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ private static StaticSessionData.AppData createAppData(IdManager idManager, AppD
749749
idManager.getAppIdentifier(),
750750
appData.versionCode,
751751
appData.versionName,
752-
idManager.getCrashlyticsInstallId(),
752+
idManager.getInstallIds().getCrashlyticsInstallId(),
753753
DeliveryMechanism.determineFrom(appData.installerPackageName).getId(),
754754
appData.developmentPlatformProvider);
755755
}

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsReportDataCapture.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ private CrashlyticsReport.Builder buildReportData() {
164164
return CrashlyticsReport.builder()
165165
.setSdkVersion(BuildConfig.VERSION_NAME)
166166
.setGmpAppId(appData.googleAppId)
167-
.setInstallationUuid(idManager.getCrashlyticsInstallId())
167+
.setInstallationUuid(idManager.getInstallIds().getCrashlyticsInstallId())
168+
.setFirebaseInstallationId(idManager.getInstallIds().getFirebaseInstallationId())
168169
.setBuildVersion(appData.versionCode)
169170
.setDisplayVersion(appData.versionName)
170171
.setPlatform(REPORT_ANDROID_PLATFORM);
@@ -188,7 +189,7 @@ private CrashlyticsReport.Session.Application populateSessionApplicationData() {
188189
.setIdentifier(idManager.getAppIdentifier())
189190
.setVersion(appData.versionCode)
190191
.setDisplayVersion(appData.versionName)
191-
.setInstallationUuid(idManager.getCrashlyticsInstallId())
192+
.setInstallationUuid(idManager.getInstallIds().getCrashlyticsInstallId())
192193
.setDevelopmentPlatform(appData.developmentPlatformProvider.getDevelopmentPlatform())
193194
.setDevelopmentPlatformVersion(
194195
appData.developmentPlatformProvider.getDevelopmentPlatformVersion());

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/IdManager.java

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import android.os.Build;
2020
import androidx.annotation.NonNull;
2121
import androidx.annotation.Nullable;
22+
import androidx.annotation.VisibleForTesting;
2223
import com.google.android.gms.tasks.Task;
2324
import com.google.firebase.crashlytics.internal.Logger;
2425
import com.google.firebase.installations.FirebaseInstallationsApi;
@@ -35,7 +36,7 @@ public class IdManager implements InstallIdProvider {
3536
static final String PREFKEY_FIREBASE_IID = "firebase.installation.id";
3637
static final String PREFKEY_LEGACY_INSTALLATION_UUID = "crashlytics.installation.id";
3738

38-
/** Regex for stripping all non-alphnumeric characters from ALL the identifier fields. */
39+
/** Regex for stripping all non-alphanumeric characters from ALL the identifier fields. */
3940
private static final Pattern ID_PATTERN = Pattern.compile("[^\\p{Alnum}]");
4041

4142
private static final String SYNTHETIC_FID_PREFIX = "SYN_";
@@ -51,8 +52,8 @@ public class IdManager implements InstallIdProvider {
5152

5253
private final DataCollectionArbiter dataCollectionArbiter;
5354

54-
// Crashlytics maintains a Crashlytics-specific install id, used in the crash processing backend
55-
private String crashlyticsInstallId;
55+
// Stores a Crashlytics-specific install id, and Firebase installation id.
56+
private InstallIds installIds;
5657

5758
/**
5859
* @param appContext Application {@link Context}
@@ -101,9 +102,9 @@ private static String formatId(String id) {
101102
*/
102103
@Override
103104
@NonNull
104-
public synchronized String getCrashlyticsInstallId() {
105-
if (crashlyticsInstallId != null) {
106-
return crashlyticsInstallId;
105+
public synchronized InstallIds getInstallIds() {
106+
if (!shouldRefresh()) {
107+
return installIds;
107108
}
108109

109110
Logger.getLogger().v("Determining Crashlytics installation ID...");
@@ -125,28 +126,37 @@ public synchronized String getCrashlyticsInstallId() {
125126

126127
if (trueFid.equals(cachedFid)) {
127128
// the current FID is the same as the cached FID, so we keep the cached Crashlytics ID
128-
crashlyticsInstallId = readCachedCrashlyticsInstallId(prefs);
129+
installIds = InstallIds.create(readCachedCrashlyticsInstallId(prefs), trueFid);
129130
} else {
130131
// the current FID has changed, so we generate a new Crashlytics ID
131-
crashlyticsInstallId = createAndCacheCrashlyticsInstallId(trueFid, prefs);
132+
installIds = InstallIds.create(createAndCacheCrashlyticsInstallId(trueFid, prefs), trueFid);
132133
}
133134
} else { // data collection is NOT enabled; we can't use the FID
134135
if (isSyntheticFid(cachedFid)) {
135136
// We already have a cached synthetic FID, so we don't need to change the Crashlytics ID
136-
crashlyticsInstallId = readCachedCrashlyticsInstallId(prefs);
137+
installIds = InstallIds.createWithoutFid(readCachedCrashlyticsInstallId(prefs));
137138
} else {
138139
// we don't have a synthetic FID, so we need to replace the cached FID with a synthetic
139140
// one and create a new Crashlytics install id.
140-
crashlyticsInstallId = createAndCacheCrashlyticsInstallId(createSyntheticFid(), prefs);
141+
installIds =
142+
InstallIds.createWithoutFid(
143+
createAndCacheCrashlyticsInstallId(createSyntheticFid(), prefs));
141144
}
142145
}
143-
if (crashlyticsInstallId == null) {
144-
// Should not happen but we don't want to throw any exceptions
145-
Logger.getLogger().w("Unable to determine Crashlytics Install Id, creating a new one.");
146-
crashlyticsInstallId = createAndCacheCrashlyticsInstallId(createSyntheticFid(), prefs);
147-
}
148-
Logger.getLogger().v("Crashlytics installation ID: " + crashlyticsInstallId);
149-
return crashlyticsInstallId;
146+
Logger.getLogger().v("Install IDs: " + installIds);
147+
return installIds;
148+
}
149+
150+
/**
151+
* Returns true if we have not cached an InstallIds, or should force refresh the fid.
152+
*
153+
* <p>We should force refresh the fid if data collection is enabled but we don't have a cached
154+
* fid. This can happen if data collection was disabled at crash time, but enabled at upload time.
155+
*/
156+
private boolean shouldRefresh() {
157+
return installIds == null
158+
|| (installIds.getFirebaseInstallationId() == null
159+
&& dataCollectionArbiter.isAutomaticDataCollectionEnabled());
150160
}
151161

152162
static String createSyntheticFid() {
@@ -163,14 +173,15 @@ private String readCachedCrashlyticsInstallId(SharedPreferences prefs) {
163173

164174
/** Makes a blocking call to query FID. If the call fails, logs a warning and returns null. */
165175
@Nullable
166-
private String fetchTrueFid() {
176+
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
177+
public String fetchTrueFid() {
167178
Task<String> currentFidTask = firebaseInstallationsApi.getId();
168179
String currentFid = null;
169180

170181
try {
171182
currentFid = Utils.awaitEvenIfOnMainThread(currentFidTask);
172183
} catch (Exception e) {
173-
Logger.getLogger().w("Failed to retrieve Firebase Installations ID.", e);
184+
Logger.getLogger().w("Failed to retrieve Firebase Installation ID.", e);
174185
}
175186
return currentFid;
176187
}

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/InstallIdProvider.java

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,33 @@
1414

1515
package com.google.firebase.crashlytics.internal.common;
1616

17+
import androidx.annotation.NonNull;
18+
import androidx.annotation.Nullable;
19+
import androidx.annotation.VisibleForTesting;
20+
import com.google.auto.value.AutoValue;
21+
1722
public interface InstallIdProvider {
1823

19-
/** @return an ID that uniquely identifies the app installation on the current device. */
20-
String getCrashlyticsInstallId();
24+
/** Returns an InstallIds that uniquely identifies the app installation on the current device. */
25+
InstallIds getInstallIds();
26+
27+
@AutoValue
28+
abstract class InstallIds {
29+
@NonNull
30+
public abstract String getCrashlyticsInstallId();
31+
32+
@Nullable
33+
public abstract String getFirebaseInstallationId();
34+
35+
/** Creates an InstallIds with just a crashlyticsInstallId, no firebaseInstallationId. */
36+
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
37+
public static InstallIds createWithoutFid(String crashlyticsInstallId) {
38+
return create(crashlyticsInstallId, /* firebaseInstallationId= */ null);
39+
}
40+
41+
static InstallIds create(String crashlyticsInstallId, @Nullable String firebaseInstallationId) {
42+
return new AutoValue_InstallIdProvider_InstallIds(
43+
crashlyticsInstallId, firebaseInstallationId);
44+
}
45+
}
2146
}

0 commit comments

Comments
 (0)