Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-15118: Convert /v2/collections APIs to annotations #2281

Merged
merged 13 commits into from
Feb 8, 2021
Merged

SOLR-15118: Convert /v2/collections APIs to annotations #2281

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
12586ee
SOLR-15118: Convert /v2/collections APIs to annotations
Jan 28, 2021
c613a1f
Use constants for param names where possible
Feb 1, 2021
472b7a4
Fix precommit, rd 1
Feb 1, 2021
f62f5b8
Merge branch 'master' into SOLR_15118_annotation_based_collections_apis
Feb 4, 2021
7b06110
Add new incremental backup/restore params to v2 POJOs
Feb 4, 2021
e4a0997
Remove unintentionally committed 'IML' file
Feb 4, 2021
f68afd1
Address review comments, rd 1
Feb 4, 2021
253676f
Address review feedback, rd 1 (cont'd)
Feb 4, 2021
6c616ad
Fix broken tests, pt 1
Feb 4, 2021
006b5c9
Remove V2RequestSupport impl from CollectionAdminRequest
Feb 7, 2021
5ffe72d
Fix precommit, rd 1
Feb 8, 2021
2f025b7
Fix precommit, rd 2
Feb 8, 2021
7ec7382
Fix test failures, rd 1
Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lucene-solr.iml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<module version="4">
<component name="NewModuleRootManager" inherit-compiler-output="true">
<exclude-output />
<orderEntry type="sourceFolder" forTests="false" />
</component>
</module>
4 changes: 3 additions & 1 deletion solr/core/src/java/org/apache/solr/core/CoreContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,9 @@ public void load() {
createHandler(ZK_PATH, ZookeeperInfoHandler.class.getName(), ZookeeperInfoHandler.class);
createHandler(ZK_STATUS_PATH, ZookeeperStatusHandler.class.getName(), ZookeeperStatusHandler.class);
collectionsHandler = createHandler(COLLECTIONS_HANDLER_PATH, cfg.getCollectionsHandlerClass(), CollectionsHandler.class);
containerHandlers.getApiBag().registerObject(new CollectionsAPI(collectionsHandler));
final CollectionsAPI collectionsAPI = new CollectionsAPI(collectionsHandler);
containerHandlers.getApiBag().registerObject(collectionsAPI);
containerHandlers.getApiBag().registerObject(collectionsAPI.collectionsCommands);
configSetsHandler = createHandler(CONFIGSETS_HANDLER_PATH, cfg.getConfigSetsHandlerClass(), ConfigSetsHandler.class);
ClusterAPI clusterAPI = new ClusterAPI(collectionsHandler, configSetsHandler);
containerHandlers.getApiBag().registerObject(clusterAPI);
Expand Down
172 changes: 168 additions & 4 deletions solr/core/src/java/org/apache/solr/handler/CollectionsAPI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,38 @@

package org.apache.solr.handler;

import org.apache.commons.collections4.CollectionUtils;
import org.apache.solr.api.Command;
import org.apache.solr.api.EndPoint;
import org.apache.solr.api.PayloadObj;
import org.apache.solr.client.solrj.request.beans.BackupCollectionBody;
import org.apache.solr.client.solrj.request.beans.CreateAliasBody;
import org.apache.solr.client.solrj.request.beans.CreateBody;
import org.apache.solr.client.solrj.request.beans.DeleteAliasBody;
import org.apache.solr.client.solrj.request.beans.RestoreCollectionBody;
import org.apache.solr.client.solrj.request.beans.SetAliasPropertyBody;
import org.apache.solr.client.solrj.request.beans.V2ApiConstants;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.common.params.CollectionParams.CollectionAction;
import org.apache.solr.handler.admin.CollectionsHandler;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;

import static org.apache.solr.client.solrj.SolrRequest.METHOD.DELETE;
import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import static org.apache.solr.client.solrj.SolrRequest.METHOD.*;
import static org.apache.solr.client.solrj.request.beans.V2ApiConstants.ROUTER_KEY;
import static org.apache.solr.cloud.api.collections.RoutedAlias.CREATE_COLLECTION_PREFIX;
import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
import static org.apache.solr.common.params.CollectionAdminParams.ROUTER_PREFIX;
import static org.apache.solr.common.params.CommonParams.ACTION;
import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.handler.ClusterAPI.wrapParams;
import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
import static org.apache.solr.security.PermissionNameProvider.Name.COLL_READ_PERM;

Expand All @@ -36,7 +58,16 @@
*/
public class CollectionsAPI {

private final CollectionsHandler collectionsHandler;
public static final String V2_CREATE_COLLECTION_CMD = "create";
public static final String V2_BACKUP_CMD = "backup-collection";
public static final String V2_RESTORE_CMD = "restore-collection";
public static final String V2_CREATE_ALIAS_CMD = "create-alias";
public static final String V2_SET_ALIAS_PROP_CMD = "set-alias-property";
public static final String V2_DELETE_ALIAS_CMD = "delete-alias";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add other collection commands here, such as: COLLECTIONPROP, CREATE / DELETESHARD, ADD / MOVE / DELETEREPLICA, MODIFYCOLLECTION, REINDEXCOLLECTION, RENAME, SPLITSHARD ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should definitely exist somewhere, but they're not /v2/collections APIs, so I didn't do that in this PR. If you guys really want them, I'm not against adding those constants in this PR.

But even then they should probably live somewhere else, shouldn't they? (since this class is for /v2/collections APIs, and those commands are for other APIs? AFAIK this file is only for those APIs that specifically live under /v2/collections, right?)


private final CollectionsHandler collectionsHandler;

public final CollectionsCommands collectionsCommands = new CollectionsCommands();

public CollectionsAPI(CollectionsHandler collectionsHandler) {
this.collectionsHandler = collectionsHandler;
Expand All @@ -50,11 +81,144 @@ public void getCollections(SolrQueryRequest req, SolrQueryResponse rsp) throws E
CollectionsHandler.CollectionOperation.LIST_OP.execute(req, rsp, collectionsHandler);
}

@EndPoint(
path = {"/c", "/collections"},
method = POST,
permission = COLL_EDIT_PERM)
public class CollectionsCommands {

@Command(name = V2_BACKUP_CMD)
@SuppressWarnings("unchecked")
public void backupCollection(PayloadObj<BackupCollectionBody> obj) throws Exception {
final Map<String, Object> v1Params = obj.get().toMap(new HashMap<>());
v1Params.put(ACTION, CollectionAction.BACKUP.toLower());

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name = V2_RESTORE_CMD)
@SuppressWarnings("unchecked")
public void restoreBackup(PayloadObj<RestoreCollectionBody> obj) throws Exception {
final RestoreCollectionBody v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.RESTORE.toLower());
if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
final Map<String, Object> createCollParams = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
convertV2CreateCollectionMapToV1ParamMap(createCollParams);
v1Params.putAll(createCollParams);
}

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name = V2_CREATE_ALIAS_CMD)
@SuppressWarnings("unchecked")
public void createAlias(PayloadObj<CreateAliasBody> obj) throws Exception {
final CreateAliasBody v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.CREATEALIAS.toLower());
if (! CollectionUtils.isEmpty(v2Body.collections)) {
final String collectionsStr = String.join(",", v2Body.collections);
v1Params.remove(V2ApiConstants.COLLECTIONS);
v1Params.put(V2ApiConstants.COLLECTIONS, collectionsStr);
}
if (v2Body.router != null) {
Map<String, Object> routerProperties = (Map<String, Object>) v1Params.remove(V2ApiConstants.ROUTER_KEY);
flattenMapWithPrefix(routerProperties, v1Params, ROUTER_PREFIX);
}
if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
final Map<String, Object> createCollectionMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
convertV2CreateCollectionMapToV1ParamMap(createCollectionMap);
flattenMapWithPrefix(createCollectionMap, v1Params, CREATE_COLLECTION_PREFIX);
}

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name= V2_SET_ALIAS_PROP_CMD)
@SuppressWarnings("unchecked")
public void setAliasProperty(PayloadObj<SetAliasPropertyBody> obj) throws Exception {
final SetAliasPropertyBody v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.ALIASPROP.toLower());
// Flatten "properties" map into individual prefixed params
final Map<String, Object> propertiesMap = (Map<String, Object>) v1Params.remove(V2ApiConstants.PROPERTIES_KEY);
flattenMapWithPrefix(propertiesMap, v1Params, PROPERTY_PREFIX);

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name= V2_DELETE_ALIAS_CMD)
@SuppressWarnings("unchecked")
public void deleteAlias(PayloadObj<DeleteAliasBody> obj) throws Exception {
final DeleteAliasBody v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
v1Params.put(ACTION, CollectionAction.DELETEALIAS.toLower());

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@Command(name = V2_CREATE_COLLECTION_CMD)
@SuppressWarnings("unchecked")
public void create(PayloadObj<CreateBody> obj) throws Exception {
final CreateBody v2Body = obj.get();
final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());

v1Params.put(ACTION, CollectionAction.CREATE.toLower());
convertV2CreateCollectionMapToV1ParamMap(v1Params);

collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
}

