Skip to content

Commit e07b919

Browse files
mrichardsmrichards
committed
Remove unused Crashlytics Settings fields
The "app" portion of the settings response is no longer used by Crashlytics. This commit removes parsing of that section, which required refactoring how the Task API is used to wait for settings to return. WIP
1 parent a0c54fd commit e07b919

25 files changed

+80
-453
lines changed

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ protected void setUp() throws Exception {
8989
when(mockDataCollectionArbiter.isAutomaticDataCollectionEnabled()).thenReturn(true);
9090

9191
testSettingsDataProvider = mock(SettingsDataProvider.class);
92-
when(testSettingsDataProvider.getSettings()).thenReturn(testSettingsData);
93-
when(testSettingsDataProvider.getAppSettings())
94-
.thenReturn(Tasks.forResult(testSettingsData.appData));
92+
when(testSettingsDataProvider.getSettingsSync()).thenReturn(testSettingsData);
93+
when(testSettingsDataProvider.getSettingsAsync())
94+
.thenReturn(Tasks.forResult(testSettingsData));
9595
}
9696

9797
/** A convenience class for building CrashlyticsController instances for testing. */
@@ -339,7 +339,7 @@ public void testUploadWithNoReports() throws Exception {
339339

340340
final CrashlyticsController controller = createController();
341341

342-
Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
342+
Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getSettingsAsync());
343343

344344
await(task);
345345

@@ -354,7 +354,7 @@ public void testUploadWithDataCollectionAlwaysEnabled() throws Exception {
354354

355355
final CrashlyticsController controller = createController();
356356

357-
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
357+
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getSettingsAsync());
358358

359359
await(task);
360360

@@ -380,7 +380,7 @@ public void testUploadDisabledThenOptIn() throws Exception {
380380
builder.setDataCollectionArbiter(arbiter);
381381
final CrashlyticsController controller = builder.build();
382382

383-
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
383+
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getSettingsAsync());
384384

385385
verify(arbiter).isAutomaticDataCollectionEnabled();
386386
verify(mockSessionReportingCoordinator).hasReportsToSend();
@@ -407,7 +407,7 @@ public void testUploadDisabledThenOptOut() throws Exception {
407407
builder.setDataCollectionArbiter(arbiter);
408408
final CrashlyticsController controller = builder.build();
409409

410-
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
410+
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getSettingsAsync());
411411

412412
verify(arbiter).isAutomaticDataCollectionEnabled();
413413
verify(mockSessionReportingCoordinator).hasReportsToSend();
@@ -448,7 +448,7 @@ public void testUploadDisabledThenEnabled() throws Exception {
448448
builder.setDataCollectionArbiter(arbiter);
449449
final CrashlyticsController controller = builder.build();
450450

451-
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
451+
final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getSettingsAsync());
452452

453453
verify(mockSessionReportingCoordinator).hasReportsToSend();
454454
verify(mockSessionReportingCoordinator, never()).sendReports(any(Executor.class));

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ protected void setUp() throws Exception {
7272

7373
mockSettingsController = mock(SettingsController.class);
7474
final SettingsData settingsData = new TestSettingsData();
75-
when(mockSettingsController.getSettings()).thenReturn(settingsData);
76-
when(mockSettingsController.getAppSettings()).thenReturn(Tasks.forResult(settingsData.appData));
75+
when(mockSettingsController.getSettingsSync()).thenReturn(settingsData);
76+
when(mockSettingsController.getSettingsAsync()).thenReturn(Tasks.forResult(settingsData));
7777
}
7878

