cache key for JarClassHasher includes artifact classifier (#2813)

cache key for JarClassHasher includes artifact classifier

changelog/@unreleased/pr-2813.v2.yml

@@ -0,0 +1,5 @@
+type: fix
+fix:
+  description: cache key for JarClassHasher includes artifact classifier
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2813

gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/ClassUniquenessArtifactIdentifier.java

@@ -0,0 +1,29 @@
+/*
+ * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.palantir.baseline.services;
+
+import java.util.Optional;
+import org.gradle.api.artifacts.ModuleVersionIdentifier;
+import org.immutables.value.Value;
+
+@Value.Immutable
+@Value.Style(optionalAcceptNullable = true)
+public interface ClassUniquenessArtifactIdentifier {
+    ModuleVersionIdentifier moduleVersionIdentifier();
+
+    Optional<String> classifier();
+}

gradle-baseline-java/src/main/groovy/com/palantir/baseline/services/JarClassHasher.java

@@ -18,6 +18,7 @@
 
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.hash.HashCode;
@@ -32,14 +33,14 @@
 import java.util.jar.JarEntry;
 import java.util.jar.JarInputStream;
 import java.util.stream.Collectors;
-import org.gradle.api.artifacts.ModuleVersionIdentifier;
 import org.gradle.api.artifacts.ResolvedArtifact;
 import org.gradle.api.services.BuildService;
 import org.gradle.api.services.BuildServiceParameters;
 import org.slf4j.Logger;
 
 public abstract class JarClassHasher implements BuildService<BuildServiceParameters.None>, AutoCloseable {
-    private final Cache<ModuleVersionIdentifier, Result> cache =
+
+    private final Cache<ClassUniquenessArtifactIdentifier, Result> cache =
             Caffeine.newBuilder().build();
 
     public static final class Result {
@@ -59,7 +60,11 @@ public static Result empty() {
     }
 
     public final Result hashClasses(ResolvedArtifact resolvedArtifact, Logger logger) {
-        return cache.get(resolvedArtifact.getModuleVersion().getId(), _moduleId -> {
+        ClassUniquenessArtifactIdentifier key = ImmutableClassUniquenessArtifactIdentifier.builder()
+                .moduleVersionIdentifier(resolvedArtifact.getModuleVersion().getId())
+                .classifier(Strings.nullToEmpty(resolvedArtifact.getClassifier()))
+                .build();
+        return cache.get(key, _moduleId -> {
             File file = resolvedArtifact.getFile();
             if (!file.exists()) {
                 return Result.empty();

gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessLockTask.java

@@ -19,6 +19,7 @@
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSortedMap;
+import com.palantir.baseline.services.ClassUniquenessArtifactIdentifier;
 import com.palantir.baseline.services.JarClassHasher;
 import com.palantir.gradle.failurereports.exceptions.ExceptionWithSuggestion;
 import difflib.DiffUtils;
@@ -37,7 +38,6 @@
 import org.gradle.api.GradleException;
 import org.gradle.api.Task;
 import org.gradle.api.artifacts.Configuration;
-import org.gradle.api.artifacts.ModuleVersionIdentifier;
 import org.gradle.api.provider.Property;
 import org.gradle.api.provider.SetProperty;
 import org.gradle.api.specs.Spec;
@@ -111,7 +111,8 @@ public final void doIt() {
                             ClassUniquenessAnalyzer analyzer = new ClassUniquenessAnalyzer(
                                     jarClassHasher.get(), getProject().getLogger());
                             analyzer.analyzeConfiguration(configuration);
-                            Collection<Set<ModuleVersionIdentifier>> problemJars = analyzer.getDifferingProblemJars();
+                            Collection<Set<ClassUniquenessArtifactIdentifier>> problemJars =
+                                    analyzer.getDifferingProblemJars();
 
                             if (problemJars.isEmpty()) {
                                 return Optional.empty();
@@ -148,16 +149,23 @@ public final void doIt() {
         }
     }
 
-    private String clashingClasses(ClassUniquenessAnalyzer analyzer, Set<ModuleVersionIdentifier> clashingJars) {
+    private String clashingClasses(
+            ClassUniquenessAnalyzer analyzer, Set<ClassUniquenessArtifactIdentifier> clashingJars) {
         return analyzer.getDifferingSharedClassesInProblemJars(clashingJars).stream()
                 .sorted()
                 .map(className -> String.format("  - %s", className))
                 .collect(Collectors.joining("\n"));
     }
 
-    private String clashingJarHeader(Set<ModuleVersionIdentifier> clashingJars) {
+    private String clashingJarHeader(Set<ClassUniquenessArtifactIdentifier> clashingJars) {
         return clashingJars.stream()
-                .map(mvi -> mvi.getGroup() + ":" + mvi.getName())
+                .map(ident -> {
+                    String mvi = ident.moduleVersionIdentifier().getGroup() + ":"
+                            + ident.moduleVersionIdentifier().getName();
+                    return ident.classifier().isEmpty()
+                            ? mvi
+                            : mvi + " (classifier=" + ident.classifier().get() + ")";
+                })
                 .sorted()
                 .collect(Collectors.joining(", ", "[", "]"));
     }

gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java

@@ -23,6 +23,8 @@
 import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.SetMultimap;
 import com.google.common.hash.HashCode;
+import com.palantir.baseline.services.ClassUniquenessArtifactIdentifier;
+import com.palantir.baseline.services.ImmutableClassUniquenessArtifactIdentifier;
 import com.palantir.baseline.services.JarClassHasher;
 import java.io.File;
 import java.time.Duration;
@@ -32,13 +34,12 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.gradle.api.artifacts.Configuration;
-import org.gradle.api.artifacts.ModuleVersionIdentifier;
 import org.gradle.api.artifacts.ResolvedArtifact;
 import org.slf4j.Logger;
 
 public final class ClassUniquenessAnalyzer {
     private final JarClassHasher jarHasher;
-    private final SetMultimap<Set<ModuleVersionIdentifier>, String> jarsToClasses = HashMultimap.create();
+    private final SetMultimap<Set<ClassUniquenessArtifactIdentifier>, String> jarsToClasses = HashMultimap.create();
     private final SetMultimap<String, HashCode> classToHashCodes = HashMultimap.create();
     private final Logger log;
 
@@ -54,7 +55,7 @@ public void analyzeConfiguration(Configuration configuration) {
 
         // we use these temporary maps to accumulate information as we process each jar,
         // so they may include singletons which we filter out later
-        SetMultimap<String, ModuleVersionIdentifier> classToJars = HashMultimap.create();
+        SetMultimap<String, ClassUniquenessArtifactIdentifier> classToJars = HashMultimap.create();
         SetMultimap<String, HashCode> tempClassToHashCodes = HashMultimap.create();
 
         for (ResolvedArtifact resolvedArtifact : dependencies) {
@@ -70,7 +71,13 @@ public void analyzeConfiguration(Configuration configuration) {
             for (Map.Entry<String, HashCode> entry : hashes.entries()) {
                 String className = entry.getKey();
                 HashCode hashValue = entry.getValue();
-                classToJars.put(className, resolvedArtifact.getModuleVersion().getId());
+                classToJars.put(
+                        className,
+                        ImmutableClassUniquenessArtifactIdentifier.builder()
+                                .moduleVersionIdentifier(
+                                        resolvedArtifact.getModuleVersion().getId())
+                                .classifier(resolvedArtifact.getClassifier())
+                                .build());
                 tempClassToHashCodes.put(className, hashValue);
             }
         }
@@ -97,24 +104,24 @@ public void analyzeConfiguration(Configuration configuration) {
      * Any groups jars that all contain some identically named classes. Note: may contain non-scary duplicates - class
      * files which are 100% identical, so their clashing name doesn't have any effect.
      */
-    private Collection<Set<ModuleVersionIdentifier>> getProblemJars() {
+    private Collection<Set<ClassUniquenessArtifactIdentifier>> getProblemJars() {
         return jarsToClasses.keySet();
     }
 
     /** Class names that appear in all of the given jars. */
-    public Set<String> getSharedClassesInProblemJars(Set<ModuleVersionIdentifier> problemJars) {
+    public Set<String> getSharedClassesInProblemJars(Set<ClassUniquenessArtifactIdentifier> problemJars) {
         return jarsToClasses.get(problemJars);
     }
 
     /** Jars which contain identically named classes with non-identical implementations. */
-    public Collection<Set<ModuleVersionIdentifier>> getDifferingProblemJars() {
+    public Collection<Set<ClassUniquenessArtifactIdentifier>> getDifferingProblemJars() {
         return getProblemJars().stream()
                 .filter(jars -> getDifferingSharedClassesInProblemJars(jars).size() > 0)
                 .collect(Collectors.toSet());
     }
 
     /** Class names which appear in all of the given jars and also have non-identical implementations. */
-    public Set<String> getDifferingSharedClassesInProblemJars(Set<ModuleVersionIdentifier> problemJars) {
+    public Set<String> getDifferingSharedClassesInProblemJars(Set<ClassUniquenessArtifactIdentifier> problemJars) {
         return getSharedClassesInProblemJars(problemJars).stream()
                 .filter(classToHashCodes::containsKey)
                 .collect(toSet());

gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy

@@ -99,6 +99,36 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest {
         lockfile.text == expected.text
     }
 
+    def 'detect duplicates in two external jars with the same ModuleVersionIdentifier but different classifiers'() {
+        File lockfile = new File(projectDir, 'baseline-class-uniqueness.lock')
+
+        when:
+        buildFile << standardBuildFile
+        buildFile << """
+        dependencies {
+            api group: 'com.google.cloud.bigdataoss', name: 'gcs-connector', version: 'hadoop3-2.2.19'
+            api group: 'com.google.cloud.bigdataoss', name: 'gcs-connector', version: 'hadoop3-2.2.19', classifier: 'shaded'
+        }
+        """.stripIndent()
+        BuildResult result = with('check', '-s').buildAndFail()
+
+        then:
+        result.getOutput().contains("baseline-class-uniqueness detected multiple jars containing identically named classes.")
+        result.getOutput().contains("[com.google.cloud.bigdataoss:gcs-connector, com.google.cloud.bigdataoss:gcs-connector (classifier=shaded)]")
+        result.getOutput().contains("com.google.cloud.hadoop.fs.gcs.FsBenchmark")
+
+        when:
+        with("checkClassUniqueness", "--write-locks").build()
+
+        then:
+        lockfile.exists()
+        File expected = new File("src/test/resources/com/palantir/baseline/baseline-class-uniqueness-with-classifier.expected.lock")
+        if (Boolean.getBoolean("recreate")) {
+            GFileUtils.writeFile(lockfile.text, expected)
+        }
+        lockfile.text == expected.text
+    }
+
     def 'detect duplicates in two external jars in non-standard configuration'() {
         when:
         buildFile << standardBuildFile

gradle-baseline-java/src/test/resources/com/palantir/baseline/baseline-class-uniqueness-with-classifier.expected.lock

@@ -0,0 +1,54 @@
+# Danger! Multiple jars contain identically named classes. This may cause different behaviour depending on classpath ordering.
+# Run ./gradlew checkClassUniqueness --fix to update this file
+
+## runtimeClasspath
+[com.google.cloud.bigdataoss:gcs-connector (classifier=shaded), com.google.cloud.bigdataoss:gcsio]
+  - com.google.cloud.hadoop.gcsio.authorization.AuthorizationHandler
+[com.google.cloud.bigdataoss:gcs-connector (classifier=shaded), com.google.cloud.bigdataoss:util-hadoop]
+  - com.google.cloud.hadoop.util.AccessTokenProvider
+  - com.google.cloud.hadoop.util.AccessTokenProvider$AccessToken
+  - com.google.cloud.hadoop.util.AccessTokenProvider$AccessTokenType
+[com.google.cloud.bigdataoss:gcs-connector (classifier=shaded), com.google.cloud.bigdataoss:util]
+  - com.google.cloud.hadoop.util.AccessBoundary
+  - com.google.cloud.hadoop.util.AccessBoundary$Action
+  - com.google.cloud.hadoop.util.AutoValue_AccessBoundary
+[com.google.cloud.bigdataoss:gcs-connector, com.google.cloud.bigdataoss:gcs-connector (classifier=shaded)]
+  - com.google.cloud.hadoop.fs.gcs.AutoValue_SyncableOutputStreamOptions
+  - com.google.cloud.hadoop.fs.gcs.AutoValue_SyncableOutputStreamOptions$1
+  - com.google.cloud.hadoop.fs.gcs.AutoValue_SyncableOutputStreamOptions$Builder
+  - com.google.cloud.hadoop.fs.gcs.CoopLockFsck
+  - com.google.cloud.hadoop.fs.gcs.CoopLockFsckRunner
+  - com.google.cloud.hadoop.fs.gcs.CoopLockFsckRunner$1
+  - com.google.cloud.hadoop.fs.gcs.CoopLockFsckRunner$2
+  - com.google.cloud.hadoop.fs.gcs.FileSystemDescriptor
+  - com.google.cloud.hadoop.fs.gcs.FsBenchmark
+  - com.google.cloud.hadoop.fs.gcs.GhfsStatistic
+  - com.google.cloud.hadoop.fs.gcs.GhfsStatisticTypeEnum
+  - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics
+  - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics$1
+  - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics$LongIterator
+  - com.google.cloud.hadoop.fs.gcs.GhfsStorageStatistics$MeanStatistic
+  - com.google.cloud.hadoop.fs.gcs.GhfsStreamStats
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFS
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFSInputStream
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystem
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$1
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$GcsFileChecksum
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$GcsFileChecksumType
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$GlobAlgorithm
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$InvocationRaisingIOE
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemBase$OutputStreamType
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopOutputStream
+  - com.google.cloud.hadoop.fs.gcs.GoogleHadoopSyncableOutputStream
+  - com.google.cloud.hadoop.fs.gcs.HadoopConfigurationProperty
+  - com.google.cloud.hadoop.fs.gcs.HadoopCredentialConfiguration
+  - com.google.cloud.hadoop.fs.gcs.InMemoryGlobberFileSystem
+  - com.google.cloud.hadoop.fs.gcs.SyncableOutputStreamOptions
+  - com.google.cloud.hadoop.fs.gcs.SyncableOutputStreamOptions$Builder
+  - com.google.cloud.hadoop.fs.gcs.auth.AbstractDelegationTokenBinding
+  - com.google.cloud.hadoop.fs.gcs.auth.AbstractDelegationTokenBinding$TokenSecretManager
+  - com.google.cloud.hadoop.fs.gcs.auth.DelegationTokenIOException
+  - com.google.cloud.hadoop.fs.gcs.auth.GcsDelegationTokens
+  - com.google.cloud.hadoop.fs.gcs.auth.GcsDtFetcher