Merge pull request #211 from ivassile/modify_blocklist_impl

Modify blocklist implementation.
wildfly-extras · Mar 22, 2024 · 75b5bb2 · 75b5bb2
2 parents 01a5917 + f99369c
commit 75b5bb2
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 32 deletions.
diff --git a/core/src/main/java/org/wildfly/channel/ChannelImpl.java b/core/src/main/java/org/wildfly/channel/ChannelImpl.java
@@ -202,6 +202,11 @@ Optional<ResolveLatestVersionResult> resolveLatestVersion(String groupId, String
         requireNonNull(artifactId);
         requireNonNull(resolver);
 
+        Set<String> blocklistedVersions = Collections.emptySet();
+        if (this.blocklist.isPresent()) {
+            blocklistedVersions = this.blocklist.get().getVersionsFor(groupId, artifactId);
+        }
+
         // first we find if there is a stream for that given (groupId, artifactId).
         Optional<Stream> foundStream = channelManifest.findStreamFor(groupId, artifactId);
         // no stream for this artifact, let's look into the required channel
@@ -214,6 +219,7 @@ Optional<ResolveLatestVersionResult> resolveLatestVersion(String groupId, String
                     foundVersions.put(found.get().version, found.get().channel);
                 }
             }
+            foundVersions.keySet().removeAll(blocklistedVersions);
             Optional<String> foundVersionInRequiredChannels = foundVersions.keySet().stream().sorted(COMPARATOR.reversed()).findFirst();
             if (foundVersionInRequiredChannels.isPresent()) {
                 return Optional.of(new ResolveLatestVersionResult(foundVersionInRequiredChannels.get(), foundVersions.get(foundVersionInRequiredChannels.get())));
@@ -223,6 +229,7 @@ Optional<ResolveLatestVersionResult> resolveLatestVersion(String groupId, String
             switch (channelDefinition.getNoStreamStrategy()) {
                 case LATEST:
                     Set<String> versions = resolver.getAllVersions(groupId, artifactId, extension, classifier);
+                    versions.removeAll(blocklistedVersions);
                     final Optional<String> latestVersion = versions.stream().sorted(COMPARATOR.reversed()).findFirst();
                     if (latestVersion.isPresent()) {
                         return Optional.of(new ResolveLatestVersionResult(latestVersion.get(), this));
@@ -231,9 +238,15 @@ Optional<ResolveLatestVersionResult> resolveLatestVersion(String groupId, String
                     }
                 case MAVEN_LATEST:
                     String latestMetadataVersion = resolver.getMetadataLatestVersion(groupId, artifactId);
+                    if (blocklistedVersions.contains(latestMetadataVersion)) {
+                        return Optional.empty();
+                    }
                     return Optional.of(new ResolveLatestVersionResult(latestMetadataVersion, this));
                 case MAVEN_RELEASE:
                     String releaseMetadataVersion = resolver.getMetadataReleaseVersion(groupId, artifactId);
+                    if (blocklistedVersions.contains(releaseMetadataVersion)) {
+                        return Optional.empty();
+                    }
                     return Optional.of(new ResolveLatestVersionResult(releaseMetadataVersion, this));
                 default:
                     return Optional.empty();
@@ -245,14 +258,13 @@ Optional<ResolveLatestVersionResult> resolveLatestVersion(String groupId, String
         // there is a stream, let's now check its version
         if (stream.getVersion() != null) {
             foundVersion = Optional.of(stream.getVersion());
+            if (foundVersion.isPresent() && blocklistedVersions.contains(foundVersion.get())) {
+                return Optional.empty();
+            }
         } else if (stream.getVersionPattern() != null) {
             // if there is a version pattern, we resolve all versions from Maven to find the latest one
             Set<String> versions = resolver.getAllVersions(groupId, artifactId, extension, classifier);
-            if (this.blocklist.isPresent()) {
-                final Set<String> blocklistedVersions = this.blocklist.get().getVersionsFor(groupId, artifactId);
-
-                versions.removeAll(blocklistedVersions);
-            }
+            versions.removeAll(blocklistedVersions);
             foundVersion = foundStream.get().getVersionComparator().matches(versions);
         }
 

diff --git a/core/src/test/java/org/wildfly/channel/ChannelRecorderTestCase.java b/core/src/test/java/org/wildfly/channel/ChannelRecorderTestCase.java
@@ -30,10 +30,11 @@
 
 import java.io.File;
 import java.nio.file.Path;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -78,7 +79,7 @@ public void testChannelRecorder() throws Exception {
         when(resolver.getAllVersions(eq("org.wildfly.core"), anyString(), eq(null), eq(null)))
                 .thenReturn(singleton("18.0.0.Final"));
         when(resolver.getAllVersions(eq("io.undertow"), anyString(), eq(null), eq(null)))
-                .thenReturn(Set.of("2.1.0.Final", "2.2.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("2.1.0.Final", "2.2.0.Final")));
         when(resolver.resolveArtifact(anyString(), anyString(), eq(null), eq(null), anyString()))
                 .thenReturn(mock(File.class));
 

diff --git a/core/src/test/java/org/wildfly/channel/ChannelSessionTestCase.java b/core/src/test/java/org/wildfly/channel/ChannelSessionTestCase.java
@@ -38,10 +38,11 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -142,7 +143,7 @@ public void testResolveLatestMavenArtifact() throws Exception {
         File resolvedArtifactFile = mock(File.class);
 
         when(factory.create(any())).thenReturn(resolver);
-        when(resolver.getAllVersions("org.wildfly", "wildfly-ee-galleon-pack", null, null)).thenReturn(Set.of("25.0.0.Final", "25.0.1.Final"));
+        when(resolver.getAllVersions("org.wildfly", "wildfly-ee-galleon-pack", null, null)).thenReturn(new HashSet<>(Arrays.asList("25.0.0.Final", "25.0.1.Final")));
         when(resolver.resolveArtifact("org.wildfly", "wildfly-ee-galleon-pack", null, null, "25.0.1.Final")).thenReturn(resolvedArtifactFile);
 
         final List<Channel> channels = mockChannel(resolver, tempDir, manifest);
@@ -440,7 +441,7 @@ public void testChannelWithLatestStrategy() throws Exception {
 
         when(factory.create(any())).thenReturn(resolver);
         when(resolver.resolveArtifact(eq("org.foo"), eq("bar"), eq(null), eq(null), eq("25.0.2.Final"))).thenReturn(resolvedArtifactFile);
-        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(Set.of("25.0.2.Final", "25.0.1.Final", "25.0.0.Final"));
+        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(new HashSet<>(Arrays.asList("25.0.2.Final", "25.0.1.Final", "25.0.0.Final")));
 
         try (ChannelSession session = new ChannelSession(channels, factory)) {
             MavenArtifact resolvedArtifact = session.resolveMavenArtifact("org.foo", "bar", null, null, "25.0.1.Final");
@@ -463,7 +464,7 @@ public void testChannelWithLatestStrategyNoArtifact() throws Exception {
         final List<Channel> channels = mockChannel(resolver, tempDir, Channel.NoStreamStrategy.LATEST, manifest);
 
         when(factory.create(any())).thenReturn(resolver);
-        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(Set.of());
+        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(new HashSet<>(Arrays.asList()));
 
         try (ChannelSession session = new ChannelSession(channels, factory)) {
             Assertions.assertThrows(UnresolvedMavenArtifactException.class, () ->
@@ -486,7 +487,7 @@ public void testChannelWithLatestStrategyWithVersionPattern() throws Exception {
         final List<Channel> channels = mockChannel(resolver, tempDir, Channel.NoStreamStrategy.LATEST, manifest);
 
         when(factory.create(any())).thenReturn(resolver);
-        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(Set.of("1.0.0"));
+        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(new HashSet<>(Arrays.asList("1.0.0")));
 
         try (ChannelSession session = new ChannelSession(channels, factory)) {
             Assertions.assertThrows(UnresolvedMavenArtifactException.class, () ->
@@ -512,7 +513,7 @@ public void testChannelWithMavenReleaseStrategy() throws Exception {
         when(factory.create(any())).thenReturn(resolver);
         when(resolver.getMetadataReleaseVersion(eq("org.foo"), eq("bar"))).thenReturn("25.0.1.Final");
         when(resolver.resolveArtifact(eq("org.foo"), eq("bar"), eq(null), eq(null), eq("25.0.1.Final"))).thenReturn(resolvedArtifactFile);
-        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(Set.of("25.0.1.Final", "25.0.0.Final"));
+        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(new HashSet<>(Arrays.asList("25.0.1.Final", "25.0.0.Final")));
 
         try (ChannelSession session = new ChannelSession(channels, factory)) {
             MavenArtifact resolvedArtifact = session.resolveMavenArtifact("org.foo", "bar", null, null, "25.0.1.Final");
@@ -537,7 +538,7 @@ public void testChannelWithMavenLatestStrategy() throws Exception {
         when(factory.create(any())).thenReturn(resolver);
         when(resolver.getMetadataLatestVersion(eq("org.foo"), eq("bar"))).thenReturn("25.0.1.Final");
         when(resolver.resolveArtifact(eq("org.foo"), eq("bar"), eq(null), eq(null), eq("25.0.1.Final"))).thenReturn(resolvedArtifactFile);
-        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(Set.of("25.0.1.Final", "25.0.0.Final"));
+        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(new HashSet<>(Arrays.asList("25.0.1.Final", "25.0.0.Final")));
 
         try (ChannelSession session = new ChannelSession(channels, factory)) {
             MavenArtifact resolvedArtifact = session.resolveMavenArtifact("org.foo", "bar", null, null, "25.0.1.Final");
@@ -561,7 +562,7 @@ public void testChannelWithStrictStrategy() throws Exception {
 
         when(factory.create(any())).thenReturn(resolver);
         when(resolver.resolveArtifact(eq("org.foo"), eq("bar"), eq(null), eq(null), eq("25.0.1.Final"))).thenReturn(resolvedArtifactFile);
-        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(Set.of("25.0.1.Final", "25.0.0.Final"));
+        when(resolver.getAllVersions("org.foo", "bar", null, null)).thenReturn(new HashSet<>(Arrays.asList("25.0.1.Final", "25.0.0.Final")));
 
         try (ChannelSession session = new ChannelSession(channels, factory)) {
             Assertions.assertThrows(UnresolvedMavenArtifactException.class, () ->

diff --git a/core/src/test/java/org/wildfly/channel/ChannelWithRequirementsTestCase.java b/core/src/test/java/org/wildfly/channel/ChannelWithRequirementsTestCase.java
@@ -29,8 +29,9 @@
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.commons.lang3.RandomUtils;
 import org.junit.jupiter.api.Assertions;
@@ -60,11 +61,11 @@ public void testChannelWhichRequiresAnotherChannel() throws Exception {
         when(factory.create(any()))
                 .thenReturn(resolver);
         when(resolver.getAllVersions("test.channels", "required-manifest", "yaml", "manifest"))
-                .thenReturn(Set.of("1", "2", "3"));
+                .thenReturn(new HashSet<>(Arrays.asList("1", "2", "3")));
         when(resolver.resolveArtifact("org.example", "required-manifest", "yaml", "manifest", "3"))
                 .thenReturn(resolvedArtifactFile);
         when(resolver.getAllVersions("org.example", "foo-bar", null, null))
-                .thenReturn(Set.of("1.0.0.Final, 1.1.0.Final", "1.2.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final, 1.1.0.Final", "1.2.0.Final")));
         when(resolver.resolveArtifact("org.example", "foo-bar", null, null, "1.2.0.Final"))
                 .thenReturn(resolvedArtifactFile);
         when(resolver.resolveChannelMetadata(any())).thenReturn(List.of(resolvedRequiredManifestURL));
@@ -182,7 +183,7 @@ public void testRequiringChannelOverridesStreamFromRequiredChannel() throws Exce
                 .thenReturn(resolver);
         // There are 2 version of foo-bar
         when(resolver.getAllVersions("org.example", "foo-bar", null, null))
-                .thenReturn(Set.of("1.0.0.Final", "1.2.0.Final", "2.0.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final", "1.2.0.Final", "2.0.0.Final")));
         when(resolver.resolveArtifact("org.example", "foo-bar", null, null, "1.0.0.Final"))
                 .thenReturn(resolvedArtifactFile100Final);
         when(resolver.resolveArtifact("org.example", "foo-bar", null, null, "1.2.0.Final"))
@@ -315,11 +316,11 @@ public void testChannelRequirementNesting() throws Exception {
         // 2 versions of im-only-in-required-channel
         // 2 versions of im-only-in-second-level
         when(resolver.getAllVersions("org.example", "foo-bar", null, null))
-                .thenReturn(Set.of("1.0.0.Final", "1.2.0.Final", "2.0.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final", "1.2.0.Final", "2.0.0.Final")));
         when(resolver.getAllVersions("org.example", "im-only-in-required-channel", null, null))
-                .thenReturn(Set.of("1.0.0.Final", "2.0.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final", "2.0.0.Final")));
         when(resolver.getAllVersions("org.example", "im-only-in-second-level", null, null))
-                .thenReturn(Set.of("1.0.0.Final", "2.0.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final", "2.0.0.Final")));
 
         when(resolver.resolveArtifact("org.example", "foo-bar", null, null, "1.0.0.Final"))
                 .thenReturn(mock(File.class));
@@ -460,9 +461,9 @@ public void testChannelMultipleRequirements() throws Exception {
                 .thenReturn(resolver);
 
         when(resolver.getAllVersions("org.example", "foo-bar", null, null))
-                .thenReturn(Set.of("1.0.0.Final", "1.2.0.Final", "2.0.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final", "1.2.0.Final", "2.0.0.Final")));
         when(resolver.getAllVersions("org.example", "im-only-in-required-channel", null, null))
-                .thenReturn(Set.of("1.0.0.Final", "2.0.0.Final"));
+                .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final", "2.0.0.Final")));
 
         when(resolver.resolveArtifact("org.example", "foo-bar", null, null, "1.0.0.Final"))
                 .thenReturn(mock(File.class));
@@ -586,11 +587,11 @@ public void testRequiredChannelIgnoresNoStreamStrategy() throws Exception {
         when(factory.create(any()))
            .thenReturn(resolver);
         when(resolver.getAllVersions("org.foo", "required-channel", "yaml", "channel"))
-           .thenReturn(Set.of("1", "2", "3"));
+           .thenReturn(new HashSet<>(Arrays.asList("1", "2", "3")));
         when(resolver.resolveArtifact("org.foo", "required-channel", "yaml", "channel", "1.2.0.Final"))
            .thenReturn(resolvedArtifactFile);
         when(resolver.getAllVersions("org.example", "foo-bar", null, null))
-           .thenReturn(Set.of("1.0.0.Final, 1.1.0.Final", "1.2.0.Final"));
+           .thenReturn(new HashSet<>(Arrays.asList("1.0.0.Final, 1.1.0.Final", "1.2.0.Final")));
         when(resolver.resolveArtifact("org.example", "foo-bar", null, null, "1.2.0.Final"))
            .thenReturn(resolvedArtifactFile);
 

diff --git a/doc/examples/channel.adoc b/doc/examples/channel.adoc
@@ -91,7 +91,7 @@ If a new version `1.2.2.Final` is added to the repository, the Channel will reso
 
 ### Block versions
 
-Blocklist allows to exclude a concrete version of an artifact from resolution while maintaining the "latest" resolution strategy. To block an artifact version we need to create a new file called `test-blocklist.yaml`:
+Blocklist allows to exclude a concrete version of an artifact from resolution while maintaining the "latest" resolution strategy. If the resolution strategy is "maven-latest" or "maven-release" and that version is in the blocklist, this will cause the artifact to be removed from the channel. To block an artifact version we need to create a new file called `test-blocklist.yaml`:
 
 [source, yaml, title="test-blocklist.yaml"]
 ----
@@ -100,7 +100,7 @@ name: "test-blocklist"
 blocks:
   - groupId: "org.test"
     artifactId: "artifact-three" #<1>
-    versions: #<1>
+    versions: #<2>
     - "1.0.1.Final"
 ...
 ----
@@ -134,6 +134,8 @@ Let's say the Maven repositories currently contain versions 1.0.0.Final and 1.0.
 
 When a new version, `1.0.3.Final`, is made available, the channel will instead resolve that version and the blocklist will have no effect.
 
+If the Maven repositories contain only version 1.0.1.Final of `org.test.artifact-three`, the artifact will be removed from the channel because this version is in the blocklist.
+
 ## Fix manifest and blocklist versions
 
 So far the channel has been using the latest available versions of manifest and blocklist. If required this can be changed to either use a specific Maven version or a file URL:

diff --git a/doc/spec.adoc b/doc/spec.adoc
@@ -231,10 +231,7 @@ A blocklist applies only to the channel that defined it, not its required channe
 
 #### Resolving artifact version
 
-During artifact version resolution, a stream matching artifact GA is located in the channel. If the stream uses concrete versions, that version of the artifact is resolved and returned to the user.
-If the stream uses `versionPattern`, the blocklist is checked for excluded versions. The excluded versions are removed from the set of available artifact versions before the latest remaining version matching the stream’s pattern is used to resolve the artifact.
-If the blocklist excludes all available artifact versions, `UnresolvedMavenArtifactException` is thrown.
-The blocklist is ignored when using `resolveDirectMavenArtifact` method.
+During artifact version resolution, a stream matching artifact GA is located in the channel. The blocklist is always checked for excluded versions, except when using `resolveDirectMavenArtifact` method. The excluded versions are removed from the set of available artifact versions before the latest remaining version matching the stream’s pattern is used to resolve the artifact. If the blocklist excludes all available artifact versions, `UnresolvedMavenArtifactException` is thrown.
 
 ### Changelog