Skip to content
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.

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

refactor: Moves configuration creation for manifest to separate class #356

Merged
merged 1 commit into from
Aug 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 21 additions & 24 deletions sdk/src/androidTest/java/com/bugsnag/android/ClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,13 @@

import static com.bugsnag.android.BugsnagTestUtils.generateClient;
import static com.bugsnag.android.BugsnagTestUtils.getSharedPrefs;
import static com.bugsnag.android.BugsnagTestUtils.streamableToJson;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import android.content.Context;
import android.content.SharedPreferences;
import android.os.Build;
import android.os.Bundle;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.SmallTest;
Expand Down Expand Up @@ -178,18 +175,18 @@ public void testClearUser() {
public void testEmptyManifestConfig() {
Bundle data = new Bundle();
Configuration protoConfig = new Configuration("api-key");
Configuration newConfig = Client.populateConfigFromManifest(protoConfig, data);

assertEquals(config.getApiKey(), newConfig.getApiKey());
assertEquals(config.getBuildUUID(), newConfig.getBuildUUID());
assertEquals(config.getAppVersion(), newConfig.getAppVersion());
assertEquals(config.getReleaseStage(), newConfig.getReleaseStage());
assertEquals(config.getEndpoint(), newConfig.getEndpoint());
assertEquals(config.getSessionEndpoint(), newConfig.getSessionEndpoint());
assertEquals(config.getSendThreads(), newConfig.getSendThreads());
assertEquals(config.getEnableExceptionHandler(), newConfig.getEnableExceptionHandler());
ConfigFactory.populateConfigFromManifest(protoConfig, data);

assertEquals(config.getApiKey(), protoConfig.getApiKey());
assertEquals(config.getBuildUUID(), protoConfig.getBuildUUID());
assertEquals(config.getAppVersion(), protoConfig.getAppVersion());
assertEquals(config.getReleaseStage(), protoConfig.getReleaseStage());
assertEquals(config.getEndpoint(), protoConfig.getEndpoint());
assertEquals(config.getSessionEndpoint(), protoConfig.getSessionEndpoint());
assertEquals(config.getSendThreads(), protoConfig.getSendThreads());
assertEquals(config.getEnableExceptionHandler(), protoConfig.getEnableExceptionHandler());
assertEquals(config.getPersistUserBetweenSessions(),
newConfig.getPersistUserBetweenSessions());
protoConfig.getPersistUserBetweenSessions());
}

@Test
Expand All @@ -212,16 +209,16 @@ public void testFullManifestConfig() {
data.putBoolean("com.bugsnag.android.AUTO_CAPTURE_SESSIONS", true);

Configuration protoConfig = new Configuration("api-key");
Configuration newConfig = Client.populateConfigFromManifest(protoConfig, data);
assertEquals(buildUuid, newConfig.getBuildUUID());
assertEquals(appVersion, newConfig.getAppVersion());
assertEquals(releaseStage, newConfig.getReleaseStage());
assertEquals(endpoint, newConfig.getEndpoint());
assertEquals(sessionEndpoint, newConfig.getSessionEndpoint());
assertEquals(false, newConfig.getSendThreads());
assertEquals(false, newConfig.getEnableExceptionHandler());
assertEquals(true, newConfig.getPersistUserBetweenSessions());
assertEquals(true, newConfig.shouldAutoCaptureSessions());
ConfigFactory.populateConfigFromManifest(protoConfig, data);
assertEquals(buildUuid, protoConfig.getBuildUUID());
assertEquals(appVersion, protoConfig.getAppVersion());
assertEquals(releaseStage, protoConfig.getReleaseStage());
assertEquals(endpoint, protoConfig.getEndpoint());
assertEquals(sessionEndpoint, protoConfig.getSessionEndpoint());
assertEquals(false, protoConfig.getSendThreads());
assertEquals(false, protoConfig.getEnableExceptionHandler());
assertEquals(true, protoConfig.getPersistUserBetweenSessions());
assertEquals(true, protoConfig.shouldAutoCaptureSessions());
}

@Test
Expand Down
108 changes: 2 additions & 106 deletions sdk/src/main/java/com/bugsnag/android/Client.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.bugsnag.android;

import static com.bugsnag.android.ConfigFactory.MF_BUILD_UUID;
import static com.bugsnag.android.MapUtils.getStringFromMap;

import android.app.Activity;
Expand All @@ -13,10 +14,8 @@
import android.content.pm.PackageManager;
import android.net.ConnectivityManager;
import android.net.NetworkInfo;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.text.TextUtils;

