From eefdd27c44767f435a94b10f73ce990066438546 Mon Sep 17 00:00:00 2001 From: John Niang Date: Wed, 30 Nov 2022 10:23:46 +0800 Subject: [PATCH] Simplify references in Attachment (#2800) #### What type of PR is this? /kind improvement /kind api-change /area core /milestone 2.0.x #### What this PR does / why we need it: This PR replace refs in Attachment extension with simple name. > **Warning** Please @halo-dev/sig-halo-console be aware of that this PR contains break changes, so that we can not use Attachment feature in the console directly. The console has to adapt the break changes. #### Does this PR introduce a user-facing change? ```release-note None ``` --- .../core/extension/attachment/Attachment.java | 13 ++++---- .../app/core/extension/attachment/Policy.java | 16 ++++----- .../extension/attachment/PolicyTemplate.java | 6 ++-- .../endpoint/AttachmentEndpoint.java | 33 +++++++------------ .../LocalAttachmentUploadHandler.java | 4 +-- .../attachment/AttachmentReconciler.java | 10 +++--- .../extensions/attachment-local-policy.yaml | 3 +- .../endpoint/AttachmentEndpointTest.java | 11 +++---- 8 files changed, 41 insertions(+), 55 deletions(-) diff --git a/src/main/java/run/halo/app/core/extension/attachment/Attachment.java b/src/main/java/run/halo/app/core/extension/attachment/Attachment.java index 2f0c4590a..8c56ccbc8 100644 --- a/src/main/java/run/halo/app/core/extension/attachment/Attachment.java +++ b/src/main/java/run/halo/app/core/extension/attachment/Attachment.java @@ -10,7 +10,6 @@ import lombok.EqualsAndHashCode; import lombok.ToString; import run.halo.app.extension.AbstractExtension; import run.halo.app.extension.GVK; -import run.halo.app.extension.Ref; @Data @ToString(callSuper = true) @@ -32,14 +31,14 @@ public class Attachment extends AbstractExtension { @Schema(description = "Display name of attachment") private String displayName; - @Schema(description = "Reference of Group") - private Ref groupRef; + @Schema(description = "Group name") + private String groupName; - @Schema(description = "Reference of Policy") - private Ref policyRef; + @Schema(description = "Policy name") + private String policyName; - @Schema(description = "Reference of User who uploads the attachment") - private Ref uploadedBy; + @Schema(description = "Name of User who uploads the attachment") + private String ownerName; @Schema(description = "Media type of attachment") private String mediaType; diff --git a/src/main/java/run/halo/app/core/extension/attachment/Policy.java b/src/main/java/run/halo/app/core/extension/attachment/Policy.java index ddb061779..450548a9f 100644 --- a/src/main/java/run/halo/app/core/extension/attachment/Policy.java +++ b/src/main/java/run/halo/app/core/extension/attachment/Policy.java @@ -1,5 +1,6 @@ package run.halo.app.core.extension.attachment; +import static io.swagger.v3.oas.annotations.media.Schema.RequiredMode.REQUIRED; import static run.halo.app.core.extension.attachment.Policy.KIND; import io.swagger.v3.oas.annotations.media.Schema; @@ -8,31 +9,30 @@ import lombok.EqualsAndHashCode; import lombok.ToString; import run.halo.app.extension.AbstractExtension; import run.halo.app.extension.GVK; -import run.halo.app.extension.Ref; @Data @ToString(callSuper = true) @EqualsAndHashCode(callSuper = true) -@GVK(group = Constant.GROUP, version = Constant.VERSION, kind = KIND, plural = "policies", - singular = "policy") +@GVK(group = Constant.GROUP, version = Constant.VERSION, kind = KIND, + plural = "policies", singular = "policy") public class Policy extends AbstractExtension { public static final String KIND = "Policy"; - @Schema(required = true) + @Schema(requiredMode = REQUIRED) private PolicySpec spec; @Data public static class PolicySpec { - @Schema(required = true, description = "Display name of policy") + @Schema(requiredMode = REQUIRED, description = "Display name of policy") private String displayName; - @Schema(description = "Reference name of Setting extension") - private Ref templateRef; + @Schema(requiredMode = REQUIRED, description = "Reference name of PolicyTemplate") + private String templateName; @Schema(description = "Reference name of ConfigMap extension") - private Ref configMapRef; + private String configMapName; } diff --git a/src/main/java/run/halo/app/core/extension/attachment/PolicyTemplate.java b/src/main/java/run/halo/app/core/extension/attachment/PolicyTemplate.java index c22f07316..509f08a70 100644 --- a/src/main/java/run/halo/app/core/extension/attachment/PolicyTemplate.java +++ b/src/main/java/run/halo/app/core/extension/attachment/PolicyTemplate.java @@ -1,13 +1,14 @@ package run.halo.app.core.extension.attachment; +import static io.swagger.v3.oas.annotations.media.Schema.RequiredMode.REQUIRED; import static run.halo.app.core.extension.attachment.PolicyTemplate.KIND; +import io.swagger.v3.oas.annotations.media.Schema; import lombok.Data; import lombok.EqualsAndHashCode; import lombok.ToString; import run.halo.app.extension.AbstractExtension; import run.halo.app.extension.GVK; -import run.halo.app.extension.Ref; @Data @ToString(callSuper = true) @@ -25,7 +26,8 @@ public class PolicyTemplate extends AbstractExtension { private String displayName; - private Ref settingRef; + @Schema(requiredMode = REQUIRED) + private String settingName; } diff --git a/src/main/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpoint.java b/src/main/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpoint.java index 57e2bd553..b1598df86 100644 --- a/src/main/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpoint.java +++ b/src/main/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpoint.java @@ -16,6 +16,7 @@ import io.swagger.v3.oas.annotations.media.Schema; import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; import lombok.extern.slf4j.Slf4j; @@ -49,7 +50,6 @@ import run.halo.app.core.extension.endpoint.SortResolver; import run.halo.app.extension.Comparators; import run.halo.app.extension.ConfigMap; import run.halo.app.extension.ReactiveExtensionClient; -import run.halo.app.extension.Ref; import run.halo.app.extension.router.IListRequest; import run.halo.app.extension.router.IListRequest.QueryListRequest; import run.halo.app.plugin.ExtensionComponentsFinder; @@ -108,11 +108,11 @@ public class AttachmentEndpoint implements CustomEndpoint { .map(UploadRequest::new) // prepare the upload option .flatMap(uploadRequest -> client.get(Policy.class, uploadRequest.getPolicyName()) - .filter(policy -> policy.getSpec().getConfigMapRef() != null) + .filter(policy -> StringUtils.hasText(policy.getSpec().getConfigMapName())) .switchIfEmpty(Mono.error(() -> new ServerWebInputException( "Please configure the attachment policy before uploading"))) .flatMap(policy -> { - var configMapName = policy.getSpec().getConfigMapRef().getName(); + var configMapName = policy.getSpec().getConfigMapName(); return client.get(ConfigMap.class, configMapName) .map(configMap -> new UploadOption(uploadRequest.getFile(), policy, configMap)); @@ -127,12 +127,12 @@ public class AttachmentEndpoint implements CustomEndpoint { spec = new Attachment.AttachmentSpec(); attachment.setSpec(spec); } - spec.setUploadedBy(Ref.of(username)); - spec.setPolicyRef(Ref.of(uploadOption.policy())); + spec.setOwnerName(username); + spec.setPolicyName(uploadOption.policy().getMetadata().getName()); var groupName = uploadRequest.getGroupName(); if (groupName != null) { // validate the group name - spec.setGroupRef(Ref.of(groupName)); + spec.setGroupName(groupName); } })) .next() @@ -239,31 +239,20 @@ public class AttachmentEndpoint implements CustomEndpoint { }).orElse(true); Predicate policyPred = attachment -> getPolicy() - .map(policy -> { - var policyRef = attachment.getSpec().getPolicyRef(); - return policyRef != null && policy.equals(policyRef.getName()); - }).orElse(true); + .map(policy -> Objects.equals(policy, attachment.getSpec().getPolicyName())) + .orElse(true); Predicate groupPred = attachment -> getGroup() - .map(group -> { - var groupRef = attachment.getSpec().getGroupRef(); - return groupRef != null && group.equals(groupRef.getName()); - }) + .map(group -> Objects.equals(group, attachment.getSpec().getGroupName())) .orElse(true); Predicate ungroupedPred = attachment -> getUngrouped() .filter(Boolean::booleanValue) - .map(ungrouped -> { - var groupRef = attachment.getSpec().getGroupRef(); - return groupRef == null || !StringUtils.hasText(groupRef.getName()); - }) + .map(ungrouped -> !StringUtils.hasText(attachment.getSpec().getGroupName())) .orElseGet(() -> groupPred.test(attachment)); Predicate uploadedByPred = attachment -> getUploadedBy() - .map(uploadedBy -> { - var uploadedByRef = attachment.getSpec().getUploadedBy(); - return uploadedByRef != null && uploadedBy.equals(uploadedByRef.getName()); - }) + .map(uploadedBy -> Objects.equals(uploadedBy, attachment.getSpec().getOwnerName())) .orElse(true); diff --git a/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java b/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java index 6e45d598e..312063cbe 100644 --- a/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java +++ b/src/main/java/run/halo/app/core/extension/attachment/endpoint/LocalAttachmentUploadHandler.java @@ -130,10 +130,10 @@ class LocalAttachmentUploadHandler implements AttachmentHandler { private boolean shouldHandle(Policy policy) { if (policy == null || policy.getSpec() == null - || policy.getSpec().getTemplateRef() == null) { + || !StringUtils.hasText(policy.getSpec().getTemplateName())) { return false; } - return "local".equals(policy.getSpec().getTemplateRef().getName()); + return "local".equals(policy.getSpec().getTemplateName()); } @Data diff --git a/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java b/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java index f17df567a..560d741e3 100644 --- a/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java +++ b/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java @@ -49,12 +49,10 @@ public class AttachmentReconciler implements Reconciler { client.fetch(Attachment.class, request.name()).ifPresent(attachment -> { // TODO Handle the finalizer if (attachment.getMetadata().getDeletionTimestamp() != null) { - Policy policy = - client.fetch(Policy.class, attachment.getSpec().getPolicyRef().getName()) - .orElseThrow(); - var configMap = - client.fetch(ConfigMap.class, policy.getSpec().getConfigMapRef().getName()) - .orElseThrow(); + Policy policy = client.fetch(Policy.class, attachment.getSpec().getPolicyName()) + .orElseThrow(); + var configMap = client.fetch(ConfigMap.class, policy.getSpec().getConfigMapName()) + .orElseThrow(); var deleteOption = new DeleteOption(attachment, policy, configMap); Flux.fromIterable(extensionComponentsFinder.getExtensions(AttachmentHandler.class)) .concatMap(handler -> handler.delete(deleteOption)).next().switchIfEmpty( diff --git a/src/main/resources/extensions/attachment-local-policy.yaml b/src/main/resources/extensions/attachment-local-policy.yaml index 9c6feba9a..68d035553 100644 --- a/src/main/resources/extensions/attachment-local-policy.yaml +++ b/src/main/resources/extensions/attachment-local-policy.yaml @@ -4,8 +4,7 @@ metadata: name: local spec: displayName: Local Storage - settingRef: - name: local-policy-template-setting + settingName: local-policy-template-setting --- apiVersion: v1alpha1 kind: Setting diff --git a/src/test/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpointTest.java b/src/test/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpointTest.java index 6af1436e1..63610887b 100644 --- a/src/test/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpointTest.java +++ b/src/test/java/run/halo/app/core/extension/attachment/endpoint/AttachmentEndpointTest.java @@ -37,7 +37,6 @@ import run.halo.app.extension.ConfigMap; import run.halo.app.extension.ListResult; import run.halo.app.extension.Metadata; import run.halo.app.extension.ReactiveExtensionClient; -import run.halo.app.extension.Ref; import run.halo.app.plugin.ExtensionComponentsFinder; @ExtendWith(MockitoExtension.class) @@ -104,7 +103,7 @@ class AttachmentEndpointTest { @Test void shouldUploadSuccessfully() { var policySpec = new PolicySpec(); - policySpec.setConfigMapRef(Ref.of("fake-configmap")); + policySpec.setConfigMapName("fake-configmap"); var policyMetadata = new Metadata(); policyMetadata.setName("fake-policy"); var policy = new Policy(); @@ -146,9 +145,9 @@ class AttachmentEndpointTest { .expectStatus().isOk() .expectBody() .jsonPath("$.metadata.name").isEqualTo("fake-attachment") - .jsonPath("$.spec.uploadedBy.name").isEqualTo("fake-user") - .jsonPath("$.spec.policyRef.name").isEqualTo("fake-policy") - .jsonPath("$.spec.groupRef.name").isEqualTo("fake-group") + .jsonPath("$.spec.ownerName").isEqualTo("fake-user") + .jsonPath("$.spec.policyName").isEqualTo("fake-policy") + .jsonPath("$.spec.groupName").isEqualTo("fake-group") ; verify(client).get(Policy.class, "fake-policy"); @@ -195,7 +194,7 @@ class AttachmentEndpointTest { assertTrue(pred.test(attachment)); - spec.setGroupRef(Ref.of("halo")); + spec.setGroupName("halo"); assertFalse(pred.test(attachment)); } }