From 42c3ddfa515f532a021a03baca8f7ede713fb599 Mon Sep 17 00:00:00 2001
From: Ivan Dlugos <dlugos.ivan@gmail.com>
Date: Wed, 4 May 2022 15:28:25 +0200
Subject: [PATCH] chore: review changes

---
 .../android/core/ManifestMetadataReader.java  | 29 ++++++++++--------
 .../core/ManifestMetadataReaderTest.kt        | 30 -------------------
 2 files changed, 16 insertions(+), 43 deletions(-)

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 2192fc1a92c..a48c29db981 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
@@ -246,20 +246,13 @@ static void applyMetadata(
             readString(metadata, logger, PROGUARD_UUID, options.getProguardUuid()));
 
         SdkVersion sdkInfo = options.getSdkVersion();
-        if (sdkInfo != null) {
-          final String sdkName = readString(metadata, logger, SDK_NAME, sdkInfo.getName());
-          if (sdkName != null) {
-            sdkInfo.setName(sdkName);
-          }
-          final String sdkVersion = readString(metadata, logger, SDK_VERSION, sdkInfo.getVersion());
-          if (sdkVersion != null) {
-            sdkInfo.setVersion(sdkVersion);
-          }
-          options.setSdkVersion(sdkInfo);
-        } else {
-          // Should always be set by the Options constructor but let's error out if that changes.
-          Objects.requireNonNull(sdkInfo, "Property options.sdkVersion must not be null.");
+        if (sdkInfo == null) {
+          // Is already set by the Options constructor, let's use an empty default otherwise.
+          sdkInfo = new SdkVersion("", "");
         }
+        sdkInfo.setName(readStringNotNull(metadata, logger, SDK_NAME, sdkInfo.getName()));
+        sdkInfo.setVersion(readStringNotNull(metadata, logger, SDK_VERSION, sdkInfo.getVersion()));
+        options.setSdkVersion(sdkInfo);
       }
 
       options
@@ -293,6 +286,16 @@ private static boolean readBool(
     return value;
   }
 
+  private static @NotNull String readStringNotNull(
+      final @NotNull Bundle metadata,
+      final @NotNull ILogger logger,
+      final @NotNull String key,
+      final @NotNull String defaultValue) {
+    final String value = metadata.getString(key, defaultValue);
+    logger.log(SentryLevel.DEBUG, "%s read: %s", key, value);
+    return value;
+  }
+
   private static @Nullable List<String> readList(
       final @NotNull Bundle metadata, final @NotNull ILogger logger, final @NotNull String key) {
     final String value = metadata.getString(key);
diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt
index 9a6cf07b870..7005208a97a 100644
--- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt
+++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt
@@ -563,21 +563,6 @@ class ManifestMetadataReaderTest {
         assertEquals(expectedValue, fixture.options.sdkVersion?.name)
     }
 
-    @Test
-    fun `applyMetadata does not override SDK name from options`() {
-        // Arrange
-        val expectedValue = "custom.sdk"
-        fixture.options.sdkVersion!!.name = expectedValue
-        val bundle = bundleOf(ManifestMetadataReader.SDK_NAME to "foo")
-        val context = fixture.getContext(metaData = bundle)
-
-        // Act
-        ManifestMetadataReader.applyMetadata(context, fixture.options)
-
-        // Assert
-        assertEquals(expectedValue, fixture.options.sdkVersion?.name)
-    }
-
     @Test
     fun `applyMetadata reads SDK version from metadata`() {
         // Arrange
@@ -593,21 +578,6 @@ class ManifestMetadataReaderTest {
         assertEquals(expectedValue, fixture.options.sdkVersion?.version)
     }
 
-    @Test
-    fun `applyMetadata does not override SDK version from options`() {
-        // Arrange
-        val expectedValue = "1.2.3-alpha.0"
-        fixture.options.sdkVersion!!.version = expectedValue
-        val bundle = bundleOf(ManifestMetadataReader.SDK_VERSION to "foo")
-        val context = fixture.getContext(metaData = bundle)
-
-        // Act
-        ManifestMetadataReader.applyMetadata(context, fixture.options)
-
-        // Assert
-        assertEquals(expectedValue, fixture.options.sdkVersion?.version)
-    }
-
     @Test
     fun `applyMetadata reads enableScopeSync to options`() {
         // Arrange