helix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hu...@apache.org
Subject [helix] 27/43: Improve MetadataStoreDirectoryAccessor endpoints and fix bugs in ZkRoutingDataReader/Writer
Date Tue, 17 Mar 2020 00:03:58 GMT
This is an automated email from the ASF dual-hosted git repository.

hulee pushed a commit to branch zooscalability
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 9e49246aa3d96a34854db22b3603793b126ac1e8
Author: Neal Sun <nealsun.0428@gmail.com>
AuthorDate: Thu Feb 27 16:41:39 2020 -0800

    Improve MetadataStoreDirectoryAccessor endpoints and fix bugs in ZkRoutingDataReader/Writer
    
    Improve the current status code design of MetadataStoreDirectoryAccessor endpoints by
more clearly translate underlying exceptions to status codes. Fix 4 bugs in Reader/Writer.
---
 .../apache/helix/rest/common/HttpConstants.java    |   2 +
 .../metadatastore/ZkMetadataStoreDirectory.java    |  31 ++++-
 .../accessor/ZkRoutingDataReader.java              |  72 +++++-----
 .../accessor/ZkRoutingDataWriter.java              |  11 +-
 .../MetadataStoreDirectoryAccessor.java            |  10 +-
 .../TestZkMetadataStoreDirectory.java              |   2 +-
 .../accessor/TestZkRoutingDataWriter.java          |   2 +-
 .../MetadataStoreDirectoryAccessorTestBase.java    |   3 +-
 .../rest/server/TestMSDAccessorLeaderElection.java |   5 +-
 .../server/TestMetadataStoreDirectoryAccessor.java | 152 +++++++++++++--------
 10 files changed, 193 insertions(+), 97 deletions(-)

diff --git a/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java b/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java
index 369f1a4..d5f3bd9 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java
@@ -26,4 +26,6 @@ public class HttpConstants {
     PUT,
     DELETE
   }
+
+  public static final String HTTP_PROTOCOL_PREFIX = "http://";
 }
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java
b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java
index 9d54c82..28a4afe 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/ZkMetadataStoreDirectory.java
@@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHashMap;
 
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.helix.msdcommon.callback.RoutingDataListener;
+import org.apache.helix.msdcommon.constant.MetadataStoreRoutingConstants;
 import org.apache.helix.msdcommon.datamodel.MetadataStoreRoutingData;
 import org.apache.helix.msdcommon.datamodel.TrieRoutingData;
 import org.apache.helix.msdcommon.exception.InvalidRoutingDataException;
@@ -37,6 +38,10 @@ import org.apache.helix.rest.metadatastore.accessor.MetadataStoreRoutingDataRead
 import org.apache.helix.rest.metadatastore.accessor.MetadataStoreRoutingDataWriter;
 import org.apache.helix.rest.metadatastore.accessor.ZkRoutingDataReader;
 import org.apache.helix.rest.metadatastore.accessor.ZkRoutingDataWriter;
+import org.apache.helix.zookeeper.api.client.HelixZkClient;
+import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer;
+import org.apache.helix.zookeeper.impl.factory.DedicatedZkClientFactory;
+import org.apache.helix.zookeeper.zkclient.exception.ZkNodeExistsException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -93,6 +98,16 @@ public class ZkMetadataStoreDirectory implements MetadataStoreDirectory,
Routing
     if (!_routingZkAddressMap.containsKey(namespace)) {
       synchronized (_routingZkAddressMap) {
         if (!_routingZkAddressMap.containsKey(namespace)) {
+          // Ensure that ROUTING_DATA_PATH exists in ZK.
+          HelixZkClient zkClient = DedicatedZkClientFactory.getInstance()
+              .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress),
+                  new HelixZkClient.ZkClientConfig().setZkSerializer(new ZNRecordSerializer()));
+          try {
+            zkClient.createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH, true);
+          } catch (ZkNodeExistsException e) {
+            // The node already exists and it's okay
+          }
+
           _routingZkAddressMap.put(namespace, zkAddress);
           _routingDataReaderMap.put(namespace, new ZkRoutingDataReader(namespace, zkAddress,
this));
           _routingDataWriterMap.put(namespace, new ZkRoutingDataWriter(namespace, zkAddress));
