Fix and polish Log4j2PluginsCacheFileTransformer (#1175)

* Copy Log4j2PluginCacheFileTransformer.java from logging-log4j-transform Updated to https://github.com/apache/logging-log4j-transform/blob/a190ba8dfd8a58f1ce95c3d06bcd6a924a416db5/log4j-transform-maven-shade-plugin-extensions/src/main/java/org/apache/logging/log4j/maven/plugins/shade/transformer/Log4j2PluginCacheFileTransformer.java * Convert Log4j2PluginCacheFileTransformer to Kotlin * Merge Log4j2PluginCacheFileTransformer into Log4j2PluginsCacheFileTransformer * Cleanups * Copy Log4j2PluginCacheFileTransformerTest.java Updated to https://github.com/apache/logging-log4j-transform/blob/8d6538acc50ed819d21987e69fe94b485ee0d368/log4j-transform-maven-shade-plugin-extensions/src/test/java/org/apache/logging/log4j/maven/plugins/shade/transformer/Log4j2PluginCacheFileTransformerTest.java * Convert Log4j2PluginCacheFileTransformerTest to Kotlin * Check element.relativePath.pathString in canTransformResource * Fix Log4j2PluginCacheFileTransformerTest * Merge Log4j2PluginCacheFileTransformerTest into Log4j2PluginsCacheFileTransformerTest * Remove redundant shouldTransform and shouldTransformForSingleFile * Cleanups * Revert changes on relocatePlugins * Add a functional test * Note this change
GradleUp · Jan 23, 2025 · 897895c · 897895c
1 parent 8ad88ed
commit 897895c
Show file tree

Hide file tree

Showing 8 changed files with 166 additions and 38 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -110,6 +110,7 @@ testing.suites {
       implementation(libs.apache.maven.modelBuilder)
       implementation(libs.moshi)
       implementation(libs.moshi.kotlin)
+      implementation(libs.apache.log4j)
     }
   }
 

diff --git a/src/docs/changes/README.md b/src/docs/changes/README.md
@@ -3,6 +3,10 @@
 
 ## [Unreleased]
 
