Fix VersionMatcher not parsing version string correctly

app/src/test/java/org/adrianvictor/lib/DefaultVersionedServiceFactoryTest.java

@@ -7,6 +7,8 @@ import org.adrianvictor.lib.versioning.VersionedServiceRegistrar;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import java.util.List;
+
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
@@ -48,7 +50,7 @@ public class DefaultVersionedServiceFactoryTest {
     void testServiceRegistrationAndRetrievalForB1_7_3() {
         // Mock VersionMatcher to return b1_7_3 suffix
         when(mockVersionMatcher.getServerVersion()).thenReturn("1.7.3");
-        when(mockVersionMatcher.getClassSuffix(eq("1.7.3"))).thenReturn("b1_7_3");
+        when(mockVersionMatcher.getClassSuffixes(eq("1.7.3"))).thenReturn(List.of("b1_7_3"));
 
         // Register the dummy service for b1_7_3
         new DummyRegistrarB1_7_3().register(factory);
@@ -65,7 +67,7 @@ public class DefaultVersionedServiceFactoryTest {
     void testServiceRegistrationAndRetrievalForR1_1() {
         // Mock VersionMatcher to return r1_1 suffix for a 1.10.2 server
         when(mockVersionMatcher.getServerVersion()).thenReturn("1.10.2");
-        when(mockVersionMatcher.getClassSuffix(eq("1.10.2"))).thenReturn("r1_1");
+        when(mockVersionMatcher.getClassSuffixes(eq("1.10.2"))).thenReturn(List.of("r1_1"));
 
         // Register the dummy service for R1_21 (which covers r1_1 compat)
         new DummyRegistrarR1_21().register(factory);
@@ -79,18 +81,17 @@ public class DefaultVersionedServiceFactoryTest {
     }
 
     @Test
-    void testServiceRegistrationAndRetrievalForR1_16_5() {
+    void testServiceFallbackCompatibility() {
-        // Mock VersionMatcher to return r1_16_5 suffix for a 1.18.1 server
+        // Mock a 1.21 server which supports r1_21, r1_16_5, and r1_1
-        when(mockVersionMatcher.getServerVersion()).thenReturn("1.18.1");
+        when(mockVersionMatcher.getServerVersion()).thenReturn("1.21.0");
-        when(mockVersionMatcher.getClassSuffix(eq("1.18.1"))).thenReturn("r1_16_5");
+        when(mockVersionMatcher.getClassSuffixes(eq("1.21.0"))).thenReturn(List.of("r1_21", "r1_16_5", "r1_1"));
 
-        // Register the dummy service for R1_21 (which covers r1_16_5 compat)
+        // Register ONLY for r1_1
-        new DummyRegistrarR1_21().register(factory);
+        factory.register(TestService.class, "r1_1", TestServiceR1_21::new);
 
-        // Retrieve the service
+        // Retrieve the service - should fall back to r1_1
         TestService service = factory.getService(TestService.class);
 
-        // Assert that the correct compatible version is returned
         assertNotNull(service);
         assertTrue(service instanceof TestServiceR1_21);
     }
@@ -99,7 +100,7 @@ public class DefaultVersionedServiceFactoryTest {
     void testServiceRegistrationAndRetrievalForR1_21() {
         // Mock VersionMatcher to return r1_21 suffix
         when(mockVersionMatcher.getServerVersion()).thenReturn("1.21.0");
-        when(mockVersionMatcher.getClassSuffix(eq("1.21.0"))).thenReturn("r1_21");
+        when(mockVersionMatcher.getClassSuffixes(eq("1.21.0"))).thenReturn(List.of("r1_21", "r1_16_5", "r1_1"));
 
         // Register the dummy service for r1_21
         new DummyRegistrarR1_21().register(factory);
@@ -116,7 +117,7 @@ public class DefaultVersionedServiceFactoryTest {
     void testNoServiceRegisteredThrowsException() {
         // Mock VersionMatcher to return an unknown suffix
         when(mockVersionMatcher.getServerVersion()).thenReturn("unknown");
-        when(mockVersionMatcher.getClassSuffix(anyString())).thenReturn("unknown_version");
+        when(mockVersionMatcher.getClassSuffixes(anyString())).thenReturn(List.of("unknown_version"));
 
         // Attempt to retrieve a service without any registration
         IllegalStateException exception = assertThrows(IllegalStateException.class, () ->
@@ -125,21 +126,4 @@ public class DefaultVersionedServiceFactoryTest {
 
         assertTrue(exception.getMessage().contains("No service registered for type"));
     }
-
-    @Test
-    void testSpecificServiceNotRegisteredForVersionThrowsException() {
-        // Mock VersionMatcher to return b1_7_3 suffix
-        when(mockVersionMatcher.getServerVersion()).thenReturn("1.7.3");
-        when(mockVersionMatcher.getClassSuffix(eq("1.7.3"))).thenReturn("b1_7_3");
-
-        // Register R1_21 service, but not B1_7_3
-        new DummyRegistrarR1_21().register(factory); // R1_21 registrar doesn't register b1_7_3
-
-        // Attempt to retrieve b1_7_3 service
-        IllegalStateException exception = assertThrows(IllegalStateException.class, () ->
-            factory.getService(TestService.class)
-        );
-
-        assertTrue(exception.getMessage().contains("No service registered for type"));
-    }
 }

build.gradle.kts

@@ -90,12 +90,40 @@ tasks.register("buildAll") {
     dependsOn(tasks.withType<Jar>())
 }
 
+// Task to merge service files
+val prepareServiceFiles = tasks.register("prepareServiceFiles") {
+    val outputDir = layout.buildDirectory.dir("generated/service-files")
+    val serviceFile = "META-INF/services/org.adrianvictor.lib.versioning.VersionedServiceRegistrar"
+    
+    // Define inputs
+    val inputFiles = mcVersions.map { ver -> file("src/$ver/resources/$serviceFile") }.filter { it.exists() }
+    inputs.files(inputFiles)
+    outputs.dir(outputDir)
+    
+    doLast {
+        val registrars = mutableSetOf<String>()
+        inputFiles.forEach { file ->
+            registrars.addAll(file.readLines().filter { it.isNotBlank() })
+        }
+        
+        val mergedFile = outputDir.get().file(serviceFile).asFile
+        mergedFile.parentFile.mkdirs()
+        mergedFile.writeText(registrars.joinToString("\n"))
+    }
+}
+
 tasks.register<Jar>("bundleAll") {
     from(sourceSets["main"].output)
     mcVersions.forEach { ver ->
         from(sourceSets[ver].output)
     }
 
+    // Include the merged service file
+    from(prepareServiceFiles) {
+        into("META-INF/services")
+        include("org.adrianvictor.lib.versioning.VersionedServiceRegistrar")
+    }
+
     duplicatesStrategy = DuplicatesStrategy.EXCLUDE
 
     archiveClassifier.set("all-implementations")

src/main/java/org/adrianvictor/lib/Main.java

@@ -19,10 +19,13 @@ public class Main extends JavaPlugin {
 
         versionedServiceFactory = new DefaultVersionedServiceFactory(versionMatcher);
 
-        ServiceLoader<VersionedServiceRegistrar> registrars = ServiceLoader.load(VersionedServiceRegistrar.class);
+        ServiceLoader<VersionedServiceRegistrar> registrars = ServiceLoader.load(VersionedServiceRegistrar.class, getClassLoader());
+        int count = 0;
         for (VersionedServiceRegistrar registrar : registrars) {
             registrar.register(versionedServiceFactory);
+            count++;
         }
+        getLogger().info("Registered " + count + " version-specific registrars.");
 
         getLogger().info("tenkumaLib has been enabled!");
     }

src/main/java/org/adrianvictor/lib/versioning/DefaultVersionedServiceFactory.java

@@ -23,12 +23,19 @@ public class DefaultVersionedServiceFactory implements VersionedServiceFactory {
     @SuppressWarnings("unchecked")
     public <T> T getService(Class<T> serviceType) {
         String serverVersion = versionMatcher.getServerVersion();
-        String versionSuffix = versionMatcher.getClassSuffix(serverVersion);
+        java.util.List<String> versionSuffixes = versionMatcher.getClassSuffixes(serverVersion);
 
         Map<String, Supplier<?>> versionSuppliers = serviceRegistrations.get(serviceType);
-        if (versionSuppliers == null || !versionSuppliers.containsKey(versionSuffix)) {
+        if (versionSuppliers == null) {
-            throw new IllegalStateException("No service registered for type " + serviceType.getName() + " and version " + versionSuffix + ". Current server version: " + serverVersion);
+            throw new IllegalStateException("No service registered for type " + serviceType.getName());
         }
-        return (T) versionSuppliers.get(versionSuffix).get();
+
+        for (String suffix : versionSuffixes) {
+            if (versionSuppliers.containsKey(suffix)) {
+                return (T) versionSuppliers.get(suffix).get();
+            }
+        }
+
+        throw new IllegalStateException("No service registered for type " + serviceType.getName() + " and versions " + versionSuffixes + ". Current server version: " + serverVersion);
     }
 }

src/main/java/org/adrianvictor/lib/versioning/VersionMatcher.java

@@ -5,7 +5,7 @@ import org.bukkit.Bukkit;
 import java.util.Comparator;
 import java.util.List;
 import java.util.ArrayList;
-import java.util.Optional;
+import java.util.Collections;
 import java.util.regex.Pattern;
 import java.util.regex.Matcher;
 
@@ -15,9 +15,9 @@ public class VersionMatcher {
 
     private record Entry(String pattern, String classSuffix, int minMajor, int minMinor, int maxMajor, int maxMinor) {}
 
-    public String getClassSuffix(String serverVersion) {
+    public List<String> getClassSuffixes(String serverVersion) {
         if (serverVersion == null) {
-            return null;
+            return Collections.emptyList();
         }
 
         List<Entry> entries = populateEntries();
@@ -30,22 +30,24 @@ public class VersionMatcher {
         for (Entry entry : entries) {
             Pattern p = Pattern.compile(entry.pattern());
             if (p.matcher(serverVersion).matches()) {
-                if (serverMajor >= entry.minMajor() && serverMinor >= entry.minMinor() &&
+                if (serverMajor > entry.minMajor() || (serverMajor == entry.minMajor() && serverMinor >= entry.minMinor())) {
-                    serverMajor <= entry.maxMajor() && serverMinor <= entry.maxMinor()) {
+                    if (serverMajor < entry.maxMajor() || (serverMajor == entry.maxMajor() && serverMinor <= entry.maxMinor())) {
                         applicableEntries.add(entry);
                     }
                 }
             }
+        }
 
-        applicableEntries.sort(Comparator
+        applicableEntries.sort((a, b) -> {
-                .comparing(Entry::maxMajor).reversed()
+            if (a.minMajor() != b.minMajor()) {
-                .thenComparing(Entry::maxMinor).reversed()
+                return Integer.compare(b.minMajor(), a.minMajor());
-                .thenComparing(Entry::minMajor).reversed()
+            }
-                .thenComparing(Entry::minMinor).reversed());
+            return Integer.compare(b.minMinor(), a.minMinor());
+        });
 
-        Optional<Entry> bestMatch = applicableEntries.stream().findFirst();
+        return applicableEntries.stream()
-
+                .map(Entry::classSuffix)
-        return bestMatch.map(Entry::classSuffix).orElse(null);
+                .toList();
     }
 
     public String getServerVersion() {
@@ -70,12 +72,12 @@ public class VersionMatcher {
     private List<Entry> populateEntries() {
         return List.of(
             new Entry("^1\\.7\\.3$", "b1_7_3", 1, 7, 1, 7),
-            // r1_1 covers versions from 1.1.x up to 1.16.4
+            // r1_1 covers versions 1.1 and above
-            new Entry("^1\\.(1[0-5]|[1-9])(\\.\\d+)*$", "r1_1", 1, 1, 1, 16),
+            new Entry("^1\\.\\d+(\\.\\d+)*$", "r1_1", 1, 1, Integer.MAX_VALUE, Integer.MAX_VALUE),
-            // r1_16_5 covers versions from 1.16.5 up to 1.20.x
+            // r1_16_5 covers versions 1.16.5 and above
-            new Entry("^1\\.(16\\.[5-9]|1[7-9]|20)(\\.\\d+)*$", "r1_16_5", 1, 16, 1, 20),
+            new Entry("^1\\.(16\\.[5-9]|1[7-9]|[2-9]\\d)(\\.\\d+)*$", "r1_16_5", 1, 16, Integer.MAX_VALUE, Integer.MAX_VALUE),
-            // r1_21 covers 1.21.x and above
+            // r1_21 covers 1.21 and above
-            new Entry("^1\\.21(\\.\\d+)*$", "r1_21", 1, 21, Integer.MAX_VALUE, Integer.MAX_VALUE)
+            new Entry("^1\\.([2-9]\\d)(\\.\\d+)*$", "r1_21", 1, 21, Integer.MAX_VALUE, Integer.MAX_VALUE)
         );
     }