@@ -173,7 +188,9 @@ public class ZkMetadataStoreDirectory implements MetadataStoreDirectory,
Routing
   @Override
   public boolean addMetadataStoreRealm(String namespace, String realm) {
     if (!_routingDataWriterMap.containsKey(namespace)) {
-      throw new IllegalArgumentException(
+      // throwing NoSuchElementException instead of IllegalArgumentException to differentiate
the
+      // status code in the Accessor level
+      throw new NoSuchElementException(
           "Failed to add metadata store realm: Namespace " + namespace + " is not found!");
     }
     return _routingDataWriterMap.get(namespace).addMetadataStoreRealm(realm);
@@ -182,7 +199,9 @@ public class ZkMetadataStoreDirectory implements MetadataStoreDirectory,
Routing
   @Override
   public boolean deleteMetadataStoreRealm(String namespace, String realm) {
     if (!_routingDataWriterMap.containsKey(namespace)) {
-      throw new IllegalArgumentException(
+      // throwing NoSuchElementException instead of IllegalArgumentException to differentiate
the
+      // status code in the Accessor level
+      throw new NoSuchElementException(
           "Failed to delete metadata store realm: Namespace " + namespace + " is not found!");
     }
     return _routingDataWriterMap.get(namespace).deleteMetadataStoreRealm(realm);
@@ -191,7 +210,9 @@ public class ZkMetadataStoreDirectory implements MetadataStoreDirectory,
Routing
   @Override
   public boolean addShardingKey(String namespace, String realm, String shardingKey) {
     if (!_routingDataWriterMap.containsKey(namespace) || !_routingDataMap.containsKey(namespace))
{
-      throw new IllegalArgumentException(
+      // throwing NoSuchElementException instead of IllegalArgumentException to differentiate
the
+      // status code in the Accessor level
+      throw new NoSuchElementException(
           "Failed to add sharding key: Namespace " + namespace + " is not found!");
     }
     if (_routingDataMap.get(namespace).containsKeyRealmPair(shardingKey, realm)) {
@@ -208,7 +229,9 @@ public class ZkMetadataStoreDirectory implements MetadataStoreDirectory,
Routing
   @Override
   public boolean deleteShardingKey(String namespace, String realm, String shardingKey) {
     if (!_routingDataWriterMap.containsKey(namespace)) {
-      throw new IllegalArgumentException(
+      // throwing NoSuchElementException instead of IllegalArgumentException to differentiate
the
+      // status code in the Accessor level
+      throw new NoSuchElementException(
           "Failed to delete sharding key: Namespace " + namespace + " is not found!");
     }
     return _routingDataWriterMap.get(namespace).deleteShardingKey(realm, shardingKey);
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataReader.java
b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataReader.java
index 76465f9..9251571 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataReader.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataReader.java
@@ -35,6 +35,7 @@ import org.apache.helix.zookeeper.zkclient.IZkChildListener;
 import org.apache.helix.zookeeper.zkclient.IZkDataListener;
 import org.apache.helix.zookeeper.zkclient.IZkStateListener;
 import org.apache.helix.zookeeper.zkclient.exception.ZkNoNodeException;
+import org.apache.helix.zookeeper.zkclient.exception.ZkNodeExistsException;
 import org.apache.zookeeper.Watcher;
 
 
@@ -57,6 +58,15 @@ public class ZkRoutingDataReader implements MetadataStoreRoutingDataReader,
IZkD
     _zkClient = DedicatedZkClientFactory.getInstance()
         .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress),
             new HelixZkClient.ZkClientConfig().setZkSerializer(new ZNRecordSerializer()));
+
+    // Ensure that ROUTING_DATA_PATH exists in ZK. If not, create
+    // create() semantic will fail if it already exists
+    try {
+      _zkClient.createPersistent(MetadataStoreRoutingConstants.ROUTING_DATA_PATH, true);
+    } catch (ZkNodeExistsException e) {
+      // This is okay
+    }
+
     _routingDataListener = routingDataListener;
     if (_routingDataListener != null) {
       // Subscribe child changes
@@ -76,8 +86,7 @@ public class ZkRoutingDataReader implements MetadataStoreRoutingDataReader,
IZkD
    * @throws InvalidRoutingDataException - when the node on
    *           MetadataStoreRoutingConstants.ROUTING_DATA_PATH is missing
    */
-  public Map<String, List<String>> getRoutingData()
-      throws InvalidRoutingDataException {
+  public Map<String, List<String>> getRoutingData() throws InvalidRoutingDataException
{
     Map<String, List<String>> routingData = new HashMap<>();
     List<String> allRealmAddresses;
     try {
@@ -87,13 +96,17 @@ public class ZkRoutingDataReader implements MetadataStoreRoutingDataReader,
IZkD
           "Routing data directory ZNode " + MetadataStoreRoutingConstants.ROUTING_DATA_PATH
               + " does not exist. Routing ZooKeeper address: " + _zkAddress);
     }
-    for (String realmAddress : allRealmAddresses) {
-      ZNRecord record =
-          _zkClient.readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + realmAddress);
-      List<String> shardingKeys =
-          record.getListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY);
-      routingData
-          .put(realmAddress, shardingKeys != null ? shardingKeys : Collections.emptyList());
+    if (allRealmAddresses != null) {
+      for (String realmAddress : allRealmAddresses) {
+        ZNRecord record = _zkClient
+            .readData(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + realmAddress);
+        if (record != null) {
+          List<String> shardingKeys =
+              record.getListField(MetadataStoreRoutingConstants.ZNRECORD_LIST_FIELD_KEY);
+          routingData
+              .put(realmAddress, shardingKeys != null ? shardingKeys : Collections.emptyList());
+        }
+      }
     }
     return routingData;
   }
@@ -113,32 +126,14 @@ public class ZkRoutingDataReader implements MetadataStoreRoutingDataReader,
IZkD
 
   @Override
   public synchronized void handleDataDeleted(String s) {
-    if (_zkClient.isClosed()) {
-      return;
-    }
-
-    // Renew subscription
-    _zkClient.subscribeChildChanges(MetadataStoreRoutingConstants.ROUTING_DATA_PATH, this);
-    for (String child : _zkClient.getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH))
{
-      _zkClient.subscribeDataChanges(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/"
+ child,
-          this);
-    }
-    _routingDataListener.refreshRoutingData(_namespace);
+    // When a child node is deleted, this and handleChildChange will both be triggered, but
the
+    // behavior is safe
+    handleResubscription();
   }
 
   @Override
   public synchronized void handleChildChange(String s, List<String> list) {
-    if (_zkClient.isClosed()) {
-      return;
-    }
-
-    // Subscribe data changes again because some children might have been deleted or added
-    _zkClient.unsubscribeAll();
-    for (String child : _zkClient.getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH))
{
-      _zkClient.subscribeDataChanges(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/"
+ child,
-          this);
-    }
-    _routingDataListener.refreshRoutingData(_namespace);
+    handleResubscription();
   }
 
   @Override
@@ -164,4 +159,19 @@ public class ZkRoutingDataReader implements MetadataStoreRoutingDataReader,
IZkD
     }
     _routingDataListener.refreshRoutingData(_namespace);
   }
+
+  private void handleResubscription() {
+    if (_zkClient.isClosed()) {
+      return;
+    }
+
+    // Renew subscription
+    _zkClient.unsubscribeAll();
+    _zkClient.subscribeChildChanges(MetadataStoreRoutingConstants.ROUTING_DATA_PATH, this);
+    for (String child : _zkClient.getChildren(MetadataStoreRoutingConstants.ROUTING_DATA_PATH))
{
+      _zkClient.subscribeDataChanges(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/"
+ child,
+          this);
+    }
+    _routingDataListener.refreshRoutingData(_namespace);
+  }
 }
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java
b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java
index 061372c..74cc14c 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java
@@ -90,7 +90,7 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter
{
     if (hostName.charAt(hostName.length() - 1) == '/') {
       hostName = hostName.substring(0, hostName.length() - 1);
     }
-    _myHostName = hostName;
+    _myHostName = HttpConstants.HTTP_PROTOCOL_PREFIX + hostName;
     ZNRecord myServerInfo = new ZNRecord(_myHostName);
 
     _leaderElection = new ZkDistributedLeaderElection(_zkClient,
@@ -247,6 +247,12 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter
{
   }
 
   protected boolean deleteZkRealm(String realm) {
+    if (!_zkClient.exists(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + realm))
{
+      LOG.warn(
+          "deleteZkRealm() called for realm: {}, but this realm already doesn't exist! Namespace:
{}",
+          realm, _namespace);
+      return true;
+    }
     return _zkClient.delete(MetadataStoreRoutingConstants.ROUTING_DATA_PATH + "/" + realm);
   }
 
@@ -339,7 +345,8 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter
{
         request = new HttpDelete(url);
         break;
       default:
-        throw new IllegalArgumentException("Unsupported request_method: " + request_method);
+        LOG.error("Unsupported request_method: " + request_method.name());
+        return false;
     }
 
     return sendRequestToLeader(request, expectedResponseCode, leaderHostName);
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
index 38b764d..0763ec1 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java
@@ -108,6 +108,8 @@ public class MetadataStoreDirectoryAccessor extends AbstractResource {
       return JSONRepresentation(new MetadataStoreShardingKey(shardingKey, realm));
     } catch (NoSuchElementException ex) {
       return notFound(ex.getMessage());
+    } catch (IllegalArgumentException e) {
+      return badRequest(e.getMessage());
     }
   }
 
@@ -176,6 +178,8 @@ public class MetadataStoreDirectoryAccessor extends AbstractResource {
       return getAllShardingKeysUnderPath(prefix);
     } catch (NoSuchElementException ex) {
       return notFound(ex.getMessage());
+    } catch (IllegalArgumentException e) {
+      return badRequest(e.getMessage());
     }
   }
 
@@ -240,6 +244,8 @@ public class MetadataStoreDirectoryAccessor extends AbstractResource {
       return getRealmShardingKeysUnderPath(realm, prefix);
     } catch (NoSuchElementException ex) {
       return notFound(ex.getMessage());
+    } catch (IllegalArgumentException e) {
+      return badRequest(e.getMessage());
     }
   }
 
@@ -252,8 +258,10 @@ public class MetadataStoreDirectoryAccessor extends AbstractResource
{
       if (!_metadataStoreDirectory.addShardingKey(_namespace, realm, shardingKey)) {
         return serverError();
       }
-    } catch (IllegalArgumentException ex) {
+    } catch (NoSuchElementException ex) {
       return notFound(ex.getMessage());
+    } catch (IllegalArgumentException e) {
+      return badRequest(e.getMessage());
     }
 
     return created();
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java
b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java
index 00a328f..8eddcea 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java
@@ -101,7 +101,7 @@ public class TestZkMetadataStoreDirectory extends AbstractTestClass {
     });
 
     System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY,