+**Fixed**
+
+- Fix `Log4j2PluginsCacheFileTransformer` not working for merging `Log4j2Plugins.dat` files. ([#1175](https://github.com/GradleUp/shadow/pull/1175))  
+
 
 ## [v9.0.0-beta5] (2025-01-21)
 

diff --git a/src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/BasePluginTest.kt b/src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/BasePluginTest.kt
@@ -334,7 +334,7 @@ abstract class BasePluginTest {
       return transform { it.task(taskPath)?.outcome }.isNotNull().isEqualTo(expectedOutcome)
     }
 
-    private fun requireResourceAsPath(name: String): Path {
+    fun requireResourceAsPath(name: String): Path {
       val resource = this::class.java.classLoader.getResource(name)
         ?: throw NoSuchFileException("Resource $name not found.")
       return resource.toURI().toPath()

diff --git a/...alTest/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/TransformersTest.kt b/...alTest/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/TransformersTest.kt
@@ -1,15 +1,20 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
+import assertk.all
 import assertk.assertThat
 import assertk.assertions.isEqualTo
+import assertk.assertions.isNotEqualTo
 import assertk.assertions.isNotNull
 import assertk.assertions.isNull
 import com.github.jengelman.gradle.plugins.shadow.util.Issue
+import com.github.jengelman.gradle.plugins.shadow.util.getStream
 import com.github.jengelman.gradle.plugins.shadow.util.isRegular
 import java.util.jar.Attributes
 import kotlin.io.path.appendText
+import kotlin.io.path.readText
 import kotlin.io.path.writeText
 import kotlin.reflect.KClass
+import org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor.PLUGIN_CACHE_FILE
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.MethodSource
@@ -65,6 +70,37 @@ class TransformersTest : BaseTransformerTest() {
     assertThat(mf.mainAttributes.getValue("New-Entry")).isNull()
   }
 
+  @Issue(
+    "https://github.com/GradleUp/shadow/issues/427",
+  )
+  @Test
+  fun canMergeLog4j2PluginCacheFiles() {
+    val content = requireResourceAsPath(PLUGIN_CACHE_FILE).readText()
+    val one = buildJarOne {
+      insert(PLUGIN_CACHE_FILE, content)
+    }
+    val two = buildJarOne {
+      insert(PLUGIN_CACHE_FILE, content)
+    }
+    projectScriptPath.appendText(
+      transform<Log4j2PluginsCacheFileTransformer>(
+        shadowJarBlock = fromJar(one, two),
+      ),
+    )
+
+    run(shadowJarTask)
+
+    val actualFileBytes = outputShadowJar.use { jar ->
+      @Suppress("Since15")
+      jar.getStream(PLUGIN_CACHE_FILE).use { it.readAllBytes() }
+    }
+    assertThat(actualFileBytes.contentHashCode()).all {
+      // Hash of the original plugin cache file.
+      isNotEqualTo(-2114104185)
+      isEqualTo(1911442937)
+    }
+  }
+
   @Test
   fun canUseCustomTransformer() {
     writeMainClass()
@@ -137,7 +173,6 @@ class TransformersTest : BaseTransformerTest() {
       "" to ComponentsXmlResourceTransformer::class,
       "" to DontIncludeResourceTransformer::class,
       "{ resource.set(\"test.file\"); file.fileValue(file(\"test/some.file\")) }" to IncludeResourceTransformer::class,
-      "" to Log4j2PluginsCacheFileTransformer::class,
       "" to ManifestAppenderTransformer::class,
       "" to ManifestResourceTransformer::class,
       "{ keyTransformer = { it.toLowerCase() } }" to PropertiesFileTransformer::class,

diff --git a/...nalTest/resources/META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat b/...nalTest/resources/META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat
diff --git a/.../github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.kt b/.../github/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformer.kt
@@ -4,85 +4,105 @@ import com.github.jengelman.gradle.plugins.shadow.ShadowStats
 import com.github.jengelman.gradle.plugins.shadow.relocation.RelocateClassContext
 import com.github.jengelman.gradle.plugins.shadow.relocation.Relocator
 import com.github.jengelman.gradle.plugins.shadow.transformers.TransformerContext.Companion.getEntryTimestamp
-import java.io.File
 import java.net.URL
+import java.nio.file.Path
 import java.util.Collections
 import java.util.Enumeration
+import kotlin.io.path.createTempFile
+import kotlin.io.path.deleteIfExists
+import kotlin.io.path.outputStream
 import org.apache.commons.io.output.CloseShieldOutputStream
 import org.apache.logging.log4j.core.config.plugins.processor.PluginCache
-import org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor
+import org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor.PLUGIN_CACHE_FILE
 import org.apache.tools.zip.ZipEntry
 import org.apache.tools.zip.ZipOutputStream
 import org.gradle.api.file.FileTreeElement
 
 /**
- * Modified from the maven equivalent to work with gradle
- *
  * Modified from [org.apache.logging.log4j.maven.plugins.shade.transformer.Log4j2PluginCacheFileTransformer.java](https://github.com/apache/logging-log4j-transform/blob/main/log4j-transform-maven-shade-plugin-extensions/src/main/java/org/apache/logging/log4j/maven/plugins/shade/transformer/Log4j2PluginCacheFileTransformer.java).
  *
  * @author Paul Nelson Baker
  * @author John Engelman
  */
 @CacheableTransformer
 public open class Log4j2PluginsCacheFileTransformer : Transformer {
-  private val temporaryFiles = mutableListOf<File>()
-  private val relocators = mutableListOf<Relocator>()
+  /**
+   * Log4j config files to share across the transformation stages.
+   */
+  private val tempFiles = mutableListOf<Path>()
+
+  /**
+   * [Relocator] instances to share across the transformation stages.
+   */
+  private val tempRelocators = mutableListOf<Relocator>()
   private var stats: ShadowStats? = null
 
   override fun canTransformResource(element: FileTreeElement): Boolean {
-    return PluginProcessor.PLUGIN_CACHE_FILE == element.name
+    return PLUGIN_CACHE_FILE == element.relativePath.pathString
   }
 
   override fun transform(context: TransformerContext) {
-    val temporaryFile = File.createTempFile("Log4j2Plugins", ".dat")
-    temporaryFile.deleteOnExit()
-    temporaryFiles.add(temporaryFile)
+    val temporaryFile = createTempFile("Log4j2Plugins", ".dat")
+    tempFiles.add(temporaryFile)
     val fos = temporaryFile.outputStream()
     context.inputStream.use {
       it.copyTo(fos)
     }
 
-    relocators.addAll(context.relocators)
+    tempRelocators.addAll(context.relocators)
 
     if (stats == null) {
       stats = context.stats
     }
   }
 
+  /**
+   * @return `true` if any dat file collected.
+   */
   override fun hasTransformedResource(): Boolean {
-    // This functionality matches the original plugin, however, I'm not clear what
-    // the exact logic is. From what I can tell temporaryFiles should be never be empty
-    // if anything has been performed.
-    return temporaryFiles.isNotEmpty() || relocators.isNotEmpty()
+    return tempFiles.isNotEmpty()
   }
 
   override fun modifyOutputStream(os: ZipOutputStream, preserveFileTimestamps: Boolean) {
-    val pluginCache = PluginCache()
-    pluginCache.loadCacheFiles(urlEnumeration)
-    relocatePlugins(pluginCache)
-    val entry = ZipEntry(PluginProcessor.PLUGIN_CACHE_FILE)
-    entry.time = getEntryTimestamp(preserveFileTimestamps, entry.time)
-    os.putNextEntry(entry)
-    pluginCache.writeCache(CloseShieldOutputStream.wrap(os))
-    temporaryFiles.clear()
+    try {
+      val aggregator = PluginCache()
+      aggregator.loadCacheFiles(urlEnumeration)
+      relocatePlugins(aggregator)
+      val entry = ZipEntry(PLUGIN_CACHE_FILE)
+      entry.time = getEntryTimestamp(preserveFileTimestamps, entry.time)
+      os.putNextEntry(entry)
+      // prevent the aggregator to close the jar output.
+      aggregator.writeCache(CloseShieldOutputStream.wrap(os))
+    } finally {
+      deleteTempFiles()
+    }
   }
 
-  private fun relocatePlugins(pluginCache: PluginCache) {
+  internal fun relocatePlugins(pluginCache: PluginCache) {
     pluginCache.allCategories.values.forEach { currentMap ->
       currentMap.values.forEach { currentPluginEntry ->
         val className = currentPluginEntry.className
         val relocateClassContext = RelocateClassContext(className, requireNotNull(stats))
-        relocators.firstOrNull { it.canRelocateClass(className) }?.let { relocator ->
+        tempRelocators.firstOrNull { it.canRelocateClass(className) }?.let { relocator ->
           // Then we perform that relocation and update the plugin entry to reflect the new value.
           currentPluginEntry.className = relocator.relocateClass(relocateClassContext)
         }
       }
     }
   }
 
+  private fun deleteTempFiles() {
+    val pathIterator = tempFiles.listIterator()
+    while (pathIterator.hasNext()) {
+      val path = pathIterator.next()
+      path.deleteIfExists()
+      pathIterator.remove()
+    }
+  }
+
   private val urlEnumeration: Enumeration<URL>
     get() {
-      val urls = temporaryFiles.map { it.toURI().toURL() }
+      val urls = tempFiles.map { it.toUri().toURL() }
       return Collections.enumeration(urls)
     }
 }
diff --git a/...hub/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformerTest.kt b/...hub/jengelman/gradle/plugins/shadow/transformers/Log4j2PluginsCacheFileTransformerTest.kt
@@ -1,29 +1,41 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
+import assertk.all
 import assertk.assertThat
 import assertk.assertions.isEqualTo
+import assertk.assertions.isFalse
+import assertk.assertions.isNotEqualTo
 import assertk.assertions.isTrue
+import assertk.fail
 import com.github.jengelman.gradle.plugins.shadow.internal.requireResourceAsStream
 import com.github.jengelman.gradle.plugins.shadow.relocation.SimpleRelocator
 import com.github.jengelman.gradle.plugins.shadow.util.SimpleRelocator
+import java.io.ByteArrayOutputStream
 import java.io.File
 import java.net.URI
+import java.net.URL
 import java.util.Collections
+import java.util.jar.JarInputStream
 import org.apache.logging.log4j.core.config.plugins.processor.PluginCache
+import org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor.PLUGIN_CACHE_FILE
 import org.apache.tools.zip.ZipOutputStream
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.Arguments
+import org.junit.jupiter.params.provider.MethodSource
 
+/**
+ * Modified from [org.apache.logging.log4j.maven.plugins.shade.transformer.Log4j2PluginCacheFileTransformerTest.java](https://github.com/apache/logging-log4j-transform/blob/main/log4j-transform-maven-shade-plugin-extensions/src/test/java/org/apache/logging/log4j/maven/plugins/shade/transformer/Log4j2PluginCacheFileTransformerTest.java).
+ */
 class Log4j2PluginsCacheFileTransformerTest : BaseTransformerTest<Log4j2PluginsCacheFileTransformer>() {
   @Test
-  fun shouldTransform() {
-    transformer.transform(context(SimpleRelocator()))
-    assertThat(transformer.hasTransformedResource()).isTrue()
-  }
-
-  @Test
-  fun shouldTransformForSingleFile() {
-    transformer.transform(context())
-    assertThat(transformer.hasTransformedResource()).isTrue()
+  fun canTransformResource() {
+    assertThat(transformer.canTransformResource("")).isFalse()
+    assertThat(transformer.canTransformResource(".")).isFalse()
+    assertThat(transformer.canTransformResource("tmp.dat")).isFalse()
+    assertThat(transformer.canTransformResource("$PLUGIN_CACHE_FILE.tmp")).isFalse()
+    assertThat(transformer.canTransformResource("tmp/$PLUGIN_CACHE_FILE")).isFalse()
+    assertThat(transformer.canTransformResource(PLUGIN_CACHE_FILE)).isTrue()
   }
 
   @Test
@@ -48,11 +60,67 @@ class Log4j2PluginsCacheFileTransformerTest : BaseTransformerTest<Log4j2PluginsC
       .isEqualTo("new.location.org.apache.logging.log4j.core.lookup.DateLookup")
   }
 
+  @Test
+  fun transformAndModifyOutputStream() {
+    assertThat(transformer.hasTransformedResource()).isFalse()
+
+    transformer.transform(context())
+    assertThat(transformer.hasTransformedResource()).isTrue()
+    transformer.transform(context())
+    assertThat(transformer.hasTransformedResource()).isTrue()
+
+    val jarBuff = ByteArrayOutputStream()
+    ZipOutputStream(jarBuff).use {
+      transformer.modifyOutputStream(it, false)
+    }
+    JarInputStream(jarBuff.toByteArray().inputStream()).use { inputStream ->
+      while (true) {
+        val jarEntry = inputStream.nextJarEntry
+        if (jarEntry == null) {
+          fail("No expected resource in the output jar.")
+        } else if (jarEntry.name == PLUGIN_CACHE_FILE) {
+          @Suppress("Since15")
+          assertThat(inputStream.readAllBytes().contentHashCode()).all {
+            // Hash of the original plugin cache file.
+            isNotEqualTo(-2114104185)
+            isEqualTo(1911442937)
+          }
+          break
+        }
+      }
+    }
+  }
+
+  @ParameterizedTest
+  @MethodSource("relocationParameters")
+  fun relocations(pattern: String, shadedPattern: String, target: String) {
+    val aggregator = PluginCache().apply {
+      val resources = Collections.enumeration(listOf(pluginCacheUrl))
+      loadCacheFiles(resources)
+    }
+    transformer.transform(context(SimpleRelocator(pattern, shadedPattern)))
+    transformer.relocatePlugins(aggregator)
+
+    for (pluginEntryMap in aggregator.allCategories.values) {
+      for (entry in pluginEntryMap.values) {
+        assertThat(entry.className.startsWith(target)).isTrue()
+      }
+    }
+  }
+
   private fun context(vararg relocator: SimpleRelocator): TransformerContext {
     return TransformerContext(PLUGIN_CACHE_FILE, requireResourceAsStream(PLUGIN_CACHE_FILE), relocator.toSet())
   }
 
   private companion object {
-    const val PLUGIN_CACHE_FILE = "META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat"
+    val pluginCacheUrl: URL = requireNotNull(this::class.java.classLoader.getResource(PLUGIN_CACHE_FILE))
+
+    @JvmStatic
+    fun relocationParameters() = listOf(
+      // test with matching relocator
+      Arguments.of("org.apache.logging", "new.location.org.apache.logging", "new.location.org.apache.logging"),
+      // test without matching relocator
+      Arguments.of("com.apache.logging", "new.location.com.apache.logging", "org.apache.logging"),
+    )
   }
 }
diff --git a/src/test/resources/META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat b/src/test/resources/META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat