From 36f61d4c5954312c4c72bf80c4c72300a8ae09a1 Mon Sep 17 00:00:00 2001 From: kbinswanger Date: Tue, 7 Jan 2020 15:51:05 -0600 Subject: [PATCH] First draft of adding a check-and-set to deleteKV The main dilemma with adding check-and-set to deleteKV is that all the delete KV methods return Response. Consequently we can't actually return whether the c-a-s worked. We can't change Response to Response without breaking a lot of existing code. This just adds the capability for check-and-set but punts on the Response problem. If callers want to see if the delete succeeded, they'll have to do another getKV. However, this is the least invasive version of this change. https://github.com/Ecwid/consul-api/issues/49 --- .../com/ecwid/consul/v1/ConsulClient.java | 16 ++++++++ .../ecwid/consul/v1/kv/KeyValueClient.java | 7 ++++ .../consul/v1/kv/KeyValueConsulClient.java | 31 +++++++++++--- .../consul/v1/kv/model/DeleteParams.java | 33 +++++++++++++++ .../v1/kv/KeyValueConsulClientTest.java | 41 ++++++++++++++++++- 5 files changed, 122 insertions(+), 6 deletions(-) create mode 100644 src/main/java/com/ecwid/consul/v1/kv/model/DeleteParams.java diff --git a/src/main/java/com/ecwid/consul/v1/ConsulClient.java b/src/main/java/com/ecwid/consul/v1/ConsulClient.java index 8d839641..c407c7d3 100644 --- a/src/main/java/com/ecwid/consul/v1/ConsulClient.java +++ b/src/main/java/com/ecwid/consul/v1/ConsulClient.java @@ -26,6 +26,7 @@ import com.ecwid.consul.v1.health.model.HealthService; import com.ecwid.consul.v1.kv.KeyValueClient; import com.ecwid.consul.v1.kv.KeyValueConsulClient; +import com.ecwid.consul.v1.kv.model.DeleteParams; import com.ecwid.consul.v1.kv.model.GetBinaryValue; import com.ecwid.consul.v1.kv.model.GetValue; import com.ecwid.consul.v1.kv.model.PutParams; @@ -759,11 +760,26 @@ public Response deleteKVValue(String key, QueryParams queryParams) { return keyValueClient.deleteKVValue(key, queryParams); } + @Override + public Response deleteKVValue(String key, DeleteParams deleteParams) { + return keyValueClient.deleteKVValue(key, deleteParams); + } + @Override public Response deleteKVValue(String key, String token, QueryParams queryParams) { return keyValueClient.deleteKVValue(key, token, queryParams); } + @Override + public Response deleteKVValue(String key, String token, DeleteParams deleteParams) { + return keyValueClient.deleteKVValue(key, token, deleteParams); + } + + @Override + public Response deleteKVValue(String key, String token, DeleteParams deleteParams, QueryParams queryParams) { + return keyValueClient.deleteKVValue(key, token, deleteParams, queryParams); + } + @Override public Response deleteKVValues(String key) { return keyValueClient.deleteKVValues(key); diff --git a/src/main/java/com/ecwid/consul/v1/kv/KeyValueClient.java b/src/main/java/com/ecwid/consul/v1/kv/KeyValueClient.java index 74ab1757..518f6cfc 100644 --- a/src/main/java/com/ecwid/consul/v1/kv/KeyValueClient.java +++ b/src/main/java/com/ecwid/consul/v1/kv/KeyValueClient.java @@ -2,6 +2,7 @@ import com.ecwid.consul.v1.QueryParams; import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.kv.model.DeleteParams; import com.ecwid.consul.v1.kv.model.GetBinaryValue; import com.ecwid.consul.v1.kv.model.GetValue; import com.ecwid.consul.v1.kv.model.PutParams; @@ -90,8 +91,14 @@ public interface KeyValueClient { public Response deleteKVValue(String key, QueryParams queryParams); + public Response deleteKVValue(String key, DeleteParams deleteParams); + public Response deleteKVValue(String key, String token, QueryParams queryParams); + public Response deleteKVValue(String key, String token, DeleteParams deleteParams); + + public Response deleteKVValue(String key, String token, DeleteParams deleteParams, QueryParams queryParams); + public Response deleteKVValues(String key); diff --git a/src/main/java/com/ecwid/consul/v1/kv/KeyValueConsulClient.java b/src/main/java/com/ecwid/consul/v1/kv/KeyValueConsulClient.java index 89d5de76..e8f63169 100644 --- a/src/main/java/com/ecwid/consul/v1/kv/KeyValueConsulClient.java +++ b/src/main/java/com/ecwid/consul/v1/kv/KeyValueConsulClient.java @@ -1,19 +1,24 @@ package com.ecwid.consul.v1.kv; -import java.util.List; - import com.ecwid.consul.ConsulException; import com.ecwid.consul.SingleUrlParameters; import com.ecwid.consul.UrlParameters; import com.ecwid.consul.json.GsonFactory; import com.ecwid.consul.transport.HttpResponse; import com.ecwid.consul.transport.TLSConfig; -import com.ecwid.consul.v1.*; +import com.ecwid.consul.v1.ConsulRawClient; +import com.ecwid.consul.v1.OperationException; +import com.ecwid.consul.v1.QueryParams; +import com.ecwid.consul.v1.Request; +import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.kv.model.DeleteParams; import com.ecwid.consul.v1.kv.model.GetBinaryValue; import com.ecwid.consul.v1.kv.model.GetValue; import com.ecwid.consul.v1.kv.model.PutParams; import com.google.gson.reflect.TypeToken; +import java.util.List; + /** * @author Vasily Vasilkov (vgv@ecwid.com) */ @@ -312,20 +317,36 @@ public Response deleteKVValue(String key) { @Override public Response deleteKVValue(String key, String token) { - return deleteKVValue(key, token, QueryParams.DEFAULT); + return deleteKVValue(key, token, null, QueryParams.DEFAULT); } @Override public Response deleteKVValue(String key, QueryParams queryParams) { - return deleteKVValue(key, null, queryParams); + return deleteKVValue(key, null, null, queryParams); + } + + @Override + public Response deleteKVValue(String key, DeleteParams deleteParams) { + return deleteKVValue(key, null, deleteParams, QueryParams.DEFAULT); } @Override public Response deleteKVValue(String key, String token, QueryParams queryParams) { + return deleteKVValue(key, null, null, queryParams); + } + + @Override + public Response deleteKVValue(String key, String token, DeleteParams deleteParams) { + return deleteKVValue(key, token, deleteParams, QueryParams.DEFAULT); + } + + @Override + public Response deleteKVValue(String key, String token, DeleteParams deleteParams, QueryParams queryParams) { Request request = Request.Builder.newBuilder() .setEndpoint("/v1/kv/" + key) .setToken(token) .addUrlParameter(queryParams) + .addUrlParameter(deleteParams) .build(); HttpResponse httpResponse = rawClient.makeDeleteRequest(request); diff --git a/src/main/java/com/ecwid/consul/v1/kv/model/DeleteParams.java b/src/main/java/com/ecwid/consul/v1/kv/model/DeleteParams.java new file mode 100644 index 00000000..94464380 --- /dev/null +++ b/src/main/java/com/ecwid/consul/v1/kv/model/DeleteParams.java @@ -0,0 +1,33 @@ +package com.ecwid.consul.v1.kv.model; + +import com.ecwid.consul.UrlParameters; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author Kevin Binswanger (kbinswanger@gmail.com) + */ +public class DeleteParams implements UrlParameters { + + private Long cas; + + public Long getCas() { + return cas; + } + + public void setCas(Long cas) { + this.cas = cas; + } + + @Override + public List toUrlParameters() { + List params = new ArrayList(); + + if (cas != null) { + params.add("cas=" + cas); + } + + return params; + } +} diff --git a/src/test/java/com/ecwid/consul/v1/kv/KeyValueConsulClientTest.java b/src/test/java/com/ecwid/consul/v1/kv/KeyValueConsulClientTest.java index adad46c1..3c2b0fda 100644 --- a/src/test/java/com/ecwid/consul/v1/kv/KeyValueConsulClientTest.java +++ b/src/test/java/com/ecwid/consul/v1/kv/KeyValueConsulClientTest.java @@ -1,10 +1,11 @@ package com.ecwid.consul.v1.kv; import com.ecwid.consul.ConsulTestConstants; +import com.ecwid.consul.v1.kv.model.DeleteParams; +import com.ecwid.consul.v1.kv.model.GetValue; import com.pszymczyk.consul.ConsulProcess; import com.pszymczyk.consul.ConsulStarterBuilder; import com.pszymczyk.consul.infrastructure.Ports; -import org.apache.commons.lang.math.RandomUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -68,6 +69,44 @@ void testDeleteKvValue() throws Exception { Assertions.assertNull(consulClient.getKVValue(testKey).getValue()); } + @Test + void testDeleteKvValueCas() { + final String testKey = "test_key"; + final String testValue = "test_value"; + final String testModifiedValue = "test_value2"; + + // Make sure there is no such key before test running + Assertions.assertNull(consulClient.getKVValue(testKey).getValue()); + + // Set the key + consulClient.setKVValue(testKey, testValue); + // Make sure test key exists + GetValue getValue = consulClient.getKVValue(testKey).getValue(); + Assertions.assertEquals(testValue, getValue.getDecodedValue()); + + // Modify the value + consulClient.setKVValue(testKey, testModifiedValue); + // Make sure it returns modified value + GetValue modifiedGetValue = consulClient.getKVValue(testKey).getValue(); + Assertions.assertEquals(testModifiedValue, modifiedGetValue.getDecodedValue()); + // Verify that consul changed the modified index + Assertions.assertNotEquals(getValue.getModifyIndex(), modifiedGetValue.getModifyIndex()); + + // Delete key with the old modify index + DeleteParams deleteOldIndex = new DeleteParams(); + deleteOldIndex.setCas(getValue.getModifyIndex()); + consulClient.deleteKVValue(testKey, deleteOldIndex); + // Make sure the key is still there + Assertions.assertEquals(testModifiedValue, consulClient.getKVValue(testKey).getValue().getDecodedValue()); + + // Delete the key with the new modify index + DeleteParams deleteNewIndex = new DeleteParams(); + deleteNewIndex.setCas(modifiedGetValue.getModifyIndex()); + consulClient.deleteKVValue(testKey, deleteNewIndex); + // Make sure the key is gone + Assertions.assertNull(consulClient.getKVValue(testKey).getValue()); + } + @Test void testDeleteKvValues() throws Exception { final String testKeyPrefix = "test_key";