-        getBaseUri().toString());
+        getBaseUri().getHost() + ":" + getBaseUri().getPort());
 
     // Create metadataStoreDirectory
     for (Map.Entry<String, String> entry : _routingZkAddrMap.entrySet()) {
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java
b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java
index e61e905..069931f 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java
@@ -63,7 +63,7 @@ public class TestZkRoutingDataWriter extends AbstractTestClass {
   public void beforeClass() {
     _baseAccessor.remove(MetadataStoreRoutingConstants.ROUTING_DATA_PATH, AccessOption.PERSISTENT);
     System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY,
-        getBaseUri().toString());
+        getBaseUri().getHost() + ":" + getBaseUri().getPort());
     _zkRoutingDataWriter = new ZkRoutingDataWriter(DUMMY_NAMESPACE, ZK_ADDR);
   }
 
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java
b/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java
index f234795..7cebbf3 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java
@@ -52,6 +52,7 @@ public class MetadataStoreDirectoryAccessorTestBase extends AbstractTestClass
{
       Arrays.asList("/sharding/key/1/d", "/sharding/key/1/e", "/sharding/key/1/f");
   protected static final String TEST_REALM_3 = "testRealm3";
   protected static final String TEST_SHARDING_KEY = "/sharding/key/1/x";
+  protected static final String INVALID_TEST_SHARDING_KEY = "sharding/key/1/x";
 
   // List of all ZK addresses, each of which corresponds to a namespace/routing ZK
   protected List<String> _zkList;
@@ -93,7 +94,7 @@ public class MetadataStoreDirectoryAccessorTestBase extends AbstractTestClass
{
     _routingDataReader = new ZkRoutingDataReader(TEST_NAMESPACE, _zkAddrTestNS, null);
 
     System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY,
-        getBaseUri().toString());
+        getBaseUri().getHost() + ":" + getBaseUri().getPort());
   }
 
   @AfterClass
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java
b/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java
index 9a122a9..951015b 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java
@@ -74,7 +74,7 @@ public class TestMSDAccessorLeaderElection extends MetadataStoreDirectoryAccesso
     int newPort = getBaseUri().getPort() + 1;
 
     // Start a second server for testing Distributed Leader Election for writes
