Skip to content

Commit

Permalink
modified state change checks in ArtifactStateExt and AbstractSqlRegis…
Browse files Browse the repository at this point in the history 
…tryStorage (Apicurio#3637)
  • Loading branch information
@renjingxiao @carlesarnal
renjingxiao authored and carlesarnal committed Nov 13, 2023
1 parent 811b68d commit ee6ad4e
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 29 deletions.
14 changes: 8 additions & 6 deletions app/src/main/java/io/apicurio/registry/storage/ArtifactStateExt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,16 @@ public void logIfDeprecated(String groupId, Object artifactId, Object version, A
}

public void applyState(Consumer<ArtifactState> 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);
}
}
}
45 changes: 22 additions & 23 deletions app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -532,24 +532,23 @@ public void updateArtifactState(String groupId, String artifactId, ArtifactState
public void updateArtifactState(String groupId, String artifactId, String version, ArtifactState state)
throws ArtifactNotFoundException, VersionNotFoundException, RegistryStorageException {
log.debug("Updating the state of artifact {} {}, version {} to {}", groupId, artifactId, version, state.name());
ArtifactVersionMetaDataDto dto = this.getArtifactVersionMetaData(groupId, artifactId, version);
var metadata = getArtifactVersionMetaData(groupId, artifactId, version);
updateArtifactVersionStateRaw(metadata.getGlobalId(), metadata.getState(), state);
}


/**
* IMPORTANT: Private methods can't be @Transactional. Callers MUST have started a transaction.
*/
private void updateArtifactVersionStateRaw(long globalId, ArtifactState oldState, ArtifactState newState) {
handles.withHandleNoException(handle -> {
long globalId = dto.getGlobalId();
ArtifactState oldState = dto.getState();
ArtifactState newState = state;
if (oldState != newState) {
artifactStateEx.applyState(s -> {
String sql = sqlStatements.updateArtifactVersionState();
int rowCount = handle.createUpdate(sql)
.bind(0, s.name())
.bind(1, tenantContext.tenantId())
.bind(2, globalId)
.execute();
if (rowCount == 0) {
throw new VersionNotFoundException(groupId, artifactId, dto.getVersion());
}
}, 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;
});
}
Expand Down Expand Up @@ -2262,7 +2261,7 @@ protected CommentDto createArtifactVersionComment(String groupId, String artifac
ArtifactVersionMetaDataDto avmdd = res.orElseThrow(() -> new VersionNotFoundException(groupId, artifactId, version));

String cid = String.valueOf(commentId.generate(handle));

sql = sqlStatements.insertComment();
handle.createUpdate(sql)
.bind(0, tenantContext.tenantId())
Expand Down Expand Up @@ -2292,7 +2291,7 @@ protected CommentDto createArtifactVersionComment(String groupId, String artifac
throw new RegistryStorageException(e);
}
}

/**
* @see io.apicurio.registry.storage.RegistryStorage#getArtifactVersionComments(java.lang.String, java.lang.String, java.lang.String)
*/
Expand All @@ -2319,7 +2318,7 @@ public List<CommentDto> getArtifactVersionComments(String groupId, String artifa
throw new RegistryStorageException(e);
}
}

/**
* @see io.apicurio.registry.storage.RegistryStorage#deleteArtifactVersionComment(java.lang.String, java.lang.String, java.lang.String, java.lang.String)
*/
Expand Down Expand Up @@ -2360,7 +2359,7 @@ public void deleteArtifactVersionComment(String groupId, String artifactId, Stri
throw new RegistryStorageException(e);
}
}

/**
* @see io.apicurio.registry.storage.RegistryStorage#updateArtifactVersionComment(java.lang.String, java.lang.String, java.lang.String, java.lang.String, java.lang.String)
*/
Expand Down Expand Up @@ -3468,7 +3467,7 @@ public List<ArtifactReferenceDto> getInboundArtifactReferences(String groupId, S
.list();
});
}

/**
* @see RegistryStorage#isArtifactExists(String, String)
*/
Expand Down Expand Up @@ -3854,7 +3853,7 @@ protected void importGroup(Handle handle, GroupEntity entity) {
log.warn("Failed to import group entity (likely it already exists).", e);
}
}

protected void importComment(Handle handle, CommentEntity entity) {
try {
String sql = sqlStatements.insertComment();
Expand Down
89 changes: 89 additions & 0 deletions app/src/test/java/io/apicurio/registry/noprofile/rest/v2/GroupsResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,15 @@

import io.apicurio.registry.AbstractResourceTestBase;
import io.apicurio.registry.rest.client.exception.RuleViolationException;
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;
import io.apicurio.registry.rest.v2.beans.EditableMetaData;
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;
Expand Down Expand Up @@ -601,6 +603,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)
Expand Down

0 comments on commit ee6ad4e

Please sign in to comment.