Skip to content

Commit 9277331

Browse files
EricWittmannEricWittmann
committed
Improvements/tweaks to DRAFT content
1 parent f9ba6e9 commit 9277331

File tree

16 files changed

+113
-20
lines changed

16 files changed

+113
-20
lines changed

‎app/src/main/java/io/apicurio/registry/rest/RestConfig.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,14 @@ public class RestConfig {
3030

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

36+
@Dynamic(label = "Update artifact version content", description = "When selected, users are permitted to update the content of artifact versions (only when in the DRAFT state).")
37+
@ConfigProperty(name = "apicurio.rest.mutability.artifact-version-content.enabled", defaultValue = "false")
38+
@Info(category = "rest", description = "Enables artifact version mutability", availableSince = "3.0.2")
39+
Supplier<Boolean> artifactVersionMutabilityEnabled;
40+
3641
public int getDownloadMaxSize() {
3742
return this.downloadMaxSize;
3843
}
@@ -53,4 +58,8 @@ public boolean isArtifactVersionDeletionEnabled() {
5358
return artifactVersionDeletionEnabled.get();
5459
}
5560

61+
public boolean isArtifactVersionMutabilityEnabled() {
62+
return artifactVersionMutabilityEnabled.get();
63+
}
64+
5665
}

‎app/src/main/java/io/apicurio/registry/rest/v3/GroupsResourceImpl.java