-    _mockBaseUri = getBaseUri().getScheme() + "://" + getBaseUri().getHost() + ":" + newPort;
+    _mockBaseUri = HttpConstants.HTTP_PROTOCOL_PREFIX + getBaseUri().getHost() + ":" + newPort;
     try {
       List<HelixRestNamespace> namespaces = new ArrayList<>();
       // Add test namespace
@@ -93,7 +93,8 @@ public class TestMSDAccessorLeaderElection extends MetadataStoreDirectoryAccesso
         Response.Status.OK.getStatusCode(), true);
 
     // Set the new uri to be used in leader election
-    System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY, _mockBaseUri);
+    System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY,
+        getBaseUri().getHost() + ":" + newPort);
 
     // Start http client for testing
     _httpClient = HttpClients.createDefault();
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestMetadataStoreDirectoryAccessor.java
b/helix-rest/src/test/java/org/apache/helix/rest/server/TestMetadataStoreDirectoryAccessor.java
index b6179aa..94641ff 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestMetadataStoreDirectoryAccessor.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestMetadataStoreDirectoryAccessor.java
@@ -99,6 +99,14 @@ public class TestMetadataStoreDirectoryAccessor extends MetadataStoreDirectoryAc
         NON_EXISTING_NAMESPACE_URI_PREFIX + "metadata-store-realms?sharding-key=" + shardingKey)
         .expectedReturnStatusCode(Response.Status.NOT_FOUND.getStatusCode()).get(this);
 
