Fixes a subtle bug with DataPrepperVersion. A recent commit to parse the version from the Gradle project updated the public parse() method to support a full version string. However, this is not an allowable version to parse for the purposes of pipeline configurations. This change fixes that by bringing back the restriction on the public parse() method to support only major or major.minor patterns. Only when reading from the version string from the VersionProvider, do we now allow reading the full version string. (opensearch-project#6199)

Signed-off-by: David Venable <dlv@amazon.com>

Author: dlvenable

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/DataPrepperVersion.java

@@ -18,8 +18,8 @@
 public class DataPrepperVersion {
     private static final String FULL_FORMAT = "%d.%d";
     private static final String SHORTHAND_FORMAT = "%d";
-    private static final String VERSION_PATTERN_STRING = "^((\\d+)(\\.(\\d+))?(\\.(\\d+))?)(-SNAPSHOT)?$";
-    private static final Pattern VERSION_PATTERN = Pattern.compile(VERSION_PATTERN_STRING);
+    private static final Pattern VERSION_PATTERN = Pattern.compile("^((\\d+)(\\.(\\d+))?)$");
+    private static final Pattern VERSION_FULL_PATTERN = Pattern.compile("^((\\d+)(\\.(\\d+))?(\\.(\\d+))?)(-SNAPSHOT)?$");
     private static final int MAJOR_VERSION_PATTERN_POSITION = 2;
     private static final int MINOR_VERSION_PATTERN_POSITION = 4;
 
@@ -38,14 +38,17 @@ public static synchronized DataPrepperVersion getCurrentVersion() {
             final String versionString = ServiceLoader.load(VersionProvider.class).findFirst()
                     .orElseThrow(() -> new RuntimeException("No Data Prepper version available."))
                     .getVersionString();
-            instance = parse(versionString);
+            instance = parse(versionString, VERSION_FULL_PATTERN);
         }
         return instance;
     }
 
     public static DataPrepperVersion parse(final String version) {
+        return parse(version, VERSION_PATTERN);
+    }
 
-        final Matcher result = VERSION_PATTERN.matcher(version);
+    private static DataPrepperVersion parse(final String version, final Pattern versionPattern) {
+        final Matcher result = versionPattern.matcher(version);
         if(result.find()) {
             String major = result.group(MAJOR_VERSION_PATTERN_POSITION);
             final int foundMajorVersion = Integer.parseInt(major);

data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/DataPrepperVersionTest.java

@@ -15,6 +15,7 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
@@ -67,11 +68,7 @@ private static Stream<Arguments> validDataPrepperVersions() {
             arguments("1.0", 1, Optional.of(0)),
             arguments("123423.0", 123423, Optional.of(0)),
             arguments("2.325", 2, Optional.of(325)),
-            arguments("3.14", 3, Optional.of(14)),
-            arguments("3.14.0", 3, Optional.of(14)),
-            arguments("3.14.1", 3, Optional.of(14)),
-            arguments("3.14.1-SNAPSHOT", 3, Optional.of(14)),
-            arguments("11-SNAPSHOT", 11, Optional.empty())
+            arguments("3.14", 3, Optional.of(14))
         );
     }
 
@@ -82,6 +79,12 @@ void testInvalidVersionsCannotBeParsed(final String version) {
         assertThrows(IllegalArgumentException.class, () -> DataPrepperVersion.parse(version));
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"3.14.1", "3.14.1-SNAPSHOT", "11-SNAPSHOT"})
+    void parse_throws_if_given_a_valid_version_that_cannot_be_a_configuration_version(final String version) {
+        assertThrows(IllegalArgumentException.class, () -> DataPrepperVersion.parse(version));
+    }
+
     @ParameterizedTest
     @MethodSource("compatibleDataPrepperVersions")
     void testCompatibleWith_equalVersions(final String versionA, final String versionB) {
@@ -190,8 +193,16 @@ void testEquals_withDifferentObject() {
     }
 
     @ParameterizedTest
-    @ValueSource(strings = {"2.11", "2.13", "3.0", "3.1"})
-    void getCurrentVersion_returns_value_from_VersionProvider(final String versionString) {
+    @CsvSource({
+            "2.11, 2.11",
+            "2.13, 2.13",
+            "3.0, 3.0",
+            "3.1, 3.1",
+            "3.14.1, 3.14",
+            "3.14.20, 3.14",
+            "3.14.1-SNAPSHOT, 3.14",
+            "11-SNAPSHOT, 11"})
+    void getCurrentVersion_returns_value_from_VersionProvider(final String versionString, final String expectedVersion) {
         final DataPrepperVersion currentVersion;
         when(serviceLoader.findFirst()).thenReturn(Optional.of(currentVersionProvider));
         when(currentVersionProvider.getVersionString()).thenReturn(versionString);
@@ -203,7 +214,7 @@ void getCurrentVersion_returns_value_from_VersionProvider(final String versionSt
         }
 
         assertThat(currentVersion, is(notNullValue()));
-        assertThat(currentVersion, is(equalTo(DataPrepperVersion.parse(versionString))));
+        assertThat(currentVersion.toString(), is(equalTo(expectedVersion)));
     }
 
     @Test
@@ -234,6 +245,19 @@ void getCurrentVersion_throws_if_no_VersionProvider() {
         }
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"2.", ".", ".1", "3.1.2.11", "3.14.1-RELEASE", "3.14.1-snapshot"})
+    void getCurrentVersion_throws_for_invalid_DataPrepperVersion(final String versionString) {
+        when(serviceLoader.findFirst()).thenReturn(Optional.of(currentVersionProvider));
+        when(currentVersionProvider.getVersionString()).thenReturn(versionString);
+        try (final MockedStatic<ServiceLoader> serviceLoaderStatic = mockStatic(ServiceLoader.class)) {
+            serviceLoaderStatic.when(() -> ServiceLoader.load(VersionProvider.class))
+                    .thenReturn(serviceLoader);
+
+            assertThrows(IllegalArgumentException.class, DataPrepperVersion::getCurrentVersion);
+        }
+    }
+
     @Test
     void testToString_shorthandVersion() {
         final DataPrepperVersion result = DataPrepperVersion.parse("2");

data-prepper-core/src/test/java/org/opensearch/dataprepper/model/configuration/DataPrepperVersionIT.java

@@ -19,11 +19,12 @@
 public class DataPrepperVersionIT {
     @Test
     void getCurrentVersion_returns_expected_value_using_Java_SPI() {
-        final String fullDataPrepperVersion = System.getProperty("project.version");
-
         final DataPrepperVersion currentVersion = DataPrepperVersion.getCurrentVersion();
         assertThat(currentVersion, notNullValue());
+
+        final String fullDataPrepperVersion = System.getProperty("project.version");
+        final String[] majorMinorPair = fullDataPrepperVersion.split("\\.");
         assertThat(fullDataPrepperVersion, containsString(currentVersion.toString()));
-        assertThat(currentVersion, equalTo(DataPrepperVersion.parse(fullDataPrepperVersion)));
+        assertThat(currentVersion, equalTo(DataPrepperVersion.parse(majorMinorPair[0] + "." + majorMinorPair[1])));
     }
 }