Skip to content

Commit a751091

Browse files
mrichardsmrichards
authored
Simplify Crashlytics settings (#3625)
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. Simplified Settings model and flattened package hierarchy.
1 parent c54c32f commit a751091

37 files changed

+397
-1193
lines changed

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

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@
3939
import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger;
4040
import com.google.firebase.crashlytics.internal.metadata.LogFileManager;
4141
import com.google.firebase.crashlytics.internal.persistence.FileStore;
42-
import com.google.firebase.crashlytics.internal.settings.SettingsDataProvider;
43-
import com.google.firebase.crashlytics.internal.settings.TestSettingsData;
44-
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
42+
import com.google.firebase.crashlytics.internal.settings.Settings;
43+
import com.google.firebase.crashlytics.internal.settings.SettingsProvider;
44+
import com.google.firebase.crashlytics.internal.settings.TestSettings;
4545
import com.google.firebase.installations.FirebaseInstallationsApi;
4646
import java.io.File;
4747
import java.util.Arrays;
@@ -58,7 +58,7 @@ public class CrashlyticsControllerTest extends CrashlyticsTestCase {
5858

5959
private Context testContext;
6060
private IdManager idManager;
61-
private SettingsDataProvider testSettingsDataProvider;
61+
private SettingsProvider testSettingsProvider;
6262
private FileStore testFileStore;
6363
private SessionReportingCoordinator mockSessionReportingCoordinator;
6464
private DataCollectionArbiter mockDataCollectionArbiter;
@@ -81,17 +81,16 @@ protected void setUp() throws Exception {
8181

8282
testFileStore = new FileStore(testContext);
8383

84-
final SettingsData testSettingsData = new TestSettingsData(3);
84+
Settings testSettings = new TestSettings(3);
8585

8686
mockSessionReportingCoordinator = mock(SessionReportingCoordinator.class);
8787

8888
mockDataCollectionArbiter = mock(DataCollectionArbiter.class);
8989
when(mockDataCollectionArbiter.isAutomaticDataCollectionEnabled()).thenReturn(true);
9090

91-
testSettingsDataProvider = mock(SettingsDataProvider.class);
92-
when(testSettingsDataProvider.getSettings()).thenReturn(testSettingsData);
93-
when(testSettingsDataProvider.getAppSettings())
94-
.thenReturn(Tasks.forResult(testSettingsData.appData));
91+
testSettingsProvider = mock(SettingsProvider.class);
92+
when(testSettingsProvider.getSettingsSync()).thenReturn(testSettings);
93+
when(testSettingsProvider.getSettingsAsync()).thenReturn(Tasks.forResult(testSettings));
9594
}
9695

9796
/** A convenience class for building CrashlyticsController instances for testing. */
@@ -185,7 +184,7 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal()
185184
.thenReturn(new TreeSet<>(Collections.singleton(sessionId)));
186185

187186
controller.writeNonFatalException(thread, nonFatal);
188-
controller.doCloseSessions(testSettingsDataProvider);
187+
controller.doCloseSessions(testSettingsProvider);
189188

190189
verify(mockSessionReportingCoordinator)
191190
.persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong());
@@ -200,7 +199,7 @@ public void testFatalException_callsSessionReportingCoordinatorPersistFatal() th
200199
when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
201200
.thenReturn(new TreeSet<>(Collections.singleton(sessionId)));
202201

203-
controller.handleUncaughtException(testSettingsDataProvider, thread, fatal);
202+
controller.handleUncaughtException(testSettingsProvider, thread, fatal);
204203

205204
verify(mockSessionReportingCoordinator)
206205
.persistFatalEvent(eq(fatal), eq(thread), eq(sessionId), anyLong());
@@ -274,7 +273,7 @@ public File getOsFile() {
274273
final CrashlyticsController controller =
275274
builder().setNativeComponent(mockNativeComponent).setLogFileManager(logFileManager).build();
276275

277-
controller.finalizeSessions(testSettingsDataProvider);
276+
controller.finalizeSessions(testSettingsProvider);
278277
verify(mockSessionReportingCoordinator)
279278
.finalizeSessionWithNativeEvent(eq(previousSessionId), any());
280279
verify(mockSessionReportingCoordinator, never())
@@ -283,7 +282,7 @@ public File getOsFile() {
283282

284283
public void testMissingNativeComponentCausesNoReports() {
285284
final CrashlyticsController controller = createController();
286-
controller.finalizeSessions(testSettingsDataProvider);
285+
controller.finalizeSessions(testSettingsProvider);
287286

288287
List<String> sessions = testFileStore.getAllOpenSessionIds();
289288
for (String sessionId : sessions) {
@@ -300,7 +299,7 @@ public void testMissingNativeComponentCausesNoReports() {
300299
public void testLoggedExceptionsAfterCrashOk() {
301300
final CrashlyticsController controller = builder().build();
302301
controller.handleUncaughtException(
303-
testSettingsDataProvider, Thread.currentThread(), new RuntimeException());
302+
testSettingsProvider, Thread.currentThread(), new RuntimeException());
304303

305304
// This should not throw.
306305
controller.writeNonFatalException(Thread.currentThread(), new RuntimeException());
@@ -314,7 +313,7 @@ public void testLoggedExceptionsAfterCrashOk() {
314313
public void testLogStringAfterCrashOk() {
315314
final CrashlyticsController controller = builder().build();
316315
controller.handleUncaughtException(
317-
testSettingsDataProvider, Thread.currentThread(), new RuntimeException());
316+
testSettingsProvider, Thread.currentThread(), new RuntimeException());
318317

319318
// This should not throw.
320319
controller.writeToLog(System.currentTimeMillis(), "Hi");
@@ -328,18 +327,18 @@ public void testLogStringAfterCrashOk() {
328327
public void testFinalizeSessionAfterCrashOk() throws Exception {
329328
final CrashlyticsController controller = builder().build();
330329
controller.handleUncaughtException(
331-
testSettingsDataProvider, Thread.currentThread(), new RuntimeException());
330+
testSettingsProvider, Thread.currentThread(), new RuntimeException());
332331

333332
// This should not throw.
334-
controller.finalizeSessions(testSettingsDataProvider);
333+
controller.finalizeSessions(testSettingsProvider);
335334
}
336335

337336
public void testUploadWithNoReports() throws Exception {
338337
when(mockSessionReportingCoordinator.hasReportsToSend()).thenReturn(false);
339338

340339
final CrashlyticsController controller = createController();
341340

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

344343
await(task);
345344

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

355354
final CrashlyticsController controller = createController();
356355

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

359358
await(task);
360359

@@ -380,7 +379,7 @@ public void testUploadDisabledThenOptIn() throws Exception {
380379
builder.setDataCollectionArbiter(arbiter);
381380
final CrashlyticsController controller = builder.build();
382381

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

385384
verify(arbiter).isAutomaticDataCollectionEnabled();
386385
verify(mockSessionReportingCoordinator).hasReportsToSend();
@@ -407,7 +406,7 @@ public void testUploadDisabledThenOptOut() throws Exception {
407406
builder.setDataCollectionArbiter(arbiter);
408407
final CrashlyticsController controller = builder.build();
409408

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

412411
verify(arbiter).isAutomaticDataCollectionEnabled();
413412
verify(mockSessionReportingCoordinator).hasReportsToSend();
@@ -448,7 +447,7 @@ public void testUploadDisabledThenEnabled() throws Exception {
448447
builder.setDataCollectionArbiter(arbiter);
449448
final CrashlyticsController controller = builder.build();
450449

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

453452
verify(mockSessionReportingCoordinator).hasReportsToSend();
454453
verify(mockSessionReportingCoordinator, never()).sendReports(any(Executor.class));
@@ -491,8 +490,8 @@ public void testFatalEvent_sendsAppExceptionEvent() {
491490

492491
controller.openSession(SESSION_ID);
493492
controller.handleUncaughtException(
494-
testSettingsDataProvider, Thread.currentThread(), new RuntimeException("Fatal"));
495-
controller.finalizeSessions(testSettingsDataProvider);
493+
testSettingsProvider, Thread.currentThread(), new RuntimeException("Fatal"));
494+
controller.finalizeSessions(testSettingsProvider);
496495

497496
assertFirebaseAnalyticsCrashEvent(mockFirebaseAnalyticsLogger);
498497
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@
3434
import com.google.firebase.crashlytics.internal.analytics.UnavailableAnalyticsEventLogger;
3535
import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource;
3636
import com.google.firebase.crashlytics.internal.persistence.FileStore;
37+
import com.google.firebase.crashlytics.internal.settings.Settings;
3738
import com.google.firebase.crashlytics.internal.settings.SettingsController;
38-
import com.google.firebase.crashlytics.internal.settings.TestSettingsData;
39-
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
39+
import com.google.firebase.crashlytics.internal.settings.TestSettings;
4040
import com.google.firebase.inject.Deferred;
4141
import com.google.firebase.installations.FirebaseInstallationsApi;
4242
import java.io.File;
@@ -71,9 +71,9 @@ protected void setUp() throws Exception {
7171
fileStore = new FileStore(getContext());
7272

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

7979
@Override

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@
3939
import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource;
4040
import com.google.firebase.crashlytics.internal.metadata.UserMetadata;
4141
import com.google.firebase.crashlytics.internal.persistence.FileStore;
42+
import com.google.firebase.crashlytics.internal.settings.Settings;
4243
import com.google.firebase.crashlytics.internal.settings.SettingsController;
43-
import com.google.firebase.crashlytics.internal.settings.TestSettingsData;
44-
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
44+
import com.google.firebase.crashlytics.internal.settings.TestSettings;
4545
import com.google.firebase.inject.Deferred;
4646
import com.google.firebase.installations.FirebaseInstallationsApi;
4747
import java.util.HashMap;
@@ -339,9 +339,9 @@ private Task<CrashlyticsCore> startCoreAsync(CrashlyticsCore crashlyticsCore) {
339339
Thread.setDefaultUncaughtExceptionHandler(NOOP_HANDLER);
340340

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

346346
AppData appData =
347347
new AppData(

0 commit comments

Comments
 (0)