+6-5
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,10 @@ public void updateArtifactVersionContent(String groupId, String artifactId, Stri
538538
requireParameter("artifactId", artifactId);
539539
requireParameter("versionExpression", versionExpression);
540540

541-
// TODO check if DRAFT content is allowed (global config property)
541+
if (!restConfig.isArtifactVersionMutabilityEnabled()) {
542+
throw new NotAllowedException("Artifact version content update operation is not enabled.",
543+
HttpMethod.GET, (String[]) null);
544+
}
542545

543546
// Resolve the GAV info
544547
var gav = VersionExpressionParser.parse(new GA(groupId, artifactId), versionExpression,
@@ -727,7 +730,7 @@ public Comment addArtifactVersionComment(String groupId, String artifactId, Stri
727730
*/
728731
@Override
729732
@Audited(extractParameters = { "0", KEY_GROUP_ID, "1", KEY_ARTIFACT_ID, "2", KEY_VERSION, "3",
730-
"comment_id" }) // TODO
733+
"comment_id" })
731734
@Authorized(style = AuthorizedStyle.GroupAndArtifact, level = AuthorizedLevel.Write)
732735
public void deleteArtifactVersionComment(String groupId, String artifactId, String versionExpression,
733736
String commentId) {
@@ -768,7 +771,7 @@ public List<Comment> getArtifactVersionComments(String groupId, String artifactI
768771
*/
769772
@Override
770773
@Audited(extractParameters = { "0", KEY_GROUP_ID, "1", KEY_ARTIFACT_ID, "2", KEY_VERSION, "3",
771-
"comment_id" }) // TODO
774+
"comment_id" })
772775
@Authorized(style = AuthorizedStyle.GroupAndArtifact, level = AuthorizedLevel.Write)
773776
public void updateArtifactVersionComment(String groupId, String artifactId, String versionExpression,
774777
String commentId, NewComment data) {
@@ -1001,8 +1004,6 @@ public VersionMetaData createArtifactVersion(String groupId, String artifactId,
10011004
String ct = data.getContent().getContentType();
10021005
boolean isDraft = data.getIsDraft() != null && data.getIsDraft();
10031006

1004-
// TODO Only allow DRAFT content if the global opt-in config property is set/enabled
1005-
10061007
// Transform the given references into dtos
10071008
final List<ArtifactReferenceDto> referencesAsDtos = toReferenceDtos(
10081009
data.getContent().getReferences());

‎app/src/main/java/io/apicurio/registry/rest/v3/IdsResourceImpl.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import io.apicurio.registry.storage.dto.ContentWrapperDto;
1717
import io.apicurio.registry.storage.dto.StoredArtifactVersionDto;
1818
import io.apicurio.registry.storage.error.ArtifactNotFoundException;
19+
import io.apicurio.registry.storage.error.ContentNotFoundException;
1920
import io.apicurio.registry.types.ArtifactMediaTypes;
2021
import io.apicurio.registry.types.ReferenceType;
2122
import io.apicurio.registry.types.VersionState;
@@ -47,7 +48,11 @@ private void checkIfDeprecated(Supplier<VersionState> stateSupplier, String arti
4748
@Override
4849
@Authorized(style = AuthorizedStyle.None, level = AuthorizedLevel.Read)
4950
public Response getContentById(long contentId) {
50-
ContentHandle content = storage.getContentById(contentId).getContent();
51+
ContentWrapperDto dto = storage.getContentById(contentId);
52+
if (dto.getContentHash() != null && dto.getContentHash().startsWith("draft:")) {
53+
throw new ContentNotFoundException(contentId);
54+
}
55+
ContentHandle content = dto.getContent();
5156
Response.ResponseBuilder builder = Response.ok(content, ArtifactMediaTypes.BINARY);
5257
return builder.build();
5358
}
@@ -60,7 +65,8 @@ public Response getContentById(long contentId) {
6065
@Authorized(style = AuthorizedStyle.GlobalId, level = AuthorizedLevel.Read)
6166
public Response getContentByGlobalId(long globalId, HandleReferencesType references) {
6267
ArtifactVersionMetaDataDto metaData = storage.getArtifactVersionMetaData(globalId);
63-
if (VersionState.DISABLED.equals(metaData.getState())) {
68+
if (VersionState.DISABLED.equals(metaData.getState())
69+
|| VersionState.DRAFT.equals(metaData.getState())) {
6470
throw new ArtifactNotFoundException(null, String.valueOf(globalId));
6571
}
6672

‎app/src/main/java/io/apicurio/registry/storage/dto/ContentWrapperDto.java

+1
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ public class ContentWrapperDto {
1717
private String contentType;
1818
private ContentHandle content;
1919
private List<ArtifactReferenceDto> references;
20+
private transient String contentHash;
2021
}

‎app/src/main/java/io/apicurio/registry/storage/impl/sql/AbstractSqlRegistryStorage.java

+22-5
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ public Pair<ArtifactMetaDataDto, ArtifactVersionMetaDataDto> createArtifact(Stri
507507
long cid = -1;
508508
if (versionContent != null) {
509509
// Put the content in the DB and get the unique content ID back.
510-
cid = ensureContentAndGetId(artifactType, versionContent);
510+
cid = ensureContentAndGetId(artifactType, versionContent, versionIsDraft);
511511
}
512512
final long contentId = cid;
513513

@@ -686,14 +686,27 @@ private void createOrUpdateSemverBranchesRaw(Handle handle, GAV gav) {
686686
* Make sure the content exists in the database (try to insert it). Regardless of whether it already
687687
* existed or not, return the contentId of the content in the DB.
688688
*/
689-
private Long ensureContentAndGetId(String artifactType, ContentWrapperDto contentDto) {
689+
private Long ensureContentAndGetId(String artifactType, ContentWrapperDto contentDto, boolean isDraft) {
690690
List<ArtifactReferenceDto> references = contentDto.getReferences();
691691
TypedContent content = TypedContent.create(contentDto.getContent(), contentDto.getContentType());
692692
String contentHash;
693693
String canonicalContentHash;
694694
String serializedReferences;
695695

696-
if (notEmpty(references)) {
696+
// Need to create the content hash and canonical content hash. If the content is DRAFT
697+
// content, then do NOT calculate those hashes because we don't want DRAFT content to
698+
// be looked up by those hashes.
699+
//
700+
// So we have three different paths to calculate the hashes:
701+
// 1. If DRAFT state
702+
// 2. If the content has references
703+
// 3. If the content has no references
704+
705+
if (isDraft) {
706+
contentHash = "draft:" + UUID.randomUUID().toString();
707+
canonicalContentHash = "draft:" + UUID.randomUUID().toString();
708+
serializedReferences = null;
709+
} else if (notEmpty(references)) {
697710
Function<List<ArtifactReferenceDto>, Map<String, TypedContent>> referenceResolver = (refs) -> {
698711
return handles.withHandle(handle -> {
699712
return resolveReferencesRaw(handle, refs);
@@ -828,7 +841,7 @@ public ArtifactVersionMetaDataDto createArtifactVersion(String groupId, String a
828841
Date createdOn = new Date();
829842

830843
// Put the content in the DB and get the unique content ID back.
831-
long contentId = ensureContentAndGetId(artifactType, content);
844+
long contentId = ensureContentAndGetId(artifactType, content, isDraft);
832845

833846
try {
834847
// Create version and return
@@ -1694,7 +1707,7 @@ public void updateArtifactVersionContent(String groupId, String artifactId, Stri
16941707
log.debug("Updating content for artifact version: {} {} @ {}", groupId, artifactId, version);
16951708

16961709
// Put the new content in the DB and get the unique content ID back.
1697-
long contentId = ensureContentAndGetId(artifactType, content);
1710+
long contentId = ensureContentAndGetId(artifactType, content, true);
16981711

16991712
String modifiedBy = securityIdentity.getPrincipal().getName();
17001713
Date modifiedOn = new Date();
@@ -1706,6 +1719,10 @@ public void updateArtifactVersionContent(String groupId, String artifactId, Stri
17061719
if (rowCount == 0) {
17071720
throw new VersionNotFoundException(groupId, artifactId, version);
17081721
}
1722+
1723+
// Updating content will typically leave a row in the content table orphaned.
1724+
deleteAllOrphanedContentRaw(handle);
1725+
17091726
return null;
17101727
});
17111728
}

‎app/src/main/java/io/apicurio/registry/storage/impl/sql/CommonSqlStatements.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -607,15 +607,17 @@ public String selectContentCountByHash() {
607607
*/
608608
@Override
609609
public String selectContentById() {
610-
return "SELECT c.content, c.contentType, c.refs FROM content c " + "WHERE c.contentId = ?";
610+
return "SELECT c.content, c.contentType, c.refs, c.contentHash FROM content c "
611+
+ "WHERE c.contentId = ?";
611612
}
612613

613614
/**
614615
* @see io.apicurio.registry.storage.impl.sql.SqlStatements#selectContentByContentHash()
615616
*/
616617
@Override
617618
public String selectContentByContentHash() {
618-
return "SELECT c.content, c.contentType, c.refs FROM content c " + "WHERE c.contentHash = ?";
619+
return "SELECT c.content, c.contentType, c.refs, c.contentHash FROM content c "
620+
+ "WHERE c.contentHash = ?";
619621
}
620622

621623
@Override

‎app/src/main/java/io/apicurio/registry/storage/impl/sql/mappers/ContentMapper.java

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public ContentWrapperDto map(ResultSet rs) throws SQLException {
2929
contentWrapperDto.setContent(content);
3030
contentWrapperDto.setContentType(rs.getString("contentType"));
3131
contentWrapperDto.setReferences(SqlUtil.deserializeReferences(rs.getString("refs")));
32+
contentWrapperDto.setContentHash(rs.getString("contentHash"));
3233
return contentWrapperDto;
3334
}
3435

‎app/src/test/java/io/apicurio/registry/noprofile/rest/v3/DraftContentTest.java

+21-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import io.apicurio.registry.AbstractResourceTestBase;
44
import io.apicurio.registry.rest.client.models.CreateArtifact;
5+
import io.apicurio.registry.rest.client.models.CreateArtifactResponse;
56
import io.apicurio.registry.rest.client.models.CreateGroup;
67
import io.apicurio.registry.rest.client.models.CreateRule;
78
import io.apicurio.registry.rest.client.models.CreateVersion;
@@ -15,8 +16,10 @@
1516
import io.apicurio.registry.rules.validity.ValidityLevel;
1617
import io.apicurio.registry.types.ArtifactType;
1718
import io.apicurio.registry.types.ContentTypes;
19+
import io.apicurio.registry.utils.tests.MutabilityEnabledProfile;
1820
import io.apicurio.registry.utils.tests.TestUtils;
1921
import io.quarkus.test.junit.QuarkusTest;
22+
import io.quarkus.test.junit.TestProfile;
2023
import org.apache.commons.io.IOUtils;
2124
import org.junit.jupiter.api.Assertions;
2225
import org.junit.jupiter.api.Test;
@@ -25,6 +28,7 @@
2528
import java.nio.charset.StandardCharsets;
2629

2730
@QuarkusTest
31+
@TestProfile(MutabilityEnabledProfile.class)
2832
public class DraftContentTest extends AbstractResourceTestBase {
2933

3034
private static final String AVRO_CONTENT_V1 = """
@@ -70,12 +74,28 @@ public void testCreateDraftArtifact() throws Exception {
7074
createArtifact.getFirstVersion().setIsDraft(true);
7175
createArtifact.getFirstVersion().setVersion("1.0.0");
7276

73-
clientV3.groups().byGroupId(groupId).artifacts().post(createArtifact);
77+
CreateArtifactResponse car = clientV3.groups().byGroupId(groupId).artifacts().post(createArtifact);
78+
Assertions.assertNotNull(car);
79+
Assertions.assertNotNull(car.getVersion());
7480

7581
VersionMetaData vmd = clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId)
7682
.versions().byVersionExpression("1.0.0").get();
7783
Assertions.assertNotNull(vmd);
7884
Assertions.assertEquals(VersionState.DRAFT, vmd.getState());
85+
86+
// Note: Should NOT be able to fetch its content by globalId (disallowed for DRAFT content)
87+
Long globalId = car.getVersion().getGlobalId();
88+
Assertions.assertNotNull(globalId);
89+
Assertions.assertThrows(ProblemDetails.class, () -> {
90+
clientV3.ids().globalIds().byGlobalId(globalId).get();
91+
});
92+
93+
// Note: Should NOT be able to fetch its content by contentId (disallowed for DRAFT content)
94+
Long contentId = car.getVersion().getContentId();
95+
Assertions.assertNotNull(contentId);
96+
Assertions.assertThrows(ProblemDetails.class, () -> {
97+
clientV3.ids().contentIds().byContentId(contentId).get();
98+
});
7999
}
80100

81101
@Test

‎app/src/test/java/io/apicurio/registry/noprofile/rest/v3/DryRunTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@
1717
import io.apicurio.registry.rules.validity.ValidityLevel;
1818
import io.apicurio.registry.types.ArtifactType;
1919
import io.apicurio.registry.types.ContentTypes;
20-
import io.apicurio.registry.utils.tests.DeletionEnabledProfile;
20+
import io.apicurio.registry.utils.tests.MutabilityEnabledProfile;
2121
import io.apicurio.registry.utils.tests.TestUtils;
2222
import io.quarkus.test.junit.QuarkusTest;
2323
import io.quarkus.test.junit.TestProfile;
2424
import org.junit.jupiter.api.Assertions;
2525
import org.junit.jupiter.api.Test;
2626

2727
@QuarkusTest
28-
@TestProfile(DeletionEnabledProfile.class)
28+
@TestProfile(MutabilityEnabledProfile.class)
2929
public class DryRunTest extends AbstractResourceTestBase {
3030

3131
private static final String SCHEMA_SIMPLE = """

‎common/src/main/resources/META-INF/openapi.json

+9
Original file line numberDiff line numberDiff line change
@@ -2357,6 +2357,9 @@
23572357
"404": {
23582358
"$ref": "#/components/responses/NotFound"
23592359
},
2360+
"405": {
2361+
"$ref": "#/components/responses/MethodNotAllowed"
2362+
},
23602363
"500": {
23612364
"$ref": "#/components/responses/ServerError"
23622365
}
@@ -2574,6 +2577,9 @@
25742577
"404": {
25752578
"$ref": "#/components/responses/NotFound"
25762579
},
2580+
"405": {
2581+
"$ref": "#/components/responses/MethodNotAllowed"
2582+
},
25772583
"500": {
25782584
"$ref": "#/components/responses/ServerError"
25792585
}
@@ -2659,6 +2665,9 @@
26592665
"404": {
26602666
"$ref": "#/components/responses/NotFound"
26612667
},
2668+
"405": {
2669+
"$ref": "#/components/responses/MethodNotAllowed"
2670+
},
26622671
"409": {
26632672
"$ref": "#/components/responses/Conflict"
26642673
},

‎docs/modules/ROOT/partials/getting-started/ref-registry-all-configs.adoc

+6-1
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ The following {registry} configuration options are available for each component
545545
|`apicurio.rest.deletion.artifact-version.enabled`
546546
|`boolean [dynamic]`
547547
|`false`
548-
|`2.4.2-SNAPSHOT`
548+
|`2.4.2`
549549
|Enables artifact version deletion
550550
|`apicurio.rest.deletion.artifact.enabled`
551551
|`boolean [dynamic]`
@@ -557,6 +557,11 @@ The following {registry} configuration options are available for each component
557557
|`false`
558558
|`3.0.0`
559559
|Enables group deletion
560+
|`apicurio.rest.mutability.artifact-version-content.enabled`
561+
|`boolean [dynamic]`
562+
|`false`
563+
|`3.0.2`
564+
|Enables artifact version mutability
560565
|===
561566

562567
== semver

‎go-sdk/pkg/registryclient-v3/groups/item_artifacts_item_versions_item_content_request_builder.go

+2
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ func (m *ItemArtifactsItemVersionsItemContentRequestBuilder) Get(ctx context.Con
8080

8181
// Put updates the content of a single version of an artifact.NOTE: the artifact must be in `DRAFT` status.Both the `artifactId` and the unique `version` number must be provided to identifythe version to update.This operation can fail for the following reasons:* No artifact with this `artifactId` exists (HTTP error `404`)* No version with this `version` exists (HTTP error `404`)* Artifact version not in `DRAFT` status (HTTP error `409`)* A server error occurred (HTTP error `500`)
8282
// returns a ProblemDetails error when the service returns a 404 status code
83+
// returns a ProblemDetails error when the service returns a 405 status code
8384
// returns a ProblemDetails error when the service returns a 409 status code
8485
// returns a ProblemDetails error when the service returns a 500 status code
8586
func (m *ItemArtifactsItemVersionsItemContentRequestBuilder) Put(ctx context.Context, body i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.VersionContentable, requestConfiguration *ItemArtifactsItemVersionsItemContentRequestBuilderPutRequestConfiguration) error {
@@ -89,6 +90,7 @@ func (m *ItemArtifactsItemVersionsItemContentRequestBuilder) Put(ctx context.Con
8990
}
9091
errorMapping := i2ae4187f7daee263371cb1c977df639813ab50ffa529013b7437480d1ec0158f.ErrorMappings{
9192
"404": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
93+
"405": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
9294
"409": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
9395
"500": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
9496
}

‎go-sdk/pkg/registryclient-v3/groups/item_artifacts_with_artifact_item_request_builder.go

+2
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func NewItemArtifactsWithArtifactItemRequestBuilder(rawUrl string, requestAdapte
5858

5959
// Delete deletes an artifact completely, resulting in all versions of the artifact also beingdeleted. This may fail for one of the following reasons:* No artifact with the `artifactId` exists (HTTP error `404`)* A server error occurred (HTTP error `500`)
6060
// returns a ProblemDetails error when the service returns a 404 status code
61+
// returns a ProblemDetails error when the service returns a 405 status code
6162
// returns a ProblemDetails error when the service returns a 500 status code
6263
func (m *ItemArtifactsWithArtifactItemRequestBuilder) Delete(ctx context.Context, requestConfiguration *ItemArtifactsWithArtifactItemRequestBuilderDeleteRequestConfiguration) error {
6364
requestInfo, err := m.ToDeleteRequestInformation(ctx, requestConfiguration)
@@ -66,6 +67,7 @@ func (m *ItemArtifactsWithArtifactItemRequestBuilder) Delete(ctx context.Context
6667
}
6768
errorMapping := i2ae4187f7daee263371cb1c977df639813ab50ffa529013b7437480d1ec0158f.ErrorMappings{
6869
"404": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
70+
"405": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
6971
"500": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
7072
}
7173
err = m.BaseRequestBuilder.RequestAdapter.SendNoContent(ctx, requestInfo, errorMapping)

‎go-sdk/pkg/registryclient-v3/groups/with_group_item_request_builder.go

+2
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func NewWithGroupItemRequestBuilder(rawUrl string, requestAdapter i2ae4187f7daee
5858

5959
// Delete deletes a group by identifier. This operation also deletes all artifacts withinthe group, so should be used very carefully.This operation can fail for the following reasons:* A server error occurred (HTTP error `500`)* The group does not exist (HTTP error `404`)
6060
// returns a ProblemDetails error when the service returns a 404 status code
61+
// returns a ProblemDetails error when the service returns a 405 status code
6162
// returns a ProblemDetails error when the service returns a 500 status code
6263
func (m *WithGroupItemRequestBuilder) Delete(ctx context.Context, requestConfiguration *WithGroupItemRequestBuilderDeleteRequestConfiguration) error {
6364
requestInfo, err := m.ToDeleteRequestInformation(ctx, requestConfiguration)
@@ -66,6 +67,7 @@ func (m *WithGroupItemRequestBuilder) Delete(ctx context.Context, requestConfigu
6667
}
6768
errorMapping := i2ae4187f7daee263371cb1c977df639813ab50ffa529013b7437480d1ec0158f.ErrorMappings{
6869
"404": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
70+
"405": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
6971
"500": i00eb2e63d156923d00d8e86fe16b5d74daf30e363c9f185a8165cb42aa2f2c71.CreateProblemDetailsFromDiscriminatorValue,
7072
}
7173
err = m.BaseRequestBuilder.RequestAdapter.SendNoContent(ctx, requestInfo, errorMapping)

0 commit comments

Comments
 (0)