helix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [1/2] helix git commit: [HELIX-789] REST2.0: Add support for update and delete for ResourceConfig
Date Wed, 14 Nov 2018 19:02:19 GMT
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 <narendly@gmail.com>
Authored: Tue Nov 13 18:20:38 2018 -0800
Committer: narendly <narendly@gmail.com>
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


Mime
View raw message