Repository: helix Updated Branches: refs/heads/master b235c4ee5 -> ceba1a55a
[HELIX-776] REST2.0: Add delete command to updateInstanceConfig For instance configs, REST2.0 did not expose the REST API for deletion of fields. This RB adds update and delete commands to updateInstanceConfig and an integration test thereof. Changelist: 1. Add delete command to updateInstanceConfig in InstanceAccessor 2. Add integration tests Project: http://git-wip-us.apache.org/repos/asf/helix/repo Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/6090732b Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/6090732b Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/6090732b Branch: refs/heads/master Commit: 6090732be6b88863017a93106fa692dc7350520b Parents: b235c4e Author: Hunter Lee <[email protected]> Authored: Wed Oct 31 14:20:18 2018 -0700 Committer: Hunter Lee <[email protected]> Committed: Wed Oct 31 14:20:18 2018 -0700 ---------------------------------------------------------------------- .../java/org/apache/helix/ConfigAccessor.java | 6 + .../resources/helix/InstanceAccessor.java | 34 +++++- .../helix/rest/server/TestInstanceAccessor.java | 114 ++++++++++++++++++- 3 files changed, 146 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/helix/blob/6090732b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java ---------------------------------------------------------------------- diff --git a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java index 2755113..53f42fb 100644 --- a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java +++ b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java @@ -765,6 +765,12 @@ public class ConfigAccessor { .forParticipant(instanceName).build(); String zkPath = scope.getZkPath(); + if (!zkClient.exists(zkPath)) { + throw new HelixException( + "updateInstanceConfig failed. Given InstanceConfig does not already exist. instance: " + + instanceName); + } + if (overwrite) { ZKUtil.createOrReplace(zkClient, zkPath, instanceConfig.getRecord(), true); } else { http://git-wip-us.apache.org/repos/asf/helix/blob/6090732b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java ---------------------------------------------------------------------- diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java index 98af0ee..38ee3b5 100644 --- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java +++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java @@ -41,10 +41,12 @@ import org.apache.helix.model.ClusterConfig; import org.apache.helix.model.CurrentState; import org.apache.helix.model.Error; import org.apache.helix.model.HealthStat; +import org.apache.helix.model.HelixConfigScope; import org.apache.helix.model.InstanceConfig; import org.apache.helix.model.LiveInstance; import org.apache.helix.model.Message; import org.apache.helix.model.ParticipantHistory; +import org.apache.helix.model.builder.HelixConfigScopeBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.codehaus.jackson.JsonNode; @@ -321,7 +323,19 @@ public class InstanceAccessor extends AbstractHelixResource { @POST @Path("{instanceName}/configs") public Response updateInstanceConfig(@PathParam("clusterId") String clusterId, - @PathParam("instanceName") String instanceName, String content) { + @PathParam("instanceName") String instanceName, @QueryParam("command") String commandStr, + String content) { + Command command; + if (commandStr == null || commandStr.isEmpty()) { + command = Command.update; // Default behavior to keep it backward-compatible + } else { + try { + command = getCommand(commandStr); + } catch (HelixException ex) { + return badRequest(ex.getMessage()); + } + } + ZNRecord record; try { record = toZNRecord(content); @@ -332,11 +346,25 @@ public class InstanceAccessor extends AbstractHelixResource { InstanceConfig instanceConfig = new InstanceConfig(record); ConfigAccessor configAccessor = getConfigAccessor(); try { - configAccessor.updateInstanceConfig(clusterId, instanceName, instanceConfig); + switch (command) { + case update: + configAccessor.updateInstanceConfig(clusterId, instanceName, instanceConfig); + break; + case delete: { + HelixConfigScope instanceScope = + new HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.PARTICIPANT) + .forCluster(clusterId).forParticipant(instanceName).build(); + configAccessor.remove(instanceScope, record); + } + break; + default: + return badRequest(String.format("Unsupported command: %s", command)); + } } catch (HelixException ex) { return notFound(ex.getMessage()); } catch (Exception ex) { - _logger.error(String.format("Error in update instance config for instance: %s", instanceName), ex); + _logger.error(String.format("Error in update instance config for instance: %s", instanceName), + ex); return serverError(ex); } return OK(); http://git-wip-us.apache.org/repos/asf/helix/blob/6090732b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java ---------------------------------------------------------------------- diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java index 24d2910..94c28b2 100644 --- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java +++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableMap; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -201,20 +202,56 @@ public class TestInstanceAccessor extends AbstractTestClass { new HashSet<>(Arrays.asList(CLUSTER_NAME + dbName + "0", CLUSTER_NAME + dbName + "3"))); } + /** + * Test "update" command for updateInstanceConfig endpoint. + * @throws IOException + */ @Test(dependsOnMethods = "updateInstance") public void updateInstanceConfig() throws IOException { System.out.println("Start test :" + TestHelper.getTestMethodName()); String instanceName = CLUSTER_NAME + "localhost_12918"; InstanceConfig instanceConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName); ZNRecord record = instanceConfig.getRecord(); - record.getSimpleFields().put("TestSimple", "value"); - record.getMapFields().put("TestMap", ImmutableMap.of("key", "value")); - record.getListFields().put("TestList", Arrays.asList("e1", "e2", "e3")); + // Generate a record containing three keys (k0, k1, k2) for all fields + String value = "value"; + for (int i = 0; i < 3; i++) { + String key = "k" + i; + record.getSimpleFields().put(key, value); + record.getMapFields().put(key, ImmutableMap.of(key, value)); + record.getListFields().put(key, Arrays.asList(key, value)); + } + + // 1. Add these fields by way of "update" Entity entity = Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE); - post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs", null, entity, - Response.Status.OK.getStatusCode()); + post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs", + Collections.singletonMap("command", "update"), entity, Response.Status.OK.getStatusCode()); + + // Check that the fields have been added + Assert.assertEquals(record.getSimpleFields(), + _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() + .getSimpleFields()); + Assert.assertEquals(record.getListFields(), + _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord().getListFields()); + Assert.assertEquals(record.getMapFields(), + _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord().getMapFields()); + + String newValue = "newValue"; + // 2. Modify the record and update + for (int i = 0; i < 3; i++) { + String key = "k" + i; + record.getSimpleFields().put(key, newValue); + record.getMapFields().put(key, ImmutableMap.of(key, newValue)); + record.getListFields().put(key, Arrays.asList(key, newValue)); + } + + entity = + Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE); + post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs", + Collections.singletonMap("command", "update"), entity, Response.Status.OK.getStatusCode()); + + // Check that the fields have been modified Assert.assertEquals(record.getSimpleFields(), _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() .getSimpleFields()); @@ -223,4 +260,71 @@ public class TestInstanceAccessor extends AbstractTestClass { Assert.assertEquals(record.getMapFields(), _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord().getMapFields()); } + + /** + * Test the "delete" command of updateInstanceConfig. + * @throws IOException + */ + @Test(dependsOnMethods = "updateInstanceConfig") + public void deleteInstanceConfig() throws IOException { + System.out.println("Start test :" + TestHelper.getTestMethodName()); + String instanceName = CLUSTER_NAME + "localhost_12918"; + ZNRecord record = new ZNRecord(instanceName); + + // Generate a record containing three keys (k1, k2, k3) for all fields for deletion + String value = "value"; + for (int i = 1; i < 4; i++) { + String key = "k" + i; + record.getSimpleFields().put(key, value); + record.getMapFields().put(key, ImmutableMap.of(key, value)); + record.getListFields().put(key, Arrays.asList(key, value)); + } + + // First, add these fields by way of "update" + Entity entity = + Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE); + post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs", + Collections.singletonMap("command", "delete"), entity, Response.Status.OK.getStatusCode()); + + // Check that the keys k1 and k2 have been deleted, and k0 remains + for (int i = 0; i < 4; i++) { + String key = "k" + i; + if (i == 0) { + Assert.assertTrue(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() + .getSimpleFields().containsKey(key)); + Assert.assertTrue(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() + .getListFields().containsKey(key)); + Assert.assertTrue(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() + .getMapFields().containsKey(key)); + continue; + } + Assert.assertFalse(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() + .getSimpleFields().containsKey(key)); + Assert.assertFalse(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() + .getListFields().containsKey(key)); + Assert.assertFalse(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord() + .getMapFields().containsKey(key)); + } + } + + /** + * Check that updateInstanceConfig fails when there is no pre-existing InstanceConfig ZNode. This + * is because InstanceConfig should have been created when the instance was added, and this REST + * endpoint is not meant for creation. + */ + @Test(dependsOnMethods = "deleteInstanceConfig") + public void checkUpdateFails() throws IOException { + System.out.println("Start test :" + TestHelper.getTestMethodName()); + String instanceName = CLUSTER_NAME + "non_existent_instance"; + InstanceConfig instanceConfig = new InstanceConfig(INSTANCE_NAME + "TEST"); + ZNRecord record = instanceConfig.getRecord(); + record.getSimpleFields().put("TestSimple", "value"); + record.getMapFields().put("TestMap", ImmutableMap.of("key", "value")); + record.getListFields().put("TestList", Arrays.asList("e1", "e2", "e3")); + + Entity entity = + Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE); + post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs", null, entity, + Response.Status.NOT_FOUND.getStatusCode()); + } }
