Skip to content

Commit

Permalink
refactor: making enablement of artifact version delete dynamic and ad…
Browse files Browse the repository at this point in the history
…ding 405 response
  • Loading branch information
Derick Rentz authored and jsenko committed Mar 6, 2023
1 parent 1d6b9b8 commit 48f6c44
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 90 deletions.
9 changes: 6 additions & 3 deletions app/src/main/java/io/apicurio/registry/rest/RestConfig.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package io.apicurio.registry.rest;

import io.apicurio.common.apps.config.Dynamic;
import org.eclipse.microprofile.config.inject.ConfigProperty;

import io.apicurio.common.apps.config.Info;

import javax.inject.Singleton;
import java.util.function.Supplier;

@Singleton
public class RestConfig {
Expand All @@ -17,16 +19,17 @@ public class RestConfig {
@Info(category = "rest", description = "Skip SSL validation when downloading artifacts from URL", availableSince = "2.2.6-SNAPSHOT")
boolean downloadSkipSSLValidation;

@ConfigProperty(name = "registry.rest.artifact.deletion.enabled", defaultValue = "true")
@Dynamic(label = "Delete artifact version", description = "When selected, users are permitted to delete artifact versions.")
@ConfigProperty(name = "registry.rest.artifact.deletion.enabled", defaultValue = "false")
@Info(category = "rest", description = "Enables artifact version deletion", availableSince = "2.4.2-SNAPSHOT")
boolean artifactVersionDeletionEnabled;
Supplier<Boolean> artifactVersionDeletionEnabled;

public int getDownloadMaxSize() { return this.downloadMaxSize; }

public boolean getDownloadSkipSSLValidation() { return this.downloadSkipSSLValidation; }

public boolean isArtifactVersionDeletionEnabled() {
return artifactVersionDeletionEnabled;
return artifactVersionDeletionEnabled.get();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
import javax.interceptor.Interceptors;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.NotAllowedException;
import javax.ws.rs.client.Client;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
Expand Down Expand Up @@ -576,9 +576,10 @@ public Response getArtifactVersion(String groupId, String artifactId, String ver
* @see io.apicurio.registry.rest.v2.GroupsResource#deleteArtifactVersion(java.lang.String, java.lang.String, java.lang.String)
*/
@Override
@Authorized(style=AuthorizedStyle.GroupAndArtifact, level=AuthorizedLevel.Write)
public void deleteArtifactVersion(String groupId, String artifactId, String version) {
if (!restConfig.isArtifactVersionDeletionEnabled()) {
throw new NotFoundException("Artifact version deletion is not enabled.");
throw new NotAllowedException("Artifact version deletion operation is not enabled.");
}

requireParameter("groupId", groupId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1003,13 +1003,16 @@
"404": {
"$ref": "#/components/responses/NotFound"
},
"405": {
"$ref": "#/components/responses/MethodNotAllowed"
},
"500": {
"$ref": "#/components/responses/ServerError"
}
},
"operationId": "deleteArtifactVersion",
"summary": "Delete artifact version",
"description": "Deletes a single version of the artifact. Both the `artifactId` and the unique `version`\nare needed. If this is the only version of the artifact, this operation is the same as \ndeleting the entire artifact.\n\nThis operation can fail for the following reasons:\n\n* No artifact with this `artifactId` exists (HTTP error `404`)\n* No version with this `version` exists (HTTP error `404`)\n* A server error occurred (HTTP error `500`)\n"
"description": "Deletes a single version of the artifact. Parameters `groupId`, `artifactId` and the unique `version`\nare needed. If this is the only version of the artifact, this operation is the same as \ndeleting the entire artifact.\n\nThis feature is disabled by default and it's discouraged for normal usage. To enable it, set the `registry.rest.artifact.deletion.enabled` property to true. This operation can fail for the following reasons:\n\n* No artifact with this `artifactId` exists (HTTP error `404`)\n* No version with this `version` exists (HTTP error `404`)\n * Feature is disabled (HTTP error `405`)\n * A server error occurred (HTTP error `500`)\n"
},
"parameters": [
{
Expand Down Expand Up @@ -4256,6 +4259,24 @@
},
"description": "Common response for all operations that can return a `404` error."
},
"MethodNotAllowed": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/Error"
},
"examples": {
"MethodNotAllowedExample": {
"value": {
"error_code": 405,
"message": "Method is currently not supported or disabled."
}
}
}
}
},
"description": "Common response for all operations that can fail due to method not allowed or disabled."
},
"ServerError": {
"content": {
"application/json": {
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,4 @@ registry.rest.artifact.download.maxSize=1000000
registry.rest.artifact.download.skipSSLValidation=false

# Artifact version deletion
registry.rest.artifact.deletion.enabled=true
registry.rest.artifact.deletion.enabled=false
Original file line number Diff line number Diff line change
Expand Up @@ -636,88 +636,6 @@ public void testGetArtifactMetaDataByContent() throws Exception {
.statusCode(400);

}

@Test
public void testDeleteArtifactVersion() throws Exception {
String artifactContent = resourceToString("openapi-empty.json");

// Create an artifact
createArtifact("testDeleteArtifactVersion/EmptyAPI", ArtifactType.OPENAPI, artifactContent);

// Update the artifact 5 times
List<Integer> versions = new ArrayList<>();
for (int idx = 0; idx < 5; idx++) {
Integer version =
given()
.when()
.contentType(CT_JSON)
.header("X-Registry-ArtifactType", ArtifactType.OPENAPI)
.pathParam("artifactId", "testDeleteArtifactVersion/EmptyAPI")
.body(artifactContent.replace("Empty API", "Empty API (Update " + idx + ")"))
.put("/registry/v1/artifacts/{artifactId}")
.then()
.statusCode(200)
.body("id", equalTo("testDeleteArtifactVersion/EmptyAPI"))
.body("type", equalTo(ArtifactType.OPENAPI))
.extract().body().path("version");

versions.add(version);
}

// Delete odd artifact versions
for (int idx = 0; idx < 5; idx++) {
if (idx % 2 == 0) {
continue;
}

Integer version = versions.get(idx);
given()
.when()
.pathParam("artifactId", "testDeleteArtifactVersion/EmptyAPI")
.pathParam("version", version)
.delete("/registry/v1/artifacts/{artifactId}/versions/{version}")
.then()
.statusCode(204);
}

// Check that the correct versions were deleted
for (int idx = 0; idx < 5; idx++) {
Integer version = versions.get(idx);
int expectedCode;
// Even indexes are still there, odd were deleted
if (idx % 2 == 0) {
expectedCode = 200;
} else {
expectedCode = 404;
}
given()
.when()
.pathParam("artifactId", "testDeleteArtifactVersion/EmptyAPI")
.pathParam("version", version)
.get("/registry/v1/artifacts/{artifactId}/versions/{version}")
.then()
.statusCode(expectedCode);
}

// Now try to delete a version that doesn't exist.
given()
.when()
.pathParam("artifactId", "testDeleteArtifactVersion/EmptyAPI")
.pathParam("version", 12345)
.get("/registry/v1/artifacts/{artifactId}/versions/{version}")
.then()
.statusCode(404);

// Try to delete a version of an artifact where the artifact doesn't exist
given()
.when()
.pathParam("artifactId", "testGetArtifactVersion/MissingAPI")
.pathParam("version", 1)
.get("/registry/v1/artifacts/{artifactId}/versions/{version}")
.then()
.statusCode(404);
}

@Test
public void testArtifactRules() throws Exception {
String artifactContent = resourceToString("openapi-empty.json");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,19 @@ public void testDeleteArtifactVersion() throws Exception {
.body("openapi", equalTo("3.0.2"))
.body("info.title", equalTo("Empty API"));

// Try to delete artifact version 1, but feature is disabled and this should fail
given()
.when()
.pathParam("groupId", GROUP)
.pathParam("artifactId", "testDeleteArtifactVersion/EmptyAPI")
.pathParam("version", "1")
.delete("/registry/v2/groups/{groupId}/artifacts/{artifactId}/versions/{version}")
.then()
.statusCode(405);

// Enable artifact version delete feature
clientV2.setConfigProperty("registry.rest.artifact.deletion.enabled", String.valueOf(true));

// Delete the artifact version 1
given()
.when()
Expand Down
23 changes: 22 additions & 1 deletion common/src/main/resources/META-INF/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1003,13 +1003,16 @@
"404": {
"$ref": "#/components/responses/NotFound"
},
"405": {
"$ref": "#/components/responses/MethodNotAllowed"
},
"500": {
"$ref": "#/components/responses/ServerError"
}
},
"operationId": "deleteArtifactVersion",
"summary": "Delete artifact version",
"description": "Deletes a single version of the artifact. Both the `artifactId` and the unique `version`\nare needed. If this is the only version of the artifact, this operation is the same as \ndeleting the entire artifact.\n\nThis operation can fail for the following reasons:\n\n* No artifact with this `artifactId` exists (HTTP error `404`)\n* No version with this `version` exists (HTTP error `404`)\n* A server error occurred (HTTP error `500`)\n"
"description": "Deletes a single version of the artifact. Parameters `groupId`, `artifactId` and the unique `version`\nare needed. If this is the only version of the artifact, this operation is the same as \ndeleting the entire artifact.\n\nThis feature is disabled by default and it's discouraged for normal usage. To enable it, set the `registry.rest.artifact.deletion.enabled` property to true. This operation can fail for the following reasons:\n\n* No artifact with this `artifactId` exists (HTTP error `404`)\n* No version with this `version` exists (HTTP error `404`)\n * Feature is disabled (HTTP error `405`)\n * A server error occurred (HTTP error `500`)\n"
},
"parameters": [
{
Expand Down Expand Up @@ -4256,6 +4259,24 @@
},
"description": "Common response for all operations that can return a `404` error."
},
"MethodNotAllowed": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/Error"
},
"examples": {
"MethodNotAllowedExample": {
"value": {
"error_code": 405,
"message": "Method is currently not supported or disabled."
}
}
}
}
},
"description": "Common response for all operations that can fail due to method not allowed or disabled."
},
"ServerError": {
"content": {
"application/json": {
Expand Down

0 comments on commit 48f6c44

Please sign in to comment.