Skip to content

Simplify Crashlytics settings #3625

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 3 commits into from
Apr 7, 2022
Merged

Simplify Crashlytics settings #3625

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e07b919
Remove unused Crashlytics Settings fields
mrichards Mar 28, 2022
a69593d
Simplified Settings model and flattened package hierarchy.
mrichards Apr 5, 2022
233c041
Removed unnecessary methods from Settings.java
mrichards Apr 7, 2022
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
47 changes: 23 additions & 24 deletions ...dTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger;
import com.google.firebase.crashlytics.internal.metadata.LogFileManager;
import com.google.firebase.crashlytics.internal.persistence.FileStore;
import com.google.firebase.crashlytics.internal.settings.SettingsDataProvider;
import com.google.firebase.crashlytics.internal.settings.TestSettingsData;
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
import com.google.firebase.crashlytics.internal.settings.Settings;
import com.google.firebase.crashlytics.internal.settings.SettingsProvider;
import com.google.firebase.crashlytics.internal.settings.TestSettings;
import com.google.firebase.installations.FirebaseInstallationsApi;
import java.io.File;
import java.util.Arrays;
Expand All @@ -58,7 +58,7 @@ public class CrashlyticsControllerTest extends CrashlyticsTestCase {

private Context testContext;
private IdManager idManager;
private SettingsDataProvider testSettingsDataProvider;
private SettingsProvider testSettingsProvider;
private FileStore testFileStore;
private SessionReportingCoordinator mockSessionReportingCoordinator;
private DataCollectionArbiter mockDataCollectionArbiter;
Expand All @@ -81,17 +81,16 @@ protected void setUp() throws Exception {

testFileStore = new FileStore(testContext);

final SettingsData testSettingsData = new TestSettingsData(3);
Settings testSettings = new TestSettings(3);

mockSessionReportingCoordinator = mock(SessionReportingCoordinator.class);

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

testSettingsDataProvider = mock(SettingsDataProvider.class);
when(testSettingsDataProvider.getSettings()).thenReturn(testSettingsData);
when(testSettingsDataProvider.getAppSettings())
.thenReturn(Tasks.forResult(testSettingsData.appData));
testSettingsProvider = mock(SettingsProvider.class);
when(testSettingsProvider.getSettingsSync()).thenReturn(testSettings);
when(testSettingsProvider.getSettingsAsync()).thenReturn(Tasks.forResult(testSettings));
}

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

controller.writeNonFatalException(thread, nonFatal);
controller.doCloseSessions(testSettingsDataProvider);
controller.doCloseSessions(testSettingsProvider);

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

controller.handleUncaughtException(testSettingsDataProvider, thread, fatal);
controller.handleUncaughtException(testSettingsProvider, thread, fatal);

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

controller.finalizeSessions(testSettingsDataProvider);
controller.finalizeSessions(testSettingsProvider);
verify(mockSessionReportingCoordinator)
.finalizeSessionWithNativeEvent(eq(previousSessionId), any());
verify(mockSessionReportingCoordinator, never())
Expand All @@ -283,7 +282,7 @@ public File getOsFile() {

public void testMissingNativeComponentCausesNoReports() {
final CrashlyticsController controller = createController();
controller.finalizeSessions(testSettingsDataProvider);
controller.finalizeSessions(testSettingsProvider);

List<String> sessions = testFileStore.getAllOpenSessionIds();
for (String sessionId : sessions) {
Expand All @@ -300,7 +299,7 @@ public void testMissingNativeComponentCausesNoReports() {
public void testLoggedExceptionsAfterCrashOk() {
final CrashlyticsController controller = builder().build();
controller.handleUncaughtException(
testSettingsDataProvider, Thread.currentThread(), new RuntimeException());
testSettingsProvider, Thread.currentThread(), new RuntimeException());

// This should not throw.
controller.writeNonFatalException(Thread.currentThread(), new RuntimeException());
Expand All @@ -314,7 +313,7 @@ public void testLoggedExceptionsAfterCrashOk() {
public void testLogStringAfterCrashOk() {
final CrashlyticsController controller = builder().build();
controller.handleUncaughtException(
testSettingsDataProvider, Thread.currentThread(), new RuntimeException());
testSettingsProvider, Thread.currentThread(), new RuntimeException());

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

// This should not throw.
controller.finalizeSessions(testSettingsDataProvider);
controller.finalizeSessions(testSettingsProvider);
}

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

final CrashlyticsController controller = createController();

Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());

await(task);

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

final CrashlyticsController controller = createController();

final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());

await(task);

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

final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());

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

final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());

verify(arbiter).isAutomaticDataCollectionEnabled();
verify(mockSessionReportingCoordinator).hasReportsToSend();
Expand Down Expand Up @@ -448,7 +447,7 @@ public void testUploadDisabledThenEnabled() throws Exception {
builder.setDataCollectionArbiter(arbiter);
final CrashlyticsController controller = builder.build();

final Task<Void> task = controller.submitAllReports(testSettingsDataProvider.getAppSettings());
final Task<Void> task = controller.submitAllReports(testSettingsProvider.getSettingsAsync());

verify(mockSessionReportingCoordinator).hasReportsToSend();
verify(mockSessionReportingCoordinator, never()).sendReports(any(Executor.class));
Expand Down Expand Up @@ -491,8 +490,8 @@ public void testFatalEvent_sendsAppExceptionEvent() {

controller.openSession(SESSION_ID);
controller.handleUncaughtException(
testSettingsDataProvider, Thread.currentThread(), new RuntimeException("Fatal"));
controller.finalizeSessions(testSettingsDataProvider);
testSettingsProvider, Thread.currentThread(), new RuntimeException("Fatal"));
controller.finalizeSessions(testSettingsProvider);

assertFirebaseAnalyticsCrashEvent(mockFirebaseAnalyticsLogger);
}
Expand Down
10 changes: 5 additions & 5 deletions ...va/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreInitializationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
import com.google.firebase.crashlytics.internal.analytics.UnavailableAnalyticsEventLogger;
import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource;
import com.google.firebase.crashlytics.internal.persistence.FileStore;
import com.google.firebase.crashlytics.internal.settings.Settings;
import com.google.firebase.crashlytics.internal.settings.SettingsController;
import com.google.firebase.crashlytics.internal.settings.TestSettingsData;
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
import com.google.firebase.crashlytics.internal.settings.TestSettings;
import com.google.firebase.inject.Deferred;
import com.google.firebase.installations.FirebaseInstallationsApi;
import java.io.File;
Expand Down Expand Up @@ -71,9 +71,9 @@ protected void setUp() throws Exception {
fileStore = new FileStore(getContext());

mockSettingsController = mock(SettingsController.class);
final SettingsData settingsData = new TestSettingsData();
when(mockSettingsController.getSettings()).thenReturn(settingsData);
when(mockSettingsController.getAppSettings()).thenReturn(Tasks.forResult(settingsData.appData));
Settings settings = new TestSettings();
when(mockSettingsController.getSettingsSync()).thenReturn(settings);
when(mockSettingsController.getSettingsAsync()).thenReturn(Tasks.forResult(settings));
}

@Override
Expand Down
10 changes: 5 additions & 5 deletions ...androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
import com.google.firebase.crashlytics.internal.breadcrumbs.DisabledBreadcrumbSource;
import com.google.firebase.crashlytics.internal.metadata.UserMetadata;
import com.google.firebase.crashlytics.internal.persistence.FileStore;
import com.google.firebase.crashlytics.internal.settings.Settings;
import com.google.firebase.crashlytics.internal.settings.SettingsController;
import com.google.firebase.crashlytics.internal.settings.TestSettingsData;
import com.google.firebase.crashlytics.internal.settings.model.SettingsData;
import com.google.firebase.crashlytics.internal.settings.TestSettings;
import com.google.firebase.inject.Deferred;
import com.google.firebase.installations.FirebaseInstallationsApi;
import java.util.HashMap;
Expand Down Expand Up @@ -339,9 +339,9 @@ private Task<CrashlyticsCore> startCoreAsync(CrashlyticsCore crashlyticsCore) {
Thread.setDefaultUncaughtExceptionHandler(NOOP_HANDLER);

SettingsController mockSettingsController = mock(SettingsController.class);
final SettingsData settings = new TestSettingsData(3);
when(mockSettingsController.getSettings()).thenReturn(settings);
when(mockSettingsController.getAppSettings()).thenReturn(Tasks.forResult(settings.appData));
final Settings settings = new TestSettings(3);
when(mockSettingsController.getSettingsSync()).thenReturn(settings);
when(mockSettingsController.getSettingsAsync()).thenReturn(Tasks.forResult(settings));

AppData appData =
new AppData(
Expand Down
45 changes: 22 additions & 23 deletions ...om/google/firebase/crashlytics/internal/persistence/CrashlyticsReportPersistenceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport.Session.Event.Application.Execution.Signal;
import com.google.firebase.crashlytics.internal.model.CrashlyticsReport.Session.Event.Application.Execution.Thread.Frame;
import com.google.firebase.crashlytics.internal.model.ImmutableList;
import com.google.firebase.crashlytics.internal.settings.SettingsDataProvider;
import com.google.firebase.crashlytics.internal.settings.model.SessionSettingsData;
import com.google.firebase.crashlytics.internal.settings.model.Settings;
import com.google.firebase.crashlytics.internal.settings.Settings;
import com.google.firebase.crashlytics.internal.settings.SettingsProvider;
import java.io.IOException;
import java.text.DecimalFormat;
import java.util.ArrayList;
Expand All @@ -48,15 +47,15 @@ public class CrashlyticsReportPersistenceTest extends CrashlyticsTestCase {
private CrashlyticsReportPersistence reportPersistence;
private FileStore fileStore;

private static SettingsDataProvider getSettingsMock(
private static SettingsProvider getSettingsMock(
int maxCompleteSessionsCount, int maxCustomExceptionEvents) {
SettingsDataProvider settingsDataProvider = mock(SettingsDataProvider.class);
SettingsProvider settingsProvider = mock(SettingsProvider.class);
Settings settingsMock = mock(Settings.class);
SessionSettingsData sessionSettingsDataMock =
new SessionSettingsData(maxCustomExceptionEvents, maxCompleteSessionsCount);
Settings.SessionData sessionSettingsDataMock =
new Settings.SessionData(maxCustomExceptionEvents, maxCompleteSessionsCount);
when(settingsMock.getSessionData()).thenReturn(sessionSettingsDataMock);
when(settingsDataProvider.getSettings()).thenReturn(settingsMock);
return settingsDataProvider;
when(settingsProvider.getSettingsSync()).thenReturn(settingsMock);
return settingsProvider;
}

@Override
Expand Down Expand Up @@ -283,15 +282,15 @@ public void testFinalizeReports_capsReports() {
}

public void testFinalizeReports_whenSettingsChanges_capsReports() throws IOException {
SettingsDataProvider settingsDataProvider = mock(SettingsDataProvider.class);
SettingsProvider settingsProvider = mock(SettingsProvider.class);
Settings settingsMock = mock(Settings.class);
SessionSettingsData sessionSettingsDataMock =
new SessionSettingsData(VERY_LARGE_UPPER_LIMIT, 4);
SessionSettingsData sessionSettingsDataMockDifferentValues =
new SessionSettingsData(VERY_LARGE_UPPER_LIMIT, 8);
Settings.SessionData sessionSettingsDataMock =
new Settings.SessionData(VERY_LARGE_UPPER_LIMIT, 4);
Settings.SessionData sessionSettingsDataMockDifferentValues =
new Settings.SessionData(VERY_LARGE_UPPER_LIMIT, 8);
when(settingsMock.getSessionData()).thenReturn(sessionSettingsDataMock);
when(settingsDataProvider.getSettings()).thenReturn(settingsMock);
reportPersistence = new CrashlyticsReportPersistence(fileStore, settingsDataProvider);
when(settingsProvider.getSettingsSync()).thenReturn(settingsMock);
reportPersistence = new CrashlyticsReportPersistence(fileStore, settingsProvider);

DecimalFormat format = new DecimalFormat("00");
for (int i = 0; i < 16; i++) {
Expand Down Expand Up @@ -542,15 +541,15 @@ public void testPersistEvent_keepsAppropriateNumberOfMostRecentEvents() throws I

public void testPersistEvent_whenSettingsChanges_keepsAppropriateNumberOfMostRecentEvents()
throws IOException {
SettingsDataProvider settingsDataProvider = mock(SettingsDataProvider.class);
SettingsProvider settingsProvider = mock(SettingsProvider.class);
Settings settingsMock = mock(Settings.class);
SessionSettingsData sessionSettingsDataMock =
new SessionSettingsData(4, VERY_LARGE_UPPER_LIMIT);
SessionSettingsData sessionSettingsDataMockDifferentValues =
new SessionSettingsData(8, VERY_LARGE_UPPER_LIMIT);
Settings.SessionData sessionSettingsDataMock =
new Settings.SessionData(4, VERY_LARGE_UPPER_LIMIT);
Settings.SessionData sessionSettingsDataMockDifferentValues =
new Settings.SessionData(8, VERY_LARGE_UPPER_LIMIT);
when(settingsMock.getSessionData()).thenReturn(sessionSettingsDataMock);
when(settingsDataProvider.getSettings()).thenReturn(settingsMock);
reportPersistence = new CrashlyticsReportPersistence(fileStore, settingsDataProvider);
when(settingsProvider.getSettingsSync()).thenReturn(settingsMock);
reportPersistence = new CrashlyticsReportPersistence(fileStore, settingsProvider);

final String sessionId = "testSession";
final CrashlyticsReport testReport = makeTestReport(sessionId);
Expand Down
Loading