Support for automatically generating SemVer branches (#5000)

* Added support for automatically generating SemVer branches * Revert a temporary snapshot version in root pom.xml * Fix a broken UI integration test
Apicurio · Aug 9, 2024 · 82b076a · 82b076a
1 parent 930434e
commit 82b076a
Show file tree

Hide file tree

Showing 18 changed files with 337 additions and 55 deletions.
diff --git a/app/pom.xml b/app/pom.xml
@@ -136,7 +136,6 @@
       <groupId>io.quarkus</groupId>
       <artifactId>quarkus-logging-json</artifactId>
     </dependency>
-
     <dependency>
       <groupId>io.quarkus</groupId>
       <artifactId>quarkus-smallrye-fault-tolerance</artifactId>
@@ -160,21 +159,23 @@
       <artifactId>quarkus-jdbc-mssql</artifactId>
     </dependency>
 
+    <!-- Third Party Libraries -->
+    <dependency>
+      <groupId>io.strimzi</groupId>
+      <artifactId>kafka-oauth-client</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jgit</groupId>
       <artifactId>org.eclipse.jgit</artifactId>
     </dependency>
-
     <dependency>
-      <groupId>commons-io</groupId>
-      <artifactId>commons-io</artifactId>
+      <groupId>org.semver4j</groupId>
+      <artifactId>semver4j</artifactId>
     </dependency>
     <dependency>
-      <groupId>io.strimzi</groupId>
-      <artifactId>kafka-oauth-client</artifactId>
+      <groupId>commons-io</groupId>
+      <artifactId>commons-io</artifactId>
     </dependency>
-
-    <!-- Third Party Libraries -->
     <dependency>
       <groupId>commons-codec</groupId>
       <artifactId>commons-codec</artifactId>
@@ -198,13 +199,11 @@
         </exclusion>
       </exclusions>
     </dependency>
-
     <dependency>
       <groupId>org.yaml</groupId>
       <artifactId>snakeyaml</artifactId>
       <version>${snakeyaml.version}</version>
     </dependency>
-
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>

diff --git a/app/src/main/java/io/apicurio/registry/semver/SemVerConfigProperties.java b/app/src/main/java/io/apicurio/registry/semver/SemVerConfigProperties.java
@@ -0,0 +1,28 @@
+package io.apicurio.registry.semver;
+
+import io.apicurio.common.apps.config.Dynamic;
+import io.apicurio.common.apps.config.Info;
+import jakarta.inject.Singleton;
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+
+import java.util.function.Supplier;
+
+@Singleton
+public class SemVerConfigProperties {
+
+    @Dynamic(label = "Ensure all version numbers are 'semver' compatible", description = "When enabled, validate that all artifact versions conform to Semantic Versioning 2 format (https://semver.org).")
+    @ConfigProperty(name = "apicurio.semver.validation.enabled", defaultValue = "false")
+    @Info(category = "semver", description = "Validate that all artifact versions conform to Semantic Versioning 2 format (https://semver.org).", availableSince = "3.0.0")
+    public Supplier<Boolean> validationEnabled;
+
+    @Dynamic(label = "Automatically create semver branches", description = "When enabled, automatically create or update branches for major ('A.x') and minor ('A.B.x') artifact versions.")
+    @ConfigProperty(name = "apicurio.semver.branching.enabled", defaultValue = "false")
+    @Info(category = "semver", description = "Automatically create or update branches for major ('A.x') and minor ('A.B.x') artifact versions.", availableSince = "3.0.0")
+    public Supplier<Boolean> branchingEnabled;
+
+    @Dynamic(label = "Coerce invalid semver versions", description = "When enabled and automatically creating semver branches, invalid versions will be coerced to Semantic Versioning 2 format (https://semver.org) if possible.", requires = "apicurio.semver.branching.enabled=true")
+    @ConfigProperty(name = "apicurio.semver.branching.coerce", defaultValue = "false")
+    @Info(category = "semver", description = "If true, invalid versions will be coerced to Semantic Versioning 2 format (https://semver.org) if possible.", availableSince = "3.0.0")
+    public Supplier<Boolean> coerceInvalidVersions;
+
+}
diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractHandleFactory.java
@@ -32,11 +32,12 @@ protected void initialize(AgroalDataSource dataSource, String dataSourceId, Logg
     public <R, X extends Exception> R withHandle(HandleCallback<R, X> callback) throws X {
         LocalState state = state();
         try {
-            // Create a new handle, or throw if one already exists (only one handle allowed at a time)
+            // Create a new handle if necessary. Increment the "level" if a handle already exists.
             if (state.handle == null) {
                 state.handle = new HandleImpl(dataSource.getConnection());
+                state.level = 0;
             } else {
-                throw new RegistryStorageException("Attempt to acquire a nested DB Handle.");
+                state.level++;
             }
 
             // Invoke the callback with the handle. This will either return a value (success)
@@ -54,32 +55,39 @@ public <R, X extends Exception> R withHandle(HandleCallback<R, X> callback) thro
             }
             throw e;
         } finally {
-            // Commit or rollback the transaction
-            try {
-                if (state.handle != null) {
-                    if (state.handle.isRollback()) {
-                        log.trace("Rollback: {} #{}", state.handle.getConnection(),
-                                state.handle.getConnection().hashCode());
-                        state.handle.getConnection().rollback();
-                    } else {
-                        log.trace("Commit: {} #{}", state.handle.getConnection(),
-                                state.handle.getConnection().hashCode());
-                        state().handle.getConnection().commit();
+            if (state.level > 0) {
+                log.trace("Exiting nested call (level {}): {} #{}", state().level,
+                        state().handle.getConnection(), state().handle.getConnection().hashCode());
+                state.level--;
+            } else {
+                // Commit or rollback the transaction
+                try {
+                    if (state.handle != null) {
+                        if (state.handle.isRollback()) {
+                            log.trace("Rollback: {} #{}", state.handle.getConnection(),
+                                    state.handle.getConnection().hashCode());
+                            state.handle.getConnection().rollback();
+                        } else {
+                            log.trace("Commit: {} #{}", state.handle.getConnection(),
+                                    state.handle.getConnection().hashCode());
+                            state().handle.getConnection().commit();
+                        }
                     }
+                } catch (Exception e) {
+                    log.error("Could not release database connection/transaction", e);
                 }
-            } catch (Exception e) {
-                log.error("Could not release database connection/transaction", e);
-            }
 
-            // Close the connection
-            try {
-                if (state.handle != null) {
-                    state.handle.close();
-                    state.handle = null;
+                // Close the connection
+                try {
+                    if (state.handle != null) {
+                        state.handle.close();
+                        state.handle = null;
+                        state.level = 0;
+                    }
+                } catch (Exception ex) {
+                    // Nothing we can do
+                    log.error("Could not close a database connection.", ex);
                 }
-            } catch (Exception ex) {
-                // Nothing we can do
-                log.error("Could not close a database connection.", ex);
             }
         }
     }
@@ -109,5 +117,6 @@ private LocalState state() {
 
     private static class LocalState {
         HandleImpl handle;
+        int level = 0;
     }
 }
diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java
@@ -11,6 +11,7 @@
 import io.apicurio.registry.model.GA;
 import io.apicurio.registry.model.GAV;
 import io.apicurio.registry.model.VersionId;
+import io.apicurio.registry.semver.SemVerConfigProperties;
 import io.apicurio.registry.storage.RegistryStorage;
 import io.apicurio.registry.storage.StorageBehaviorProperties;
 import io.apicurio.registry.storage.StorageEvent;
@@ -113,9 +114,11 @@
 import io.quarkus.security.identity.SecurityIdentity;
 import jakarta.enterprise.event.Event;
 import jakarta.inject.Inject;
+import jakarta.validation.ValidationException;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
 import org.eclipse.microprofile.config.inject.ConfigProperty;
+import org.semver4j.Semver;
 import org.slf4j.Logger;
 
 import java.sql.ResultSet;
@@ -187,6 +190,9 @@ public abstract class AbstractSqlRegistryStorage implements RegistryStorage {
     @Inject
     RegistryStorageContentUtils utils;
 
+    @Inject
+    SemVerConfigProperties semVerConfigProps;
+
     protected SqlStatements sqlStatements() {
         return sqlStatements;
     }
@@ -554,7 +560,6 @@ private ArtifactVersionMetaDataDto createArtifactVersionRaw(Handle handle, boole
                     .bind(12, contentId).execute();
 
             gav = new GAV(groupId, artifactId, finalVersion1);
-            createOrUpdateBranchRaw(handle, gav, BranchId.LATEST, true);
         } else {
             handle.createUpdate(sqlStatements.insertVersion(false)).bind(0, globalId)
                     .bind(1, normalizeGroupId(groupId)).bind(2, artifactId).bind(3, version)
@@ -571,7 +576,6 @@ private ArtifactVersionMetaDataDto createArtifactVersionRaw(Handle handle, boole
             }
 
             gav = getGAVByGlobalId(handle, globalId);
-            createOrUpdateBranchRaw(handle, gav, BranchId.LATEST, true);
         }
 
         // Insert labels into the "version_labels" table
@@ -583,6 +587,10 @@ private ArtifactVersionMetaDataDto createArtifactVersionRaw(Handle handle, boole
             });
         }
 
+        // Update system generated branches
+        createOrUpdateBranchRaw(handle, gav, BranchId.LATEST, true);
+        createOrUpdateSemverBranches(handle, gav);
+
         // Create any user defined branches
         if (branches != null && !branches.isEmpty()) {
             branches.forEach(branch -> {
@@ -595,6 +603,54 @@ private ArtifactVersionMetaDataDto createArtifactVersionRaw(Handle handle, boole
                 .map(ArtifactVersionMetaDataDtoMapper.instance).one();
     }
 
+    /**
+     * If SemVer support is enabled, create (or update) the automatic system generated semantic versioning
+     * branches.
+     * 
+     * @param handle
+     * @param gav
+     */
+    private void createOrUpdateSemverBranches(Handle handle, GAV gav) {
+        boolean validationEnabled = semVerConfigProps.validationEnabled.get();
+        boolean branchingEnabled = semVerConfigProps.branchingEnabled.get();
+        boolean coerceInvalidVersions = semVerConfigProps.coerceInvalidVersions.get();
+
+        // Validate the version if validation is enabled.
+        if (validationEnabled) {
+            Semver semver = Semver.parse(gav.getRawVersionId());
+            if (semver == null) {
+                throw new ValidationException("Version '" + gav.getRawVersionId()
+                        + "' does not conform to Semantic Versioning 2 format.");
+            }
+        }
+
+        // Create branches if branching is enabled
+        if (!branchingEnabled) {
+            return;
+        }
+
+        Semver semver = null;
+        if (coerceInvalidVersions) {
+            semver = Semver.coerce(gav.getRawVersionId());
+            if (semver == null) {
+                throw new ValidationException("Version '" + gav.getRawVersionId()
+                        + "' cannot be coerced to Semantic Versioning 2 format.");
+            }
+        } else {
+            semver = Semver.parse(gav.getRawVersionId());
+            if (semver == null) {
+                throw new ValidationException("Version '" + gav.getRawVersionId()
+                        + "' does not conform to Semantic Versioning 2 format.");
+            }
+        }
+        if (semver == null) {
+            throw new UnreachableCodeException("Unexpectedly reached unreachable code!");
+        }
+        createOrUpdateBranchRaw(handle, gav, new BranchId(semver.getMajor() + ".x"), true);
+        createOrUpdateBranchRaw(handle, gav, new BranchId(semver.getMajor() + "." + semver.getMinor() + ".x"),
+                true);
+    }
+
     /**
      * Store the content in the database and return the content ID of the new row. If the content already
      * exists, just return the content ID of the existing row.
@@ -3031,6 +3087,11 @@ public BranchMetaDataDto createBranch(GA ga, BranchId branchId, String descripti
 
     @Override
     public void updateBranchMetaData(GA ga, BranchId branchId, EditableBranchMetaDataDto dto) {
+        BranchMetaDataDto bmd = getBranchMetaData(ga, branchId);
+        if (bmd.isSystemDefined()) {
+            throw new NotAllowedException("System generated branches cannot be modified.");
+        }
+
         String modifiedBy = securityIdentity.getPrincipal().getName();
         Date modifiedOn = new Date();
         log.debug("Updating metadata for branch {} of {}/{}.", branchId, ga.getRawGroupIdWithNull(),
@@ -3220,6 +3281,11 @@ public VersionSearchResultsDto getBranchVersions(GA ga, BranchId branchId, int o
 
     @Override
     public void appendVersionToBranch(GA ga, BranchId branchId, VersionId version) {
+        BranchMetaDataDto bmd = getBranchMetaData(ga, branchId);
+        if (bmd.isSystemDefined()) {
+            throw new NotAllowedException("System generated branches cannot be modified.");
+        }
+
         try {
             handles.withHandle(handle -> {
                 appendVersionToBranchRaw(handle, ga, branchId, version);
@@ -3257,6 +3323,11 @@ private void appendVersionToBranchRaw(Handle handle, GA ga, BranchId branchId, V
 
     @Override
     public void replaceBranchVersions(GA ga, BranchId branchId, List<VersionId> versions) {
+        BranchMetaDataDto bmd = getBranchMetaData(ga, branchId);
+        if (bmd.isSystemDefined()) {
+            throw new NotAllowedException("System generated branches cannot be modified.");
+        }
+
         handles.withHandle(handle -> {
             // Delete all previous versions.
             handle.createUpdate(sqlStatements.deleteBranchVersions()).bind(0, ga.getRawGroupId())
@@ -3341,8 +3412,9 @@ private GAV getGAVByGlobalId(Handle handle, long globalId) {
 
     @Override
     public void deleteBranch(GA ga, BranchId branchId) {
-        if (BranchId.LATEST.equals(branchId)) {
-            throw new NotAllowedException("Artifact branch 'latest' cannot be deleted.");
+        BranchMetaDataDto bmd = getBranchMetaData(ga, branchId);
+        if (bmd.isSystemDefined()) {
+            throw new NotAllowedException("System generated branches cannot be deleted.");
         }
 
         handles.withHandleNoException(handle -> {

diff --git a/app/src/main/resources/application.properties b/app/src/main/resources/application.properties
@@ -127,7 +127,9 @@ apicurio.authn.basic-client-credentials.enabled.dynamic.allow=${apicurio.config.
 apicurio.rest.deletion.group.enabled.dynamic.allow=${apicurio.config.dynamic.allow-all}
 apicurio.rest.deletion.artifact.enabled.dynamic.allow=${apicurio.config.dynamic.allow-all}
 apicurio.rest.deletion.artifactVersion.enabled.dynamic.allow=${apicurio.config.dynamic.allow-all}
-
+apicurio.semver.validation.enabled.dynamic.allow=${apicurio.config.dynamic.allow-all}
+apicurio.semver.branching.enabled.dynamic.allow=${apicurio.config.dynamic.allow-all}
+apicurio.semver.branching.coerce.dynamic.allow=${apicurio.config.dynamic.allow-all}
 
 # Error
 apicurio.api.errors.include-stack-in-response=false

diff --git a/app/src/test/java/io/apicurio/registry/noprofile/rest/v3/BranchesTest.java b/app/src/test/java/io/apicurio/registry/noprofile/rest/v3/BranchesTest.java
@@ -7,6 +7,7 @@
 import io.apicurio.registry.rest.client.models.CreateBranch;
 import io.apicurio.registry.rest.client.models.CreateVersion;
 import io.apicurio.registry.rest.client.models.EditableBranchMetaData;
+import io.apicurio.registry.rest.client.models.Error;
 import io.apicurio.registry.rest.client.models.ReplaceBranchVersions;
 import io.apicurio.registry.rest.client.models.VersionMetaData;
 import io.apicurio.registry.rest.client.models.VersionSearchResults;
@@ -39,6 +40,13 @@ public void testLatestBranch() throws Exception {
         VersionSearchResults versions = clientV3.groups().byGroupId(groupId).artifacts()
                 .byArtifactId(artifactId).branches().byBranchId("latest").versions().get();
         Assertions.assertEquals(2, versions.getCount());
+
+        // Not allowed to delete the latest branch.
+        var error = Assertions.assertThrows(Error.class, () -> {
+            clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).branches()
+                    .byBranchId("latest").delete();
+        });
+        Assertions.assertEquals("System generated branches cannot be deleted.", error.getMessageEscaped());
     }
 
     @Test