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

Jump to bottom

SOLR-15749: Create v2 equivalent for 'rename' collection API #1080

Merged
merged 11 commits into from Nov 9, 2022
Merged

SOLR-15749: Create v2 equivalent for 'rename' collection API #1080

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fb50537
Created v2 equivalent of v1 'CLUSTERSTATUS' command
joshgog Oct 10, 2022
9273f9f
Created a v2 equivalent of v1 'RENAME'
AAnakhe Oct 15, 2022
edd217a
Revert "Created v2 equivalent of v1 'CLUSTERSTATUS' command"
AAnakhe Oct 15, 2022
ec887df
Test Class for V2 RenameClusterAPI
AAnakhe Oct 15, 2022
468f811
Modified v2 equivalent of v1 'RENAME'
AAnakhe Oct 18, 2022
9109a59
Update: Text method updated to V2 syntax
AAnakhe Oct 31, 2022
6f7c37b
Update: V2RenameCollectionAPItest
AAnakhe Nov 8, 2022
f497b74
Merge branch 'main' into aanakhe_2
gerlowskija Nov 8, 2022
5c86d51
Minor cleanup and updates
gerlowskija Nov 8, 2022
975d1e6
Add coverage for v2 RENAME to ref-guide
gerlowskija Nov 9, 2022
50e3ac9
Add CHANGES.txt entry
gerlowskija Nov 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 2 additions & 19 deletions solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
View file
Open in desktop
Expand Up @@ -200,25 +200,7 @@
import org.apache.solr.core.snapshots.CollectionSnapshotMetaData;
import org.apache.solr.core.snapshots.SolrSnapshotManager;
import org.apache.solr.handler.RequestHandlerBase;
import org.apache.solr.handler.admin.api.AddReplicaAPI;
import org.apache.solr.handler.admin.api.AddReplicaPropertyAPI;
import org.apache.solr.handler.admin.api.AdminAPIBase;
import org.apache.solr.handler.admin.api.BalanceShardUniqueAPI;
import org.apache.solr.handler.admin.api.CollectionStatusAPI;
import org.apache.solr.handler.admin.api.CreateShardAPI;
import org.apache.solr.handler.admin.api.DeleteCollectionAPI;
import org.apache.solr.handler.admin.api.DeleteReplicaAPI;
import org.apache.solr.handler.admin.api.DeleteReplicaPropertyAPI;
import org.apache.solr.handler.admin.api.DeleteShardAPI;
import org.apache.solr.handler.admin.api.ForceLeaderAPI;
import org.apache.solr.handler.admin.api.MigrateDocsAPI;
import org.apache.solr.handler.admin.api.ModifyCollectionAPI;
import org.apache.solr.handler.admin.api.MoveReplicaAPI;
import org.apache.solr.handler.admin.api.RebalanceLeadersAPI;
import org.apache.solr.handler.admin.api.ReloadCollectionAPI;
import org.apache.solr.handler.admin.api.SetCollectionPropertyAPI;
import org.apache.solr.handler.admin.api.SplitShardAPI;
import org.apache.solr.handler.admin.api.SyncShardAPI;
import org.apache.solr.handler.admin.api.*;
import org.apache.solr.handler.api.V2ApiUtils;
import org.apache.solr.jersey.SolrJerseyResponse;
import org.apache.solr.logging.MDCLoggingContext;
Expand Down Expand Up @@ -2094,6 +2076,7 @@ public Collection<Api> getApis() {
apis.addAll(AnnotatedApi.getApis(new ReloadCollectionAPI(this)));
apis.addAll(AnnotatedApi.getApis(new SetCollectionPropertyAPI(this)));
apis.addAll(AnnotatedApi.getApis(new CollectionStatusAPI(this)));
apis.addAll(AnnotatedApi.getApis(new RenameCollectionAPI(this)));
return apis;
}

Expand Down
83 changes: 83 additions & 0 deletions solr/core/src/java/org/apache/solr/handler/admin/api/RenameCollectionAPI.java
View file
Open in desktop
@@ -0,0 +1,83 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.solr.handler.admin.api;

