Repository: helix Updated Branches: refs/heads/master d5bf3ad41 -> abc6969d7
[HELIX-789] REST2.0: Add support for update and delete for ResourceConfig Previous implementation of updateResourceConfig did not allow deletion of fields in ResourceConfig in ZK. This RB refactors the REST endpoint. Changelist: 1. Add command support for updateResourceConfig 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/22fa03f3 Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/22fa03f3 Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/22fa03f3 Branch: refs/heads/master Commit: 22fa03f3d8bb0863913f2a6614574443130e500a Parents: d5bf3ad Author: narendly <[email protected]> Authored: Tue Nov 13 18:20:38 2018 -0800 Committer: narendly <[email protected]> Committed: Tue Nov 13 18:21:49 2018 -0800 ---------------------------------------------------------------------- .../resources/helix/InstanceAccessor.java | 3 +- .../resources/helix/ResourceAccessor.java | 33 ++++++- .../helix/rest/server/TestResourceAccessor.java | 95 ++++++++++++++++++++ 3 files changed, 126 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/helix/blob/22fa03f3/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 38ee3b5..f55ee89 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 @@ -350,12 +350,11 @@ public class InstanceAccessor extends AbstractHelixResource { case update: configAccessor.updateInstanceConfig(clusterId, instanceName, instanceConfig); break; - case delete: { + 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)); http://git-wip-us.apache.org/repos/asf/helix/blob/22fa03f3/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ResourceAccessor.java ---------------------------------------------------------------------- diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ResourceAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ResourceAccessor.java index a54cd8d..75d561b 100644 --- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ResourceAccessor.java +++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ResourceAccessor.java @@ -43,9 +43,11 @@ import org.apache.helix.PropertyPathBuilder; import org.apache.helix.ZNRecord; import org.apache.helix.manager.zk.client.HelixZkClient; import org.apache.helix.model.ExternalView; +import org.apache.helix.model.HelixConfigScope; import org.apache.helix.model.IdealState; import org.apache.helix.model.ResourceConfig; import org.apache.helix.model.StateModelDefinition; +import org.apache.helix.model.builder.HelixConfigScopeBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.codehaus.jackson.node.ArrayNode; @@ -294,7 +296,19 @@ public class ResourceAccessor extends AbstractHelixResource { @POST @Path("{resourceName}/configs") public Response updateResourceConfig(@PathParam("clusterId") String clusterId, - @PathParam("resourceName") String resourceName, String content) { + @PathParam("resourceName") String resourceName, @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); @@ -305,11 +319,24 @@ public class ResourceAccessor extends AbstractHelixResource { ResourceConfig resourceConfig = new ResourceConfig(record); ConfigAccessor configAccessor = getConfigAccessor(); try { - configAccessor.updateResourceConfig(clusterId, resourceName, resourceConfig); + switch (command) { + case update: + configAccessor.updateResourceConfig(clusterId, resourceName, resourceConfig); + break; + case delete: + HelixConfigScope resourceScope = + new HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.RESOURCE) + .forCluster(clusterId).forResource(resourceName).build(); + configAccessor.remove(resourceScope, 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 resource config for resource: %s", resourceName), ex); + _logger.error(String.format("Error in update resource config for resource: %s", resourceName), + ex); return serverError(ex); } return OK(); http://git-wip-us.apache.org/repos/asf/helix/blob/22fa03f3/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java ---------------------------------------------------------------------- diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java index e8888a2..db5c902 100644 --- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java +++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestResourceAccessor.java @@ -23,6 +23,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.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -36,6 +37,7 @@ import org.apache.helix.HelixManager; import org.apache.helix.HelixManagerFactory; import org.apache.helix.InstanceType; import org.apache.helix.TestHelper; +import org.apache.helix.ZNRecord; import org.apache.helix.model.ExternalView; import org.apache.helix.model.IdealState; import org.apache.helix.model.ResourceConfig; @@ -249,6 +251,99 @@ public class TestResourceAccessor extends AbstractTestClass { } /** + * Test "update" command of updateResourceConfig. + * @throws Exception + */ + @Test(dependsOnMethods = "testResourceHealth") + public void updateResourceConfig() throws Exception { + // Get ResourceConfig + ResourceConfig resourceConfig = _configAccessor.getResourceConfig(CLUSTER_NAME, RESOURCE_NAME); + ZNRecord record = resourceConfig.getRecord(); + + // Generate a record containing three keys (k0, k1, k2) for all fields + String value = "RESOURCE_TEST"; + 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 + "/resources/" + RESOURCE_NAME + "/configs", + Collections.singletonMap("command", "update"), entity, Response.Status.OK.getStatusCode()); + + // Check that the fields have been added + ResourceConfig updatedConfig = _configAccessor.getResourceConfig(CLUSTER_NAME, RESOURCE_NAME); + Assert.assertEquals(record.getSimpleFields(), updatedConfig.getRecord().getSimpleFields()); + Assert.assertEquals(record.getListFields(), updatedConfig.getRecord().getListFields()); + Assert.assertEquals(record.getMapFields(), updatedConfig.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 + "/resources/" + RESOURCE_NAME + "/configs", + Collections.singletonMap("command", "update"), entity, Response.Status.OK.getStatusCode()); + + updatedConfig = _configAccessor.getResourceConfig(CLUSTER_NAME, RESOURCE_NAME); + // Check that the fields have been modified + Assert.assertEquals(record.getSimpleFields(), updatedConfig.getRecord().getSimpleFields()); + Assert.assertEquals(record.getListFields(), updatedConfig.getRecord().getListFields()); + Assert.assertEquals(record.getMapFields(), updatedConfig.getRecord().getMapFields()); + } + + /** + * Test "delete" command of updateResourceConfig. + * @throws Exception + */ + @Test(dependsOnMethods = "updateResourceConfig") + public void deleteFromResourceConfig() throws Exception { + ZNRecord record = new ZNRecord(RESOURCE_NAME); + + // 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 + "/resources/" + RESOURCE_NAME + "/configs", + Collections.singletonMap("command", "delete"), entity, Response.Status.OK.getStatusCode()); + + ResourceConfig configAfterDelete = + _configAccessor.getResourceConfig(CLUSTER_NAME, RESOURCE_NAME); + + // 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(configAfterDelete.getRecord().getSimpleFields().containsKey(key)); + Assert.assertTrue(configAfterDelete.getRecord().getListFields().containsKey(key)); + Assert.assertTrue(configAfterDelete.getRecord().getMapFields().containsKey(key)); + continue; + } + Assert.assertFalse(configAfterDelete.getRecord().getSimpleFields().containsKey(key)); + Assert.assertFalse(configAfterDelete.getRecord().getListFields().containsKey(key)); + Assert.assertFalse(configAfterDelete.getRecord().getMapFields().containsKey(key)); + } + } + + /** * Creates a setup where the health API can be tested. * * @param clusterName
