Added new "drafts" system branch

Apicurio · Oct 11, 2024 · db9adeb · db9adeb
1 parent b2ca216
commit db9adeb
Show file tree

Hide file tree

Showing 7 changed files with 127 additions and 17 deletions.
diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/kafkasql/serde/KafkaSqlMessageIndex.java b/app/src/main/java/io/apicurio/registry/storage/impl/kafkasql/serde/KafkaSqlMessageIndex.java
@@ -53,7 +53,9 @@
 import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateArtifactMetaData3Message;
 import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateArtifactRule4Message;
 import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateArtifactVersionComment5Message;
+import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateArtifactVersionContent5Message;
 import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateArtifactVersionMetaData4Message;
+import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateArtifactVersionState5Message;
 import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateBranchMetaData3Message;
 import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateContentCanonicalHash3Message;
 import io.apicurio.registry.storage.impl.kafkasql.messages.UpdateGlobalRule2Message;
@@ -106,6 +108,7 @@ private static void indexMessageClasses(Class<? extends KafkaSqlMessage>... mcla
                 UpdateArtifactVersionMetaData4Message.class, UpdateBranchMetaData3Message.class,
                 UpdateContentCanonicalHash3Message.class, UpdateGlobalRule2Message.class,
                 UpdateGroupMetaData2Message.class, UpdateRoleMapping2Message.class,
+                UpdateArtifactVersionState5Message.class, UpdateArtifactVersionContent5Message.class,
                 UpdateGroupRule3Message.class, DeleteGroupRule2Message.class, DeleteGroupRules1Message.class,
                 ImportGroupRule1Message.class, ExecuteSqlStatement1Message.class);
     }

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
@@ -647,8 +647,12 @@ private ArtifactVersionMetaDataDto createArtifactVersionRaw(Handle handle, boole
         }
 
         // Update system generated branches
