diff --git a/src/main/java/run/halo/app/extension/JSONExtensionConverter.java b/src/main/java/run/halo/app/extension/JSONExtensionConverter.java index a77b3c4dc..0cfa1be10 100644 --- a/src/main/java/run/halo/app/extension/JSONExtensionConverter.java +++ b/src/main/java/run/halo/app/extension/JSONExtensionConverter.java @@ -53,8 +53,6 @@ public class JSONExtensionConverter implements ExtensionConverter { var scheme = schemeManager.get(gvk); try { - var data = objectMapper.writeValueAsBytes(extension); - var validation = new ValidationData<>(extension); var extensionJsonNode = objectMapper.valueToTree(extension); var validator = getValidator(scheme); @@ -68,6 +66,7 @@ public class JSONExtensionConverter implements ExtensionConverter { var version = extension.getMetadata().getVersion(); var storeName = ExtensionUtil.buildStoreName(scheme, extension.getMetadata().getName()); + var data = objectMapper.writeValueAsBytes(extensionJsonNode); return new ExtensionStore(storeName, data, version); } catch (IOException e) { throw new ExtensionConvertException("Failed write Extension as bytes", e); diff --git a/src/main/java/run/halo/app/extension/JsonExtension.java b/src/main/java/run/halo/app/extension/JsonExtension.java new file mode 100644 index 000000000..67ef64fc8 --- /dev/null +++ b/src/main/java/run/halo/app/extension/JsonExtension.java @@ -0,0 +1,232 @@ +package run.halo.app.extension; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.JsonSerializer; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.fasterxml.jackson.databind.node.LongNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.node.TextNode; +import java.io.IOException; +import java.time.Instant; +import java.util.Map; +import java.util.Set; + +/** + * JsonExtension is representation an extension using ObjectNode. This extension is preparing for + * patching in the future. + * + * @author johnniang + */ +@JsonSerialize(using = JsonExtension.ObjectNodeExtensionSerializer.class) +@JsonDeserialize(using = JsonExtension.ObjectNodeExtensionDeSerializer.class) +class JsonExtension implements Extension { + + private final ObjectMapper objectMapper; + + private final ObjectNode objectNode; + + public JsonExtension(ObjectMapper objectMapper) { + this(objectMapper, objectMapper.createObjectNode()); + } + + public JsonExtension(ObjectMapper objectMapper, ObjectNode objectNode) { + this.objectMapper = objectMapper; + this.objectNode = objectNode; + } + + public JsonExtension(ObjectMapper objectMapper, Extension e) { + this(objectMapper, (ObjectNode) objectMapper.valueToTree(e)); + } + + @Override + public MetadataOperator getMetadata() { + var metadataNode = objectNode.get("metadata"); + if (metadataNode == null) { + return null; + } + return new ObjectNodeMetadata((ObjectNode) metadataNode); + } + + @Override + public String getApiVersion() { + var apiVersionNode = objectNode.get("apiVersion"); + return apiVersionNode == null ? null : apiVersionNode.asText(); + } + + @Override + public String getKind() { + return objectNode.get("kind").asText(); + } + + @Override + public void setApiVersion(String apiVersion) { + objectNode.set("apiVersion", new TextNode(apiVersion)); + } + + @Override + public void setKind(String kind) { + objectNode.set("kind", new TextNode(kind)); + } + + @Override + public void setMetadata(MetadataOperator metadata) { + objectNode.set("metadata", objectMapper.valueToTree(metadata)); + } + + public static class ObjectNodeExtensionSerializer extends JsonSerializer { + + @Override + public void serialize(JsonExtension value, JsonGenerator gen, + SerializerProvider serializers) throws IOException { + gen.writeTree(value.objectNode); + } + } + + public static class ObjectNodeExtensionDeSerializer + extends JsonDeserializer { + + @Override + public JsonExtension deserialize(JsonParser p, DeserializationContext ctxt) + throws IOException { + var mapper = (ObjectMapper) p.getCodec(); + var treeNode = mapper.readTree(p); + return new JsonExtension(mapper, (ObjectNode) treeNode); + } + } + + /** + * Get internal representation. + * + * @return internal representation + */ + public ObjectNode getInternal() { + return objectNode; + } + + public MetadataOperator getMetadataOrCreate() { + var metadataNode = objectMapper.createObjectNode(); + objectNode.set("metadata", metadataNode); + return new ObjectNodeMetadata(metadataNode); + } + + class ObjectNodeMetadata implements MetadataOperator { + + private final ObjectNode objectNode; + + public ObjectNodeMetadata(ObjectNode objectNode) { + this.objectNode = objectNode; + } + + @Override + public String getName() { + var nameNode = objectNode.get("name"); + return objectMapper.convertValue(nameNode, String.class); + } + + @Override + public String getGenerateName() { + var generateNameNode = objectNode.get("generateName"); + return objectMapper.convertValue(generateNameNode, String.class); + } + + @Override + public Map getLabels() { + var labelsNode = objectNode.get("labels"); + return objectMapper.convertValue(labelsNode, new TypeReference<>() { + }); + } + + @Override + public Map getAnnotations() { + var annotationsNode = objectNode.get("annotations"); + return objectMapper.convertValue(annotationsNode, new TypeReference<>() { + }); + } + + @Override + public Long getVersion() { + JsonNode versionNode = objectNode.get("version"); + return objectMapper.convertValue(versionNode, Long.class); + } + + @Override + public Instant getCreationTimestamp() { + return objectMapper.convertValue(objectNode.get("creationTimestamp"), Instant.class); + } + + @Override + public Instant getDeletionTimestamp() { + return objectMapper.convertValue(objectNode.get("deletionTimestamp"), Instant.class); + } + + @Override + public Set getFinalizers() { + return objectMapper.convertValue(objectNode.get("finalizers"), new TypeReference<>() { + }); + } + + @Override + public void setName(String name) { + if (name != null) { + objectNode.set("name", TextNode.valueOf(name)); + } + } + + @Override + public void setGenerateName(String generateName) { + if (generateName != null) { + objectNode.set("generateName", TextNode.valueOf(generateName)); + } + } + + @Override + public void setLabels(Map labels) { + if (labels != null) { + objectNode.set("labels", objectMapper.valueToTree(labels)); + } + } + + @Override + public void setAnnotations(Map annotations) { + if (annotations != null) { + objectNode.set("annotations", objectMapper.valueToTree(annotations)); + } + } + + @Override + public void setVersion(Long version) { + if (version != null) { + objectNode.set("version", LongNode.valueOf(version)); + } + } + + @Override + public void setCreationTimestamp(Instant creationTimestamp) { + if (creationTimestamp != null) { + objectNode.set("creationTimestamp", objectMapper.valueToTree(creationTimestamp)); + } + } + + @Override + public void setDeletionTimestamp(Instant deletionTimestamp) { + if (deletionTimestamp != null) { + objectNode.set("deletionTimestamp", objectMapper.valueToTree(deletionTimestamp)); + } + } + + @Override + public void setFinalizers(Set finalizers) { + if (finalizers != null) { + objectNode.set("finalizers", objectMapper.valueToTree(finalizers)); + } + } + } +} diff --git a/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java b/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java index c07abd880..7466e958b 100644 --- a/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java +++ b/src/main/java/run/halo/app/extension/ReactiveExtensionClientImpl.java @@ -3,11 +3,13 @@ package run.halo.app.extension; import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic; import static org.springframework.util.StringUtils.hasText; +import com.fasterxml.jackson.databind.ObjectMapper; import java.time.Duration; import java.time.Instant; import java.util.Comparator; import java.util.List; import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.data.util.Predicates; @@ -29,11 +31,14 @@ public class ReactiveExtensionClientImpl implements ReactiveExtensionClient { private final Watcher.WatcherComposite watchers; + private final ObjectMapper objectMapper; + public ReactiveExtensionClientImpl(ReactiveExtensionStoreClient client, - ExtensionConverter converter, SchemeManager schemeManager) { + ExtensionConverter converter, SchemeManager schemeManager, ObjectMapper objectMapper) { this.client = client; this.converter = converter; this.schemeManager = schemeManager; + this.objectMapper = objectMapper; this.watchers = new Watcher.WatcherComposite(); } @@ -110,7 +115,7 @@ public class ReactiveExtensionClientImpl implements ReactiveExtensionClient { if (!hasText(metadata.getGenerateName())) { throw new IllegalArgumentException( "The metadata.generateName must not be blank when metadata.name is " - + "blank"); + + "blank"); } // generate name with random text metadata.setName(metadata.getGenerateName() + randomAlphabetic(5)); @@ -124,40 +129,57 @@ public class ReactiveExtensionClientImpl implements ReactiveExtensionClient { .retryWhen(Retry.backoff(3, Duration.ofMillis(100)) // retry when generateName is set .filter(t -> t instanceof DataIntegrityViolationException - && hasText(extension.getMetadata().getGenerateName()))); + && hasText(extension.getMetadata().getGenerateName()))); } @Override public Mono update(E extension) { - // overwrite some fields - Mono mono; - if (extension instanceof Unstructured unstructured) { - mono = get(unstructured.groupVersionKind(), extension.getMetadata().getName()); - } else { - mono = get(extension.getClass(), extension.getMetadata().getName()); - } - return mono - .flatMap(old -> { - // reset some fields - var oldMetadata = old.getMetadata(); - var newMetadata = extension.getMetadata(); + // Refactor the atomic reference if we have a better solution. + final var statusChangeOnly = new AtomicBoolean(false); + return getLatest(extension) + .map(old -> new JsonExtension(objectMapper, old)) + .flatMap(oldJsonExt -> { + var newJsonExt = new JsonExtension(objectMapper, extension); + // reset some mandatory fields + var oldMetadata = oldJsonExt.getMetadata(); + var newMetadata = newJsonExt.getMetadata(); newMetadata.setCreationTimestamp(oldMetadata.getCreationTimestamp()); - newMetadata.setDeletionTimestamp(oldMetadata.getDeletionTimestamp()); - extension.setMetadata(newMetadata); - if (Objects.equals(old, extension)) { + newMetadata.setGenerateName(oldMetadata.getGenerateName()); + + var oldObjectNode = oldJsonExt.getInternal().deepCopy(); + var newObjectNode = newJsonExt.getInternal().deepCopy(); + if (Objects.equals(oldObjectNode, newObjectNode)) { + // if no data were changed, just skip updating. return Mono.empty(); } - return Mono.just(extension); + // check status is changed + oldObjectNode.remove("status"); + newObjectNode.remove("status"); + if (Objects.equals(oldObjectNode, newObjectNode)) { + statusChangeOnly.set(true); + } + return Mono.just(newJsonExt); }) .map(converter::convertTo) .flatMap(extensionStore -> client.update(extensionStore.getName(), extensionStore.getVersion(), extensionStore.getData())) .map(updated -> converter.convertFrom((Class) extension.getClass(), updated)) - .doOnNext(updated -> watchers.onUpdate(extension, updated)) + .doOnNext(updated -> { + if (!statusChangeOnly.get()) { + watchers.onUpdate(extension, updated); + } + }) .switchIfEmpty(Mono.defer(() -> Mono.just(extension))); } + private Mono getLatest(Extension extension) { + if (extension instanceof Unstructured unstructured) { + return get(unstructured.groupVersionKind(), unstructured.getMetadata().getName()); + } + return get(extension.getClass(), extension.getMetadata().getName()); + } + @Override public Mono delete(E extension) { // set deletionTimestamp diff --git a/src/test/java/run/halo/app/extension/JsonExtensionTest.java b/src/test/java/run/halo/app/extension/JsonExtensionTest.java new file mode 100644 index 000000000..62c5596ac --- /dev/null +++ b/src/test/java/run/halo/app/extension/JsonExtensionTest.java @@ -0,0 +1,80 @@ +package run.halo.app.extension; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.node.TextNode; +import org.json.JSONException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.skyscreamer.jsonassert.JSONAssert; + +class JsonExtensionTest { + + ObjectMapper objectMapper; + + @BeforeEach + void setUp() { + objectMapper = JsonMapper.builder().build(); + } + + @Test + void serializeEmptyExt() throws JsonProcessingException, JSONException { + var ext = new JsonExtension(objectMapper); + var json = objectMapper.writeValueAsString(ext); + JSONAssert.assertEquals("{}", json, true); + } + + @Test + void serializeExt() throws JsonProcessingException, JSONException { + var ext = new JsonExtension(objectMapper); + ext.setApiVersion("fake.halo.run/v1alpha"); + ext.setKind("Fake"); + var metadata = ext.getMetadataOrCreate(); + metadata.setName("fake-name"); + + ext.getInternal().set("data", TextNode.valueOf("halo")); + + JSONAssert.assertEquals(""" + { + "apiVersion": "fake.halo.run/v1alpha", + "kind": "Fake", + "metadata": { + "name": "fake-name" + }, + "data": "halo" + }""", objectMapper.writeValueAsString(ext), true); + } + + @Test + void deserialize() throws JsonProcessingException { + var json = """ + { + "apiVersion": "fake.halo.run/v1alpha1", + "kind": "Fake", + "metadata": { + "name": "faker" + }, + "otherProperty": "otherPropertyValue" + }"""; + + var ext = objectMapper.readValue(json, JsonExtension.class); + + assertEquals("fake.halo.run/v1alpha1", ext.getApiVersion()); + assertEquals("Fake", ext.getKind()); + assertNotNull(ext.getMetadata()); + assertEquals("faker", ext.getMetadata().getName()); + assertNull(ext.getMetadata().getVersion()); + assertNull(ext.getMetadata().getFinalizers()); + assertNull(ext.getMetadata().getAnnotations()); + assertNull(ext.getMetadata().getLabels()); + assertNull(ext.getMetadata().getGenerateName()); + assertNull(ext.getMetadata().getCreationTimestamp()); + assertNull(ext.getMetadata().getDeletionTimestamp()); + assertEquals("otherPropertyValue", ext.getInternal().get("otherProperty").asText()); + } +} \ No newline at end of file diff --git a/src/test/java/run/halo/app/extension/ReactiveExtensionClientTest.java b/src/test/java/run/halo/app/extension/ReactiveExtensionClientTest.java index abf01c8d1..7e60e74f6 100644 --- a/src/test/java/run/halo/app/extension/ReactiveExtensionClientTest.java +++ b/src/test/java/run/halo/app/extension/ReactiveExtensionClientTest.java @@ -20,6 +20,9 @@ import static org.mockito.Mockito.when; import static run.halo.app.extension.GroupVersionKind.fromAPIVersionAndKind; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import java.util.List; import java.util.Map; import org.junit.jupiter.api.BeforeEach; @@ -29,6 +32,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.dao.DataIntegrityViolationException; import reactor.core.Exceptions; @@ -53,6 +57,11 @@ class ReactiveExtensionClientTest { @Mock SchemeManager schemeManager; + @Spy + ObjectMapper objectMapper = JsonMapper.builder() + .addModule(new JavaTimeModule()) + .build(); + @InjectMocks ReactiveExtensionClientImpl client; @@ -419,7 +428,7 @@ class ReactiveExtensionClientTest { .verifyComplete(); verify(storeClient).fetchByName(storeName); - verify(converter).convertTo(eq(fake)); + verify(converter).convertTo(isA(JsonExtension.class)); verify(converter, times(2)).convertFrom(same(FakeExtension.class), any()); verify(storeClient) .update(eq("/registry/fake.halo.run/fakes/fake"), eq(2L), any()); @@ -433,6 +442,7 @@ class ReactiveExtensionClientTest { Mono.just(createExtensionStore(storeName, 1L))); var oldFake = createFakeExtension("fake", 2L); + when(converter.convertFrom(same(FakeExtension.class), any())).thenReturn(oldFake); StepVerifier.create(client.update(fake)) @@ -470,7 +480,7 @@ class ReactiveExtensionClientTest { .verifyComplete(); verify(storeClient).fetchByName(name); - verify(converter).convertTo(eq(fake)); + verify(converter).convertTo(isA(JsonExtension.class)); verify(converter, times(2)).convertFrom(same(Unstructured.class), any()); verify(storeClient) .update(eq("/registry/fake.halo.run/fakes/fake"), eq(12345L), any());