Merge pull request #243 from DataDog/nogorodnikov/merge-release-1.13.…

…1-into-main Merge release `1.13.1` into `main` branch
DataDog · Apr 12, 2024 · 5c2bc6f · 5c2bc6f
2 parents ccdd034 + 86b6c28
commit 5c2bc6f
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# 1.13.1 / 2024-04-11
+
+* [BUGFIX] Avoid eager fetching of Variant values. See [#240](https://github.com/DataDog/dd-sdk-android-gradle-plugin/pull/240)
+
 # 1.13.0 / 2024-04-09
 
 * [FEATURE] Inject build ID into mapping file upload and SDK host application as well. See [#211](https://github.com/DataDog/dd-sdk-android-gradle-plugin/pull/211)

diff --git a/buildSrc/src/main/kotlin/com/datadog/gradle/config/MavenConfig.kt b/buildSrc/src/main/kotlin/com/datadog/gradle/config/MavenConfig.kt
@@ -20,7 +20,7 @@ import java.net.URI
 
 object MavenConfig {
 
-    val VERSION = Version(1, 13, 0, Version.Type.Release)
+    val VERSION = Version(1, 13, 1, Version.Type.Release)
     const val GROUP_ID = "com.datadoghq"
     const val PUBLICATION = "pluginMaven"
 }

diff --git a/...-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt b/...-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt
@@ -16,7 +16,6 @@ import org.gradle.api.Plugin
 import org.gradle.api.Project
 import org.gradle.api.Task
 import org.gradle.api.logging.Logging
-import org.gradle.api.provider.Provider
 import org.gradle.api.provider.ProviderFactory
 import org.gradle.api.tasks.TaskContainer
 import org.gradle.api.tasks.TaskProvider
@@ -181,65 +180,65 @@ class DdAndroidGradlePlugin @Inject constructor(
         buildIdGenerationTask: TaskProvider<GenerateBuildIdTask>,
         apiKey: ApiKey,
         extension: DdExtension
-    ): Task {
-        val extensionConfiguration = resolveExtensionConfiguration(extension, variant)
-        val flavorName = variant.flavorName
+    ): TaskProvider<DdMappingFileUploadTask> {
         val uploadTaskName = UPLOAD_TASK_NAME + variant.name.capitalize()
-        // TODO RUMM-2382 use tasks.register
-        val uploadTask = target.tasks.create(
+        val uploadTask = target.tasks.register(
             uploadTaskName,
             DdMappingFileUploadTask::class.java,
             GitRepositoryDetector(execOps)
-        )
-
-        configureVariantTask(uploadTask, apiKey, flavorName, extensionConfiguration, variant)
+        ).apply {
+            configure { uploadTask ->
+                val extensionConfiguration = resolveExtensionConfiguration(extension, variant)
+                configureVariantTask(uploadTask, apiKey, variant.flavorName, extensionConfiguration, variant)
+
+                // upload task shouldn't depend on the build ID generation task, but only read its property,
+                // because upload task may be triggered after assemble task and we don't want to re-generate
+                // build ID, because it will be different then from the one which is already embedded in
+                // the application package
+                uploadTask.buildId = buildIdGenerationTask.flatMap {
+                    it.buildIdFile.flatMap {
+                        providerFactory.provider { it.asFile.readText().trim() }
+                    }
+                }
+                uploadTask.mappingFilePath = resolveMappingFilePath(extensionConfiguration, target, variant)
+                uploadTask.mappingFilePackagesAliases =
+                    filterMappingFileReplacements(
+                        extensionConfiguration.mappingFilePackageAliases,
+                        variant.applicationId
+                    )
+                uploadTask.mappingFileTrimIndents = extensionConfiguration.mappingFileTrimIndents
+                if (!extensionConfiguration.ignoreDatadogCiFileConfig) {
+                    uploadTask.datadogCiFile = DdTaskUtils.findDatadogCiFile(target.projectDir)
+                }
 
-        // upload task shouldn't depend on the build ID generation task, but only read its property,
-        // because upload task may be triggered after assemble task and we don't want to re-generate
-        // build ID, because it will be different then from the one which is already embedded in
-        // the application package
-        uploadTask.buildId = buildIdGenerationTask.flatMap {
-            it.buildIdFile.flatMap {
-                providerFactory.provider { it.asFile.readText().trim() }
-            }
-        }
-        uploadTask.mappingFilePath = resolveMappingFilePath(extensionConfiguration, target, variant)
-        uploadTask.mappingFilePackagesAliases =
-            filterMappingFileReplacements(
-                extensionConfiguration.mappingFilePackageAliases,
-                variant.applicationId
-            )
-        uploadTask.mappingFileTrimIndents = extensionConfiguration.mappingFileTrimIndents
-        if (!extensionConfiguration.ignoreDatadogCiFileConfig) {
-            uploadTask.datadogCiFile = DdTaskUtils.findDatadogCiFile(target.projectDir)
-        }
+                uploadTask.repositoryFile = DdTaskUtils.resolveDatadogRepositoryFile(target)
 
-        uploadTask.repositoryFile = DdTaskUtils.resolveDatadogRepositoryFile(target)
+                val roots = mutableListOf<File>()
+                variant.sourceSets.forEach {
+                    roots.addAll(it.javaDirectories)
+                    @Suppress("MagicNumber")
+                    if (DdTaskUtils.isAgpAbove(7, 0, 0)) {
+                        roots.addAll(it.kotlinDirectories)
+                    }
+                }
 
-        val roots = mutableListOf<File>()
-        variant.sourceSets.forEach {
-            roots.addAll(it.javaDirectories)
-            @Suppress("MagicNumber")
-            if (DdTaskUtils.isAgpAbove(7, 0, 0)) {
-                roots.addAll(it.kotlinDirectories)
+                // it can be an overlap between java and kotlin directories and since File doesn't override
+                // equals for set comparison, we will remove duplicates manually
+                uploadTask.sourceSetRoots = roots.map { it.absolutePath }
+                    .distinct()
+                    .map { File(it) }
             }
         }
 
-        // it can be an overlap between java and kotlin directories and since File doesn't override
-        // equals for set comparison, we will remove duplicates manually
-        uploadTask.sourceSetRoots = roots.map { it.absolutePath }
-            .distinct()
-            .map { File(it) }
-
         return uploadTask
     }
 
-    @Suppress("DefaultLocale", "ReturnCount")
+    @Suppress("ReturnCount")
     internal fun configureVariantForSdkCheck(
         target: Project,
         variant: ApplicationVariant,
         extension: DdExtension
-    ): Provider<DdCheckSdkDepsTask>? {
+    ): TaskProvider<DdCheckSdkDepsTask>? {
         if (!extension.enabled) {
             LOGGER.info("Extension disabled for variant ${variant.name}, no sdk check task created")
             return null
@@ -347,7 +346,7 @@ class DdAndroidGradlePlugin @Inject constructor(
 
         uploadTask.site = extensionConfiguration.site ?: ""
         uploadTask.versionName = extensionConfiguration.versionName ?: variant.versionName
-        uploadTask.versionCode = variant.versionCode
+        uploadTask.versionCode = providerFactory.provider { variant.versionCode }
         uploadTask.serviceName = extensionConfiguration.serviceName ?: variant.applicationId
         uploadTask.remoteRepositoryUrl = extensionConfiguration.remoteRepositoryUrl ?: ""
     }

diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdFileUploadTask.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdFileUploadTask.kt
@@ -28,7 +28,7 @@ import javax.inject.Inject
  * Proguard/R8 files, etc.)..
  */
 abstract class DdFileUploadTask @Inject constructor(
-    providerFactory: ProviderFactory,
+    private val providerFactory: ProviderFactory,
     @get:Internal internal val repositoryDetector: RepositoryDetector
 ) : DefaultTask() {
     @get:Internal
@@ -62,10 +62,11 @@ abstract class DdFileUploadTask @Inject constructor(
     var versionName: String = ""
 
     /**
-     * The version code of the application.
+     * The version code of the application. Need to be a provider, because resolution during
+     * configuration phase may cause incompatibility with other plugins if legacy Variant API is used.
      */
     @get:Input
-    var versionCode: Int = 0
+    var versionCode: Provider<Int> = providerFactory.provider { 0 }
 
     /**
      * The service name of the application (by default, it is your app's package name).
@@ -164,7 +165,7 @@ abstract class DdFileUploadTask @Inject constructor(
                     DdAppIdentifier(
                         serviceName = serviceName,
                         version = versionName,
-                        versionCode = versionCode,
+                        versionCode = versionCode.get(),
                         variant = variantName,
                         buildId = buildId.get()
                     ),
@@ -199,7 +200,7 @@ abstract class DdFileUploadTask @Inject constructor(
         site = extensionConfiguration.site ?: ""
 
         versionName = variant.versionName ?: ""
-        versionCode = variant.versionCode
+        versionCode = providerFactory.provider { variant.versionCode }
         serviceName = extensionConfiguration.serviceName ?: variant.applicationId
         variantName = variant.flavorName
         remoteRepositoryUrl = extensionConfiguration.remoteRepositoryUrl ?: ""

diff --git a/...roid-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt b/...roid-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt
@@ -33,6 +33,7 @@ import org.gradle.api.Project
 import org.gradle.api.Transformer
 import org.gradle.api.artifacts.Configuration
 import org.gradle.api.provider.Provider
+import org.gradle.api.provider.ProviderFactory
 import org.gradle.api.tasks.TaskProvider
 import org.gradle.testfixtures.ProjectBuilder
 import org.junit.jupiter.api.BeforeEach
@@ -43,12 +44,14 @@ import org.mockito.Mock
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.junit.jupiter.MockitoSettings
 import org.mockito.kotlin.any
+import org.mockito.kotlin.doAnswer
 import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
 import java.io.File
 import java.util.UUID
+import java.util.concurrent.Callable
 
 @Extensions(
     ExtendWith(MockitoExtension::class),
@@ -90,7 +93,16 @@ internal class DdAndroidGradlePluginTest {
         fakeFlavorNames = fakeFlavorNames.take(5) // A D F G A♭ A A♭ G F
         fakeBuildId = forge.getForgery<UUID>().toString()
         fakeProject = ProjectBuilder.builder().build()
-        testedPlugin = DdAndroidGradlePlugin(mock(), mock())
+        val mockProviderFactory = mock<ProviderFactory>()
+        whenever(mockProviderFactory.provider(any<Callable<*>>())) doAnswer {
+            val argument = it.getArgument<Callable<*>>(0)
+            fakeProject.provider(argument)
+        }
+        testedPlugin = DdAndroidGradlePlugin(
+            execOps = mock(),
+            providerFactory = mockProviderFactory
+        )
+
         setEnv(DdAndroidGradlePlugin.DD_API_KEY, "")
         setEnv(DdAndroidGradlePlugin.DATADOG_API_KEY, "")
     }
@@ -124,7 +136,7 @@ internal class DdAndroidGradlePluginTest {
             mockBuildIdGenerationTask(fakeBuildId),
             fakeApiKey,
             fakeExtension
-        )
+        ).get()
 
         // Then
         check(task is DdMappingFileUploadTask)
@@ -170,7 +182,7 @@ internal class DdAndroidGradlePluginTest {
             mockBuildIdGenerationTask(fakeBuildId),
             fakeApiKey,
             fakeExtension
-        )
+        ).get()
 
         // Then
         check(task is DdMappingFileUploadTask)
@@ -228,7 +240,7 @@ internal class DdAndroidGradlePluginTest {
             mockBuildIdGenerationTask(fakeBuildId),
             fakeApiKey,
             fakeExtension
-        )
+        ).get()
 
         // Then
         check(task is DdMappingFileUploadTask)
@@ -284,7 +296,7 @@ internal class DdAndroidGradlePluginTest {
             mockBuildIdGenerationTask(fakeBuildId),
             fakeApiKey,
             fakeExtension
-        )
+        ).get()
 
         // Then
         check(task is DdMappingFileUploadTask)
@@ -342,7 +354,7 @@ internal class DdAndroidGradlePluginTest {
             mockBuildIdGenerationTask(fakeBuildId),
             fakeApiKey,
             fakeExtension
-        )
+        ).get()
 
         // Then
         check(task is DdMappingFileUploadTask)
@@ -400,7 +412,7 @@ internal class DdAndroidGradlePluginTest {
             mockBuildIdGenerationTask(fakeBuildId),
             fakeApiKey,
             fakeExtension
-        )
+        ).get()
 
         // Then
         check(task is DdMappingFileUploadTask)
@@ -593,7 +605,7 @@ internal class DdAndroidGradlePluginTest {
             mockBuildIdGenerationTask(fakeBuildId),
             fakeApiKey,
             fakeExtension
-        )
+        ).get()
 
         // Then
         check(task is DdMappingFileUploadTask)

diff --git a/...id-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt b/...id-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt
@@ -119,7 +119,9 @@ internal class DdMappingFileUploadTaskTest {
         testedTask.apiKeySource = fakeApiKey.source
         testedTask.variantName = fakeVariant
         testedTask.versionName = fakeVersion
-        testedTask.versionCode = fakeVersionCode
+        testedTask.versionCode = mock<Provider<Int>>().apply {
+            whenever(get()) doReturn fakeVersionCode
+        }
         testedTask.serviceName = fakeService
         testedTask.site = fakeSite.name
         testedTask.buildId = mock<Provider<String>>().apply {