-        createOrUpdateBranchRaw(handle, gav, BranchId.LATEST, true);
-        createOrUpdateSemverBranchesRaw(handle, gav);
+        if (isDraft) {
+            createOrUpdateBranchRaw(handle, gav, BranchId.DRAFTS, true);
+        } else {
+            createOrUpdateBranchRaw(handle, gav, BranchId.LATEST, true);
+            createOrUpdateSemverBranchesRaw(handle, gav);
+        }
 
         // Create any user defined branches
         if (branches != null && !branches.isEmpty()) {
@@ -2046,20 +2050,29 @@ public void updateArtifactVersionState(String groupId, String artifactId, String
                 handle.setRollback(true);
             }
 
-            int rowCount = handle.createUpdate(sqlStatements.updateArtifactVersionStateByGAV())
-                    .bind(0, newState.name()).bind(1, normalizeGroupId(groupId)).bind(2, artifactId)
-                    .bind(3, version).execute();
-            if (rowCount == 0) {
-                throw new VersionNotFoundException(groupId, artifactId, version);
-            }
+            Optional<VersionState> res = handle
+                    .createQuery(sqlStatements.selectArtifactVersionStateForUpdate())
+                    .bind(0, normalizeGroupId(groupId)).bind(1, artifactId).bind(2, version)
+                    .map(VersionStateMapper.instance).findOne();
+            VersionState currentState = res
+                    .orElseThrow(() -> new VersionNotFoundException(groupId, artifactId, version));
+
+            handle.createUpdate(sqlStatements.updateArtifactVersionStateByGAV()).bind(0, newState.name())
+                    .bind(1, normalizeGroupId(groupId)).bind(2, artifactId).bind(3, version).execute();
+
             String modifiedBy = securityIdentity.getPrincipal().getName();
             Date modifiedOn = new Date();
+            handle.createUpdate(sqlStatements.updateArtifactVersionModifiedByOn()).bind(0, modifiedBy)
+                    .bind(1, modifiedOn).bind(2, normalizeGroupId(groupId)).bind(3, artifactId)
+                    .bind(4, version).execute();
 
-            rowCount = handle.createUpdate(sqlStatements.updateArtifactVersionModifiedByOn())
-                    .bind(0, modifiedBy).bind(1, modifiedOn).bind(2, normalizeGroupId(groupId))
-                    .bind(3, artifactId).bind(4, version).execute();
-            if (rowCount == 0) {
-                throw new VersionNotFoundException(groupId, artifactId, version);
+            // If transitioning from DRAFT state to something else, then we need to maintain
+            // the system branches.
+            if (currentState == VersionState.DRAFT) {
+                GAV gav = new GAV(groupId, artifactId, version);
+                createOrUpdateBranchRaw(handle, gav, BranchId.LATEST, true);
+                createOrUpdateSemverBranchesRaw(handle, gav);
+                removeVersionFromBranchRaw(handle, gav, BranchId.DRAFTS);
             }
 
             return null;
@@ -3633,6 +3646,19 @@ private void createOrUpdateBranchRaw(Handle handle, GAV gav, BranchId branchId,
         appendVersionToBranchRaw(handle, gav, branchId, gav.getVersionId());
     }
 
+    /**
+     * Removes a version from the given branch.
+     * 
+     * @param handle
+     * @param gav
+     * @param branchId
+     */
+    private void removeVersionFromBranchRaw(Handle handle, GAV gav, BranchId branchId) {
+        handle.createUpdate(sqlStatements.deleteVersionFromBranch()).bind(0, gav.getRawGroupIdWithNull())
+                .bind(1, gav.getRawArtifactId()).bind(2, branchId.getRawBranchId())
+                .bind(3, gav.getRawVersionId()).execute();
+    }
+
     private void updateBranchModifiedTimeRaw(Handle handle, GA ga, BranchId branchId) {
         String user = securityIdentity.getPrincipal().getName();
         Date now = new Date();

diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/CommonSqlStatements.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/CommonSqlStatements.java
@@ -182,6 +182,12 @@ public String selectArtifactVersionState() {
                 + "WHERE v.groupId = ? AND v.artifactId = ? AND v.version = ?";
     }
 
+    @Override
+    public String selectArtifactVersionStateForUpdate() {
+        return "SELECT v.state FROM versions v "
+                + "WHERE v.groupId = ? AND v.artifactId = ? AND v.version = ? FOR UPDATE";
+    }
+
     /**
      * @see io.apicurio.registry.storage.impl.sql.SqlStatements#selectArtifactVersionMetaDataByContentHash()
      */
@@ -1142,12 +1148,17 @@ public String appendBranchVersion() {
 
     @Override
     public String deleteBranchVersions() {
-        return "DELETE FROM branch_versions " + "WHERE groupId = ? AND artifactId = ? AND branchId = ?";
+        return "DELETE FROM branch_versions WHERE groupId = ? AND artifactId = ? AND branchId = ?";
+    }
+
+    @Override
+    public String deleteVersionFromBranch() {
+        return "DELETE FROM branch_versions WHERE groupId = ? AND artifactId = ? AND branchId = ? AND version = ?";
     }
 
     @Override
     public String deleteBranch() {
-        return "DELETE FROM branches " + "WHERE groupId = ? AND artifactId = ? AND branchId = ?";
+        return "DELETE FROM branches WHERE groupId = ? AND artifactId = ? AND branchId = ?";
     }
 
     @Override

diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/SQLServerSqlStatements.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/SQLServerSqlStatements.java
@@ -129,6 +129,12 @@ public String deleteAllOrphanedContent() {
         return "DELETE FROM content WHERE NOT EXISTS (SELECT 1 FROM versions v WHERE v.contentId = contentId )";
     }
 
+    @Override
+    public String selectArtifactVersionStateForUpdate() {
+        return "SELECT v.state FROM versions v WITH (UPDLOCK, ROWLOCK)"
+                + "WHERE v.groupId = ? AND v.artifactId = ? AND v.version = ?";
+    }
+
     @Override
     public String createDataSnapshot() {
         throw new IllegalStateException("Snapshot creation is not supported for Sqlserver storage");

diff --git a/app/src/main/java/io/apicurio/registry/storage/impl/sql/SqlStatements.java b/app/src/main/java/io/apicurio/registry/storage/impl/sql/SqlStatements.java
@@ -455,6 +455,11 @@ public interface SqlStatements {
      */
     public String selectArtifactVersionState();
 
+    /**
+     * A statement used to select the state of a version.
+     */
+    public String selectArtifactVersionStateForUpdate();
+
     /*
      * The next few statements support globalId and contentId management.
      */
@@ -631,11 +636,18 @@ public interface SqlStatements {
 
     public String deleteAllBranches();
 
+    public String deleteVersionFromBranch();
+
+    // ========== Snapshots ==========
+
     public String createDataSnapshot();
 
     public String restoreFromSnapshot();
 
-    String createOutboxEvent();
+    // ========== Events ==========
+
+    public String createOutboxEvent();
+
+    public String deleteOutboxEvent();
 
-    String deleteOutboxEvent();
 }
diff --git a/app/src/test/java/io/apicurio/registry/noprofile/rest/v3/DraftContentTest.java b/app/src/test/java/io/apicurio/registry/noprofile/rest/v3/DraftContentTest.java
@@ -324,4 +324,55 @@ public void testCreateInvalidDraftVersion() throws Exception {
         Assertions.assertEquals("Syntax violation for Avro artifact.", error.getTitle());
     }
 
+    @Test
+    public void testDraftVersionsWithBranches() throws Exception {
+        String content = resourceToString("openapi-empty.json");
+        String groupId = TestUtils.generateGroupId();
+        String artifactId = TestUtils.generateArtifactId();
+
+        // First version is ENABLED
+        CreateArtifact createArtifact = TestUtils.clientCreateArtifact(artifactId, ArtifactType.OPENAPI,
+                content, ContentTypes.APPLICATION_JSON);
+        createArtifact.getFirstVersion().setIsDraft(false);
+        createArtifact.getFirstVersion().setVersion("1.0.0");
+
+        clientV3.groups().byGroupId(groupId).artifacts().post(createArtifact);
+
+        VersionSearchResults latestBranch = clientV3.groups().byGroupId(groupId).artifacts()
+                .byArtifactId(artifactId).branches().byBranchId("latest").versions().get();
+        Assertions.assertEquals(1, latestBranch.getVersions().size());
+        ProblemDetails problemDetails = Assertions.assertThrows(ProblemDetails.class, () -> {
+            clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).branches()
+                    .byBranchId("drafts").versions().get();
+        });
+        Assertions.assertEquals("BranchNotFoundException", problemDetails.getName());
+
+        // Second version is DRAFT
+        CreateVersion createVersion = TestUtils.clientCreateVersion(content, ContentTypes.APPLICATION_JSON);
+        createVersion.setVersion("1.0.1");
+        createVersion.setIsDraft(true);
+        clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).versions()
+                .post(createVersion);
+
+        latestBranch = clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).branches()
+                .byBranchId("latest").versions().get();
+        Assertions.assertEquals(1, latestBranch.getVersions().size());
+        VersionSearchResults draftsBranch = clientV3.groups().byGroupId(groupId).artifacts()
+                .byArtifactId(artifactId).branches().byBranchId("drafts").versions().get();
+        Assertions.assertEquals(1, draftsBranch.getVersions().size());
+
+        // Transition draft content to enabled
+        WrappedVersionState enabled = new WrappedVersionState();
+        enabled.setState(VersionState.ENABLED);
+        clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).versions()
+                .byVersionExpression("1.0.1").state().put(enabled);
+
+        latestBranch = clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).branches()
+                .byBranchId("latest").versions().get();
+        Assertions.assertEquals(2, latestBranch.getVersions().size());
+        draftsBranch = clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).branches()
+                .byBranchId("drafts").versions().get();
+        Assertions.assertEquals(0, draftsBranch.getVersions().size());
+    }
+
 }
diff --git a/common/src/main/java/io/apicurio/registry/model/BranchId.java b/common/src/main/java/io/apicurio/registry/model/BranchId.java
@@ -17,6 +17,7 @@ public final class BranchId {
     private static final Pattern VALID_PATTERN = Pattern.compile("[a-zA-Z0-9._\\-+]{1,256}");
 
     public static final BranchId LATEST = new BranchId("latest");
+    public static final BranchId DRAFTS = new BranchId("drafts");
 
     private final String rawBranchId;