+    new JerseyUriRequestBuilder(
+        TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms?sharding-key=" + TEST_SHARDING_KEY)
+        .expectedReturnStatusCode(Response.Status.NOT_FOUND.getStatusCode()).get(this);
+
+    new JerseyUriRequestBuilder(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms?sharding-key="
+        + INVALID_TEST_SHARDING_KEY)
+        .expectedReturnStatusCode(Response.Status.BAD_REQUEST.getStatusCode()).get(this);
+
     String responseBody = new JerseyUriRequestBuilder(
         TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms?sharding-key=" + shardingKey)
         .isBodyReturnExpected(true).get(this);
@@ -133,6 +141,11 @@ public class TestMetadataStoreDirectoryAccessor extends MetadataStoreDirectoryAc
         Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
         Response.Status.CREATED.getStatusCode());
 
+    // Second addition also succeeds.
+    put(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_3, null,
+        Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
+        Response.Status.CREATED.getStatusCode());
+
     expectedRealmsSet.add(TEST_REALM_3);
     Assert.assertEquals(getAllRealms(), expectedRealmsSet);
   }
@@ -151,6 +164,10 @@ public class TestMetadataStoreDirectoryAccessor extends MetadataStoreDirectoryAc
     delete(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_3,
         Response.Status.OK.getStatusCode());
 
+    // Second deletion also succeeds.
+    delete(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_3,
+        Response.Status.OK.getStatusCode());
+
     Set<String> updateRealmsSet = getAllRealms();
     expectedRealmsSet.remove(TEST_REALM_3);
     Assert.assertEquals(updateRealmsSet, expectedRealmsSet);
@@ -190,9 +207,65 @@ public class TestMetadataStoreDirectoryAccessor extends MetadataStoreDirectoryAc
   }
 
   /*
-   * Tests REST endpoint: "GET /routing-data"
+   * Tests REST endpoint: "GET /sharding-keys?prefix={prefix}"
    */
+  @SuppressWarnings("unchecked")
   @Test(dependsOnMethods = "testGetShardingKeysInNamespace")
