modrinth: version-from-modrinth-projects improved logging and error handling

src/main/java/me/itzg/helpers/modrinth/ProjectRef.java

@@ -12,6 +12,8 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
+
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.ToString;
 import me.itzg.helpers.errors.InvalidParameterException;
@@ -20,6 +22,7 @@
 
 @Getter
 @ToString
+@EqualsAndHashCode
 public class ProjectRef {
     private static final Pattern VERSIONS = Pattern.compile("[a-zA-Z0-9]{8}");
     private static final Pattern MODPACK_PAGE_URL = Pattern.compile(

src/main/java/me/itzg/helpers/modrinth/VersionFromModrinthProjectsCommand.java

@@ -5,12 +5,17 @@
 
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import lombok.extern.slf4j.Slf4j;
 import me.itzg.helpers.errors.GenericException;
+import me.itzg.helpers.errors.InvalidParameterException;
 import me.itzg.helpers.http.SharedFetchArgs;
 import me.itzg.helpers.modrinth.model.VersionType;
 import picocli.CommandLine.ArgGroup;
@@ -58,6 +63,11 @@ public class VersionFromModrinthProjectsCommand implements Callable<Integer> {
 
     @Override
     public Integer call() throws Exception {
+
+        if (projects == null || projects.isEmpty()) {
+            throw new InvalidParameterException("No Modrinth projects provided, please provide at least one Modrinth project");
+        }
+
         try (ModrinthApiClient modrinthApiClient = new ModrinthApiClient(baseUrl, "modrinth", sharedFetchArgs.options())) {
             final String version = versionFromProjects(modrinthApiClient, projects, loader, defaultVersionType);
 
@@ -72,12 +82,20 @@ public Integer call() throws Exception {
         }
     }
 
-    static String versionFromProjects(ModrinthApiClient modrinthApiClient, List<String> projectRefs, Loader defaultLoader, VersionType defaultVersionType) {
+    static String versionFromProjects(ModrinthApiClient modrinthApiClient, List<String> projectRefs, Loader defaultLoader, VersionType defaultVersionType) throws InvalidParameterException {
         // Parse all refs and separate optional from required
         final List<ProjectRef> allRefs = projectRefs.stream()
+            .filter(Objects::nonNull)
+            .map(String::trim)
+            .filter(s -> !s.isEmpty())
             .map(ProjectRef::parse)
+            .distinct()
             .collect(Collectors.toList());
 
+        if (allRefs.isEmpty()) {
+            throw new InvalidParameterException("No Modrinth projects parsed successfully, please ensure projects follow \"<loader>:<project ID>|<slug>\" and are delimited by commas");
+        }
+
         final List<ProjectRef> requiredRefs = allRefs.stream()
             .filter(ref -> !ref.isOptional())
             .collect(Collectors.toList());
@@ -99,27 +117,30 @@ static String versionFromProjects(ModrinthApiClient modrinthApiClient, List<Stri
         }
 
         final List<List<String>> allGameVersions = Flux.fromIterable(effectiveRefs)
-            .flatMap(projectRef -> {
+            .flatMapSequential(projectRef -> {
                 final Loader loader = projectRef.getLoader() != null ? projectRef.getLoader() : defaultLoader;
                 final VersionType allowedVersionType = projectRef.hasVersionType()
                     ? projectRef.getVersionType()
                     : defaultVersionType;
 
-                return modrinthApiClient.resolveProjectGameVersions(projectRef, loader, null, allowedVersionType);
+                return modrinthApiClient.resolveProjectGameVersions(projectRef, loader, null, allowedVersionType)
+                    .onErrorMap(error -> new GenericException("Failed to resolve project version for " + projectRef.getIdOrSlug()));
             })
             .collectList()
             .block();
 
         if (allGameVersions != null) {
-            return processGameVersions(allGameVersions);
+            return processGameVersions(allGameVersions, effectiveRefs);
         }
         else {
             throw new GenericException("Unable to retrieve game versions for projects " + projectRefs);
         }
     }
 
-    static String processGameVersions(List<List<String>> allGameVersions) {
-        final Map<String, Integer> gameVersionCounts = new HashMap<>();
+    static String processGameVersions(List<List<String>> allGameVersions, List<ProjectRef> projects) {
+
+        final Map<String, int[]> gameVersionPositions = new HashMap<>();
+        final Set<String> loggedBlockedVersions = new HashSet<>();
 
         final int projectCount = allGameVersions.size();
 
@@ -139,15 +160,42 @@ static String processGameVersions(List<List<String>> allGameVersions) {
                 if (positions[i] >= 0) {
                     final int position = positions[i]--;
                     final String version = allGameVersions.get(i).get(position);
-                    final Integer result = gameVersionCounts.compute(version, (k, count) -> count == null ? 1 : count + 1);
+
+                    final int[] projectPositions = gameVersionPositions.computeIfAbsent(version, ignored -> {
+                        final int[] result = new int[projectCount];
+                        Arrays.fill(result, -1);
+                        return result;
+                    });
+
+                    // Prevent duplicate entries from the same project counting twice.
+                    if (projectPositions[i] < 0) {
+                        projectPositions[i] = position;
+                    }
+
                     // did this version slot indicate match for all?
-                    if (result == projectCount) {
+                    if (Arrays.stream(projectPositions).allMatch(projectPosition -> projectPosition >= 0)) {
                         return version;
                     }
+
+                    if (log.isDebugEnabled() && !loggedBlockedVersions.contains(version)) {
+                        final List<Integer> blockingProjects = IntStream.range(0, projectCount)
+                            .filter(projectIndex -> !allGameVersions.get(projectIndex).contains(version))
+                            .boxed()
+                            .collect(Collectors.toList());
+
+                        if (!blockingProjects.isEmpty()) {
+                            loggedBlockedVersions.add(version);
+                            log.debug("Minecraft version {} is blocked by projects {}", version, 
+                                    blockingProjects.stream()
+                                    .map(projects::get)
+                                    .map(p -> p.getIdOrSlug())
+                                    .collect(Collectors.toList()));
+                        }
+                    }
                 }
             }
         }
 
         return null;
     }
-}
+}

src/test/java/me/itzg/helpers/modrinth/VersionFromModrinthProjectsCommandTest.java

@@ -9,6 +9,12 @@
 import com.github.stefanbirkner.systemlambda.SystemLambda;
 import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
 import com.github.tomakehurst.wiremock.junit5.WireMockTest;
+
+import me.itzg.helpers.LatchingExecutionExceptionHandler;
+import me.itzg.helpers.McImageHelper;
+import me.itzg.helpers.errors.InvalidParameterException;
+
+import java.util.ArrayList;
 import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -23,7 +29,16 @@ class VersionFromModrinthProjectsCommandTest {
     @ParameterizedTest
     @FieldSource("processGameVersionsArgs")
     void processGameVersions(List<List<String>> versions, String expected) {
-        final String result = VersionFromModrinthProjectsCommand.processGameVersions(versions);
+
+        List<ProjectRef> projects = new ArrayList<>();
+
+        for (int i = 0; i < versions.size(); ++i) {
+            projects.add(new ProjectRef("test-project-" + Integer.toString(i), null));
+        }
+
+        System.out.println(projects.toString());
+
+        final String result = VersionFromModrinthProjectsCommand.processGameVersions(versions, projects);
 
         if (expected != null) {
             assertThat(result)
@@ -143,6 +158,36 @@ void testCommandFabric(WireMockRuntimeInfo wmInfo) throws Exception {
         assertThat(out).isEqualToNormalizingNewlines("1.21.4\n");
     }
 
+    @Test
+    void testCommandHanlesEmptyProjects() throws Exception {
+
+        final LatchingExecutionExceptionHandler exceptionHandler = new LatchingExecutionExceptionHandler();
+
+        new CommandLine(new McImageHelper())
+            .setExecutionExceptionHandler(exceptionHandler)
+            .execute(
+                 "version-from-modrinth-projects",
+                "--projects="
+                );
+
+        assertThat(exceptionHandler.getExecutionException())
+            .isInstanceOf(InvalidParameterException.class)
+            .hasMessageContaining("No Modrinth projects parsed successfully, please ensure projects follow \"<loader>:<project ID>|<slug>\" and are delimited by commas");
+    }
+
+    @Test
+    void testCommandHandlesNullProjects() throws Exception {
+        final LatchingExecutionExceptionHandler exceptionHandler = new LatchingExecutionExceptionHandler();
+
+        new CommandLine(new McImageHelper())
+            .setExecutionExceptionHandler(exceptionHandler)
+            .execute("version-from-modrinth-projects");
+
+        assertThat(exceptionHandler.getExecutionException())
+            .isInstanceOf(InvalidParameterException.class)
+            .hasMessageContaining("No Modrinth projects provided, please provide at least one Modrinth project");
+    }
+
     @Test
     void testCommandWithProjectQualifiers(WireMockRuntimeInfo wmInfo) throws Exception {