pkuwm commented on a change in pull request #761: Add REST read endpoints to helix-rest for metadata store directory URL: https://github.com/apache/helix/pull/761#discussion_r381000691
########## File path: helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java ########## @@ -111,41 +135,74 @@ public Response deleteMetadataStoreRealm(@PathParam("realm") String realm) { } /** - * Gets sharding keys mapped at path "HTTP GET /sharding-keys" which returns all sharding keys in - * a namespace, or path "HTTP GET /sharding-keys?realm={realmName}" which returns sharding keys in - * a realm. + * Gets all sharding keys mapped at paths: + * - "HTTP GET /sharding-keys" which returns all sharding keys in a namespace. + * - "HTTP GET /sharding-keys?realm={realm}" which returns sharding keys in the realm. + * - "HTTP GET /sharding-keys?prefix={prefix}" which returns sharding keys that have the prefix. + * -- JSON response example for this path: + * { + * "prefix": "/sharding/key", + * "shardingKeys": [{ + * "realm": "testRealm2", + * "shardingKey": "/sharding/key/1/f" + * }, { + * "realm": "testRealm2", + * "shardingKey": "/sharding/key/1/e" + * }, { + * "realm": "testRealm1", + * "shardingKey": "/sharding/key/1/b" + * }, { + * "realm": "testRealm1", + * "shardingKey": "/sharding/key/1/a" + * }] + * } * - * @param realm Query param in endpoint path - * @return Json representation of a map: shardingKeys -> collection of sharding keys. + * - "HTTP GET /sharding-keys?realm={realm}&prefix={prefix}" which returns sharding keys in the + * realm and that have the prefix. + * + * @param realm Query param in endpoint path: metadata store realm. + * @param prefix Query param in endpoint path: prefix substring of sharding key. + * @return Json representation for the sharding keys. */ @GET @Path("/sharding-keys") - public Response getShardingKeys(@QueryParam("realm") String realm) { - Map<String, Object> responseMap; - Collection<String> shardingKeys; + public Response getShardingKeys(@QueryParam("realm") String realm, + @QueryParam("prefix") String prefix) { try { - // If realm is not set in query param, the endpoint is: "/sharding-keys" - // to get all sharding keys in a namespace. - if (realm == null) { - shardingKeys = _metadataStoreDirectory.getAllShardingKeys(_namespace); - // To avoid allocating unnecessary resource, limit the map's capacity only for - // SHARDING_KEYS. - responseMap = new HashMap<>(1); + if (realm == null && prefix == null) { + // For endpoint: "/sharding-keys" to get all sharding keys in a namespace. + return getAllShardingKeys(); + } else if (prefix == null) { + // For endpoint: "/sharding-keys?realm={realm}" + return getAllShardingKeysInRealm(realm); + } else if (realm == null) { + // For endpoint: "/sharding-keys?prefix={prefix}" + return getAllShardingKeysUnderPath(prefix); } else { - // For endpoint: "/sharding-keys?realm={realmName}" - shardingKeys = _metadataStoreDirectory.getAllShardingKeysInRealm(_namespace, realm); - // To avoid allocating unnecessary resource, limit the map's capacity only for - // SHARDING_KEYS and SINGLE_METADATA_STORE_REALM. - responseMap = new HashMap<>(2); - responseMap.put(MetadataStoreRoutingConstants.SINGLE_METADATA_STORE_REALM, realm); + // For endpoint: "/sharding-keys?realm={realm}&prefix={prefix}" + return getRealmShardingKeysUnderPath(realm, prefix); } } catch (NoSuchElementException ex) { return notFound(ex.getMessage()); } + } - responseMap.put(MetadataStoreRoutingConstants.SHARDING_KEYS, shardingKeys); - - return JSONRepresentation(responseMap); + /** + * Gets all path-based sharding keys for a queried realm at endpoint: + * "GET /metadata-store-realms/{realm}/sharding-keys". This endpoint is equivalent to + * the endpoint: "GET /sharding-keys?realm={realm}". + * + * @param realm Queried metadata store realm to get sharding keys. + * @return All path-based sharding keys in the queried realm. + */ + @GET + @Path("/metadata-store-realms/{realm}/sharding-keys") + public Response getRealmShardingKeys(@PathParam("realm") String realm) { + try { + return getAllShardingKeysInRealm(realm); + } catch (NoSuchElementException ex) { + return notFound(ex.getMessage()); Review comment: @NealSun96 Thanks for reviewing. As you referred in the link, `400 Bad Request`: The request could not be understood by the server due to **malformed syntax**. If you request a POST/PUT with json payload which has syntax error, I believe `Bad Request` is more suitable. `404 Not Found` means the endpoint is correct but resource is not found in server, which matches exactly our case here `NoSuchElementException`. (refs as an examples: - https://stackoverflow.com/questions/26968479/404-not-found-or-bad-request; - https://stackoverflow.com/questions/19671317/400-bad-request-http-error-code-meaning) What do you think? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org