From 8b0af693286724f83cf39ad288936a8c0d36d17b Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 12 Dec 2019 14:31:32 -0800 Subject: [PATCH 1/4] added executor for caching values out of the main thread --- .../core/DefaultAndroidEventProcessor.java | 91 +++++++++++++++++-- 1 file changed, 82 insertions(+), 9 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 904e34145..959a37ec7 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -46,10 +46,15 @@ import java.util.Arrays; import java.util.Calendar; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Properties; import java.util.TimeZone; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import org.jetbrains.annotations.Nullable; final class DefaultAndroidEventProcessor implements EventProcessor { @@ -59,12 +64,43 @@ final class DefaultAndroidEventProcessor implements EventProcessor { final Context context; private final SentryOptions options; + private final Future> contextData; + public DefaultAndroidEventProcessor(Context context, SentryOptions options) { this.context = Objects.requireNonNull( context != null ? context.getApplicationContext() : null, "The application context is required."); this.options = Objects.requireNonNull(options, "The SentryOptions is required."); + + ExecutorService executorService = Executors.newSingleThreadExecutor(); + contextData = executorService.submit(this::loadContextData); + + executorService.shutdown(); + } + + private Map loadContextData() { + Map map = new HashMap<>(); + String[] proGuardUuids = getProGuardUuids(); + if (proGuardUuids != null) { + map.put("proGuardUuids", proGuardUuids); + } + + map.put("rooted", isRooted()); + + String androidId = getAndroidId(); + if (androidId != null) { + map.put("androidId", androidId); + } + + String kernelVersion = getKernelVersion(); + if (kernelVersion != null) { + map.put("kernelVersion", kernelVersion); + } + + map.put("emulator", isEmulator()); + + return map; } @Override @@ -118,7 +154,16 @@ private void processNonCachedEvent(SentryEvent event) { } private List getDebugImages() { - String[] uuids = getProGuardUuids(); + String[] uuids = null; + try { + Object proGuardUuids = contextData.get().get("proGuardUuids"); + if (proGuardUuids != null) { + uuids = (String[]) proGuardUuids; + } + } catch (Exception e) { + log(SentryLevel.ERROR, "Error getting proGuardUuids.", e); + return null; + } if (uuids == null || uuids.length == 0) { return null; @@ -252,7 +297,15 @@ private Device getDevice() { } device.setOnline(isConnected()); device.setOrientation(getOrientation()); - device.setSimulator(isEmulator()); + + try { + Object emulator = contextData.get().get("emulator"); + if (emulator != null) { + device.setSimulator((Boolean) emulator); + } + } catch (Exception e) { + log(SentryLevel.ERROR, "Error getting emulator.", e); + } ActivityManager.MemoryInfo memInfo = getMemInfo(); if (memInfo != null) { @@ -583,11 +636,11 @@ private StatFs getExternalStorageStat(File internalStorage) { } @SuppressWarnings("ObsoleteSdkInt") - public File[] getExternalFilesDirs(String type) { + private File[] getExternalFilesDirs() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { - return context.getExternalFilesDirs(type); + return context.getExternalFilesDirs(null); } else { - File single = context.getExternalFilesDir(type); + File single = context.getExternalFilesDir(null); if (single != null) { return new File[] {single}; } @@ -596,7 +649,7 @@ public File[] getExternalFilesDirs(String type) { } private File getExternalStorageDep(File internalStorage) { - File[] externalFilesDirs = getExternalFilesDirs(null); + File[] externalFilesDirs = getExternalFilesDirs(); if (externalFilesDirs != null) { // return the 1st file which is not the emulated internal storage @@ -677,8 +730,20 @@ private OperatingSystem getOperatingSystem() { os.setName("Android"); os.setVersion(Build.VERSION.RELEASE); os.setBuild(Build.DISPLAY); - os.setKernelVersion(getKernelVersion()); - os.setRooted(isRooted()); + + try { + Object kernelVersion = contextData.get().get("kernelVersion"); + if (kernelVersion != null) { + os.setKernelVersion((String) kernelVersion); + } + + Object rooted = contextData.get().get("rooted"); + if (rooted != null) { + os.setRooted((Boolean) rooted); + } + } catch (Exception e) { + log(SentryLevel.ERROR, "Error getting OperatingSystem.", e); + } return os; } @@ -794,7 +859,15 @@ private String getApplicationName() { public User getUser() { User user = new User(); - user.setId(getAndroidId()); + try { + Object androidId = contextData.get().get("androidId"); + + if (androidId != null) { + user.setId((String) androidId); + } + } catch (Exception e) { + log(SentryLevel.ERROR, "Error getting androidId.", e); + } return user; } From 93b6b7f3289e3af86ca5273e47a21fc095c5c407 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 12 Dec 2019 14:52:27 -0800 Subject: [PATCH 2/4] fix lint issue due to a bug on lint --- .../io/sentry/android/core/DefaultAndroidEventProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 959a37ec7..8ff03d82a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -74,7 +74,7 @@ public DefaultAndroidEventProcessor(Context context, SentryOptions options) { this.options = Objects.requireNonNull(options, "The SentryOptions is required."); ExecutorService executorService = Executors.newSingleThreadExecutor(); - contextData = executorService.submit(this::loadContextData); + contextData = executorService.submit(() -> loadContextData()); executorService.shutdown(); } From 36b925582e3bf10f30d40483ff3db3bc08a23782 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 13 Dec 2019 08:19:52 -0800 Subject: [PATCH 3/4] adding const to stirng keys --- .../core/DefaultAndroidEventProcessor.java | 27 ++++++++++++------- .../android/core/ManifestMetadataReader.java | 5 +++- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 8ff03d82a..43f54b815 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -59,6 +59,12 @@ final class DefaultAndroidEventProcessor implements EventProcessor { + private static final String PROGUARD_UUID = "proGuardUuids"; + private static final String ROOTED = "rooted"; + private static final String ANDROID_ID = "androidId"; + private static final String KERNEL_VERSION = "kernelVersion"; + private static final String EMULATOR = "emulator"; + // it could also be a parameter and get from Sentry.init(...) private static final Date appStartTime = DateUtils.getCurrentDateTime(); final Context context; @@ -83,22 +89,23 @@ private Map loadContextData() { Map map = new HashMap<>(); String[] proGuardUuids = getProGuardUuids(); if (proGuardUuids != null) { - map.put("proGuardUuids", proGuardUuids); + map.put(PROGUARD_UUID, proGuardUuids); } - map.put("rooted", isRooted()); + map.put(ROOTED, isRooted()); String androidId = getAndroidId(); if (androidId != null) { - map.put("androidId", androidId); + map.put(ANDROID_ID, androidId); } String kernelVersion = getKernelVersion(); if (kernelVersion != null) { - map.put("kernelVersion", kernelVersion); + map.put(KERNEL_VERSION, kernelVersion); } - map.put("emulator", isEmulator()); + // its not IO, but it has been cached in the old version as well + map.put(EMULATOR, isEmulator()); return map; } @@ -156,7 +163,7 @@ private void processNonCachedEvent(SentryEvent event) { private List getDebugImages() { String[] uuids = null; try { - Object proGuardUuids = contextData.get().get("proGuardUuids"); + Object proGuardUuids = contextData.get().get(PROGUARD_UUID); if (proGuardUuids != null) { uuids = (String[]) proGuardUuids; } @@ -299,7 +306,7 @@ private Device getDevice() { device.setOrientation(getOrientation()); try { - Object emulator = contextData.get().get("emulator"); + Object emulator = contextData.get().get(EMULATOR); if (emulator != null) { device.setSimulator((Boolean) emulator); } @@ -732,12 +739,12 @@ private OperatingSystem getOperatingSystem() { os.setBuild(Build.DISPLAY); try { - Object kernelVersion = contextData.get().get("kernelVersion"); + Object kernelVersion = contextData.get().get(KERNEL_VERSION); if (kernelVersion != null) { os.setKernelVersion((String) kernelVersion); } - Object rooted = contextData.get().get("rooted"); + Object rooted = contextData.get().get(ROOTED); if (rooted != null) { os.setRooted((Boolean) rooted); } @@ -860,7 +867,7 @@ public User getUser() { User user = new User(); try { - Object androidId = contextData.get().get("androidId"); + Object androidId = contextData.get().get(ANDROID_ID); if (androidId != null) { user.setId((String) androidId); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index c7f50cf36..e741de958 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -56,7 +56,10 @@ static void applyMetadata(Context context, SentryAndroidOptions options) { String dsn = metadata.getString(DSN_KEY, null); if (dsn == null) { - logIfNotNull(options.getLogger(), SentryLevel.FATAL, "DSN is required. Use empty string to disable SDK."); + logIfNotNull( + options.getLogger(), + SentryLevel.FATAL, + "DSN is required. Use empty string to disable SDK."); } else if (dsn.isEmpty()) { logIfNotNull( options.getLogger(), SentryLevel.DEBUG, "DSN is empty, disabling sentry-android"); From cc4dbbc182cd980ec651c1e28b69fde9c1fea929 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 13 Dec 2019 09:03:32 -0800 Subject: [PATCH 4/4] added unit test for executor sercice --- .../core/DefaultAndroidEventProcessor.java | 17 ++++++++++------- .../test/assets/sentry-debug-meta.properties | 1 + .../core/DefaultAndroidEventProcessorTest.kt | 19 ++++++++++++++++++- 3 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 sentry-android-core/src/test/assets/sentry-debug-meta.properties diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 43f54b815..a9d348698 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -56,21 +56,24 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; final class DefaultAndroidEventProcessor implements EventProcessor { - private static final String PROGUARD_UUID = "proGuardUuids"; - private static final String ROOTED = "rooted"; - private static final String ANDROID_ID = "androidId"; - private static final String KERNEL_VERSION = "kernelVersion"; - private static final String EMULATOR = "emulator"; + @TestOnly static final String PROGUARD_UUID = "proGuardUuids"; + @TestOnly static final String ROOTED = "rooted"; + @TestOnly static final String ANDROID_ID = "androidId"; + @TestOnly static final String KERNEL_VERSION = "kernelVersion"; + @TestOnly static final String EMULATOR = "emulator"; // it could also be a parameter and get from Sentry.init(...) private static final Date appStartTime = DateUtils.getCurrentDateTime(); - final Context context; + + @TestOnly final Context context; + private final SentryOptions options; - private final Future> contextData; + @TestOnly final Future> contextData; public DefaultAndroidEventProcessor(Context context, SentryOptions options) { this.context = diff --git a/sentry-android-core/src/test/assets/sentry-debug-meta.properties b/sentry-android-core/src/test/assets/sentry-debug-meta.properties new file mode 100644 index 000000000..0634e9353 --- /dev/null +++ b/sentry-android-core/src/test/assets/sentry-debug-meta.properties @@ -0,0 +1 @@ +io.sentry.ProguardUuids=test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index bb09637ba..2c9a0901d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -5,6 +5,11 @@ import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.nhaarman.mockitokotlin2.doReturn import com.nhaarman.mockitokotlin2.mock +import io.sentry.android.core.DefaultAndroidEventProcessor.ANDROID_ID +import io.sentry.android.core.DefaultAndroidEventProcessor.EMULATOR +import io.sentry.android.core.DefaultAndroidEventProcessor.KERNEL_VERSION +import io.sentry.android.core.DefaultAndroidEventProcessor.PROGUARD_UUID +import io.sentry.android.core.DefaultAndroidEventProcessor.ROOTED import io.sentry.core.ILogger import io.sentry.core.SentryEvent import io.sentry.core.SentryOptions @@ -68,7 +73,7 @@ class DefaultAndroidEventProcessorTest { event = processor.process(event, null) assertNotNull(event.user) assertNotNull(event.contexts.app) -// assertNotNull(event.debugMeta) file doesnt exist + assertEquals("test", event.debugMeta.images[0].uuid) assertNotNull(event.sdk) assertNotNull(event.release) assertNotNull(event.dist) @@ -88,4 +93,16 @@ class DefaultAndroidEventProcessorTest { assertNull(event.release) assertNull(event.dist) } + + @Test + fun `Executor service should be called on ctor`() { + val processor = DefaultAndroidEventProcessor(context, fixture.options) + val contextData = processor.contextData.get() + assertNotNull(contextData) + assertEquals("test", (contextData[PROGUARD_UUID] as Array<*>)[0]) + assertNotNull(contextData[ROOTED]) + assertNotNull(contextData[ANDROID_ID]) + assertNotNull(contextData[KERNEL_VERSION]) + assertNotNull(contextData[EMULATOR]) + } }