@SuppressWarnings("unchecked")
private void convertV2CreateCollectionMapToV1ParamMap(Map<String, Object> v2MapVals) {
// Keys are copied so that map can be modified as keys are looped through.
final Set<String> v2Keys = v2MapVals.keySet().stream().collect(Collectors.toSet());
for (String key : v2Keys) {
switch (key) {
case V2ApiConstants.PROPERTIES_KEY:
final Map<String, Object> propertiesMap = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.PROPERTIES_KEY);
flattenMapWithPrefix(propertiesMap, v2MapVals, PROPERTY_PREFIX);
break;
case ROUTER_KEY:
final Map<String, Object> routerProperties = (Map<String, Object>) v2MapVals.remove(V2ApiConstants.ROUTER_KEY);
flattenMapWithPrefix(routerProperties, v2MapVals, CollectionAdminParams.ROUTER_PREFIX);
break;
case V2ApiConstants.CONFIG:
v2MapVals.put(CollectionAdminParams.COLL_CONF, v2MapVals.remove(V2ApiConstants.CONFIG));
break;
case V2ApiConstants.SHUFFLE_NODES:
v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM, v2MapVals.remove(V2ApiConstants.SHUFFLE_NODES));
break;
case V2ApiConstants.NODE_SET:
final List<String> nodeSetList = (List<String>) v2MapVals.remove(V2ApiConstants.NODE_SET);
final String nodeSetStr = String.join(",", nodeSetList);
v2MapVals.put(CollectionAdminParams.CREATE_NODE_SET_PARAM, nodeSetStr);
break;
default:
break;
}
}
}

