epugh commented on code in PR #4177: URL: https://github.com/apache/solr/pull/4177#discussion_r2874172569
########## solr/core/src/test/org/apache/solr/handler/admin/api/UpdateAPITest.java: ########## @@ -0,0 +1,125 @@ +/* + * 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.core.CoreContainer.ALLOW_PATHS_SYSPROP; + +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.request.GenericV2SolrRequest; +import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.client.solrj.request.RequestWriter; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.EnvUtils; +import org.apache.solr.util.ExternalPaths; +import org.apache.solr.util.SolrJettyTestRule; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +/** + * Integration tests for the v2 update API endpoints implemented via JAX-RS in {@link + * org.apache.solr.handler.admin.api.UpdateAPI}. + */ +public class UpdateAPITest extends SolrTestCaseJ4 { + + @ClassRule public static SolrJettyTestRule solrTestRule = new SolrJettyTestRule(); + + private static final String CORE_NAME = DEFAULT_TEST_COLLECTION_NAME; + + @BeforeClass + public static void beforeClass() throws Exception { + EnvUtils.setProperty( + ALLOW_PATHS_SYSPROP, ExternalPaths.SERVER_HOME.toAbsolutePath().toString()); + solrTestRule.startSolr(createTempDir()); + solrTestRule + .newCollection(CORE_NAME) + .withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET) + .create(); + } + + @Test + public void testUpdateViaV2Api() throws Exception { + final SolrClient client = solrTestRule.getSolrClient(CORE_NAME); + + // POST a JSON array of documents via the V2 /update endpoint (rewrites to /update/json/docs) + final GenericV2SolrRequest addReq = + new GenericV2SolrRequest(SolrRequest.METHOD.POST, "/cores/" + CORE_NAME + "/update"); + addReq.setContentWriter( + new RequestWriter.StringPayloadContentWriter( + "[{\"id\":\"v2update1\",\"title\":\"V2 update test\"}]", "application/json")); + client.request(addReq); + + // Commit via standard SolrJ commit (v2 /update is docs-only and does not support commands) + client.commit(CORE_NAME); + + // Verify the document was indexed + final ModifiableSolrParams queryParams = new ModifiableSolrParams(); Review Comment: is there a more concise way of doing this? ########## solr/core/src/test/org/apache/solr/handler/admin/api/UpdateAPITest.java: ########## @@ -0,0 +1,125 @@ +/* + * 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.core.CoreContainer.ALLOW_PATHS_SYSPROP; + +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.request.GenericV2SolrRequest; +import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.client.solrj.request.RequestWriter; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.EnvUtils; +import org.apache.solr.util.ExternalPaths; +import org.apache.solr.util.SolrJettyTestRule; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +/** + * Integration tests for the v2 update API endpoints implemented via JAX-RS in {@link + * org.apache.solr.handler.admin.api.UpdateAPI}. + */ +public class UpdateAPITest extends SolrTestCaseJ4 { + + @ClassRule public static SolrJettyTestRule solrTestRule = new SolrJettyTestRule(); + + private static final String CORE_NAME = DEFAULT_TEST_COLLECTION_NAME; + + @BeforeClass + public static void beforeClass() throws Exception { + EnvUtils.setProperty( + ALLOW_PATHS_SYSPROP, ExternalPaths.SERVER_HOME.toAbsolutePath().toString()); + solrTestRule.startSolr(createTempDir()); + solrTestRule + .newCollection(CORE_NAME) + .withConfigSet(ExternalPaths.TECHPRODUCTS_CONFIGSET) + .create(); + } + + @Test + public void testUpdateViaV2Api() throws Exception { + final SolrClient client = solrTestRule.getSolrClient(CORE_NAME); + + // POST a JSON array of documents via the V2 /update endpoint (rewrites to /update/json/docs) Review Comment: While we CAN do it this way, can't we also do it with the solrj generaed code? ########## solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java: ########## @@ -17,52 +17,93 @@ package org.apache.solr.handler.admin.api; -import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST; import static org.apache.solr.common.params.CommonParams.PATH; import static org.apache.solr.security.PermissionNameProvider.Name.UPDATE_PERM; -import org.apache.solr.api.EndPoint; +import jakarta.inject.Inject; +import org.apache.solr.api.JerseyResource; +import org.apache.solr.client.api.endpoint.UpdateApi; +import org.apache.solr.client.api.model.SolrJerseyResponse; +import org.apache.solr.common.SolrException; +import org.apache.solr.core.SolrCore; import org.apache.solr.handler.UpdateRequestHandler; +import org.apache.solr.jersey.APIConfigProvider; +import org.apache.solr.jersey.PermissionName; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; /** - * All v2 APIs that share a prefix of /update + * V2 API implementation for indexing documents. * - * <p>Most of these v2 APIs are implemented as pure "pass-throughs" to the v1 code paths, but there - * are a few exceptions: /update and /update/json are both rewritten to /update/json/docs. + * <p>These APIs delegate to the v1 {@link UpdateRequestHandler}. The {@code /update} and {@code + * /update/json} paths are rewritten to {@code /update/json/docs} so that JSON arrays of documents + * are processed by the JSON loader rather than the update-command loader. */ -public class UpdateAPI { +public class UpdateAPI extends JerseyResource implements UpdateApi { + private final UpdateRequestHandler updateRequestHandler; + private final SolrQueryRequest solrQueryRequest; + private final SolrQueryResponse solrQueryResponse; + + @Inject + public UpdateAPI( + UpdateRequestHandlerConfig handlerConfig, + SolrQueryRequest solrQueryRequest, + SolrQueryResponse solrQueryResponse) { + this.updateRequestHandler = handlerConfig.updateRequestHandler; + this.solrQueryRequest = solrQueryRequest; + this.solrQueryResponse = solrQueryResponse; + } - public UpdateAPI(UpdateRequestHandler updateRequestHandler) { - this.updateRequestHandler = updateRequestHandler; + @Override + @PermissionName(UPDATE_PERM) + public SolrJerseyResponse update() throws Exception { + return handleUpdate(UpdateRequestHandler.DOC_PATH); } - @EndPoint(method = POST, path = "/update", permission = UPDATE_PERM) - public void update(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - req.getContext().put(PATH, "/update/json/docs"); - updateRequestHandler.handleRequest(req, rsp); + @Override + @PermissionName(UPDATE_PERM) + public SolrJerseyResponse updateJson() { + return handleUpdate(UpdateRequestHandler.DOC_PATH); } - @EndPoint(method = POST, path = "/update/xml", permission = UPDATE_PERM) - public void updateXml(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - updateRequestHandler.handleRequest(req, rsp); + @Override + @PermissionName(UPDATE_PERM) + public SolrJerseyResponse updateXml() { + return handleUpdate(null); } - @EndPoint(method = POST, path = "/update/csv", permission = UPDATE_PERM) - public void updateCsv(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - updateRequestHandler.handleRequest(req, rsp); + @Override + @PermissionName(UPDATE_PERM) + public SolrJerseyResponse updateCsv() { + return handleUpdate(null); } - @EndPoint(method = POST, path = "/update/json", permission = UPDATE_PERM) - public void updateJson(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - req.getContext().put(PATH, "/update/json/docs"); - updateRequestHandler.handleRequest(req, rsp); + @Override + @PermissionName(UPDATE_PERM) + public SolrJerseyResponse updateBin() { + return handleUpdate(null); } - @EndPoint(method = POST, path = "/update/bin", permission = UPDATE_PERM) - public void updateJavabin(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - updateRequestHandler.handleRequest(req, rsp); + private SolrJerseyResponse handleUpdate(String pathOverride) { + final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class); + if (pathOverride != null) { + solrQueryRequest.getContext().put(PATH, pathOverride); + } + SolrCore.preDecorateResponse(solrQueryRequest, solrQueryResponse); + updateRequestHandler.handleRequest(solrQueryRequest, solrQueryResponse); + SolrCore.postDecorateResponse(updateRequestHandler, solrQueryRequest, solrQueryResponse); Review Comment: one of the tests was failing with out this... ########## solr/core/src/java/org/apache/solr/jersey/container/ContainerRequestUtils.java: ########## @@ -140,4 +144,20 @@ private static String getServerAddress(URI baseUri) { } return serverAddress; } + + /** + * Normalizes the {@code /c/} collection alias to {@code /collections/} so that JAX-RS path + * matching works correctly. The V2 API supports {@code /c/} as a shorthand for {@code + * /collections/}, but JAX-RS path templates use the regex {@code cores|collections} to match the + * index-type segment. + */ + static String normalizeCAlias(String uri) { + if (uri.startsWith("/c/")) { + return "/collections/" + uri.substring(3); + } + if (uri.equals("/c")) { + return "/collections"; + } + return uri; Review Comment: I think we may revmoe this code so won't worry about it. ########## solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java: ########## @@ -17,52 +17,93 @@ package org.apache.solr.handler.admin.api; -import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST; import static org.apache.solr.common.params.CommonParams.PATH; import static org.apache.solr.security.PermissionNameProvider.Name.UPDATE_PERM; -import org.apache.solr.api.EndPoint; +import jakarta.inject.Inject; +import org.apache.solr.api.JerseyResource; +import org.apache.solr.client.api.endpoint.UpdateApi; +import org.apache.solr.client.api.model.SolrJerseyResponse; +import org.apache.solr.common.SolrException; +import org.apache.solr.core.SolrCore; import org.apache.solr.handler.UpdateRequestHandler; +import org.apache.solr.jersey.APIConfigProvider; +import org.apache.solr.jersey.PermissionName; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; /** - * All v2 APIs that share a prefix of /update + * V2 API implementation for indexing documents. * - * <p>Most of these v2 APIs are implemented as pure "pass-throughs" to the v1 code paths, but there - * are a few exceptions: /update and /update/json are both rewritten to /update/json/docs. + * <p>These APIs delegate to the v1 {@link UpdateRequestHandler}. The {@code /update} and {@code + * /update/json} paths are rewritten to {@code /update/json/docs} so that JSON arrays of documents + * are processed by the JSON loader rather than the update-command loader. */ -public class UpdateAPI { +public class UpdateAPI extends JerseyResource implements UpdateApi { + private final UpdateRequestHandler updateRequestHandler; + private final SolrQueryRequest solrQueryRequest; + private final SolrQueryResponse solrQueryResponse; + + @Inject + public UpdateAPI( + UpdateRequestHandlerConfig handlerConfig, + SolrQueryRequest solrQueryRequest, + SolrQueryResponse solrQueryResponse) { + this.updateRequestHandler = handlerConfig.updateRequestHandler; + this.solrQueryRequest = solrQueryRequest; + this.solrQueryResponse = solrQueryResponse; + } Review Comment: updated the title! -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