import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -43,25 +42,10 @@ public class Client extends Observable implements Observer {

private static final boolean BLOCKING = true;
private static final String SHARED_PREF_KEY = "com.bugsnag.android";
private static final String BUGSNAG_NAMESPACE = "com.bugsnag.android";
private static final String USER_ID_KEY = "user.id";
private static final String USER_NAME_KEY = "user.name";
private static final String USER_EMAIL_KEY = "user.email";

static final String MF_API_KEY = BUGSNAG_NAMESPACE + ".API_KEY";
static final String MF_BUILD_UUID = BUGSNAG_NAMESPACE + ".BUILD_UUID";
static final String MF_APP_VERSION = BUGSNAG_NAMESPACE + ".APP_VERSION";
static final String MF_ENDPOINT = BUGSNAG_NAMESPACE + ".ENDPOINT";
static final String MF_SESSIONS_ENDPOINT = BUGSNAG_NAMESPACE + ".SESSIONS_ENDPOINT";
static final String MF_RELEASE_STAGE = BUGSNAG_NAMESPACE + ".RELEASE_STAGE";
static final String MF_SEND_THREADS = BUGSNAG_NAMESPACE + ".SEND_THREADS";
static final String MF_ENABLE_EXCEPTION_HANDLER =
BUGSNAG_NAMESPACE + ".ENABLE_EXCEPTION_HANDLER";
static final String MF_PERSIST_USER_BETWEEN_SESSIONS =
BUGSNAG_NAMESPACE + ".PERSIST_USER_BETWEEN_SESSIONS";
static final String MF_AUTO_CAPTURE_SESSIONS =
BUGSNAG_NAMESPACE + ".AUTO_CAPTURE_SESSIONS";

@NonNull
protected final Configuration config;
final Context appContext;
Expand Down Expand Up @@ -117,7 +101,7 @@ public Client(@NonNull Context androidContext,
@Nullable String apiKey,
boolean enableExceptionHandler) {
this(androidContext,
createNewConfiguration(androidContext, apiKey, enableExceptionHandler));
ConfigFactory.createNewConfiguration(androidContext, apiKey, enableExceptionHandler));
}

/**
Expand Down Expand Up @@ -258,94 +242,6 @@ public void update(Observable observable, Object arg) {
}
}

/**
* Creates a new configuration object based on the provided parameters
* will read the API key and other configuration values from the manifest if it is not provided
*
* @param androidContext The context of the application
* @param apiKey The API key to use
* @param enableExceptionHandler should we automatically handle uncaught exceptions?
* @return The created config
*/
@NonNull
private static Configuration createNewConfiguration(@NonNull Context androidContext,
String apiKey,
boolean enableExceptionHandler) {
Context appContext = androidContext.getApplicationContext();

// Attempt to load API key and other config from AndroidManifest.xml, if not passed in
boolean loadFromManifest = TextUtils.isEmpty(apiKey);

if (loadFromManifest) {
try {
PackageManager packageManager = appContext.getPackageManager();
String packageName = appContext.getPackageName();
ApplicationInfo ai =
packageManager.getApplicationInfo(packageName, PackageManager.GET_META_DATA);
Bundle data = ai.metaData;
apiKey = data.getString(MF_API_KEY);
} catch (Exception ignore) {
Logger.warn("Bugsnag is unable to read api key from manifest.");
}
}

if (apiKey == null) {
throw new NullPointerException("You must provide a Bugsnag API key");
}

// Build a configuration object
Configuration newConfig = new Configuration(apiKey);
newConfig.setEnableExceptionHandler(enableExceptionHandler);

if (loadFromManifest) {
try {
PackageManager packageManager = appContext.getPackageManager();
String packageName = appContext.getPackageName();
ApplicationInfo ai =
packageManager.getApplicationInfo(packageName, PackageManager.GET_META_DATA);
Bundle data = ai.metaData;
populateConfigFromManifest(newConfig, data);
} catch (Exception ignore) {
Logger.warn("Bugsnag is unable to read config from manifest.");
}
}
return newConfig;
}

/**
* Populates the config with meta-data values supplied from the manifest as a Bundle.
*
* @param config the config to mutate
* @param data the manifest bundle
* @return the updated config
*/
@NonNull
static Configuration populateConfigFromManifest(@NonNull Configuration config,
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the default access modifier that's applied here? Does moving this have any external implications?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The package-level modifier will be applied, so anything with the com.bugsnag.android namespace can access this. I don't believe this has any external implications as I'm not aware of any other notifiers which call this method.

@NonNull Bundle data) {
config.setBuildUUID(data.getString(MF_BUILD_UUID));
config.setAppVersion(data.getString(MF_APP_VERSION));
config.setReleaseStage(data.getString(MF_RELEASE_STAGE));

if (data.containsKey(MF_ENDPOINT)) {
String endpoint = data.getString(MF_ENDPOINT);
String sessionEndpoint = data.getString(MF_SESSIONS_ENDPOINT);
//noinspection ConstantConditions (pass in null/empty as this function will warn)
config.setEndpoints(endpoint, sessionEndpoint);
}

config.setSendThreads(data.getBoolean(MF_SEND_THREADS, true));
config.setPersistUserBetweenSessions(
data.getBoolean(MF_PERSIST_USER_BETWEEN_SESSIONS, false));

if (data.containsKey(MF_AUTO_CAPTURE_SESSIONS)) {
config.setAutoCaptureSessions(data.getBoolean(MF_AUTO_CAPTURE_SESSIONS));
}

config.setEnableExceptionHandler(
data.getBoolean(MF_ENABLE_EXCEPTION_HANDLER, true));
return config;
}