+  public void testGetShardingKeysUnderPath() throws IOException {
+    new JerseyUriRequestBuilder(
+        TEST_NAMESPACE_URI_PREFIX + "/sharding-keys?prefix=" + INVALID_TEST_SHARDING_KEY)
+        .expectedReturnStatusCode(Response.Status.BAD_REQUEST.getStatusCode()).get(this);
+
+    // Test non existed prefix and empty sharding keys in response.
+    String responseBody = new JerseyUriRequestBuilder(
+        TEST_NAMESPACE_URI_PREFIX + "/sharding-keys?prefix=/non/Existed/Prefix")
+        .isBodyReturnExpected(true).get(this);
+
+    Map<String, Object> queriedShardingKeysMap = OBJECT_MAPPER.readValue(responseBody,
Map.class);
+    Collection<Map<String, String>> emptyKeysList =
+        (Collection<Map<String, String>>) queriedShardingKeysMap
+            .get(MetadataStoreRoutingConstants.SHARDING_KEYS);
+    Assert.assertTrue(emptyKeysList.isEmpty());
+
+    // Success response with non empty sharding keys.
+    String shardingKeyPrefix = "/sharding/key";
+    responseBody = new JerseyUriRequestBuilder(
+        TEST_NAMESPACE_URI_PREFIX + "/sharding-keys?prefix=" + shardingKeyPrefix)
+        .isBodyReturnExpected(true).get(this);
+
+    queriedShardingKeysMap = OBJECT_MAPPER.readValue(responseBody, Map.class);
+
+    // Check fields.
+    Assert.assertEquals(queriedShardingKeysMap.keySet(), ImmutableSet
+        .of(MetadataStoreRoutingConstants.SHARDING_KEY_PATH_PREFIX,
+            MetadataStoreRoutingConstants.SHARDING_KEYS));
+
+    // Check sharding key prefix in json response.
+    Assert.assertEquals(
+        queriedShardingKeysMap.get(MetadataStoreRoutingConstants.SHARDING_KEY_PATH_PREFIX),
+        shardingKeyPrefix);
+
+    Collection<Map<String, String>> queriedShardingKeys =
+        (Collection<Map<String, String>>) queriedShardingKeysMap
+            .get(MetadataStoreRoutingConstants.SHARDING_KEYS);
+    Set<Map<String, String>> queriedShardingKeysSet = new HashSet<>(queriedShardingKeys);
+    Set<Map<String, String>> expectedShardingKeysSet = new HashSet<>();
+
+    TEST_SHARDING_KEYS_1.forEach(key -> expectedShardingKeysSet.add(ImmutableMap
+        .of(MetadataStoreRoutingConstants.SINGLE_SHARDING_KEY, key,
+            MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM, TEST_REALM_1)));
+
+    TEST_SHARDING_KEYS_2.forEach(key -> expectedShardingKeysSet.add(ImmutableMap
+        .of(MetadataStoreRoutingConstants.SINGLE_SHARDING_KEY, key,
+            MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM, TEST_REALM_2)));
+
+    Assert.assertEquals(queriedShardingKeysSet, expectedShardingKeysSet);
+  }
+
+  /*
+   * Tests REST endpoint: "GET /routing-data"
+   */
+  @Test(dependsOnMethods = "testGetShardingKeysUnderPath")
   public void testGetRoutingData() throws IOException {
     /*
      * responseBody:
@@ -258,63 +331,15 @@ public class TestMetadataStoreDirectoryAccessor extends MetadataStoreDirectoryAc
   }
 
   /*
-   * Tests REST endpoint: "GET /sharding-keys?prefix={prefix}"
-   */
-  @SuppressWarnings("unchecked")
-  @Test(dependsOnMethods = "testGetShardingKeysInRealm")
-  public void testGetShardingKeysUnderPath() throws IOException {
-    // Test non existed prefix and empty sharding keys in response.
-    String responseBody = new JerseyUriRequestBuilder(
-        TEST_NAMESPACE_URI_PREFIX + "/sharding-keys?prefix=/non/Existed/Prefix")
-        .isBodyReturnExpected(true).get(this);
-
-    Map<String, Object> queriedShardingKeysMap = OBJECT_MAPPER.readValue(responseBody,
Map.class);
-    Collection<Map<String, String>> emptyKeysList =
-        (Collection<Map<String, String>>) queriedShardingKeysMap
-            .get(MetadataStoreRoutingConstants.SHARDING_KEYS);
-    Assert.assertTrue(emptyKeysList.isEmpty());
-
-    // Success response with non empty sharding keys.
-    String shardingKeyPrefix = "/sharding/key";
-    responseBody = new JerseyUriRequestBuilder(
-        TEST_NAMESPACE_URI_PREFIX + "/sharding-keys?prefix=" + shardingKeyPrefix)
-        .isBodyReturnExpected(true).get(this);
-
-    queriedShardingKeysMap = OBJECT_MAPPER.readValue(responseBody, Map.class);
-
-    // Check fields.
-    Assert.assertEquals(queriedShardingKeysMap.keySet(), ImmutableSet
-        .of(MetadataStoreRoutingConstants.SHARDING_KEY_PATH_PREFIX,
-            MetadataStoreRoutingConstants.SHARDING_KEYS));
-
-    // Check sharding key prefix in json response.
-    Assert.assertEquals(
-        queriedShardingKeysMap.get(MetadataStoreRoutingConstants.SHARDING_KEY_PATH_PREFIX),
-        shardingKeyPrefix);
-
-    Collection<Map<String, String>> queriedShardingKeys =
-        (Collection<Map<String, String>>) queriedShardingKeysMap
-            .get(MetadataStoreRoutingConstants.SHARDING_KEYS);
-    Set<Map<String, String>> queriedShardingKeysSet = new HashSet<>(queriedShardingKeys);
-    Set<Map<String, String>> expectedShardingKeysSet = new HashSet<>();
-
-    TEST_SHARDING_KEYS_1.forEach(key -> expectedShardingKeysSet.add(ImmutableMap
-        .of(MetadataStoreRoutingConstants.SINGLE_SHARDING_KEY, key,
-            MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM, TEST_REALM_1)));
-
-    TEST_SHARDING_KEYS_2.forEach(key -> expectedShardingKeysSet.add(ImmutableMap
-        .of(MetadataStoreRoutingConstants.SINGLE_SHARDING_KEY, key,
-            MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM, TEST_REALM_2)));
-
-    Assert.assertEquals(queriedShardingKeysSet, expectedShardingKeysSet);
-  }
-
-  /*
    * Tests REST endpoint: "GET /metadata-store-realms/{realm}/sharding-keys?prefix={prefix}"
    */
   @SuppressWarnings("unchecked")
-  @Test(dependsOnMethods = "testGetShardingKeysUnderPath")
+  @Test(dependsOnMethods = "testGetShardingKeysInRealm")
   public void testGetRealmShardingKeysUnderPath() throws IOException {
+    new JerseyUriRequestBuilder(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_1
+        + "/sharding-keys?prefix=" + INVALID_TEST_SHARDING_KEY)
+        .expectedReturnStatusCode(Response.Status.BAD_REQUEST.getStatusCode()).get(this);
+
     // Test non existed prefix and empty sharding keys in response.
     String responseBody = new JerseyUriRequestBuilder(
         TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_1
@@ -380,11 +405,26 @@ public class TestMetadataStoreDirectoryAccessor extends MetadataStoreDirectoryAc
         null, Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
         Response.Status.NOT_FOUND.getStatusCode());
 
+    put(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_1 + "/sharding-keys"
+            + "//" + INVALID_TEST_SHARDING_KEY, null,
+        Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
+        Response.Status.BAD_REQUEST.getStatusCode());
+
     // Successful request.
     put(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_1 + "/sharding-keys"
             + TEST_SHARDING_KEY, null, Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
         Response.Status.CREATED.getStatusCode());
 
+    // Idempotency
+    put(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_1 + "/sharding-keys"
+            + TEST_SHARDING_KEY, null, Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
+        Response.Status.CREATED.getStatusCode());
+
+    // Invalid
+    put(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_2 + "/sharding-keys"
+            + TEST_SHARDING_KEY, null, Entity.entity("", MediaType.APPLICATION_JSON_TYPE),
+        Response.Status.BAD_REQUEST.getStatusCode());
+
     expectedShardingKeysSet.add(TEST_SHARDING_KEY);
     Assert.assertEquals(getAllShardingKeysInTestRealm1(), expectedShardingKeysSet);
   }
@@ -404,6 +444,10 @@ public class TestMetadataStoreDirectoryAccessor extends MetadataStoreDirectoryAc
     delete(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_1 + "/sharding-keys"
         + TEST_SHARDING_KEY, Response.Status.OK.getStatusCode());
 
+    // Idempotency
+    delete(TEST_NAMESPACE_URI_PREFIX + "/metadata-store-realms/" + TEST_REALM_1 + "/sharding-keys"
+        + TEST_SHARDING_KEY, Response.Status.OK.getStatusCode());
+
     expectedShardingKeysSet.remove(TEST_SHARDING_KEY);
     Assert.assertEquals(getAllShardingKeysInTestRealm1(), expectedShardingKeysSet);
   }


Mime
View raw message