Fix lockfile refs for binary-less and cached artifacts

private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolver.java

@@ -207,7 +207,7 @@ private ResolutionResult parseDependencies(
         artifactsByNode.computeIfAbsent(coordinates, k -> new ArrayList<>()).add(artifact);
 
         File artifactFile = artifact.getFile();
-        if (artifactFile != null && artifactFile.exists()) {
+        if (artifactFileMatchesCoordinates(coordinates, artifactFile)) {
           paths.put(coordinates, artifactFile.toPath());
         }
 
@@ -294,29 +294,15 @@ private ResolutionResult parseDependencies(
       }
 
       File bestFile = null;
-      // Prefer jar/aar with name matching artifactId-version to avoid picking wrong version
+      // Prefer the artifact file matching the resolved coordinates to avoid picking wrong versions
+      // or metadata files such as POMs.
       for (GradleResolvedArtifact artifact : entry.getValue()) {
         File file = artifact.getFile();
-        if (file == null || !file.exists()) {
-          continue;
-        }
-        String name = file.getName();
-        boolean isJarOrAar = name.endsWith(".jar") || name.endsWith(".aar");
-        if (isJarOrAar && name.contains(coords.getArtifactId() + "-" + coords.getVersion())) {
+        if (artifactFileMatchesCoordinates(coords, file)) {
           bestFile = file;
           break;
         }
       }
-      // Fallback: any existing file (including pom)
-      if (bestFile == null) {
-        for (GradleResolvedArtifact artifact : entry.getValue()) {
-          File file = artifact.getFile();
-          if (file != null && file.exists()) {
-            bestFile = file;
-            break;
-          }
-        }
-      }
       if (bestFile != null) {
         paths.put(coords, bestFile.toPath());
       }
@@ -337,6 +323,15 @@ private String makeDepKey(String group, String artifact, String version) {
     return group + ":" + artifact + ":" + version;
   }
 
+  private boolean artifactFileMatchesCoordinates(Coordinates coordinates, File file) {
+    if (file == null || !file.exists()) {
+      return false;
+    }
+
+    Path expectedFileName = Paths.get(coordinates.toRepoPath()).getFileName();
+    return expectedFileName != null && expectedFileName.toString().equals(file.getName());
+  }
+
   private void addDependency(
       MutableGraph<Coordinates> graph,
       Coordinates parent,

private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V3LockFile.java

@@ -258,9 +258,13 @@ public Map<String, Object> render() {
           }
         });
 
+    Map<String, Map<String, Object>> finalizedArtifacts =
+        ensureArtifactsAllHaveAtLeastOneShaSum(artifacts);
+    Set<String> artifactKeys = artifactKeys(finalizedArtifacts);
+
     Map<String, Object> lock = new LinkedHashMap<>();
-    lock.put("artifacts", ensureArtifactsAllHaveAtLeastOneShaSum(artifacts));
-    lock.put("dependencies", removeEmptyItems(deps));
+    lock.put("artifacts", finalizedArtifacts);
+    lock.put("dependencies", removeEmptyItems(filterDependencyKeys(deps, artifactKeys)));
     if (renderPackages) {
       lock.put("packages", removeEmptyItems(packages));
     }
@@ -270,7 +274,7 @@ public Map<String, Object> render() {
     }
     // repos is a LinkedHashSet which is iterated in insertion order.
     // Meaning the order from the Starlark repositories array will be preserved.
-    lock.put("repositories", repos);
+    lock.put("repositories", filterRepositoryKeys(repos, artifactKeys));
 
     lock.put("skipped", skipped);
     if (conflicts != null && !conflicts.isEmpty()) {
@@ -289,6 +293,55 @@ public Map<String, Object> render() {
     return lock;
   }
 
+  private Set<String> artifactKeys(Map<String, Map<String, Object>> artifacts) {
+    Set<String> keys = new TreeSet<>();
+    for (Map.Entry<String, Map<String, Object>> entry : artifacts.entrySet()) {
+      String root = entry.getKey();
+      @SuppressWarnings("unchecked")
+      Map<String, Object> shasums = (Map<String, Object>) entry.getValue().get("shasums");
+      if (shasums == null) {
+        continue;
+      }
+
+      boolean isJarType = root.chars().filter(ch -> ch == ':').count() == 1;
+      for (String type : shasums.keySet()) {
+        String suffix = "jar".equals(type) ? "" : (isJarType ? ":jar" : "") + ":" + type;
+        keys.add(root + suffix);
+      }
+    }
+    return keys;
+  }
+
+  private Map<String, Set<String>> filterRepositoryKeys(
+      Map<String, Set<String>> repos, Set<String> artifactKeys) {
+    Map<String, Set<String>> filtered = new LinkedHashMap<>();
+    repos.forEach(
+        (repo, artifacts) ->
+            filtered.put(
+                repo,
+                artifacts.stream()
+                    .filter(artifactKeys::contains)
+                    .collect(Collectors.toCollection(TreeSet::new))));
+    return filtered;
+  }
+
+  private Map<String, Set<String>> filterDependencyKeys(
+      Map<String, Set<String>> deps, Set<String> artifactKeys) {
+    Map<String, Set<String>> filtered = new TreeMap<>();
+    deps.forEach(
+        (dep, dependencies) -> {
+          if (!artifactKeys.contains(dep)) {
+            return;
+          }
+          filtered.put(
+              dep,
+              dependencies.stream()
+                  .filter(artifactKeys::contains)
+                  .collect(Collectors.toCollection(TreeSet::new)));
+        });
+    return filtered;
+  }
+
   private Map<String, Map<String, Object>> ensureArtifactsAllHaveAtLeastOneShaSum(
       Map<String, Map<String, Object>> artifacts) {
     for (Map<String, Object> item : artifacts.values()) {

private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/remote/Downloader.java

@@ -125,7 +125,7 @@ private DownloadResult performDownload(Coordinates coordsToUse, String path) {
     Path pathInRepo = null;
     Path knownPath = knownPaths.get(coordsToUse);
 
-    if (knownPath != null && Files.exists(knownPath)) {
+    if (knownPath != null && knownPathMatchesRequest(knownPath, path)) {
       pathInRepo = knownPath;
     } else {
       // Check the local cache for the path first
@@ -179,6 +179,13 @@ private DownloadResult performDownload(Coordinates coordsToUse, String path) {
     return new DownloadResult(coordsToUse, Set.copyOf(repos), pathInRepo, sha256);
   }
 
+  private boolean knownPathMatchesRequest(Path knownPath, String path) {
+    Path requestedFileName = Paths.get(path).getFileName();
+    return requestedFileName != null
+        && requestedFileName.equals(knownPath.getFileName())
+        && Files.exists(knownPath);
+  }
+
   private URI buildUri(URI baseUri, String pathInRepo) {
     String path = baseUri.getPath();
     if (!path.endsWith("/")) {

tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java

@@ -86,6 +86,40 @@ public void resolvesSimpleJvmVariant() throws IOException, XMLStreamException {
     assertEquals(Set.of(baseCoordinates, jvmCoordinates), resolved.nodes());
   }
 
+  @Test
+  public void doesNotUsePomAsArtifactPathForAvailableAtModule()
+      throws IOException, XMLStreamException {
+    Coordinates baseCoordinates = new Coordinates("com.example:sample:1.0");
+    Coordinates jvmCoordinates = new Coordinates("com.example:sample-jvm:1.0");
+    MavenRepo mavenRepo = MavenRepo.create();
+    GradleModuleMetadataHelper moduleMetadataHelper = new GradleModuleMetadataHelper(mavenRepo);
+
+    Runfiles runfiles =
+        Runfiles.preload().withSourceRepository(AutoBazelRepository_GradleResolverTest.NAME);
+    Path baseMetadataPath =
+        Paths.get(
+            runfiles.rlocation(
+                "rules_jvm_external/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/fixtures/simpleJvmVariant/sample-1.0.module"));
+    String baseMetadata = Files.readString(baseMetadataPath);
+    moduleMetadataHelper.addToMavenRepo(baseCoordinates.setExtension("pom"), baseMetadata);
+
+    Path jvmMetadataPath =
+        Paths.get(
+            runfiles.rlocation(
+                "rules_jvm_external/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/fixtures/simpleJvmVariant/sample-jvm-1.0.module"));
+    String jvmMetadata = Files.readString(jvmMetadataPath);
+    moduleMetadataHelper.addToMavenRepo(jvmCoordinates, jvmMetadata);
+
+    ResolutionResult result =
+        resolver.resolve(prepareRequestFor(mavenRepo.getPath().toUri(), baseCoordinates));
+
+    assertEquals(Set.of(baseCoordinates, jvmCoordinates), result.getResolution().nodes());
+    assertTrue(result.getArtifacts().get(baseCoordinates).getPath().isEmpty());
+    assertEquals(
+        "sample-jvm-1.0.jar",
+        result.getArtifacts().get(jvmCoordinates).getPath().get().getFileName().toString());
+  }
+
   @Test
   public void resolvesJvmButNotAndroidVariant() throws IOException, XMLStreamException {
     // This test validates a scenario similar to

tests/com/github/bazelbuild/rules_jvm_external/resolver/lockfile/V3LockFileTest.java

@@ -18,6 +18,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import com.github.bazelbuild.rules_jvm_external.Coordinates;
 import com.github.bazelbuild.rules_jvm_external.resolver.Conflict;
@@ -27,6 +28,7 @@
 import com.google.gson.GsonBuilder;
 import java.io.IOException;
 import java.net.URI;
+import java.nio.file.Paths;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -65,6 +67,174 @@ public void shouldRenderAggregatingJarsAsJarWithNullShasum() {
     assertEquals(expected, shasums);
   }
 
+  @Test
+  public void shouldKeepStandaloneAggregatingJarReferences() {
+    Coordinates depCoordinates = new Coordinates("com.example:dep:1.0.0");
+    DependencyInfo aggregator =
+        new DependencyInfo(
+            new Coordinates("com.example:aggregator:1.0.0"),
+            repos,
+            Optional.empty(),
+            Optional.empty(),
+            Set.of(depCoordinates),
+            Set.of(),
+            Set.of(),
+            new TreeMap<>());
+    DependencyInfo dep =
+        new DependencyInfo(
+            depCoordinates,
+            repos,
+            Optional.of(Paths.get("dep-1.0.0.jar")),
+            Optional.of("77e7c2db478e09882e42a74fb2bc821646cbd4e91c20c616fbd5a8c7b7f350b0"),
+            Set.of(),
+            Set.of(),
+            Set.of(),
+            new TreeMap<>());
+
+    Map<String, Object> rendered =
+        new V3LockFile(repos, Set.of(aggregator, dep), Set.of(), true).render();
+
+    Map<?, ?> artifacts = (Map<?, ?>) rendered.get("artifacts");
+    Map<?, ?> data = (Map<?, ?>) artifacts.get("com.example:aggregator");
+    Map<?, ?> shasums = (Map<?, ?>) data.get("shasums");
+
+    HashMap<Object, Object> expected = new HashMap<>();
+    expected.put("jar", null);
+    assertEquals(expected, shasums);
+
+    Map<?, ?> dependencies = (Map<?, ?>) rendered.get("dependencies");
+    assertEquals(Set.of("com.example:dep"), dependencies.get("com.example:aggregator"));
+
+    Map<?, ?> repositories = (Map<?, ?>) rendered.get("repositories");
+    assertTrue(
+        ((Set<?>) repositories.get(defaultRepo.toString())).contains("com.example:aggregator"));
+
+    assertLockReferencesOnlyArtifactKeys(rendered);
+    assertTrue(AbstractMain.calculateArtifactHash(rendered).containsKey("com.example:aggregator"));
+  }
+
+  @Test
+  public void shouldRemoveNonEmittedAggregatorKeysWhenClassifiedSiblingOwnsShasum() {
+    // A binary-less aggregator can share its `group:artifact` short key with a classified sibling.
+    // The sibling owns the only shasum, so the plain key is not emitted as an artifact target.
+    Coordinates umbrellaCoordinates = new Coordinates("com.example:umbrella:1.0.0");
+    Coordinates unshadedCoordinates =
+        new Coordinates("com.example", "umbrella", "jar", "unshaded", "1.0.0");
+    Coordinates depCoordinates = new Coordinates("com.example:dep:1.0.0");
+    Coordinates rootCoordinates = new Coordinates("com.example:root:1.0.0");
+
+    DependencyInfo umbrella =
+        new DependencyInfo(
+            umbrellaCoordinates,
+            repos,
+            Optional.empty(),
+            Optional.empty(),
+            Set.of(depCoordinates),
+            Set.of(),
+            Set.of(),
+            new TreeMap<>());
+    DependencyInfo unshadedSibling =
+        new DependencyInfo(
+            unshadedCoordinates,
+            repos,
+            Optional.of(Paths.get("umbrella-1.0.0-unshaded.jar")),
+            Optional.of("52b70baa4650255f6c06b4401f9f5ab74038c4d0f50357077033c6bfd504f2aa"),
+            Set.of(depCoordinates),
+            Set.of(),
+            Set.of(),
+            new TreeMap<>());
+    DependencyInfo dep =
+        new DependencyInfo(
+            new Coordinates("com.example:dep:1.0.0"),
+            repos,
+            Optional.of(Paths.get("dep-1.0.0.jar")),
+            Optional.of("77e7c2db478e09882e42a74fb2bc821646cbd4e91c20c616fbd5a8c7b7f350b0"),
+            Set.of(),
+            Set.of(),
+            Set.of(),
+            new TreeMap<>());
+    DependencyInfo root =
+        new DependencyInfo(
+            rootCoordinates,
+            repos,
+            Optional.of(Paths.get("root-1.0.0.jar")),
+            Optional.of("55eb963f66c63e0513f8f0898f24a5d9933b7634b1ac50811f305ec19c926bb8"),
+            Set.of(umbrellaCoordinates, unshadedCoordinates, depCoordinates),
+            Set.of(),
+            Set.of(),
+            new TreeMap<>());
+
+    Map<String, Object> rendered =
+        new V3LockFile(repos, Set.of(umbrella, unshadedSibling, dep, root), Set.of(), true)
+            .render();
+
+    Map<?, ?> artifacts = (Map<?, ?>) rendered.get("artifacts");
+    Map<?, ?> umbrellaData = (Map<?, ?>) artifacts.get("com.example:umbrella");
+    Map<?, ?> shasums = (Map<?, ?>) umbrellaData.get("shasums");
+    assertEquals(Set.of("unshaded"), shasums.keySet());
+
+    Map<?, ?> repositories = (Map<?, ?>) rendered.get("repositories");
+    Set<?> repoArtifacts = (Set<?>) repositories.get(defaultRepo.toString());
+    assertFalse(repoArtifacts.contains("com.example:umbrella"));
+    assertTrue(repoArtifacts.contains("com.example:umbrella:jar:unshaded"));
+
+    Map<?, ?> dependencies = (Map<?, ?>) rendered.get("dependencies");
+    assertFalse(dependencies.containsKey("com.example:umbrella"));
+    Set<?> rootDependencies = (Set<?>) dependencies.get("com.example:root");
+    assertFalse(rootDependencies.contains("com.example:umbrella"));
+    assertTrue(rootDependencies.contains("com.example:umbrella:jar:unshaded"));
+
+    assertLockReferencesOnlyArtifactKeys(rendered);
+    Map<String, Integer> hash = AbstractMain.calculateArtifactHash(rendered);
+
+    assertTrue(hash.containsKey("com.example:umbrella:jar:unshaded"));
+    assertTrue(hash.containsKey("com.example:dep"));
+    assertTrue(hash.containsKey("com.example:root"));
+    assertFalse(hash.containsKey("com.example:umbrella"));
+  }
+
+  @SuppressWarnings("unchecked")
+  private static void assertLockReferencesOnlyArtifactKeys(Map<String, Object> rendered) {
+    Set<String> artifactKeys = artifactKeys(rendered);
+
+    Map<String, Set<String>> repositories = (Map<String, Set<String>>) rendered.get("repositories");
+    for (Map.Entry<String, Set<String>> repo : repositories.entrySet()) {
+      for (String artifact : repo.getValue()) {
+        assertTrue(
+            "Repository references non-emitted artifact: " + artifact,
+            artifactKeys.contains(artifact));
+      }
+    }
+
+    Map<String, Set<String>> dependencies = (Map<String, Set<String>>) rendered.get("dependencies");
+    for (Map.Entry<String, Set<String>> dep : dependencies.entrySet()) {
+      assertTrue(
+          "Dependency key is not an emitted artifact: " + dep.getKey(),
+          artifactKeys.contains(dep.getKey()));
+      for (String target : dep.getValue()) {
+        assertTrue(
+            "Dependency references non-emitted artifact: " + target, artifactKeys.contains(target));
+      }
+    }
+  }
+
+  @SuppressWarnings("unchecked")
+  private static Set<String> artifactKeys(Map<String, Object> rendered) {
+    Map<String, Map<String, Object>> artifacts =
+        (Map<String, Map<String, Object>>) rendered.get("artifacts");
+    Set<String> keys = new TreeSet<>();
+    for (Map.Entry<String, Map<String, Object>> entry : artifacts.entrySet()) {
+      String root = entry.getKey();
+      Map<String, Object> shasums = (Map<String, Object>) entry.getValue().get("shasums");
+      boolean isJarType = root.chars().filter(ch -> ch == ':').count() == 1;
+      for (String type : shasums.keySet()) {
+        String suffix = "jar".equals(type) ? "" : (isJarType ? ":jar" : "") + ":" + type;
+        keys.add(root + suffix);
+      }
+    }
+    return keys;
+  }
+
   @Test
   public void shouldRoundTripASimpleSetOfDependencies() {
     V3LockFile roundTripped = roundTrip(new V3LockFile(repos, Set.of(), Set.of(), true));