You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ge...@apache.org on 2023/03/15 13:46:11 UTC
[solr] 02/02: SOLR-16391: Migrate collprop APIs to JAX-RS (#1441)
This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
commit 3a0771202cfe7b3841a1aca249b39b35b5df54c0
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Mon Mar 13 12:45:23 2023 -0400
SOLR-16391: Migrate collprop APIs to JAX-RS (#1441)
This commit makes various cosmetic improvements to Solr's v2 collprop
CRUD APIs, to bring them more into line with the more REST-ful v2
design. In the process it also converts these APIs to our JAX-RS
framework.
As of this commit, our v2 collprop APIs are now:
- PUT /api/collections/collName/properties/propName {value: newVal}
- DELETE /api/collections/collName/properties/propName
---
solr/CHANGES.txt | 4 +
.../solr/handler/admin/CollectionsHandler.java | 31 +++----
.../handler/admin/api/CollectionPropertyAPI.java | 100 +++++++++++++++++++++
.../admin/api/SetCollectionPropertyAPI.java | 71 ---------------
.../solr/handler/admin/TestCollectionAPIs.java | 7 --
.../admin/api/V2CollectionAPIMappingTest.java | 18 ----
.../pages/collection-management.adoc | 23 +++--
.../beans/SetCollectionPropertyPayload.java | 27 ------
8 files changed, 135 insertions(+), 146 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a6373a82a54..fd3dcbb1439 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -152,6 +152,10 @@ Improvements
* SOLR-16397: /mlt now has a v2 API available at `GET /api/collections/collName/mlt` (Ameer Albahem via Jason Gerlowski)
+* SOLR-16391: The path of the v2 "collprop" API has been tweaked slightly to be more intuitive. Endpoints are now
+ available under the `PUT` and `DELETE` verbs at `/api/collections/collName/properties/propName` depending on whether the property is
+ being upserted or deleted. (Jason Gerlowski)
+
Optimizations
---------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index 42430bf39e7..e154824ea29 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
@@ -171,7 +171,6 @@ import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.ClusterProperties;
import org.apache.solr.common.cloud.ClusterState;
-import org.apache.solr.common.cloud.CollectionProperties;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.DocCollection.CollectionStateProps;
import org.apache.solr.common.cloud.ImplicitDocRouter;
@@ -209,6 +208,7 @@ 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.CollectionPropertyAPI;
import org.apache.solr.handler.admin.api.CollectionStatusAPI;
import org.apache.solr.handler.admin.api.CreateShardAPI;
import org.apache.solr.handler.admin.api.DeleteCollectionAPI;
@@ -226,7 +226,6 @@ import org.apache.solr.handler.admin.api.RebalanceLeadersAPI;
import org.apache.solr.handler.admin.api.ReloadCollectionAPI;
import org.apache.solr.handler.admin.api.RenameCollectionAPI;
import org.apache.solr.handler.admin.api.ReplaceNodeAPI;
-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.api.V2ApiUtils;
@@ -1049,18 +1048,20 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
COLLECTIONPROP_OP(
COLLECTIONPROP,
(req, rsp, h) -> {
- String extCollection = req.getParams().required().get(NAME);
- String collection =
- h.coreContainer
- .getZkController()
- .getZkStateReader()
- .getAliases()
- .resolveSimpleAlias(extCollection);
- String name = req.getParams().required().get(PROPERTY_NAME);
- String val = req.getParams().get(PROPERTY_VALUE);
- CollectionProperties cp =
- new CollectionProperties(h.coreContainer.getZkController().getZkClient());
- cp.setCollectionProperty(collection, name, val);
+ final String collection = req.getParams().required().get(NAME);
+ final String propName = req.getParams().required().get(PROPERTY_NAME);
+ final String val = req.getParams().get(PROPERTY_VALUE);
+
+ final CollectionPropertyAPI setCollPropApi =
+ new CollectionPropertyAPI(h.coreContainer, req, rsp);
+ final SolrJerseyResponse setPropRsp =
+ (val != null)
+ ? setCollPropApi.createOrUpdateCollectionProperty(
+ collection,
+ propName,
+ new CollectionPropertyAPI.UpdateCollectionPropertyRequestBody(val))
+ : setCollPropApi.deleteCollectionProperty(collection, propName);
+ V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, setPropRsp);
return null;
}),
REQUESTSTATUS_OP(
@@ -2077,6 +2078,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
DeleteReplicaPropertyAPI.class,
ListCollectionsAPI.class,
ReplaceNodeAPI.class,
+ CollectionPropertyAPI.class,
DeleteNodeAPI.class,
ListAliasesAPI.class);
}
@@ -2097,7 +2099,6 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
apis.addAll(AnnotatedApi.getApis(new MoveReplicaAPI(this)));
apis.addAll(AnnotatedApi.getApis(new RebalanceLeadersAPI(this)));
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;
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/CollectionPropertyAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/CollectionPropertyAPI.java
new file mode 100644
index 00000000000..72d3c1e8145
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/handler/admin/api/CollectionPropertyAPI.java
@@ -0,0 +1,100 @@
+/*
+ * 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 static org.apache.solr.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.io.IOException;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+import org.apache.solr.common.cloud.CollectionProperties;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for modifying collection-level properties.
+ *
+ * <p>These APIs (PUT and DELETE /api/collections/collName/properties/propName) are analogous to the
+ * v1 /admin/collections?action=COLLECTIONPROP command.
+ */
+@Path("/collections/{collName}/properties/{propName}")
+public class CollectionPropertyAPI extends AdminAPIBase {
+
+ public CollectionPropertyAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @PUT
+ @PermissionName(COLL_EDIT_PERM)
+ @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML, BINARY_CONTENT_TYPE_V2})
+ public SolrJerseyResponse createOrUpdateCollectionProperty(
+ @PathParam("collName") String collName,
+ @PathParam("propName") String propName,
+ UpdateCollectionPropertyRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ recordCollectionForLogAndTracing(collName, solrQueryRequest);
+ modifyCollectionProperty(collName, propName, requestBody.value);
+ return response;
+ }
+
+ @DELETE
+ @PermissionName(COLL_EDIT_PERM)
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ public SolrJerseyResponse deleteCollectionProperty(
+ @PathParam("collName") String collName, @PathParam("propName") String propName)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ recordCollectionForLogAndTracing(collName, solrQueryRequest);
+ modifyCollectionProperty(collName, propName, null);
+ return response;
+ }
+
+ private void modifyCollectionProperty(
+ String collection, String propertyName, String propertyValue /* May be null for deletes */)
+ throws IOException {
+ String resolvedCollection = coreContainer.getAliases().resolveSimpleAlias(collection);
+ CollectionProperties cp =
+ new CollectionProperties(coreContainer.getZkController().getZkClient());
+ cp.setCollectionProperty(resolvedCollection, propertyName, propertyValue);
+ }
+
+ public static class UpdateCollectionPropertyRequestBody implements JacksonReflectMapWriter {
+ public UpdateCollectionPropertyRequestBody() {}
+
+ public UpdateCollectionPropertyRequestBody(String value) {
+ this.value = value;
+ }
+
+ @JsonProperty(required = true)
+ public String value;
+ }
+}
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/SetCollectionPropertyAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/SetCollectionPropertyAPI.java
deleted file mode 100644
index ea3c47abf03..00000000000
--- a/solr/core/src/java/org/apache/solr/handler/admin/api/SetCollectionPropertyAPI.java
+++ /dev/null
@@ -1,71 +0,0 @@
-/*
- * 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 static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
-import static org.apache.solr.common.params.CollectionAdminParams.COLLECTION;
-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 java.util.HashMap;
-import java.util.Map;
-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.SetCollectionPropertyPayload;
-import org.apache.solr.common.params.CollectionParams;
-import org.apache.solr.handler.admin.CollectionsHandler;
-
-/**
- * V2 API for modifying collection-level properties.
- *
- * <p>This API (POST /v2/collections/collectionName {'set-collection-property': {...}}) is analogous
- * to the v1 /admin/collections?action=COLLECTIONPROP command.
- *
- * @see SetCollectionPropertyPayload
- */
-@EndPoint(
- path = {"/c/{collection}", "/collections/{collection}"},
- method = POST,
- permission = COLL_EDIT_PERM)
-public class SetCollectionPropertyAPI {
- private static final String V2_SET_COLLECTION_PROPERTY_CMD = "set-collection-property";
-
- private final CollectionsHandler collectionsHandler;
-
- public SetCollectionPropertyAPI(CollectionsHandler collectionsHandler) {
- this.collectionsHandler = collectionsHandler;
- }
-
- @Command(name = V2_SET_COLLECTION_PROPERTY_CMD)
- public void setCollectionProperty(PayloadObj<SetCollectionPropertyPayload> obj) throws Exception {
- final SetCollectionPropertyPayload v2Body = obj.get();
- final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
-
- v1Params.put("propertyName", v1Params.remove("name"));
- if (v2Body.value != null) {
- v1Params.put("propertyValue", v2Body.value);
- }
- v1Params.put(ACTION, CollectionParams.CollectionAction.COLLECTIONPROP.toLower());
- v1Params.put(NAME, obj.getRequest().getPathTemplateValues().get(COLLECTION));
-
- collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
- }
-}
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java b/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
index 680eaa95747..326066239ad 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
@@ -223,13 +223,6 @@ public class TestCollectionAPIs extends SolrTestCaseJ4 {
POST,
"{migrate-docs : {forwardTimeout: 1800, target: coll2, splitKey: 'a123!'} }",
"{operation : migrate ,collection : coll1, target.collection:coll2, forward.timeout:1800, split.key:'a123!'}");
-
- compareOutput(
- apiBag,
- "/collections/coll1",
- POST,
- "{set-collection-property : {name: 'foo', value:'bar'} }",
- "{operation : collectionprop, name : coll1, propertyName:'foo', propertyValue:'bar'}");
}
static ZkNodeProps compareOutput(
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java
index e2ead1e452d..fa943c80740 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java
@@ -60,7 +60,6 @@ public class V2CollectionAPIMappingTest extends V2ApiMappingTest<CollectionsHand
apiBag.registerObject(new MoveReplicaAPI(collectionsHandler));
apiBag.registerObject(new RebalanceLeadersAPI(collectionsHandler));
apiBag.registerObject(new ReloadCollectionAPI(collectionsHandler));
- apiBag.registerObject(new SetCollectionPropertyAPI(collectionsHandler));
apiBag.registerObject(new CollectionStatusAPI(collectionsHandler));
apiBag.registerObject(new RenameCollectionAPI(collectionsHandler));
}
@@ -226,21 +225,4 @@ public class V2CollectionAPIMappingTest extends V2ApiMappingTest<CollectionsHand
assertEquals(123, v1Params.getPrimitiveInt("maxAtOnce"));
assertEquals(456, v1Params.getPrimitiveInt("maxWaitSeconds"));
}
-
- @Test
- public void testSetCollectionPropertyAllProperties() throws Exception {
- final SolrParams v1Params =
- captureConvertedV1Params(
- "/collections/collName",
- "POST",
- "{ 'set-collection-property': {"
- + "'name': 'somePropertyName', "
- + "'value': 'somePropertyValue' "
- + "}}");
-
- assertEquals(CollectionParams.CollectionAction.COLLECTIONPROP.lowerName, v1Params.get(ACTION));
- assertEquals("collName", v1Params.get(NAME));
- assertEquals("somePropertyName", v1Params.get("propertyName"));
- assertEquals("somePropertyValue", v1Params.get("propertyValue"));
- }
}
diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc
index cc330d9f36b..aeb9c249b9d 100644
--- a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc
+++ b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc
@@ -703,23 +703,28 @@ http://localhost:8983/solr/admin/collections?action=COLLECTIONPROP&name=techprod
====
[.tab-label]*V2 API*
+To create or update a collection property:
[source,bash]
----
-curl -X POST http://localhost:8983/api/collections/techproducts_v2 -H 'Content-Type: application/json' -d '
+curl -X PUT http://localhost:8983/api/collections/techproducts_v2/properties/foo -H 'Content-Type: application/json' -d '
{
- "set-collection-property": {
- "name": "foo",
- "value": "bar"
- }
+ "value": "bar"
}
'
----
+
+To delete an existing collection property:
+
+[source,bash]
+----
+curl -X DELETE http://localhost:8983/api/collections/techproducts_v2/properties/foo
+----
====
--
=== COLLECTIONPROP Parameters
-`name`::
+`name` (v1)::
+
[%autowidth,frame=none]
|===
@@ -727,8 +732,9 @@ curl -X POST http://localhost:8983/api/collections/techproducts_v2 -H 'Content-T
|===
+
The name of the collection for which the property would be set.
+Appears in the path of v2 requests.
-`propertyName` (v1), `name` (v2)::
+`propertyName` (v1)::
+
[%autowidth,frame=none]
|===
@@ -736,6 +742,7 @@ The name of the collection for which the property would be set.
|===
+
The name of the property.
+Appears in the path of v2 requests.
`propertyValue` (v1), `value` (v2)::
+
@@ -745,7 +752,7 @@ The name of the property.
|===
+
The value of the property.
-When not provided, the property is deleted.
+When not provided in v1 requests, the property is deleted.
=== COLLECTIONPROP Response
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/SetCollectionPropertyPayload.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/SetCollectionPropertyPayload.java
deleted file mode 100644
index 2719266c975..00000000000
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/SetCollectionPropertyPayload.java
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * 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 SetCollectionPropertyPayload implements ReflectMapWriter {
- @JsonProperty(required = true)
- public String name;
-
- @JsonProperty public String value = null;
-}