import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.solr.api.EndPoint;
import org.apache.solr.client.solrj.request.beans.RenameCollectionPayload;
import org.apache.solr.common.params.CollectionAdminParams;
import org.apache.solr.common.params.CollectionParams;
import org.apache.solr.common.util.ContentStream;
import org.apache.solr.handler.admin.CollectionsHandler;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.util.SolrJacksonAnnotationInspector;

import java.util.Locale;

import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
import static org.apache.solr.common.cloud.ZkStateReader.ALIASES;
import static org.apache.solr.common.params.CollectionAdminParams.TARGET;
import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
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;


public class RenameCollectionAPI {
gerlowskija marked this conversation as resolved.
Show resolved Hide resolved



private final CollectionsHandler collectionsHandler;
public RenameCollectionAPI(CollectionsHandler collectionsHandler) {
this.collectionsHandler = collectionsHandler;
}

public static final String RENAME_COLLECTION_CMD = "rename";
AAnakhe marked this conversation as resolved.
Show resolved Hide resolved

private static final ObjectMapper mapper = SolrJacksonAnnotationInspector.createObjectMapper()
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
.disable(MapperFeature.AUTO_DETECT_FIELDS);

@EndPoint(
path = {"/collections/{collection}/command/rename"},
AAnakhe marked this conversation as resolved.
Show resolved Hide resolved
method = POST,
permission = COLL_EDIT_PERM)
public void renameCollection(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception{

final ContentStream cs = req.getContentStreams().iterator().next();
RenameCollectionPayload v2Body = mapper.readValue(cs.getStream(), RenameCollectionPayload.class);

req =
wrapParams(
req,
ACTION,
CollectionParams.CollectionAction.RENAME.name().toLowerCase(Locale.ROOT),
NAME,
req.getPathTemplateValues().get(CollectionAdminParams.COLLECTION),
TARGET,
v2Body.to,
ASYNC,
v2Body.async,
ALIASES,
AAnakhe marked this conversation as resolved.
Show resolved Hide resolved
v2Body.followAliases);
collectionsHandler.handleRequestBody(req, rsp);
}
}
64 changes: 60 additions & 4 deletions solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java
View file
Open in desktop
Expand Up @@ -17,20 +17,28 @@

package org.apache.solr.handler.admin.api;

import static org.apache.solr.common.params.CollectionAdminParams.COLLECTION;
import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
import static org.apache.solr.common.params.CollectionAdminParams.*;
import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
import static org.apache.solr.common.params.CommonParams.ACTION;
import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.common.params.CommonParams.*;
import static org.apache.solr.common.params.CoreAdminParams.SHARD;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.google.common.collect.Maps;
import org.apache.solr.api.Api;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CollectionParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.CommandOperation;
import org.apache.solr.handler.admin.CollectionsHandler;
import org.apache.solr.handler.admin.TestCollectionAPIs;
import org.apache.solr.handler.admin.V2ApiMappingTest;
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
import org.junit.Test;

/**
Expand Down Expand Up @@ -62,6 +70,7 @@ public void populateApiBag() {
apiBag.registerObject(new ReloadCollectionAPI(collectionsHandler));
apiBag.registerObject(new SetCollectionPropertyAPI(collectionsHandler));
apiBag.registerObject(new CollectionStatusAPI(collectionsHandler));
apiBag.registerObject(new RenameCollectionAPI(collectionsHandler));
}

@Override
Expand All @@ -85,6 +94,20 @@ public void testGetCollectionStatus() throws Exception {
assertEquals("shard2", v1Params.get(SHARD));
}


@Test
public void testRenameCollectionAllParams(){
final SolrQueryRequest request = runRenameCollectionsApi("/collections/collName");
Copy link
Contributor

@joshgog joshgog Nov 1, 2022 •
edited

Choose a reason for hiding this comment

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

Test result returns java.lang.AssertionError: expected:<rename> but was:<null>

What if we mock SolrQueryRequest and SolrQueryResponse , pass them to thepublic void renameCollection(SolrQueryRequest req, SolrQueryResponse rsp) method of the API then perform the assertions

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 will try to implement that, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test result returns java.lang.AssertionError: expected:<rename> but was:<null>

What if we mock SolrQueryRequest and SolrQueryResponse , pass them to thepublic void renameCollection(SolrQueryRequest req, SolrQueryResponse rsp) method of the API then perform the assertions

Please I need more clarification on this

Copy link
Contributor

@joshgog joshgog Nov 1, 2022 •
edited

Choose a reason for hiding this comment

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

I'm not sure what exactly you would like to be clarified. If its about mocks checkout AddReplicaPropertyAPITest which uses mocked SolrQueryRequest and SolrQueryResponse

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to what Josh said. AddReplicaPropertyAPITest is a good example on how to use mocks, but it's setup for APIs that use the JAX-RS framework, so you wouldn't want to follow it too closely. But it's good if you're just looking for an example on the mock stuff.

And I'm sure we'd both be happy to clarify if you can elaborate a bit on what you need clarified.

[Q] One thing that jumps out at me on this line is the runRenameCollectionsAPI: is there a reason you've created a separate method for this instead of reusing one of the captureConvertedV1Params methods that's already out there. If there's not some reason for runRenameCollectionsAPI that I'm missing, you could presumably just replace its usage here with a snippet like:

    final SolrParams v1Params = captureConvertedV1Params("/collections/collName/commands/rename", "POST",
            "{\"to\": \"targetColl\", \"async\": \"requestTrackingId\", \"followAliases\": true}");

The second thing that jumps out at me here is the path being sent as an argument to this method - it doesn't match the path of the API you created in this PR (see here).

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 thought you said we were moving away from the old captureConvertedV1Params syntax, I wasn't aware there are different method implementations

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, not quite. Or at least, I didn't mean to suggest that?

What I probably meant to say is that Solr is moving away from its traditional v2 API framework to a framework based on JAX-RS. captureConvertedV1Params is a nice utility for testing APIs implemented using the "traditional" (i.e. non JAX-RS) framework.

In the long run I'd expect to see usage of captureConvertedV1Params decline, but not because it's deprecated or shouldn't be used - just because things will switch over to JAX-RS as time goes by. This PR uses the traditional framework (which is absolutely still valid), so it's fine/normal to use captureConvertedV1Params in your tests.

I'll push up a commit here to help get the unit test over the line and then hopefully we can get this merged 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's clearer now, and I'm so glad to hear that this will be merged thank you.


assertEquals("rename", request.getContext().get(ACTION));
assertEquals("collName", request.getContext().get(NAME));
assertEquals("targetColl", request.getContext().get(TARGET));
assertEquals("requestTrackingId", request.getContext().get(ASYNC));
assertEquals(true, request.getParams().getPrimitiveBool("followAliases"));

}


@Test
public void testModifyCollectionAllProperties() throws Exception {
final SolrParams v1Params =
Expand Down Expand Up @@ -247,4 +270,37 @@ public void testSetCollectionPropertyAllProperties() throws Exception {
assertEquals("somePropertyName", v1Params.get("propertyName"));
assertEquals("somePropertyValue", v1Params.get("propertyValue"));
}






private SolrQueryRequest runRenameCollectionsApi(String path) {
final HashMap<String, String> parts = new HashMap<>();
final Api api = apiBag.lookup(path, "POST", parts);
final SolrQueryResponse rsp = new SolrQueryResponse();
final LocalSolrQueryRequest req =
new LocalSolrQueryRequest(null, Maps.newHashMap()) {
@Override
public List<CommandOperation> getCommands(boolean validateInput) {
return Collections.emptyList();
}

@Override
public Map<String, String> getPathTemplateValues() {
return parts;
}

@Override
public String getHttpMethod() {
return "POST";
}
};
req.getContext().put(PATH, path);

api.call(req, rsp);

return req;
}
}
31 changes: 31 additions & 0 deletions solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/RenameCollectionPayload.java
View file
Open in desktop
@@ -0,0 +1,31 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.solr.client.solrj.request.beans;

import org.apache.solr.common.annotation.JsonProperty;
import org.apache.solr.common.util.ReflectMapWriter;

public class RenameCollectionPayload implements ReflectMapWriter {
@JsonProperty
public String async;

@JsonProperty
public Boolean followAliases;

@JsonProperty(required = true)
public String to;
}