From c7f47b2d5b7580a90555360a239d6c05af47fd3a Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 22 Sep 2023 13:55:07 +0200 Subject: [PATCH] Polishing. Add author and since tags. Refine nullability and toString rendering. Original pull request: gh-808 See gh-789 --- .../core/VaultKeyValueMetadataTemplate.java | 9 +-- .../VaultVersionedKeyValueOperations.java | 1 + .../core/VaultVersionedKeyValueTemplate.java | 10 +-- .../vault/support/VaultMetadataRequest.java | 74 +++++++++++-------- .../vault/support/VaultMetadataResponse.java | 46 ++++++------ .../vault/support/Versioned.java | 60 ++++++++------- ...ionedKeyValueTemplateIntegrationTests.java | 7 +- 7 files changed, 112 insertions(+), 95 deletions(-) diff --git a/spring-vault-core/src/main/java/org/springframework/vault/core/VaultKeyValueMetadataTemplate.java b/spring-vault-core/src/main/java/org/springframework/vault/core/VaultKeyValueMetadataTemplate.java index 9be44522..f5caee03 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/core/VaultKeyValueMetadataTemplate.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/core/VaultKeyValueMetadataTemplate.java @@ -36,6 +36,7 @@ import org.springframework.vault.support.Versioned; * * @author Zakaria Amine * @author Mark Paluch + * @author Jeroen Willemsen * @since 2.3 */ class VaultKeyValueMetadataTemplate implements VaultKeyValueMetadataOperations { @@ -120,12 +121,8 @@ class VaultKeyValueMetadataTemplate implements VaultKeyValueMetadataOperations { .createdAt(createdTime) .deletedAt(deletionTime) .destroyed(destroyed) - .version(kvVersion); - - if (versionData.get("custom_metadata") != null) { - Map customMetadata = (Map) versionData.get("custom_metadata"); - builder.customMetadata(customMetadata); - } + .version(kvVersion) + .customMetadata((Map) versionData.get("custom_metadata")); return builder.build(); } diff --git a/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueOperations.java b/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueOperations.java index f9df8f7d..8bbfb8ea 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueOperations.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueOperations.java @@ -43,6 +43,7 @@ public interface VaultVersionedKeyValueOperations extends VaultKeyValueOperation * @param path must not be {@literal null}. * @return the data. May be {@literal null} if the path does not exist. */ + @Override @Nullable default Versioned> get(String path) { return get(path, Version.unversioned()); diff --git a/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueTemplate.java b/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueTemplate.java index dde6be23..e5980ac4 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueTemplate.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/core/VaultVersionedKeyValueTemplate.java @@ -46,6 +46,7 @@ import org.springframework.web.client.HttpStatusCodeException; * * @author Mark Paluch * @author Maciej Drozdzowski + * @author Jeroen Willemsen * @since 2.1 */ public class VaultVersionedKeyValueTemplate extends VaultKeyValue2Accessor implements VaultVersionedKeyValueOperations { @@ -159,6 +160,7 @@ public class VaultVersionedKeyValueTemplate extends VaultKeyValue2Accessor imple return getMetadata(response.getRequiredData()); } + @SuppressWarnings("unchecked") private static Metadata getMetadata(Map responseMetadata) { MetadataBuilder builder = Metadata.builder(); @@ -176,12 +178,8 @@ public class VaultVersionedKeyValueTemplate extends VaultKeyValue2Accessor imple } Integer version = (Integer) responseMetadata.get("version"); - builder.version(Version.from(version)); - - if (responseMetadata.get("custom_metadata") != null) { - Map customMetadata = (Map) responseMetadata.get("custom_metadata"); - builder.customMetadata(customMetadata); - } + builder.version(Version.from(version)) + .customMetadata((Map) responseMetadata.get("custom_metadata")); return builder.build(); } diff --git a/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataRequest.java b/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataRequest.java index 88a5dcec..7f114da3 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataRequest.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataRequest.java @@ -26,6 +26,7 @@ import org.springframework.lang.Nullable; * Value object to bind Vault HTTP kv metadata update API requests. * * @author Zakaria Amine + * @author Jeroen Willemsen * @see Update * Metadata @@ -43,28 +44,21 @@ public class VaultMetadataRequest { private final String deleteVersionAfter; @JsonProperty("custom_metadata") - private final Map customMetadata; + private final @Nullable Map customMetadata; - private VaultMetadataRequest(int maxVersions, boolean casRequired, @Nullable Duration deleteVersionAfter, - @Nullable Map customMetadata) { - this.maxVersions = maxVersions; + private VaultMetadataRequest(boolean casRequired, @Nullable Map customMetadata, + @Nullable Duration deleteVersionAfter, int maxVersions) { this.casRequired = casRequired; + this.customMetadata = customMetadata; this.deleteVersionAfter = DurationParser .formatDuration(deleteVersionAfter != null ? deleteVersionAfter : Duration.ZERO); - this.customMetadata = customMetadata; + this.maxVersions = maxVersions; } public static VaultMetadataRequestBuilder builder() { return new VaultMetadataRequestBuilder(); } - /** - * @return The number of versions to keep per key. - */ - public int getMaxVersions() { - return this.maxVersions; - } - /** * @return If true all keys will require the cas parameter to be set on all write * requests. @@ -73,6 +67,11 @@ public class VaultMetadataRequest { return this.casRequired; } + @Nullable + public Map getCustomMetadata() { + return this.customMetadata; + } + /** * @return the deletion_time for all new versions written to this key. Accepts * Go duration format string. @@ -81,31 +80,33 @@ public class VaultMetadataRequest { return this.deleteVersionAfter; } - @Nullable - public Map getCustomMetadata() { - return this.customMetadata; + /** + * @return The number of versions to keep per key. + */ + public int getMaxVersions() { + return this.maxVersions; } public static class VaultMetadataRequestBuilder { - private int maxVersions; - private boolean casRequired; - @Nullable - private Duration deleteVersionAfter; - @Nullable private Map customMetadata; + @Nullable + private Duration deleteVersionAfter; + + private int maxVersions; + /** - * Set the number of versions to keep per key. - * @param maxVersions + * Set the cas_required parameter to {@code true} to require the cas parameter to + * be set on all write requests. * @return {@link VaultMetadataRequest} + * @since 3.1 */ - public VaultMetadataRequestBuilder maxVersions(int maxVersions) { - this.maxVersions = maxVersions; - return this; + public VaultMetadataRequestBuilder casRequired() { + return casRequired(true); } /** @@ -119,6 +120,17 @@ public class VaultMetadataRequest { return this; } + /** + * Sets the custom Metadata for the metadata request. + * @param customMetadata + * @return {@link VaultMetadataRequest} + * @since 3.1 + */ + public VaultMetadataRequestBuilder customMetadata(Map customMetadata) { + this.customMetadata = customMetadata; + return this; + } + /** * Sets the deletion time for all new versions written to this key. * @param deleteVersionAfter @@ -130,12 +142,12 @@ public class VaultMetadataRequest { } /** - * Sets the custom Metadata for the metadatarequest - * @param customMetadata + * Set the number of versions to keep per key. + * @param maxVersions * @return {@link VaultMetadataRequest} */ - public VaultMetadataRequestBuilder customMetadata(Map customMetadata) { - this.customMetadata = customMetadata; + public VaultMetadataRequestBuilder maxVersions(int maxVersions) { + this.maxVersions = maxVersions; return this; } @@ -143,8 +155,8 @@ public class VaultMetadataRequest { * @return a new {@link VaultMetadataRequest} */ public VaultMetadataRequest build() { - return new VaultMetadataRequest(this.maxVersions, this.casRequired, this.deleteVersionAfter, - this.customMetadata); + return new VaultMetadataRequest(this.casRequired, this.customMetadata, this.deleteVersionAfter, + this.maxVersions); } } diff --git a/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataResponse.java b/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataResponse.java index 1b8aafa1..15d83d66 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataResponse.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/support/VaultMetadataResponse.java @@ -18,15 +18,18 @@ package org.springframework.vault.support; import java.time.Duration; import java.time.Instant; import java.time.Period; +import java.util.Collections; import java.util.List; import java.util.Map; import org.springframework.lang.Nullable; +import org.springframework.vault.support.Versioned.Metadata; /** * Value object to bind Vault HTTP kv read metadata API responses. * * @author Zakaria Amine + * @author Jeroen Willemsen * @since 2.3 */ public class VaultMetadataResponse { @@ -37,6 +40,8 @@ public class VaultMetadataResponse { private final int currentVersion; + private final Map customMetadata; + private final Duration deleteVersionAfter; private final int maxVersions; @@ -47,20 +52,19 @@ public class VaultMetadataResponse { private final List versions; - private final Map customMetadata; - private VaultMetadataResponse(boolean casRequired, Instant createdTime, int currentVersion, - Duration deleteVersionAfter, int maxVersions, int oldestVersion, Instant updatedTime, - List versions, Map customMetadata) { + Map customMetadata, Duration deleteVersionAfter, int maxVersions, int oldestVersion, + Instant updatedTime, List versions) { + this.casRequired = casRequired; this.createdTime = createdTime; + this.customMetadata = customMetadata; this.currentVersion = currentVersion; this.deleteVersionAfter = deleteVersionAfter; this.maxVersions = maxVersions; this.oldestVersion = oldestVersion; this.updatedTime = updatedTime; this.versions = versions; - this.customMetadata = customMetadata; } public static VaultMetadataResponseBuilder builder() { @@ -98,9 +102,9 @@ public class VaultMetadataResponse { } /** - * @return KV of customMetadata. Entries can be any arbitrary key-value pairs + * @return the custom metadata. Entries can be any arbitrary key-value pairs + * @since 3.1 */ - @Nullable public Map getCustomMetadata() { return this.customMetadata; } @@ -127,12 +131,10 @@ public class VaultMetadataResponse { } /** - * Follows the following format. - * - * "versions": { "1": { "created_time": "2020-05-18T12:23:09.895587932Z", - * "deletion_time": "2020-05-18T12:31:00.66257744Z", "destroyed": false }, "2": { - * "created_time": "2020-05-18T12:23:10.122081788Z", "deletion_time": "", "destroyed": - * false } } + * Follows the following format. "versions": { "1": { "created_time": + * "2020-05-18T12:23:09.895587932Z", "deletion_time": "2020-05-18T12:31:00.66257744Z", + * "destroyed": false }, "2": { "created_time": "2020-05-18T12:23:10.122081788Z", + * "deletion_time": "", "destroyed": false } } * @return the key versions and their details */ public List getVersions() { @@ -143,6 +145,8 @@ public class VaultMetadataResponse { private boolean casRequired; + private Map customMetadata; + private Instant createdTime; private int currentVersion; @@ -157,8 +161,6 @@ public class VaultMetadataResponse { private List versions; - private Map customMetadata; - public VaultMetadataResponseBuilder casRequired(boolean casRequired) { this.casRequired = casRequired; return this; @@ -174,6 +176,11 @@ public class VaultMetadataResponse { return this; } + public VaultMetadataResponseBuilder customMetadata(@Nullable Map customMetadata) { + this.customMetadata = customMetadata != null ? customMetadata : Collections.emptyMap(); + return this; + } + public VaultMetadataResponseBuilder deleteVersionAfter(Duration deleteVersionAfter) { this.deleteVersionAfter = deleteVersionAfter; return this; @@ -199,15 +206,10 @@ public class VaultMetadataResponse { return this; } - public VaultMetadataResponseBuilder customMetadata(Map customMetadata) { - this.customMetadata = customMetadata; - return this; - } - public VaultMetadataResponse build() { return new VaultMetadataResponse(this.casRequired, this.createdTime, this.currentVersion, - this.deleteVersionAfter, this.maxVersions, this.oldestVersion, this.updatedTime, this.versions, - this.customMetadata); + this.customMetadata, this.deleteVersionAfter, this.maxVersions, this.oldestVersion, + this.updatedTime, this.versions); } } diff --git a/spring-vault-core/src/main/java/org/springframework/vault/support/Versioned.java b/spring-vault-core/src/main/java/org/springframework/vault/support/Versioned.java index 950a7ac2..4d9d7129 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/support/Versioned.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/support/Versioned.java @@ -16,17 +16,19 @@ package org.springframework.vault.support; import java.time.Instant; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.Optional; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; /** * Value object representing versioned secrets along {@link Version} metadata. A versioned * object can hold various states to represent: - * *
    *
  • Initial (not yet versioned) secrets via {@link Versioned#create(Object)}
    • *
  • Versioned secrets via {@link Versioned#create(Object, Version)}
    • @@ -35,7 +37,6 @@ import org.springframework.util.Assert; *
*

* Versioned secrets follow a lifecycle that spans from creation to destruction: - * *

    *
  1. Creation of an unversioned secret: Secret is not yet persisted.
    2. *
  2. Versioned secret: Secret is persisted.
    3. @@ -45,6 +46,7 @@ import org.springframework.util.Assert; *
* * @author Mark Paluch + * @author Jeroen Willemsen * @see Version * @see Metadata * @since 2.1 @@ -218,21 +220,22 @@ public class Versioned { private final Instant createdAt; + private final @Nullable Map customMetadata; + private final @Nullable Instant deletedAt; private final boolean destroyed; private final Version version; - private final @Nullable Map customMetadata; + private Metadata(Instant createdAt, @Nullable Map customMetadata, @Nullable Instant deletedAt, + boolean destroyed, Version version) { - private Metadata(Instant createdAt, @Nullable Instant deletedAt, boolean destroyed, Version version, - @Nullable Map customMetadata) { this.createdAt = createdAt; + this.customMetadata = customMetadata; this.deletedAt = deletedAt; this.destroyed = destroyed; this.version = version; - this.customMetadata = customMetadata; } /** @@ -243,6 +246,13 @@ public class Versioned { return new MetadataBuilder(); } + /** + * @return custom metadata, if provided. + */ + public Map getCustomMetadata() { + return customMetadata == null ? Collections.emptyMap() : customMetadata; + } + /** * @return {@link Instant} at which the version was created. */ @@ -280,27 +290,19 @@ public class Versioned { return this.destroyed; } - /** - * @return Metadata . - */ - @Nullable - public Map getCustomMetadata() { - return customMetadata; - } - @Override public String toString() { - String customMetadataString = ""; + StringBuilder customMetadataString = new StringBuilder(", customMetadata=["); if (customMetadata != null && customMetadata.keySet().size() > 0) { - StringBuilder metadataPrintBuilder = new StringBuilder(", customMetadata=Map["); + for (String key : customMetadata.keySet()) { - metadataPrintBuilder.append(key).append(":").append(customMetadata.get(key)).append(" "); + customMetadataString.append(key).append(":").append(customMetadata.get(key)).append(" "); } - metadataPrintBuilder.append("]"); - customMetadataString = metadataPrintBuilder.toString(); } + customMetadataString.append("]"); + return getClass().getSimpleName() + " [createdAt=" + this.createdAt + ", deletedAt=" + this.deletedAt + ", destroyed=" + this.destroyed + ", version=" + this.version + customMetadataString + ']'; } @@ -379,12 +381,17 @@ public class Versioned { return this; } + /** + * Configure the custom metadata map. + * @param customMetadata must not be {@literal null} and not empty. + * @return {@code this} {@link MetadataBuilder}. + * @since 3.1 + */ public MetadataBuilder customMetadata(Map customMetadata) { - Assert.notNull(customMetadata, "customMetadata should not be null"); - Assert.notEmpty(customMetadata.keySet(), "customMetadata should have at least one key"); - Assert.notEmpty(customMetadata.values(), "customMetadata should have at least one value"); - this.customMetadata = customMetadata; + this.customMetadata = customMetadata != null && !CollectionUtils.isEmpty(customMetadata) + ? new LinkedHashMap<>(customMetadata) : null; + return this; } @@ -399,7 +406,7 @@ public class Versioned { Assert.notNull(this.createdAt, "CreatedAt must not be null"); Assert.notNull(this.version, "Version must not be null"); - return new Metadata(this.createdAt, this.deletedAt, this.destroyed, this.version, this.customMetadata); + return new Metadata(this.createdAt, this.customMetadata, this.deletedAt, this.destroyed, this.version); } } @@ -469,10 +476,9 @@ public class Versioned { public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof Version)) + if (!(o instanceof Version other)) return false; - Version version1 = (Version) o; - return this.version == version1.version; + return this.version == other.version; } @Override diff --git a/spring-vault-core/src/test/java/org/springframework/vault/core/VaultVersionedKeyValueTemplateIntegrationTests.java b/spring-vault-core/src/test/java/org/springframework/vault/core/VaultVersionedKeyValueTemplateIntegrationTests.java index 1e49cd24..13d859dc 100644 --- a/spring-vault-core/src/test/java/org/springframework/vault/core/VaultVersionedKeyValueTemplateIntegrationTests.java +++ b/spring-vault-core/src/test/java/org/springframework/vault/core/VaultVersionedKeyValueTemplateIntegrationTests.java @@ -45,6 +45,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; * Integration tests for {@link VaultVersionedKeyValueTemplate}. * * @author Mark Paluch + * @author Jeroen Willemsen */ @ExtendWith(SpringExtension.class) @RequiresVaultVersion(VaultInitializer.VERSIONING_INTRODUCED_WITH_VALUE) @@ -88,6 +89,7 @@ class VaultVersionedKeyValueTemplateIntegrationTests extends IntegrationTestSupp Versioned versioned = this.versionedOperations.get(key, Person.class); assertThat(versioned.getRequiredData()).isEqualTo(person); + assertThat(versioned.getRequiredMetadata().getCustomMetadata()).isEmpty(); } @Test @@ -107,6 +109,7 @@ class VaultVersionedKeyValueTemplateIntegrationTests extends IntegrationTestSupp @Test void shouldWriteSecretWithCustomMetadata() { + Person person = new Person(); person.setFirstname("Walter"); person.setLastname("White"); @@ -124,9 +127,7 @@ class VaultVersionedKeyValueTemplateIntegrationTests extends IntegrationTestSupp this.versionedOperations.opsForKeyValueMetadata().put(key, request); Versioned versioned = this.versionedOperations.get(key, Person.class); - - assertThat(versioned.getMetadata().getCustomMetadata().get("foo")).isEqualTo("bar"); - + assertThat(versioned.getRequiredMetadata().getCustomMetadata()).containsEntry("foo", "bar"); } @Test