Polishing.

Align Object Mapping with imperative approach. Refactor read methods to improve empty handling without the use of exceptions. Revert code reorganization. Improve encapsulation.

Reformat code. Simplify tests.

See gh-576
Original pull request: gh-807

Author: mp911de

spring-vault-core/src/main/java/org/springframework/vault/client/VaultResponses.java

@@ -33,7 +33,6 @@
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 import org.springframework.vault.VaultException;
-import org.springframework.vault.support.VaultResponseDataVersion2;
 import org.springframework.vault.support.VaultResponseSupport;
 import org.springframework.web.client.HttpStatusCodeException;
 
@@ -135,68 +134,6 @@ public Type getType() {
 		};
 	}
 
-	public static <T> ParameterizedTypeReference<VaultResponseSupport<T>> getTypeReference(
-			final ParameterizedTypeReference<T> responseType) {
-
-		Assert.notNull(responseType, "Response type must not be null");
-
-		final Type supportType = new ParameterizedType() {
-
-			@Override
-			public Type[] getActualTypeArguments() {
-				return new Type[] { responseType.getType() };
-			}
-
-			@Override
-			public Type getRawType() {
-				return VaultResponseSupport.class;
-			}
-
-			@Override
-			public Type getOwnerType() {
-				return VaultResponseSupport.class;
-			}
-		};
-
-		return new ParameterizedTypeReference<VaultResponseSupport<T>>() {
-			@Override
-			public Type getType() {
-				return supportType;
-			}
-		};
-	}
-
-	public static <T> ParameterizedTypeReference<VaultResponseDataVersion2<T>> getDataTypeReference(
-			final Class<T> responseType) {
-
-		Assert.notNull(responseType, "Response type must not be null");
-
-		final Type supportType = new ParameterizedType() {
-
-			@Override
-			public Type[] getActualTypeArguments() {
-				return new Type[] { responseType };
-			}
-
-			@Override
-			public Type getRawType() {
-				return VaultResponseDataVersion2.class;
-			}
-
-			@Override
-			public Type getOwnerType() {
-				return VaultResponseDataVersion2.class;
-			}
-		};
-
-		return new ParameterizedTypeReference<VaultResponseDataVersion2<T>>() {
-			@Override
-			public Type getType() {
-				return supportType;
-			}
-		};
-	}
-
 	/**
 	 * Obtain the error message from a JSON response.
 	 * @param json must not be {@literal null}.

spring-vault-core/src/main/java/org/springframework/vault/core/VaultKeyValueUtilities.java renamed to spring-vault-core/src/main/java/org/springframework/vault/core/KeyValueUtilities.java

@@ -4,10 +4,15 @@
 import java.time.Instant;
 import java.time.format.DateTimeFormatter;
 import java.time.temporal.TemporalAccessor;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 import org.springframework.vault.support.DurationParser;
 import org.springframework.vault.support.VaultMetadataResponse;
@@ -16,8 +21,16 @@
 import org.springframework.vault.support.Versioned.Metadata.MetadataBuilder;
 import org.springframework.vault.support.Versioned.Version;
 
-class VaultKeyValueUtilities {
+/**
+ * Common utility methods to map raw vault data structures to Spring Vault objects
+ *
+ * @author Timothy R. Weiand
+ * @author Mark Paluch
+ * @since 3.1
+ */
+class KeyValueUtilities {
 
+	@SuppressWarnings("unchecked")
 	static Metadata getMetadata(Map<String, Object> responseMetadata) {
 
 		MetadataBuilder builder = Metadata.builder();
@@ -34,6 +47,8 @@ static Metadata getMetadata(Map<String, Object> responseMetadata) {
 			builder.destroyed();
 		}
 
+		builder.customMetadata((Map) responseMetadata.get("custom_metadata"));
+
 		Integer version = (Integer) responseMetadata.get("version");
 		builder.version(Version.from(version));
 
@@ -95,4 +110,24 @@ private static Versioned.Metadata buildVersion(String version, Map<String, Objec
 			.build();
 	}
 
+	static Map<String, Object> createPatchRequest(Map<String, ?> patch, Map<String, Object> previous,
+			Map<String, Object> metadata) {
+
+		Map<String, Object> result = new LinkedHashMap<>(previous);
+		result.putAll(patch);
+
+		Map<String, Object> body = new HashMap<>();
+		body.put("data", result);
+		body.put("options", Collections.singletonMap("cas", metadata.get("version")));
+
+		return body;
+	}
+
+	static String normalizeListPath(String path) {
+
+		Assert.notNull(path, "Path must not be null");
+
+		return path.equals("/") ? "" : path.endsWith("/") ? path : path + "/";
+	}
+
 }

spring-vault-core/src/main/java/org/springframework/vault/core/ReactiveKeyValueHelper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2022 the original author or authors.
+ * Copyright 2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,24 +16,28 @@
 package org.springframework.vault.core;
 
 import java.util.Map;
-import org.springframework.core.ParameterizedTypeReference;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
 import org.springframework.util.Assert;
 import org.springframework.vault.core.VaultKeyValueOperationsSupport.KeyValueBackend;
 import org.springframework.vault.support.VaultResponse;
 import org.springframework.vault.support.VaultResponseSupport;
-import org.springframework.web.reactive.function.client.WebClientResponseException;
-import reactor.core.publisher.Flux;
-import reactor.core.publisher.Mono;
 
 /**
  * Default implementation of {@link ReactiveVaultKeyValueOperations} for the Key/Value
  * backend version 1.
  *
  * @author Timothy R. Weiand
+ * @author Mark Paluch
  * @since 3.1
  */
 class ReactiveVaultKeyValue1Template extends ReactiveVaultKeyValueAccessor implements ReactiveVaultKeyValueOperations {
 
+	private final String path;
+
 	/**
 	 * Create a new {@link ReactiveVaultKeyValue1Template} given
 	 * {@link ReactiveVaultOperations} and the mount {@code path}.
@@ -43,6 +47,7 @@ class ReactiveVaultKeyValue1Template extends ReactiveVaultKeyValueAccessor imple
 	public ReactiveVaultKeyValue1Template(ReactiveVaultOperations vaultOperations, String path) {
 
 		super(vaultOperations, path);
+		this.path = path;
 	}
 
 	@Override
@@ -53,22 +58,28 @@ public Flux<String> list(String path) {
 	@Override
 	@SuppressWarnings("unchecked")
 	public Mono<VaultResponse> get(String path) {
-		ParameterizedTypeReference<Map<String, Object>> ref = new ParameterizedTypeReference<>() {
-		};
-
-		return doRead(path, ref).onErrorResume(WebClientResponseException.NotFound.class, e -> Mono.empty())
-			.map(response -> {
-				VaultResponse vaultResponse = new VaultResponse();
-				VaultResponseSupport.updateWithoutData(vaultResponse, response);
-				vaultResponse.setData(response.getData());
-				return vaultResponse;
-			});
+
+		return doRead(path, Map.class, (response, map) -> {
+			VaultResponse vaultResponse = new VaultResponse();
+			VaultResponseSupport.updateWithoutData(vaultResponse, response);
+			vaultResponse.setData(map);
+			return vaultResponse;
+		});
 	}
 
 	@Override
+	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public <T> Mono<VaultResponseSupport<T>> get(String path, Class<T> responseType) {
 
-		return doRead(path, responseType).onErrorResume(WebClientResponseException.NotFound.class, e -> Mono.empty());
+		Assert.hasText(path, "Path must not be empty");
+		Assert.notNull(responseType, "Response type must not be null");
+
+		return doRead(path, responseType, (response, data) -> {
+
+			VaultResponseSupport result = response;
+			result.setData(data);
+			return result;
+		});
 	}
 
 	@Override
@@ -89,6 +100,11 @@ public KeyValueBackend getApiVersion() {
 		return KeyValueBackend.KV_1;
 	}
 
+	@Override
+	JsonNode getJsonNode(VaultResponseSupport<JsonNode> response) {
+		return response.getRequiredData();
+	}
+
 	@Override
 	String createDataPath(String path) {
 		return String.format("%s/%s", this.path, path);

spring-vault-core/src/main/java/org/springframework/vault/core/ReactiveVaultKeyValue1Template.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2022 the original author or authors.
+ * Copyright 2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,23 +16,24 @@
 package org.springframework.vault.core;
 
 import java.util.List;
-import java.util.Map;
-import org.springframework.core.ParameterizedTypeReference;
-import org.springframework.vault.client.VaultResponses;
+
 import org.springframework.vault.core.VaultKeyValueOperationsSupport.KeyValueBackend;
 import org.springframework.vault.support.VaultResponseSupport;
+
+import com.fasterxml.jackson.databind.JsonNode;
 import reactor.core.publisher.Flux;
 
 /**
  * Support class to build accessor methods for the Vault key-value backend version 2.
  *
  * @author Timothy R. Weiand
+ * @author Mark Paluch
  * @since 3.1
  * @see KeyValueBackend#KV_2
  */
 abstract class ReactiveVaultKeyValue2Accessor extends ReactiveVaultKeyValueAccessor {
 
-	final String path;
+	private final String path;
 
 	/**
 	 * Create a new {@link ReactiveVaultKeyValue2Accessor} given {@link VaultOperations}
@@ -51,22 +52,13 @@ abstract class ReactiveVaultKeyValue2Accessor extends ReactiveVaultKeyValueAcces
 	@SuppressWarnings("unchecked")
 	public Flux<String> list(String path) {
 
-		String pathToUse = path.equals("/") ? "" : path.endsWith("/") ? path : (path + "/");
-
-		// TODO: to test - null returns empty
-		ParameterizedTypeReference<VaultResponseSupport<Map<String, Object>>> type = VaultResponses
-			.getTypeReference(new ParameterizedTypeReference<>() {
-			});
-
-		return doReadRaw(String.format("%s?list=true", createBackendPath("metadata", pathToUse)), type, false)
-			.flatMap(ReactiveKeyValueHelper::getRequiredData)
+		return doRead(
+				String.format("%s?list=true", createBackendPath("metadata", KeyValueUtilities.normalizeListPath(path))),
+				VaultListResponse.class)
 			.flatMapMany(response -> {
-				final List<String> list = (List<String>) response.get("keys");
-				if (null == list) {
-					return Flux.empty();
-				}
-				return Flux.fromIterable(list);
 
+				List<String> list = (List<String>) response.getRequiredData().get("keys");
+				return null == list ? Flux.empty() : Flux.fromIterable(list);
 			});
 	}
 
@@ -75,6 +67,12 @@ public KeyValueBackend getApiVersion() {
 		return KeyValueBackend.KV_2;
 	}
 
+	@Override
+	JsonNode getJsonNode(VaultResponseSupport<JsonNode> response) {
+		return response.getRequiredData().at("/data");
+	}
+
+	@Override
 	String createDataPath(String path) {
 		return createBackendPath("data", path);
 	}