From 14c1f8736f461cb42692afaa8129e952fc7173b0 Mon Sep 17 00:00:00 2001 From: renjingxiao Date: Mon, 4 Sep 2023 13:34:12 +0100 Subject: [PATCH] modified state change checks in ArtifactStateExt and AbstractSqlRegistryStorage --- .../registry/storage/ArtifactStateExt.java | 14 +-- .../impl/sql/AbstractSqlRegistryStorage.java | 22 ++--- .../noprofile/rest/v2/GroupsResourceTest.java | 89 +++++++++++++++++++ 3 files changed, 105 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/io/apicurio/registry/storage/ArtifactStateExt.java b/app/src/main/java/io/apicurio/registry/storage/ArtifactStateExt.java index 317dff9ad3..17422a5884 100644 --- a/app/src/main/java/io/apicurio/registry/storage/ArtifactStateExt.java +++ b/app/src/main/java/io/apicurio/registry/storage/ArtifactStateExt.java @@ -66,14 +66,16 @@ public void logIfDeprecated(String groupId, Object artifactId, Object version, A } public void applyState(Consumer consumer, ArtifactState previousState, ArtifactState newState) { - if (previousState != null) { - if (canTransition(previousState, newState)) { - consumer.accept(newState); + if ( previousState != newState) { + if (previousState != null) { + if (canTransition(previousState, newState)) { + consumer.accept(newState); + } else { + throw new InvalidArtifactStateException(previousState, newState); + } } else { - throw new InvalidArtifactStateException(previousState, newState); + consumer.accept(newState); } - } else { - consumer.accept(newState); } } } 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 index 79142279cb..fb58d1b342 100644 --- 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 @@ -451,21 +451,15 @@ public void updateArtifactState(String groupId, String artifactId, String versio /** * IMPORTANT: Private methods can't be @Transactional. Callers MUST have started a transaction. */ - private void updateArtifactVersionStateRaw(long globalId, ArtifactState oldState, ArtifactState newState) - throws VersionNotFoundException { + private void updateArtifactVersionStateRaw(long globalId, ArtifactState oldState, ArtifactState newState) { handles.withHandleNoException(handle -> { - if (oldState != newState) { - artifactStateEx.applyState(s -> { - int rowCount = handle.createUpdate(sqlStatements.updateArtifactVersionState()) - .bind(0, s.name()) - .bind(1, tenantContext.tenantId()) - .bind(2, globalId) - .execute(); - if (rowCount == 0) { - throw new VersionNotFoundException(globalId); - } - }, oldState, newState); - } + artifactStateEx.applyState(s -> { + handle.createUpdate(sqlStatements.updateArtifactVersionState()) + .bind(0, s.name()) + .bind(1, tenantContext.tenantId()) + .bind(2, globalId) + .execute(); + }, oldState, newState); return null; }); } diff --git a/app/src/test/java/io/apicurio/registry/noprofile/rest/v2/GroupsResourceTest.java b/app/src/test/java/io/apicurio/registry/noprofile/rest/v2/GroupsResourceTest.java index 1943480410..8d51022527 100644 --- a/app/src/test/java/io/apicurio/registry/noprofile/rest/v2/GroupsResourceTest.java +++ b/app/src/test/java/io/apicurio/registry/noprofile/rest/v2/GroupsResourceTest.java @@ -62,6 +62,7 @@ import io.apicurio.registry.AbstractResourceTestBase; import io.apicurio.registry.rest.client.exception.RuleViolationException; import io.apicurio.registry.rest.v2.beans.ArtifactOwner; +import io.apicurio.registry.types.ArtifactState; import io.apicurio.registry.rest.v2.beans.ArtifactMetaData; import io.apicurio.registry.rest.v2.beans.ArtifactReference; import io.apicurio.registry.rest.v2.beans.Comment; @@ -69,6 +70,7 @@ import io.apicurio.registry.rest.v2.beans.IfExists; import io.apicurio.registry.rest.v2.beans.NewComment; import io.apicurio.registry.rest.v2.beans.Rule; +import io.apicurio.registry.rest.v2.beans.UpdateState; import io.apicurio.registry.rest.v2.beans.VersionMetaData; import io.apicurio.registry.rules.compatibility.jsonschema.diff.DiffType; import io.apicurio.registry.storage.impl.sql.SqlUtil; @@ -655,6 +657,93 @@ public void testUpdateArtifact() throws Exception { } + @Test + public void testUpdateArtifactState() throws Exception { + String oaiArtifactContent = resourceToString("openapi-empty.json"); + createArtifact("testUpdateArtifactState", "testUpdateArtifactState/EmptyAPI/1",ArtifactType.OPENAPI, oaiArtifactContent); + + UpdateState updateState = new UpdateState(); + updateState.setState(ArtifactState.DEPRECATED); + + // Update the artifact state to DEPRECATED. + given() + .when() + .contentType(CT_JSON) + .pathParam("groupId", "testUpdateArtifactState") + .pathParam("artifactId", "testUpdateArtifactState/EmptyAPI/1") + .body(updateState) + .put("/registry/v2/groups/{groupId}/artifacts/{artifactId}/state") + .then() + .statusCode(204); + + // Update the artifact state to DEPRECATED again. + given() + .when() + .contentType(CT_JSON) + .pathParam("groupId", "testUpdateArtifactState") + .pathParam("artifactId", "testUpdateArtifactState/EmptyAPI/1") + .body(updateState) + .put("/registry/v2/groups/{groupId}/artifacts/{artifactId}/state") + .then() + .statusCode(204); + + // Send a GET request to check if the artifact state is DEPRECATED. + given() + .when() + .contentType(CT_JSON) + .pathParam("groupId", "testUpdateArtifactState") + .pathParam("artifactId", "testUpdateArtifactState/EmptyAPI/1") + .get("/registry/v2/groups/{groupId}/artifacts/{artifactId}") + .then() + .statusCode(200) + .header("X-Registry-Deprecated", "true"); + } + + @Test + public void testUpdateArtifactVersionState() throws Exception { + String oaiArtifactContent = resourceToString("openapi-empty.json"); + createArtifact("testUpdateArtifactVersionState", "testUpdateArtifactVersionState/EmptyAPI",ArtifactType.OPENAPI, oaiArtifactContent); + + UpdateState updateState = new UpdateState(); + updateState.setState(ArtifactState.DEPRECATED); + + // Update the artifact state to DEPRECATED. + given() + .when() + .contentType(CT_JSON) + .pathParam("groupId", "testUpdateArtifactVersionState") + .pathParam("artifactId", "testUpdateArtifactVersionState/EmptyAPI") + .pathParam("versionId", "1") + .body(updateState) + .put("/registry/v2/groups/{groupId}/artifacts/{artifactId}/versions/{versionId}/state") + .then() + .statusCode(204); + + // Update the artifact state to DEPRECATED again. + given() + .when() + .contentType(CT_JSON) + .pathParam("groupId", "testUpdateArtifactVersionState") + .pathParam("artifactId", "testUpdateArtifactVersionState/EmptyAPI") + .pathParam("versionId", "1") + .body(updateState) + .put("/registry/v2/groups/{groupId}/artifacts/{artifactId}/versions/{versionId}/state") + .then() + .statusCode(204); + + // Send a GET request to check if the artifact state is DEPRECATED. + given() + .when() + .contentType(CT_JSON) + .pathParam("groupId", "testUpdateArtifactVersionState") + .pathParam("artifactId", "testUpdateArtifactVersionState/EmptyAPI") + .pathParam("versionId", "1") + .get("/registry/v2/groups/{groupId}/artifacts/{artifactId}/versions/{versionId}") + .then() + .statusCode(200) + .header("X-Registry-Deprecated", "true"); + } + @Test @DisabledIfEnvironmentVariable(named = CURRENT_ENV, matches = CURRENT_ENV_MAS_REGEX) @DisabledOnOs(OS.WINDOWS)