From b6f20e8d021e06d6083c6237475822cbf483f832 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 31 Oct 2022 11:12:43 +0100 Subject: [PATCH 1/9] Implement modules loader for Android and Java --- .../core/AndroidOptionsInitializer.java | 9 +++ .../internal/modules/AssetsModulesLoader.java | 72 +++++++++++++++++++ .../java/io/sentry/MainEventProcessor.java | 9 +++ sentry/src/main/java/io/sentry/Sentry.java | 9 +++ .../main/java/io/sentry/SentryOptions.java | 25 ++++++- .../internal/modules/IModulesLoader.java | 10 +++ .../internal/modules/NoOpModulesLoader.java | 19 +++++ .../modules/ResourcesModulesLoader.java | 70 ++++++++++++++++++ 8 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java create mode 100644 sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java create mode 100644 sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java create mode 100644 sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 8a6c244a4b..bcc649fb26 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -12,15 +12,23 @@ import io.sentry.SendFireAndForgetOutboxSender; import io.sentry.SentryLevel; import io.sentry.android.core.cache.AndroidEnvelopeCache; +import io.sentry.android.core.internal.modules.AssetsModulesLoader; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; +import io.sentry.util.CollectionUtils; import io.sentry.util.Objects; import java.io.BufferedInputStream; +import java.io.BufferedReader; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.util.HashMap; +import java.util.Map; import java.util.Properties; +import java.util.TreeMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -154,6 +162,7 @@ static void init( options.setTransportGate(new AndroidTransportGate(context, options.getLogger())); options.setTransactionProfiler( new AndroidTransactionProfiler(context, options, buildInfoProvider)); + options.setModulesLoader(new AssetsModulesLoader(context, options.getLogger())); } private static void installDefaultIntegrations( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java new file mode 100644 index 0000000000..9a131b380a --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java @@ -0,0 +1,72 @@ +package io.sentry.android.core.internal.modules; + +import android.content.Context; +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import io.sentry.internal.modules.IModulesLoader; +import java.io.BufferedReader; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.lang.ref.WeakReference; +import java.nio.charset.Charset; +import java.util.Map; +import java.util.TreeMap; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class AssetsModulesLoader implements IModulesLoader { + + private @Nullable Map cachedModules = null; + private final @NotNull ILogger logger; + private final @NotNull WeakReference contextRef; + + public AssetsModulesLoader(final @NotNull Context context, final @NotNull ILogger logger) { + this.logger = logger; + this.contextRef = new WeakReference<>(context); + } + + @Override public @Nullable Map getOrLoadModules() { + if (cachedModules != null) { + return cachedModules; + } + cachedModules = extractModules(); + return cachedModules; + } + + @SuppressWarnings("CharsetObjectCanBeUsed") + private Map extractModules() { + final Map modules = new TreeMap<>(); + try { + final Context context = contextRef.get(); + if (context == null) { + return modules; + } + + try (final BufferedReader reader = + new BufferedReader(new InputStreamReader(context.getAssets().open("sentry-external-modules.txt"), Charset.forName("UTF-8")))) { + String module = reader.readLine(); + while (module != null) { + int sep = module.lastIndexOf(':'); + final String group = module.substring(0, sep); + final String version = module.substring(sep + 1); + modules.put(group, version); + module = reader.readLine(); + } + logger.log(SentryLevel.DEBUG, "Extracted %d modules from resources.", modules.size()); + } catch (FileNotFoundException e) { + logger.log(SentryLevel.INFO, "sentry-external-modules.txt file was not found."); + } catch (IOException e) { + logger.log(SentryLevel.ERROR, "Error extracting modules.", e); + } catch (RuntimeException e) { + logger.log(SentryLevel.ERROR, "sentry-external-modules.txt file is malformed.", e); + } + } catch (SecurityException e) { + logger.log(SentryLevel.INFO, "Access to resources denied.", e); + } + return modules; + } +} diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 5e559b64f9..ba5d498a49 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.hints.Cached; +import io.sentry.internal.modules.NoOpModulesLoader; import io.sentry.protocol.DebugImage; import io.sentry.protocol.DebugMeta; import io.sentry.protocol.SentryException; @@ -60,6 +61,7 @@ public MainEventProcessor(final @NotNull SentryOptions options) { setCommons(event); setExceptions(event); setDebugMeta(event); + setModules(event); if (shouldApplyScopeData(event, hint)) { processNonCachedEvent(event); @@ -90,6 +92,13 @@ private void setDebugMeta(final @NotNull SentryEvent event) { } } + private void setModules(final @NotNull SentryEvent event) { + final Map modules = event.getModules(); + if (modules == null) { + event.setModules(options.getModulesLoader().getOrLoadModules()); + } + } + private boolean shouldApplyScopeData( final @NotNull SentryBaseEvent event, final @NotNull Hint hint) { if (HintUtils.shouldApplyScopeData(hint)) { diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index d616aa03b5..2f43ea60ae 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -3,6 +3,9 @@ import io.sentry.cache.EnvelopeCache; import io.sentry.cache.IEnvelopeCache; import io.sentry.config.PropertiesProviderFactory; +import io.sentry.internal.modules.IModulesLoader; +import io.sentry.internal.modules.NoOpModulesLoader; +import io.sentry.internal.modules.ResourcesModulesLoader; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; import io.sentry.transport.NoOpEnvelopeCache; @@ -270,6 +273,12 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) }); } + final IModulesLoader modulesLoader = options.getModulesLoader(); + // only override the ModulesLoader if it's not already set by Android + if (modulesLoader instanceof NoOpModulesLoader) { + options.setModulesLoader(new ResourcesModulesLoader(options.getLogger())); + } + return true; } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 7867b5bf85..5605d8b62d 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -5,7 +5,10 @@ import io.sentry.clientreport.ClientReportRecorder; import io.sentry.clientreport.IClientReportRecorder; import io.sentry.clientreport.NoOpClientReportRecorder; +import io.sentry.internal.modules.IModulesLoader; +import io.sentry.internal.modules.NoOpModulesLoader; import io.sentry.protocol.SdkVersion; +import io.sentry.transport.ITransport; import io.sentry.transport.ITransportGate; import io.sentry.transport.NoOpEnvelopeCache; import io.sentry.transport.NoOpTransportGate; @@ -181,7 +184,7 @@ public class SentryOptions { private final @NotNull List inAppIncludes = new CopyOnWriteArrayList<>(); /** - * The transport factory creates instances of {@link io.sentry.transport.ITransport} - internal + * The transport factory creates instances of {@link ITransport} - internal * construct of the client that abstracts away the event sending. */ private @NotNull ITransportFactory transportFactory = NoOpTransportFactory.getInstance(); @@ -355,6 +358,11 @@ public class SentryOptions { /** ClientReportRecorder to track count of lost events / transactions / ... * */ @NotNull IClientReportRecorder clientReportRecorder = new ClientReportRecorder(this); + /** + * Modules (dependencies, packages) that will be send along with each event. + */ + private @NotNull IModulesLoader modulesLoader = NoOpModulesLoader.getInstance(); + /** * Adds an event processor * @@ -1745,6 +1753,21 @@ public void setSendClientReports(boolean sendClientReports) { return clientReportRecorder; } + /** + * Returns a ModulesLoader to load external modules (dependencies/packages) of the program. + * + * @return a modules loader or no-op + */ + @ApiStatus.Internal + public @NotNull IModulesLoader getModulesLoader() { + return modulesLoader; + } + + @ApiStatus.Internal + public void setModulesLoader(final @NotNull IModulesLoader modulesLoader) { + this.modulesLoader = modulesLoader; + } + /** The BeforeSend callback */ public interface BeforeSendCallback { diff --git a/sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java new file mode 100644 index 0000000000..2c8eed272a --- /dev/null +++ b/sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java @@ -0,0 +1,10 @@ +package io.sentry.internal.modules; + +import java.util.Map; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public interface IModulesLoader { + @Nullable Map getOrLoadModules(); +} diff --git a/sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java new file mode 100644 index 0000000000..af76ef31c2 --- /dev/null +++ b/sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java @@ -0,0 +1,19 @@ +package io.sentry.internal.modules; + +import java.util.Map; +import org.jetbrains.annotations.Nullable; + +public final class NoOpModulesLoader implements IModulesLoader { + + private static final NoOpModulesLoader instance = new NoOpModulesLoader(); + + public static NoOpModulesLoader getInstance() { + return instance; + } + + private NoOpModulesLoader() {} + + @Override public @Nullable Map getOrLoadModules() { + return null; + } +} diff --git a/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java new file mode 100644 index 0000000000..5227fbc333 --- /dev/null +++ b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java @@ -0,0 +1,70 @@ +package io.sentry.internal.modules; + +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import java.io.BufferedReader; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.util.Map; +import java.util.TreeMap; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class ResourcesModulesLoader implements IModulesLoader { + + private @Nullable Map cachedModules = null; + private final @NotNull ILogger logger; + + public ResourcesModulesLoader(final @NotNull ILogger logger) { + this.logger = logger; + } + + @Override public @Nullable Map getOrLoadModules() { + if (cachedModules != null) { + return cachedModules; + } + cachedModules = extractModules(); + return cachedModules; + } + + @SuppressWarnings("CharsetObjectCanBeUsed") + private Map extractModules() { + final Map modules = new TreeMap<>(); + try { + final InputStream resourcesStream = + getClass().getClassLoader().getResourceAsStream("sentry-external-modules.txt"); + + if (resourcesStream == null) { + logger.log(SentryLevel.DEBUG, "sentry-external-modules.txt file was not found."); + return modules; + } + + try (final BufferedReader reader = + new BufferedReader(new InputStreamReader(resourcesStream, Charset.forName("UTF-8")))) { + String module = reader.readLine(); + while (module != null) { + int sep = module.lastIndexOf(':'); + final String group = module.substring(0, sep); + final String version = module.substring(sep + 1); + modules.put(group, version); + module = reader.readLine(); + } + logger.log(SentryLevel.DEBUG, "Extracted %d modules from resources.", modules.size()); + } catch (FileNotFoundException e) { + logger.log(SentryLevel.INFO, "sentry-external-modules.txt file was not found."); + } catch (IOException e) { + logger.log(SentryLevel.ERROR, "Error extracting modules.", e); + } catch (RuntimeException e) { + logger.log(SentryLevel.ERROR, "sentry-external-modules.txt file is malformed.", e); + } + } catch (SecurityException e) { + logger.log(SentryLevel.INFO, "Access to resources denied.", e); + } + return modules; + } +} From 25e6b110627d6dd1820ff042a0e9b9ba4e7e3fde Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 31 Oct 2022 17:44:03 +0100 Subject: [PATCH 2/9] Common logic into abstract ModulesLoader --- .../internal/modules/AssetsModulesLoader.java | 51 ++++------------ .../internal/modules/ModulesLoader.java | 61 +++++++++++++++++++ .../modules/ResourcesModulesLoader.java | 47 ++------------ 3 files changed, 80 insertions(+), 79 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java index 9a131b380a..48048447e4 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java @@ -4,6 +4,7 @@ import io.sentry.ILogger; import io.sentry.SentryLevel; import io.sentry.internal.modules.IModulesLoader; +import io.sentry.internal.modules.ModulesLoader; import java.io.BufferedReader; import java.io.FileNotFoundException; import java.io.IOException; @@ -18,54 +19,28 @@ import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public final class AssetsModulesLoader implements IModulesLoader { +public final class AssetsModulesLoader extends ModulesLoader { - private @Nullable Map cachedModules = null; - private final @NotNull ILogger logger; private final @NotNull WeakReference contextRef; public AssetsModulesLoader(final @NotNull Context context, final @NotNull ILogger logger) { - this.logger = logger; + super(logger); this.contextRef = new WeakReference<>(context); } - @Override public @Nullable Map getOrLoadModules() { - if (cachedModules != null) { - return cachedModules; + @Override + protected Map loadModules() { + final Map modules = new TreeMap<>(); + final Context context = contextRef.get(); + if (context == null) { + return modules; } - cachedModules = extractModules(); - return cachedModules; - } - @SuppressWarnings("CharsetObjectCanBeUsed") - private Map extractModules() { - final Map modules = new TreeMap<>(); try { - final Context context = contextRef.get(); - if (context == null) { - return modules; - } - - try (final BufferedReader reader = - new BufferedReader(new InputStreamReader(context.getAssets().open("sentry-external-modules.txt"), Charset.forName("UTF-8")))) { - String module = reader.readLine(); - while (module != null) { - int sep = module.lastIndexOf(':'); - final String group = module.substring(0, sep); - final String version = module.substring(sep + 1); - modules.put(group, version); - module = reader.readLine(); - } - logger.log(SentryLevel.DEBUG, "Extracted %d modules from resources.", modules.size()); - } catch (FileNotFoundException e) { - logger.log(SentryLevel.INFO, "sentry-external-modules.txt file was not found."); - } catch (IOException e) { - logger.log(SentryLevel.ERROR, "Error extracting modules.", e); - } catch (RuntimeException e) { - logger.log(SentryLevel.ERROR, "sentry-external-modules.txt file is malformed.", e); - } - } catch (SecurityException e) { - logger.log(SentryLevel.INFO, "Access to resources denied.", e); + final InputStream stream = context.getAssets().open(EXTERNAL_MODULES_FILENAME); + return parseStream(stream); + } catch (IOException e) { + logger.log(SentryLevel.ERROR, "Error extracting modules.", e); } return modules; } diff --git a/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java new file mode 100644 index 0000000000..acb09afd61 --- /dev/null +++ b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java @@ -0,0 +1,61 @@ +package io.sentry.internal.modules; + +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import java.io.BufferedReader; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.util.Map; +import java.util.TreeMap; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public abstract class ModulesLoader implements IModulesLoader { + + public static final String EXTERNAL_MODULES_FILENAME = "sentry-external-modules.txt"; + protected final @NotNull ILogger logger; + private @Nullable Map cachedModules = null; + + public ModulesLoader(final @NotNull ILogger logger) { + this.logger = logger; + } + + @Override public @Nullable Map getOrLoadModules() { + if (cachedModules != null) { + return cachedModules; + } + cachedModules = loadModules(); + return cachedModules; + } + + protected abstract Map loadModules(); + + @SuppressWarnings("CharsetObjectCanBeUsed") + protected Map parseStream(final @NotNull InputStream stream) { + final Map modules = new TreeMap<>(); + try (final BufferedReader reader = + new BufferedReader(new InputStreamReader(stream, Charset.forName("UTF-8")))) { + String module = reader.readLine(); + while (module != null) { + int sep = module.lastIndexOf(':'); + final String group = module.substring(0, sep); + final String version = module.substring(sep + 1); + modules.put(group, version); + module = reader.readLine(); + } + logger.log(SentryLevel.DEBUG, "Extracted %d modules from resources.", modules.size()); + } catch (FileNotFoundException e) { + logger.log(SentryLevel.INFO, "%s file was not found.", EXTERNAL_MODULES_FILENAME); + } catch (IOException e) { + logger.log(SentryLevel.ERROR, "Error extracting modules.", e); + } catch (RuntimeException e) { + logger.log(SentryLevel.ERROR, e, "%s file is malformed.", EXTERNAL_MODULES_FILENAME); + } + return modules; + } +} diff --git a/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java index 5227fbc333..08e1647fe2 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java @@ -2,66 +2,31 @@ import io.sentry.ILogger; import io.sentry.SentryLevel; -import java.io.BufferedReader; -import java.io.FileNotFoundException; -import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.Charset; import java.util.Map; import java.util.TreeMap; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public final class ResourcesModulesLoader implements IModulesLoader { - - private @Nullable Map cachedModules = null; - private final @NotNull ILogger logger; +public final class ResourcesModulesLoader extends ModulesLoader { public ResourcesModulesLoader(final @NotNull ILogger logger) { - this.logger = logger; + super(logger); } - @Override public @Nullable Map getOrLoadModules() { - if (cachedModules != null) { - return cachedModules; - } - cachedModules = extractModules(); - return cachedModules; - } - - @SuppressWarnings("CharsetObjectCanBeUsed") - private Map extractModules() { + @Override protected Map loadModules() { final Map modules = new TreeMap<>(); try { final InputStream resourcesStream = - getClass().getClassLoader().getResourceAsStream("sentry-external-modules.txt"); + getClass().getClassLoader().getResourceAsStream(EXTERNAL_MODULES_FILENAME); if (resourcesStream == null) { - logger.log(SentryLevel.DEBUG, "sentry-external-modules.txt file was not found."); + logger.log(SentryLevel.DEBUG, "%s file was not found.", EXTERNAL_MODULES_FILENAME); return modules; } - try (final BufferedReader reader = - new BufferedReader(new InputStreamReader(resourcesStream, Charset.forName("UTF-8")))) { - String module = reader.readLine(); - while (module != null) { - int sep = module.lastIndexOf(':'); - final String group = module.substring(0, sep); - final String version = module.substring(sep + 1); - modules.put(group, version); - module = reader.readLine(); - } - logger.log(SentryLevel.DEBUG, "Extracted %d modules from resources.", modules.size()); - } catch (FileNotFoundException e) { - logger.log(SentryLevel.INFO, "sentry-external-modules.txt file was not found."); - } catch (IOException e) { - logger.log(SentryLevel.ERROR, "Error extracting modules.", e); - } catch (RuntimeException e) { - logger.log(SentryLevel.ERROR, "sentry-external-modules.txt file is malformed.", e); - } + return parseStream(resourcesStream); } catch (SecurityException e) { logger.log(SentryLevel.INFO, "Access to resources denied.", e); } From 5a6755b48e6aa5eefa443a6b71fe05b4f3064b1a Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 1 Nov 2022 22:54:34 +0100 Subject: [PATCH 3/9] Add tests --- .../core/AndroidOptionsInitializer.java | 11 +- .../internal/modules/AssetsModulesLoader.java | 7 +- .../core/AndroidOptionsInitializerTest.kt | 8 ++ .../modules/AssetsModulesLoaderTest.kt | 104 ++++++++++++++++++ .../main/java/io/sentry/SentryOptions.java | 4 +- .../internal/modules/ModulesLoader.java | 2 - .../modules/ResourcesModulesLoader.java | 11 +- .../java/io/sentry/MainEventProcessorTest.kt | 55 +++++++-- sentry/src/test/java/io/sentry/SentryTest.kt | 32 ++++++ .../modules/ResourcesModulesLoaderTest.kt | 96 ++++++++++++++++ 10 files changed, 303 insertions(+), 27 deletions(-) create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt create mode 100644 sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index bcc649fb26..3f3f3c65d6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -1,7 +1,5 @@ package io.sentry.android.core; -import static io.sentry.android.core.NdkIntegration.SENTRY_NDK_CLASS_NAME; - import android.app.Application; import android.content.Context; import android.content.pm.PackageInfo; @@ -15,23 +13,18 @@ import io.sentry.android.core.internal.modules.AssetsModulesLoader; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; -import io.sentry.util.CollectionUtils; import io.sentry.util.Objects; import java.io.BufferedInputStream; -import java.io.BufferedReader; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; -import java.nio.charset.Charset; -import java.util.HashMap; -import java.util.Map; import java.util.Properties; -import java.util.TreeMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import static io.sentry.android.core.NdkIntegration.SENTRY_NDK_CLASS_NAME; + /** * Android Options initializer, it reads configurations from AndroidManifest and sets to the * SentryAndroidOptions. It also adds default values for some fields. diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java index 48048447e4..a278e26453 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java @@ -3,20 +3,15 @@ import android.content.Context; import io.sentry.ILogger; import io.sentry.SentryLevel; -import io.sentry.internal.modules.IModulesLoader; import io.sentry.internal.modules.ModulesLoader; -import java.io.BufferedReader; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; import java.lang.ref.WeakReference; -import java.nio.charset.Charset; import java.util.Map; import java.util.TreeMap; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; @ApiStatus.Internal public final class AssetsModulesLoader extends ModulesLoader { @@ -39,6 +34,8 @@ protected Map loadModules() { try { final InputStream stream = context.getAssets().open(EXTERNAL_MODULES_FILENAME); return parseStream(stream); + } catch (FileNotFoundException e) { + logger.log(SentryLevel.INFO, "%s file was not found.", EXTERNAL_MODULES_FILENAME); } catch (IOException e) { logger.log(SentryLevel.ERROR, "Error extracting modules.", e); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 2f26c4317a..36096b2460 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -11,6 +11,7 @@ import io.sentry.ILogger import io.sentry.MainEventProcessor import io.sentry.SentryOptions import io.sentry.android.core.cache.AndroidEnvelopeCache +import io.sentry.android.core.internal.modules.AssetsModulesLoader import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration import org.junit.runner.RunWith @@ -371,4 +372,11 @@ class AndroidOptionsInitializerTest { assertTrue { fixture.sentryOptions.envelopeDiskCache is AndroidEnvelopeCache } } + + @Test + fun `AssetsModulesLoader is set to options`() { + fixture.initSut() + + assertTrue { fixture.sentryOptions.modulesLoader is AssetsModulesLoader } + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt new file mode 100644 index 0000000000..61f073f42d --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt @@ -0,0 +1,104 @@ +package io.sentry.android.core.internal.modules + +import android.content.Context +import android.content.res.AssetManager +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.times +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.ILogger +import java.io.FileNotFoundException +import java.nio.charset.Charset +import kotlin.test.Test +import kotlin.test.assertEquals + +class AssetsModulesLoaderTest { + + class Fixture { + val context = mock() + val assets = mock() + val logger = mock() + + fun getSut( + fileName: String = "sentry-external-modules.txt", + content: String? = null, + throws: Boolean = false + ): AssetsModulesLoader { + if (content != null) { + whenever(assets.open(fileName)).thenReturn( + content.byteInputStream(Charset.defaultCharset()) + ) + } + if (throws) { + whenever(assets.open(fileName)).thenThrow(FileNotFoundException()) + } + whenever(context.assets).thenReturn(assets) + return AssetsModulesLoader(context, logger) + } + } + + private val fixture = Fixture() + + @Test + fun `reads modules from assets into map`() { + val sut = fixture.getSut( + content = + """ + com.squareup.okhttp3:okhttp:3.14.9 + com.squareup.okio:okio:1.17.2 + """.trimIndent() + ) + + assertEquals( + mapOf( + "com.squareup.okhttp3:okhttp" to "3.14.9", + "com.squareup.okio:okio" to "1.17.2" + ), + sut.orLoadModules + ) + } + + @Test + fun `caches modules after first read`() { + val sut = fixture.getSut( + content = + """ + com.squareup.okhttp3:okhttp:3.14.9 + com.squareup.okio:okio:1.17.2 + """.trimIndent() + ) + + // first, call method to get modules cached + sut.orLoadModules + + // then call it second time + assertEquals( + mapOf( + "com.squareup.okhttp3:okhttp" to "3.14.9", + "com.squareup.okio:okio" to "1.17.2" + ), + sut.orLoadModules + ) + // the context only called once when there's no in-memory cache + verify(fixture.context, times(1)).assets + } + + @Test + fun `when file does not exist, swallows exception and returns empty map`() { + val sut = fixture.getSut(throws = true) + + assertEquals(emptyMap(), sut.orLoadModules) + } + + @Test + fun `when content is malformed, swallows exception and returns empty map`() { + val sut = fixture.getSut( + content = + """ + com.squareup.okhttp3;3.14.9 + """.trimIndent() + ) + + assertEquals(emptyMap(), sut.orLoadModules) + } +} diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 5605d8b62d..8f6aa7fa45 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1764,8 +1764,8 @@ public void setSendClientReports(boolean sendClientReports) { } @ApiStatus.Internal - public void setModulesLoader(final @NotNull IModulesLoader modulesLoader) { - this.modulesLoader = modulesLoader; + public void setModulesLoader(final @Nullable IModulesLoader modulesLoader) { + this.modulesLoader = modulesLoader != null ? modulesLoader : NoOpModulesLoader.getInstance(); } /** The BeforeSend callback */ diff --git a/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java index acb09afd61..36e6980baf 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java @@ -49,8 +49,6 @@ protected Map parseStream(final @NotNull InputStream stream) { module = reader.readLine(); } logger.log(SentryLevel.DEBUG, "Extracted %d modules from resources.", modules.size()); - } catch (FileNotFoundException e) { - logger.log(SentryLevel.INFO, "%s file was not found.", EXTERNAL_MODULES_FILENAME); } catch (IOException e) { logger.log(SentryLevel.ERROR, "Error extracting modules.", e); } catch (RuntimeException e) { diff --git a/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java index 08e1647fe2..0208d9d803 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java @@ -11,18 +11,25 @@ @ApiStatus.Internal public final class ResourcesModulesLoader extends ModulesLoader { + private final @NotNull ClassLoader classLoader; + public ResourcesModulesLoader(final @NotNull ILogger logger) { + this(logger, ResourcesModulesLoader.class.getClassLoader()); + } + + ResourcesModulesLoader(final @NotNull ILogger logger, final @NotNull ClassLoader classLoader) { super(logger); + this.classLoader = classLoader; } @Override protected Map loadModules() { final Map modules = new TreeMap<>(); try { final InputStream resourcesStream = - getClass().getClassLoader().getResourceAsStream(EXTERNAL_MODULES_FILENAME); + classLoader.getResourceAsStream(EXTERNAL_MODULES_FILENAME); if (resourcesStream == null) { - logger.log(SentryLevel.DEBUG, "%s file was not found.", EXTERNAL_MODULES_FILENAME); + logger.log(SentryLevel.INFO, "%s file was not found.", EXTERNAL_MODULES_FILENAME); return modules; } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index 3f5e40dba4..d4440cb41d 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -13,7 +13,6 @@ import io.sentry.protocol.User import io.sentry.util.HintUtils import org.awaitility.kotlin.await import org.mockito.Mockito -import java.lang.RuntimeException import java.net.InetAddress import kotlin.test.AfterTest import kotlin.test.Test @@ -37,12 +36,25 @@ class MainEventProcessorTest { lateinit var sentryTracer: SentryTracer private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java) - fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map = emptyMap(), sendDefaultPii: Boolean? = null, serverName: String? = "server", host: String? = null, resolveHostDelay: Long? = null, hostnameCacheDuration: Long = 10, proguardUuid: String? = null): MainEventProcessor { + fun getSut( + attachThreads: Boolean = true, + attachStackTrace: Boolean = true, + environment: String? = "environment", + tags: Map = emptyMap(), + sendDefaultPii: Boolean? = null, + serverName: String? = "server", + host: String? = null, + resolveHostDelay: Long? = null, + hostnameCacheDuration: Long = 10, + proguardUuid: String? = null, + modules: Map? = null + ): MainEventProcessor { sentryOptions.isAttachThreads = attachThreads sentryOptions.isAttachStacktrace = attachStackTrace sentryOptions.isAttachServerName = true sentryOptions.environment = environment sentryOptions.serverName = serverName + sentryOptions.setModulesLoader { modules } if (sendDefaultPii != null) { sentryOptions.isSendDefaultPii = sendDefaultPii } @@ -348,7 +360,8 @@ class MainEventProcessorTest { @Test fun `uses cache to retrieve servername for subsequent events`() { - val processor = fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000) + val processor = + fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000) val firstEvent = SentryEvent() processor.process(firstEvent, Hint()) assertEquals("aHost", firstEvent.serverName) @@ -477,9 +490,37 @@ class MainEventProcessorTest { } } - private fun generateCrashedEvent(crashedThread: Thread = Thread.currentThread()) = SentryEvent().apply { - val mockThrowable = mock() - val actualThrowable = UncaughtExceptionHandlerIntegration.getUnhandledThrowable(crashedThread, mockThrowable) - throwable = actualThrowable + @Test + fun `when event has modules, does not override them`() { + val sut = fixture.getSut(modules = mapOf("group1:artifact1" to "2.0.0")) + + var event = SentryEvent().apply { + modules = mapOf("group:artifact" to "1.0.0") + } + event = sut.process(event, Hint()) + + assertEquals(1, event.modules!!.size) + assertEquals("1.0.0", event.modules!!["group:artifact"]) + } + + @Test + fun `sets event modules`() { + val sut = fixture.getSut(modules = mapOf("group1:artifact1" to "2.0.0")) + + var event = SentryEvent() + event = sut.process(event, Hint()) + + assertEquals(1, event.modules!!.size) + assertEquals("2.0.0", event.modules!!["group1:artifact1"]) } + + private fun generateCrashedEvent(crashedThread: Thread = Thread.currentThread()) = + SentryEvent().apply { + val mockThrowable = mock() + val actualThrowable = UncaughtExceptionHandlerIntegration.getUnhandledThrowable( + crashedThread, + mockThrowable + ) + throwable = actualThrowable + } } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 75658d06e6..6336a81a39 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -7,6 +7,8 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import io.sentry.cache.EnvelopeCache import io.sentry.cache.IEnvelopeCache +import io.sentry.internal.modules.IModulesLoader +import io.sentry.internal.modules.ResourcesModulesLoader import io.sentry.protocol.SentryId import org.junit.rules.TemporaryFolder import java.io.File @@ -19,6 +21,7 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertTrue + class SentryTest { private val dsn = "http://key@localhost/proj" @@ -295,6 +298,35 @@ class SentryTest { assertTrue { sentryOptions!!.envelopeDiskCache is CustomEnvelopCache } } + @Test + fun `overrides modules loader if it's not set`() { + var sentryOptions: SentryOptions? = null + + Sentry.init { + it.dsn = dsn + sentryOptions = it + } + + assertTrue { sentryOptions!!.modulesLoader is ResourcesModulesLoader } + } + + @Test + fun `does not override modules loader if it's already set`() { + var sentryOptions: SentryOptions? = null + + Sentry.init { + it.dsn = dsn + it.setModulesLoader(CustomModulesLoader()) + sentryOptions = it + } + + assertTrue { sentryOptions!!.modulesLoader is CustomModulesLoader } + } + + private class CustomModulesLoader: IModulesLoader { + override fun getOrLoadModules(): MutableMap? = null + } + private class CustomEnvelopCache : IEnvelopeCache { override fun iterator(): MutableIterator = TODO() override fun store(envelope: SentryEnvelope, hint: Hint) = Unit diff --git a/sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt b/sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt new file mode 100644 index 0000000000..377b594591 --- /dev/null +++ b/sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt @@ -0,0 +1,96 @@ +package io.sentry.internal.modules + +import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.times +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.ILogger +import java.nio.charset.Charset +import kotlin.test.Test +import kotlin.test.assertEquals + +class ResourcesModulesLoaderTest { + + class Fixture { + val logger = mock() + val classLoader = mock() + + fun getSut( + fileName: String = "sentry-external-modules.txt", + content: String? = null + ): ResourcesModulesLoader { + if (content != null) { + whenever(classLoader.getResourceAsStream(fileName)).thenReturn( + content.byteInputStream(Charset.defaultCharset()) + ) + } + return ResourcesModulesLoader(logger, classLoader) + } + } + + private val fixture = Fixture() + + @Test + fun `reads modules from resources into map`() { + val sut = fixture.getSut( + content = + """ + com.squareup.okhttp3:okhttp:3.14.9 + com.squareup.okio:okio:1.17.2 + """.trimIndent() + ) + + assertEquals( + mapOf( + "com.squareup.okhttp3:okhttp" to "3.14.9", + "com.squareup.okio:okio" to "1.17.2" + ), + sut.orLoadModules + ) + } + + @Test + fun `caches modules after first read`() { + val sut = fixture.getSut( + content = + """ + com.squareup.okhttp3:okhttp:3.14.9 + com.squareup.okio:okio:1.17.2 + """.trimIndent() + ) + + // first, call method to get modules cached + sut.orLoadModules + + // then call it second time + assertEquals( + mapOf( + "com.squareup.okhttp3:okhttp" to "3.14.9", + "com.squareup.okio:okio" to "1.17.2" + ), + sut.orLoadModules + ) + // the classloader only called once when there's no in-memory cache + verify(fixture.classLoader, times(1)).getResourceAsStream(any()) + } + + @Test + fun `when file does not exist, returns empty map`() { + val sut = fixture.getSut() + + assertEquals(emptyMap(), sut.orLoadModules) + } + + @Test + fun `when content is malformed, swallows exception and returns empty map`() { + val sut = fixture.getSut( + content = + """ + com.squareup.okhttp3;3.14.9 + """.trimIndent() + ) + + assertEquals(emptyMap(), sut.orLoadModules) + } +} From cb5cbdbaa41524721d4b4a5c88e2290590f970ee Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 1 Nov 2022 22:54:53 +0100 Subject: [PATCH 4/9] Spotless --- .../io/sentry/android/core/AndroidOptionsInitializer.java | 4 ++-- sentry/src/main/java/io/sentry/MainEventProcessor.java | 1 - sentry/src/main/java/io/sentry/SentryOptions.java | 8 +++----- .../java/io/sentry/internal/modules/IModulesLoader.java | 3 ++- .../java/io/sentry/internal/modules/ModulesLoader.java | 6 +++--- .../io/sentry/internal/modules/NoOpModulesLoader.java | 3 ++- .../sentry/internal/modules/ResourcesModulesLoader.java | 5 +++-- sentry/src/test/java/io/sentry/SentryTest.kt | 2 +- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 3f3f3c65d6..c857af078a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -1,5 +1,7 @@ package io.sentry.android.core; +import static io.sentry.android.core.NdkIntegration.SENTRY_NDK_CLASS_NAME; + import android.app.Application; import android.content.Context; import android.content.pm.PackageInfo; @@ -23,8 +25,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import static io.sentry.android.core.NdkIntegration.SENTRY_NDK_CLASS_NAME; - /** * Android Options initializer, it reads configurations from AndroidManifest and sets to the * SentryAndroidOptions. It also adds default values for some fields. diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index ba5d498a49..6dd0e3b9d6 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -1,7 +1,6 @@ package io.sentry; import io.sentry.hints.Cached; -import io.sentry.internal.modules.NoOpModulesLoader; import io.sentry.protocol.DebugImage; import io.sentry.protocol.DebugMeta; import io.sentry.protocol.SentryException; diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 8f6aa7fa45..e7c0980c2b 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -184,8 +184,8 @@ public class SentryOptions { private final @NotNull List inAppIncludes = new CopyOnWriteArrayList<>(); /** - * The transport factory creates instances of {@link ITransport} - internal - * construct of the client that abstracts away the event sending. + * The transport factory creates instances of {@link ITransport} - internal construct of the + * client that abstracts away the event sending. */ private @NotNull ITransportFactory transportFactory = NoOpTransportFactory.getInstance(); @@ -358,9 +358,7 @@ public class SentryOptions { /** ClientReportRecorder to track count of lost events / transactions / ... * */ @NotNull IClientReportRecorder clientReportRecorder = new ClientReportRecorder(this); - /** - * Modules (dependencies, packages) that will be send along with each event. - */ + /** Modules (dependencies, packages) that will be send along with each event. */ private @NotNull IModulesLoader modulesLoader = NoOpModulesLoader.getInstance(); /** diff --git a/sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java index 2c8eed272a..f8679363b7 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/IModulesLoader.java @@ -6,5 +6,6 @@ @ApiStatus.Internal public interface IModulesLoader { - @Nullable Map getOrLoadModules(); + @Nullable + Map getOrLoadModules(); } diff --git a/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java index 36e6980baf..553b9279ba 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/ModulesLoader.java @@ -3,7 +3,6 @@ import io.sentry.ILogger; import io.sentry.SentryLevel; import java.io.BufferedReader; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -25,7 +24,8 @@ public ModulesLoader(final @NotNull ILogger logger) { this.logger = logger; } - @Override public @Nullable Map getOrLoadModules() { + @Override + public @Nullable Map getOrLoadModules() { if (cachedModules != null) { return cachedModules; } @@ -39,7 +39,7 @@ public ModulesLoader(final @NotNull ILogger logger) { protected Map parseStream(final @NotNull InputStream stream) { final Map modules = new TreeMap<>(); try (final BufferedReader reader = - new BufferedReader(new InputStreamReader(stream, Charset.forName("UTF-8")))) { + new BufferedReader(new InputStreamReader(stream, Charset.forName("UTF-8")))) { String module = reader.readLine(); while (module != null) { int sep = module.lastIndexOf(':'); diff --git a/sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java index af76ef31c2..c0dd1e56f7 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/NoOpModulesLoader.java @@ -13,7 +13,8 @@ public static NoOpModulesLoader getInstance() { private NoOpModulesLoader() {} - @Override public @Nullable Map getOrLoadModules() { + @Override + public @Nullable Map getOrLoadModules() { return null; } } diff --git a/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java index 0208d9d803..491678118a 100644 --- a/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java +++ b/sentry/src/main/java/io/sentry/internal/modules/ResourcesModulesLoader.java @@ -22,11 +22,12 @@ public ResourcesModulesLoader(final @NotNull ILogger logger) { this.classLoader = classLoader; } - @Override protected Map loadModules() { + @Override + protected Map loadModules() { final Map modules = new TreeMap<>(); try { final InputStream resourcesStream = - classLoader.getResourceAsStream(EXTERNAL_MODULES_FILENAME); + classLoader.getResourceAsStream(EXTERNAL_MODULES_FILENAME); if (resourcesStream == null) { logger.log(SentryLevel.INFO, "%s file was not found.", EXTERNAL_MODULES_FILENAME); diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 6336a81a39..9a34f12acf 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -323,7 +323,7 @@ class SentryTest { assertTrue { sentryOptions!!.modulesLoader is CustomModulesLoader } } - private class CustomModulesLoader: IModulesLoader { + private class CustomModulesLoader : IModulesLoader { override fun getOrLoadModules(): MutableMap? = null } From dda4b6cf56ad82dd5b71ab9c388e6a5efc1a889f Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 1 Nov 2022 23:03:19 +0100 Subject: [PATCH 5/9] api dump --- sentry/api/sentry.api | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 8e398c641a..4d5c9ee87f 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1379,6 +1379,7 @@ public class io/sentry/SentryOptions { public fun getMaxRequestBodySize ()Lio/sentry/SentryOptions$RequestSize; public fun getMaxSpans ()I public fun getMaxTraceFileSize ()J + public fun getModulesLoader ()Lio/sentry/internal/modules/IModulesLoader; public fun getOutboxPath ()Ljava/lang/String; public fun getProfilesSampleRate ()Ljava/lang/Double; public fun getProfilesSampler ()Lio/sentry/SentryOptions$ProfilesSamplerCallback; @@ -1457,6 +1458,7 @@ public class io/sentry/SentryOptions { public fun setMaxRequestBodySize (Lio/sentry/SentryOptions$RequestSize;)V public fun setMaxSpans (I)V public fun setMaxTraceFileSize (J)V + public fun setModulesLoader (Lio/sentry/internal/modules/IModulesLoader;)V public fun setPrintUncaughtStackTrace (Z)V public fun setProfilesSampleRate (Ljava/lang/Double;)V public fun setProfilesSampler (Lio/sentry/SentryOptions$ProfilesSamplerCallback;)V @@ -2190,6 +2192,28 @@ public final class io/sentry/instrumentation/file/SentryFileWriter : java/io/Out public fun (Ljava/lang/String;Z)V } +public abstract interface class io/sentry/internal/modules/IModulesLoader { + public abstract fun getOrLoadModules ()Ljava/util/Map; +} + +public abstract class io/sentry/internal/modules/ModulesLoader : io/sentry/internal/modules/IModulesLoader { + public static final field EXTERNAL_MODULES_FILENAME Ljava/lang/String; + protected final field logger Lio/sentry/ILogger; + public fun (Lio/sentry/ILogger;)V + public fun getOrLoadModules ()Ljava/util/Map; + protected abstract fun loadModules ()Ljava/util/Map; + protected fun parseStream (Ljava/io/InputStream;)Ljava/util/Map; +} + +public final class io/sentry/internal/modules/NoOpModulesLoader : io/sentry/internal/modules/IModulesLoader { + public static fun getInstance ()Lio/sentry/internal/modules/NoOpModulesLoader; + public fun getOrLoadModules ()Ljava/util/Map; +} + +public final class io/sentry/internal/modules/ResourcesModulesLoader : io/sentry/internal/modules/ModulesLoader { + public fun (Lio/sentry/ILogger;)V +} + public final class io/sentry/protocol/App : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public static final field TYPE Ljava/lang/String; public fun ()V From 5cdbc68262603530cfecd40bf79cbd2834dfe47e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 2 Nov 2022 09:08:49 +0100 Subject: [PATCH 6/9] Changelog --- CHANGELOG.md | 1 + sentry-android-ndk/sentry-native | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 048422482e..7ec68d4cd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Customizable fragment lifecycle breadcrumbs ([#2299](https://github.com/getsentry/sentry-java/pull/2299)) - Provide hook for Jetpack Compose navigation instrumentation ([#2320](https://github.com/getsentry/sentry-java/pull/2320)) +- Populate `event.modules` with dependencies metadata ([#2324](https://github.com/getsentry/sentry-java/pull/2324)) ### Dependencies diff --git a/sentry-android-ndk/sentry-native b/sentry-android-ndk/sentry-native index 28be51f5e3..1a4c7c15f0 160000 --- a/sentry-android-ndk/sentry-native +++ b/sentry-android-ndk/sentry-native @@ -1 +1 @@ -Subproject commit 28be51f5e3acb01327b1164206d3145464577670 +Subproject commit 1a4c7c15f086644d1838c44209c8213a8cc3b497 From eed2c859aa27959021a5661d7438257b07d2bed7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 2 Nov 2022 22:56:41 +0100 Subject: [PATCH 7/9] Address PR reviews --- .../core/internal/modules/AssetsModulesLoader.java | 9 ++------- .../core/internal/modules/AssetsModulesLoaderTest.kt | 8 ++++---- .../internal/modules/ResourcesModulesLoaderTest.kt | 8 ++++---- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java index a278e26453..c381bacf0a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/modules/AssetsModulesLoader.java @@ -7,7 +7,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.lang.ref.WeakReference; import java.util.Map; import java.util.TreeMap; import org.jetbrains.annotations.ApiStatus; @@ -16,20 +15,16 @@ @ApiStatus.Internal public final class AssetsModulesLoader extends ModulesLoader { - private final @NotNull WeakReference contextRef; + private final @NotNull Context context; public AssetsModulesLoader(final @NotNull Context context, final @NotNull ILogger logger) { super(logger); - this.contextRef = new WeakReference<>(context); + this.context = context; } @Override protected Map loadModules() { final Map modules = new TreeMap<>(); - final Context context = contextRef.get(); - if (context == null) { - return modules; - } try { final InputStream stream = context.getAssets().open(EXTERNAL_MODULES_FILENAME); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt index 61f073f42d..04006ca2bf 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/modules/AssetsModulesLoaderTest.kt @@ -3,7 +3,6 @@ package io.sentry.android.core.internal.modules import android.content.Context import android.content.res.AssetManager import com.nhaarman.mockitokotlin2.mock -import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import io.sentry.ILogger @@ -11,6 +10,7 @@ import java.io.FileNotFoundException import java.nio.charset.Charset import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertTrue class AssetsModulesLoaderTest { @@ -80,14 +80,14 @@ class AssetsModulesLoaderTest { sut.orLoadModules ) // the context only called once when there's no in-memory cache - verify(fixture.context, times(1)).assets + verify(fixture.context).assets } @Test fun `when file does not exist, swallows exception and returns empty map`() { val sut = fixture.getSut(throws = true) - assertEquals(emptyMap(), sut.orLoadModules) + assertTrue(sut.orLoadModules!!.isEmpty()) } @Test @@ -99,6 +99,6 @@ class AssetsModulesLoaderTest { """.trimIndent() ) - assertEquals(emptyMap(), sut.orLoadModules) + assertTrue(sut.orLoadModules!!.isEmpty()) } } diff --git a/sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt b/sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt index 377b594591..34605984cd 100644 --- a/sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt +++ b/sentry/src/test/java/io/sentry/internal/modules/ResourcesModulesLoaderTest.kt @@ -2,13 +2,13 @@ package io.sentry.internal.modules import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.mock -import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import io.sentry.ILogger import java.nio.charset.Charset import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertTrue class ResourcesModulesLoaderTest { @@ -72,14 +72,14 @@ class ResourcesModulesLoaderTest { sut.orLoadModules ) // the classloader only called once when there's no in-memory cache - verify(fixture.classLoader, times(1)).getResourceAsStream(any()) + verify(fixture.classLoader).getResourceAsStream(any()) } @Test fun `when file does not exist, returns empty map`() { val sut = fixture.getSut() - assertEquals(emptyMap(), sut.orLoadModules) + assertTrue(sut.orLoadModules!!.isEmpty()) } @Test @@ -91,6 +91,6 @@ class ResourcesModulesLoaderTest { """.trimIndent() ) - assertEquals(emptyMap(), sut.orLoadModules) + assertTrue(sut.orLoadModules!!.isEmpty()) } } From 2f998c091cc465475e10df64d0652b5bdae9eb04 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 3 Nov 2022 15:55:33 +0100 Subject: [PATCH 8/9] Append modules if there are existing ones on the event --- .../src/main/java/io/sentry/MainEventProcessor.java | 11 +++++++++-- .../src/test/java/io/sentry/MainEventProcessorTest.kt | 5 +++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 6dd0e3b9d6..26d21bfa6d 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -92,9 +92,16 @@ private void setDebugMeta(final @NotNull SentryEvent event) { } private void setModules(final @NotNull SentryEvent event) { - final Map modules = event.getModules(); + final Map modules = options.getModulesLoader().getOrLoadModules(); if (modules == null) { - event.setModules(options.getModulesLoader().getOrLoadModules()); + return; + } + + final Map eventModules = event.getModules(); + if (eventModules == null) { + event.setModules(modules); + } else { + eventModules.putAll(modules); } } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index d4440cb41d..baa58ada73 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -491,7 +491,7 @@ class MainEventProcessorTest { } @Test - fun `when event has modules, does not override them`() { + fun `when event has modules, appends to them`() { val sut = fixture.getSut(modules = mapOf("group1:artifact1" to "2.0.0")) var event = SentryEvent().apply { @@ -499,8 +499,9 @@ class MainEventProcessorTest { } event = sut.process(event, Hint()) - assertEquals(1, event.modules!!.size) + assertEquals(2, event.modules!!.size) assertEquals("1.0.0", event.modules!!["group:artifact"]) + assertEquals("2.0.0", event.modules!!["group1:artifact1"]) } @Test From 361a46778636c139ef1bc0cfafa28f2321be4665 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 3 Nov 2022 22:07:02 +0100 Subject: [PATCH 9/9] Revert submodule --- sentry-android-ndk/sentry-native | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-ndk/sentry-native b/sentry-android-ndk/sentry-native index 1a4c7c15f0..28be51f5e3 160000 --- a/sentry-android-ndk/sentry-native +++ b/sentry-android-ndk/sentry-native @@ -1 +1 @@ -Subproject commit 1a4c7c15f086644d1838c44209c8213a8cc3b497 +Subproject commit 28be51f5e3acb01327b1164206d3145464577670