private void flattenMapWithPrefix(Map<String, Object> toFlatten, Map<String, Object> destination,
String additionalPrefix) {
if (toFlatten == null || toFlatten.isEmpty() || destination == null) {
return;
}

toFlatten.forEach((k, v) -> destination.put(additionalPrefix + k, v));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make this recursive to support potentially nested v2 payloads?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually tried this initially but found the result to be a lot like the code in CollectionsApiMapping.Meta, which I personally find tough to read with all the map-literals for renaming properties and prefixing properties. I guessed it was more readable to just call this non-recursive version twice. That wouldn't work as well on APIs with deeper nesting, but seemed feasible here.

I'm open to going back that direction though if you prefer it. Did you have a particular signature in mind?

}
}

@EndPoint(path = {"/c/{collection}", "/collections/{collection}"},
method = DELETE,
permission = COLL_EDIT_PERM)
public void deleteCollection(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
req = ClusterAPI.wrapParams(req, "action",
req = wrapParams(req, ACTION,
CollectionAction.DELETE.toString(),
NAME, req.getPathTemplateValues().get(ZkStateReader.COLLECTION_PROP));
collectionsHandler.handleRequestBody(req, rsp);
Expand Down
2 changes: 2 additions & 0 deletions solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ public void testCommands() throws Exception {
ApiBag apiBag;
try (MockCollectionsHandler collectionsHandler = new MockCollectionsHandler()) {
apiBag = new ApiBag(false);
final CollectionsAPI collectionsAPI = new CollectionsAPI(collectionsHandler);
apiBag.registerObject(new CollectionsAPI(collectionsHandler));
apiBag.registerObject(collectionsAPI.collectionsCommands);
Collection<Api> apis = collectionsHandler.getApis();
for (Api api : apis) apiBag.register(api, Collections.emptyMap());

Expand Down
Loading