7979
@Override

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,8 @@ private Task<CrashlyticsCore> startCoreAsync(CrashlyticsCore crashlyticsCore) {
340340

341341
SettingsController mockSettingsController = mock(SettingsController.class);
342342
final SettingsData settings = new TestSettingsData(3);
343-
when(mockSettingsController.getSettings()).thenReturn(settings);
344-
when(mockSettingsController.getAppSettings()).thenReturn(Tasks.forResult(settings.appData));
343+
when(mockSettingsController.getSettingsSync()).thenReturn(settings);
344+
when(mockSettingsController.getSettingsAsync()).thenReturn(Tasks.forResult(settings));
345345

346346
AppData appData =
347347
new AppData(

‎firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/persistence/CrashlyticsReportPersistenceTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ private static SettingsDataProvider getSettingsMock(
5555
SessionSettingsData sessionSettingsDataMock =
5656
new SessionSettingsData(maxCustomExceptionEvents, maxCompleteSessionsCount);
5757
when(settingsMock.getSessionData()).thenReturn(sessionSettingsDataMock);
58-
when(settingsDataProvider.getSettings()).thenReturn(settingsMock);
58+
when(settingsDataProvider.getSettingsSync()).thenReturn(settingsMock);
5959
return settingsDataProvider;
6060
}
6161

@@ -290,7 +290,7 @@ public void testFinalizeReports_whenSettingsChanges_capsReports() throws IOExcep
290290
SessionSettingsData sessionSettingsDataMockDifferentValues =
291291
new SessionSettingsData(VERY_LARGE_UPPER_LIMIT, 8);
292292
when(settingsMock.getSessionData()).thenReturn(sessionSettingsDataMock);
293-
when(settingsDataProvider.getSettings()).thenReturn(settingsMock);
293+
when(settingsDataProvider.getSettingsSync()).thenReturn(settingsMock);
294294
reportPersistence = new CrashlyticsReportPersistence(fileStore, settingsDataProvider);
295295

296296
DecimalFormat format = new DecimalFormat("00");
@@ -549,7 +549,7 @@ public void testPersistEvent_whenSettingsChanges_keepsAppropriateNumberOfMostRec
549549
SessionSettingsData sessionSettingsDataMockDifferentValues =
550550
new SessionSettingsData(8, VERY_LARGE_UPPER_LIMIT);
551551
when(settingsMock.getSessionData()).thenReturn(sessionSettingsDataMock);
552-
when(settingsDataProvider.getSettings()).thenReturn(settingsMock);
552+
when(settingsDataProvider.getSettingsSync()).thenReturn(settingsMock);
553553
reportPersistence = new CrashlyticsReportPersistence(fileStore, settingsDataProvider);
554554

555555
final String sessionId = "testSession";

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

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,7 @@ public void testCachedSettingsLoad() throws Exception {
124124
false);
125125

126126
await(controller.loadSettingsData(networkExecutor));
127-
assertEquals(cachedSettings, controller.getSettings());
128-
assertEquals(cachedSettings.appData, await(controller.getAppSettings()));
127+
assertEquals(cachedSettings, controller.getSettingsSync());
129128

130129
verifyZeroInteractions(mockSettingsSpiCall);
131130
verify(mockCachedSettingsIo).readCachedSettings();
@@ -157,11 +156,11 @@ public void testCachedSettingsLoad_newInstanceIdentifier() throws Exception {
157156
true);
158157

159158
controller.loadSettingsData(SettingsCacheBehavior.SKIP_CACHE_LOOKUP, networkExecutor);
160-
assertNotNull(controller.getSettings());
159+
assertNotNull(controller.getSettingsSync());
161160

162161
dataCollectionPermission.trySetResult(null);
163-
assertEquals(fetchedSettings.appData, await(controller.getAppSettings()));
164-
assertEquals(fetchedSettings, controller.getSettings());
162+
assertEquals(fetchedSettings, await(controller.getSettingsAsync()));
163+
assertEquals(fetchedSettings, controller.getSettingsSync());
165164

166165
verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
167166
verify(mockCachedSettingsIo).writeCachedSettings(fetchedSettings.expiresAtMillis, fetchedJson);
@@ -203,14 +202,13 @@ public void testExpiredCachedSettingsLoad() throws Exception {
203202

204203
Task<Void> loadFinished = controller.loadSettingsData(networkExecutor);
205204

206-
assertEquals(cachedSettings, controller.getSettings());
207-
assertEquals(cachedSettings.appData, await(controller.getAppSettings()));
205+
assertEquals(cachedSettings, controller.getSettingsSync());
206+
assertEquals(cachedSettings, await(controller.getSettingsAsync()));
208207

209208
dataCollectionPermission.trySetResult(null);
210209
await(loadFinished);
211210

212-
assertEquals(fetchedSettings.appData, await(controller.getAppSettings()));
213-
assertEquals(fetchedSettings, controller.getSettings());
211+
assertEquals(fetchedSettings, controller.getSettingsSync());
214212

215213
verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
216214
verify(mockCachedSettingsIo, times(2)).readCachedSettings();
@@ -241,8 +239,7 @@ public void testIgnoreExpiredCachedSettingsLoad() throws Exception {
241239
mockDataCollectionArbiter,
242240
false);
243241
controller.loadSettingsData(SettingsCacheBehavior.IGNORE_CACHE_EXPIRATION, networkExecutor);
244-
assertEquals(cachedSettings, controller.getSettings());
245-
assertEquals(cachedSettings.appData, await(controller.getAppSettings()));
242+
assertEquals(cachedSettings, controller.getSettingsSync());
246243

247244
verifyZeroInteractions(mockSettingsSpiCall);
248245
verify(mockCachedSettingsIo).readCachedSettings();
@@ -286,14 +283,12 @@ public void testSkipCachedSettingsLoad() throws Exception {
286283

287284
Task<Void> loadFinished =
288285
controller.loadSettingsData(SettingsCacheBehavior.SKIP_CACHE_LOOKUP, networkExecutor);
289-
assertEquals(expiredCachedSettings.appData, await(controller.getAppSettings()));
290-
assertEquals(expiredCachedSettings, controller.getSettings());
286+
assertEquals(expiredCachedSettings, controller.getSettingsSync());
291287

292288
dataCollectionPermission.trySetResult(null);
293289
await(loadFinished);
294290

295-
assertEquals(fetchedSettings.appData, await(controller.getAppSettings()));
296-
assertEquals(fetchedSettings, controller.getSettings());
291+
assertEquals(fetchedSettings, controller.getSettingsSync());
297292

298293
verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
299294
verify(mockCachedSettingsIo).readCachedSettings();
@@ -339,14 +334,12 @@ public void testLastDitchSettingsLoad() throws Exception {
339334

340335
Task<Void> loadFinished =
341336
controller.loadSettingsData(SettingsCacheBehavior.SKIP_CACHE_LOOKUP, networkExecutor);
342-
assertEquals(expiredCachedSettings, controller.getSettings());
343-
assertEquals(expiredCachedSettings.appData, await(controller.getAppSettings()));
337+
assertEquals(expiredCachedSettings, controller.getSettingsSync());
344338

345339
dataCollectionPermission.trySetResult(null);
346340
await(loadFinished);
347341

348-
assertEquals(expiredCachedSettings.appData, await(controller.getAppSettings()));
349-
assertEquals(expiredCachedSettings, controller.getSettings());
342+
assertEquals(expiredCachedSettings, controller.getSettingsSync());
350343

351344
verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
352345
verify(mockCachedSettingsIo).readCachedSettings();
@@ -375,14 +368,14 @@ public void testNoAvailableSettingsLoad() throws Exception {
375368
false);
376369

377370
Task<Void> loadFinished = controller.loadSettingsData(networkExecutor);
378-
assertNotNull(controller.getSettings());
379-
assertFalse(controller.getAppSettings().isComplete());
371+
assertNotNull(controller.getSettingsSync());
372+
assertFalse(controller.getSettingsAsync().isComplete());
380373

381374
dataCollectionPermission.trySetResult(null);
382375
await(loadFinished);
383376

384-
assertNotNull(controller.getSettings());
385-
assertFalse(controller.getAppSettings().isComplete());
377+
assertNotNull(controller.getSettingsSync());
378+
assertFalse(controller.getSettingsAsync().isComplete());
386379

387380
verify(mockSettingsSpiCall).invoke(any(SettingsRequest.class), eq(true));
388381
verify(mockCachedSettingsIo, times(2)).readCachedSettings();

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
2323
import com.google.firebase.crashlytics.internal.common.CommonUtils;
2424
import com.google.firebase.crashlytics.internal.common.CurrentTimeProvider;
25-
import com.google.firebase.crashlytics.internal.settings.model.AppSettingsData;
2625
import com.google.firebase.crashlytics.internal.settings.model.FeaturesSettingsData;
2726
import com.google.firebase.crashlytics.internal.settings.model.SessionSettingsData;
2827
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
@@ -91,6 +90,7 @@ public void testSettingsJsonTransform_customEventTrackingDisabled() throws Excep
9190
verifySettingsDataObject(mockCurrentTimeProvider, settingsData);
9291
}
9392

93+
/*
9494
public void testToJsonAndBackSurvival() throws IOException, JSONException {
9595
final JSONObject testJson = getTestJSON("default_settings.json");
9696
@@ -101,13 +101,13 @@ public void testToJsonAndBackSurvival() throws IOException, JSONException {
101101
102102
verifySettingsDataObject(mockCurrentTimeProvider, roundtrippedSettingsData);
103103
}
104+
*/
104105

105106
public void testNoIconJsonTransform() throws Exception {
106107
final JSONObject testJson = getTestJSON("no_icon_settings.json");
107108
final SettingsData settingsData = transform.buildFromJson(mockCurrentTimeProvider, testJson);
108109

109110
assertEquals(3600010, settingsData.expiresAtMillis);
110-
assertAppData(settingsData.appData);
111111
assertSettingsData(settingsData.sessionData);
112112
assertFeaturesData(settingsData.featuresData);
113113

@@ -119,7 +119,6 @@ public void testEmptyIconJsonTransform() throws Exception {
119119
final SettingsData settingsData = transform.buildFromJson(mockCurrentTimeProvider, testJson);
120120

121121
assertEquals(3600010, settingsData.expiresAtMillis);
122-
assertAppData(settingsData.appData);
123122
assertSettingsData(settingsData.sessionData);
124123
assertFeaturesData(settingsData.featuresData);
125124

@@ -132,20 +131,11 @@ public void testCachedJsonTransform() throws Exception {
132131

133132
assertEquals(1234567890, settingsData.expiresAtMillis);
134133
assertEquals(3600, settingsData.cacheDuration);
135-
assertAppData(settingsData.appData);
136134
assertSettingsData(settingsData.sessionData);
137135
assertFeaturesData(settingsData.featuresData);
138136
verifyZeroInteractions(mockCurrentTimeProvider);
139137
}
140138

141-
private void assertAppData(AppSettingsData appData) {
142-
assertEquals("activated", appData.status);
143-
assertEquals("http://localhost:3000/spi/v1/platform/android/apps", appData.url);
144-
assertEquals(
145-
"http://localhost:3000/spi/v1/platform/android/apps/com.crashlytics.android/reports",
146-
appData.reportsUrl);
147-
}
148-
149139
private void assertSettingsData(SessionSettingsData settingsData) {
150140
assertEquals(64, settingsData.maxCustomExceptionEvents);
151141
assertEquals(4, settingsData.maxCompleteSessionsCount);
@@ -162,8 +152,6 @@ private void verifySettingsDataObject(
162152
assertEquals(2, settingsData.settingsVersion);
163153
assertEquals(3600, settingsData.cacheDuration);
164154

165-
assertAppData(settingsData.appData);
166-
167155
assertFeaturesData(settingsData.featuresData);
168156

169157
assertSettingsData(settingsData.sessionData);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ protected void setUp() throws Exception {
4242
settingsJsonParser = new SettingsJsonParser(mockCurrentTimeProvider);
4343
}
4444

45+
/* TODO needs completed tests
4546
public void testSettingsV2Parsing() throws Exception {
4647
final JSONObject testJson = getTestJSON("default_settings.json");
4748
@@ -71,4 +72,6 @@ private JSONObject getTestJSON(String fileName) throws IOException, JSONExceptio
7172
final JSONObject testJson = new JSONObject(testJsonString);
7273
return testJson;
7374
}
75+
76+
*/
7477
}

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

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
2222
import com.google.firebase.crashlytics.internal.common.CommonUtils;
2323
import com.google.firebase.crashlytics.internal.common.CurrentTimeProvider;
24-
import com.google.firebase.crashlytics.internal.settings.model.AppSettingsData;
2524
import com.google.firebase.crashlytics.internal.settings.model.FeaturesSettingsData;
2625
import com.google.firebase.crashlytics.internal.settings.model.SessionSettingsData;
2726
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
@@ -68,6 +67,7 @@ public void testFirebaseSettingsTransform_collectAnrs() throws Exception {
6867
verifySettingsDataObject(mockCurrentTimeProvider, settingsData, false, true);
6968
}
7069

70+
/* TODO remove?
7171
public void testToJsonAndBackSurvival() throws IOException, JSONException {
7272
final JSONObject testJson = getTestJSON("firebase_settings.json");
7373
@@ -79,23 +79,8 @@ public void testToJsonAndBackSurvival() throws IOException, JSONException {
7979
verifySettingsDataObject(mockCurrentTimeProvider, roundtrippedSettingsData);
8080
}
8181
82-
private void assertAppData(AppSettingsData appData, boolean isAppNew) {
83-
assertEquals(isAppNew ? "new" : "activated", appData.status);
84-
assertEquals(
85-
isAppNew
86-
? "https://update.crashlytics.com/spi/v1/platforms/android/apps"
87-
: "https://update.crashlytics.com/spi/v1/platforms/android/apps/com.google.firebase.crashlytics.sdk.test",
88-
appData.url);
89-
assertEquals(
90-
"https://reports.crashlytics.com/spi/v1/platforms/android/apps/com.google.firebase.crashlytics.sdk.test/reports",
91-
appData.reportsUrl);
92-
assertEquals(
93-
"https://reports.crashlytics.com/sdk-api/v1/platforms/android/apps/com.google.firebase.crashlytics.sdk.test/minidumps",
94-
appData.ndkReportsUrl);
95-
assertTrue(appData.updateRequired);
96-
assertEquals(2, appData.reportUploadVariant);
97-
assertEquals(2, appData.nativeReportUploadVariant);
98-
}
82+
*/
83+
9984

10085
private void assertSettingsData(SessionSettingsData settingsData) {
10186
assertEquals(8, settingsData.maxCustomExceptionEvents);
@@ -127,10 +112,7 @@ private void verifySettingsDataObject(
127112
assertEquals(3, settingsData.settingsVersion);
128113
assertEquals(7200, settingsData.cacheDuration);
129114

130-
assertAppData(settingsData.appData, isAppNew);
131-
132115
assertFeaturesData(settingsData.featuresData, collectAnrs);
133-
134116
assertSettingsData(settingsData.sessionData);
135117

136118
verify(mockCurrentTimeProvider).getCurrentTimeMillis();

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

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

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

17-
import com.google.firebase.crashlytics.internal.settings.model.AppSettingsData;
1817
import com.google.firebase.crashlytics.internal.settings.model.FeaturesSettingsData;
1918
import com.google.firebase.crashlytics.internal.settings.model.SessionSettingsData;
2019
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
@@ -31,16 +30,7 @@ public TestSettingsData(int settingsVersion) {
3130

3231
public TestSettingsData(
3332
int settingsVersion, int reportUploadVariant, int nativeReportUploadVariant) {
34-
super(
35-
5,
36-
buildAppData(reportUploadVariant, nativeReportUploadVariant),
37-
buildSettingsData(),
38-
buildFeaturesData(),
39-
settingsVersion,
40-
3600,
41-
10,
42-
1.2,
43-
60);
33+
super(5, buildSettingsData(), buildFeaturesData(), settingsVersion, 3600, 10, 1.2, 60);
4434
}
4535

4636
private static FeaturesSettingsData buildFeaturesData() {
@@ -50,18 +40,4 @@ private static FeaturesSettingsData buildFeaturesData() {
5040
private static SessionSettingsData buildSettingsData() {
5141
return new SessionSettingsData(64, 4);
5242
}
53-
54-
private static AppSettingsData buildAppData(
55-
int reportUploadVariant, int nativeReportUploadVariant) {
56-
return new AppSettingsData(
57-
AppSettingsData.STATUS_ACTIVATED,
58-
"http://localhost",
59-
"http://localhost",
60-
"http://localhost",
61-
"testBundleId",
62-
"testOrganizationId",
63-
false,
64-
reportUploadVariant,
65-
nativeReportUploadVariant);
66-
}
6743
}

0 commit comments

Comments
 (0)