/**
* Manually starts tracking a new session.
*
Expand Down
112 changes: 112 additions & 0 deletions sdk/src/main/java/com/bugsnag/android/ConfigFactory.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package com.bugsnag.android;

import android.content.Context;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.text.TextUtils;

class ConfigFactory {

private static final String BUGSNAG_NAMESPACE = "com.bugsnag.android";
private static final String MF_APP_VERSION = BUGSNAG_NAMESPACE + ".APP_VERSION";
private static final String MF_ENDPOINT = BUGSNAG_NAMESPACE + ".ENDPOINT";
private static final String MF_SESSIONS_ENDPOINT = BUGSNAG_NAMESPACE + ".SESSIONS_ENDPOINT";
private static final String MF_RELEASE_STAGE = BUGSNAG_NAMESPACE + ".RELEASE_STAGE";
private static final String MF_SEND_THREADS = BUGSNAG_NAMESPACE + ".SEND_THREADS";
private static final String MF_ENABLE_EXCEPTION_HANDLER =
BUGSNAG_NAMESPACE + ".ENABLE_EXCEPTION_HANDLER";
private static final String MF_PERSIST_USER_BETWEEN_SESSIONS =
BUGSNAG_NAMESPACE + ".PERSIST_USER_BETWEEN_SESSIONS";
private static final String MF_AUTO_CAPTURE_SESSIONS =
BUGSNAG_NAMESPACE + ".AUTO_CAPTURE_SESSIONS";
private static final String MF_API_KEY = BUGSNAG_NAMESPACE + ".API_KEY";
static final String MF_BUILD_UUID = BUGSNAG_NAMESPACE + ".BUILD_UUID";

/**
* Creates a new configuration object based on the provided parameters
* will read the API key and other configuration values from the manifest if it is not provided
*
* @param androidContext The context of the application
* @param apiKey The API key to use
* @param enableExceptionHandler should we automatically handle uncaught exceptions?
* @return The created config
*/
@NonNull
static Configuration createNewConfiguration(@NonNull Context androidContext,
String apiKey,
boolean enableExceptionHandler) {
Context appContext = androidContext.getApplicationContext();

// Attempt to load API key and other config from AndroidManifest.xml, if not passed in
boolean loadFromManifest = TextUtils.isEmpty(apiKey);

if (loadFromManifest) {
try {
PackageManager packageManager = appContext.getPackageManager();
String packageName = appContext.getPackageName();
ApplicationInfo ai =
packageManager.getApplicationInfo(packageName, PackageManager.GET_META_DATA);
Bundle data = ai.metaData;
apiKey = data.getString(MF_API_KEY);
} catch (Exception ignore) {
Logger.warn("Bugsnag is unable to read api key from manifest.");
}
}

if (apiKey == null) {
throw new NullPointerException("You must provide a Bugsnag API key");
}

// Build a configuration object
Configuration newConfig = new Configuration(apiKey);
newConfig.setEnableExceptionHandler(enableExceptionHandler);

if (loadFromManifest) {
try {
PackageManager packageManager = appContext.getPackageManager();
String packageName = appContext.getPackageName();
ApplicationInfo ai =
packageManager.getApplicationInfo(packageName, PackageManager.GET_META_DATA);
Bundle data = ai.metaData;
populateConfigFromManifest(newConfig, data);
} catch (Exception ignore) {
Logger.warn("Bugsnag is unable to read config from manifest.");
}
}
return newConfig;
}

/**
* Populates the config with meta-data values supplied from the manifest as a Bundle.
*
* @param config the config to mutate
* @param data the manifest bundle
*/
static void populateConfigFromManifest(@NonNull Configuration config,
@NonNull Bundle data) {
config.setBuildUUID(data.getString(MF_BUILD_UUID));
config.setAppVersion(data.getString(MF_APP_VERSION));
config.setReleaseStage(data.getString(MF_RELEASE_STAGE));

if (data.containsKey(MF_ENDPOINT)) {
String endpoint = data.getString(MF_ENDPOINT);
String sessionEndpoint = data.getString(MF_SESSIONS_ENDPOINT);
//noinspection ConstantConditions (pass in null/empty as this function will warn)
config.setEndpoints(endpoint, sessionEndpoint);
}

config.setSendThreads(data.getBoolean(MF_SEND_THREADS, true));
config.setPersistUserBetweenSessions(
data.getBoolean(MF_PERSIST_USER_BETWEEN_SESSIONS, false));

if (data.containsKey(MF_AUTO_CAPTURE_SESSIONS)) {
config.setAutoCaptureSessions(data.getBoolean(MF_AUTO_CAPTURE_SESSIONS));
}

config.setEnableExceptionHandler(
data.getBoolean(MF_ENABLE_EXCEPTION_HANDLER, true));
}

}