janhoy commented on code in PR #4177:
URL: https://github.com/apache/solr/pull/4177#discussion_r4102957107
##########
solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java:
##########
@@ -17,52 +17,168 @@
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 java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.client.api.endpoint.UpdateApi;
+import org.apache.solr.client.api.model.UpdateResponse;
+import org.apache.solr.client.api.model.VersionedDocument;
+import org.apache.solr.client.api.model.VersionedQuery;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.NamedList;
+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;
+ }
+
+ // Query parameters like commit, overwrite, etc are declared as method
arguments for the
+ // JAX-RS/OpenAPI contract and via magic are read in by the handler.
+ @Override
+ @PermissionName(UPDATE_PERM)
+ public UpdateResponse update(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody)
+ throws Exception {
+ return handleUpdate(null);
+ }
- public UpdateAPI(UpdateRequestHandler updateRequestHandler) {
- this.updateRequestHandler = updateRequestHandler;
+ @Override
+ @PermissionName(UPDATE_PERM)
+ public UpdateResponse updateJson(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ 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 UpdateResponse updateXml(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ return handleUpdate(null);
}
- @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 UpdateResponse updateCsv(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ 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 UpdateResponse updateJavabin(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ return handleUpdate(UpdateRequestHandler.BIN_PATH);
}
- @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);
+ private UpdateResponse handleUpdate(String pathOverride) {
+ final UpdateResponse response =
instantiateJerseyResponse(UpdateResponse.class);
+ if (pathOverride != null) {
+ solrQueryRequest.getContext().put(PATH, pathOverride);
+ }
+ // The distributed update processor writes replication metadata into the
legacy response
+ // header while handling the request. Initialize it for the handler, then
leave serialization
+ // to the typed Jersey response so only one responseHeader is returned to
the client.
+ SolrCore.preDecorateResponse(solrQueryRequest, solrQueryResponse);
+ try {
+ updateRequestHandler.handleRequest(solrQueryRequest, solrQueryResponse);
Review Comment:
Have not checked this myself, but Claude claims the following:
----
Metrics end up counted twice. `PluginBag:278` maps `UpdateAPI` ->
`V2UpdateRequestHandler`, so `PreRequestMetricsFilter` has already incremented
`requests` and started a timer before this line; `handleRequest` then does both
again on the same handler. Errors double too: `processErrorMetricsOnException`
here, then again via `CatchAllExceptionMapper`. Other JAX-RS resources avoid
this by not re-entering the v1 handler.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java:
##########
@@ -17,52 +17,168 @@
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 java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.client.api.endpoint.UpdateApi;
+import org.apache.solr.client.api.model.UpdateResponse;
+import org.apache.solr.client.api.model.VersionedDocument;
+import org.apache.solr.client.api.model.VersionedQuery;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.NamedList;
+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;
+ }
+
+ // Query parameters like commit, overwrite, etc are declared as method
arguments for the
+ // JAX-RS/OpenAPI contract and via magic are read in by the handler.
+ @Override
+ @PermissionName(UPDATE_PERM)
+ public UpdateResponse update(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody)
+ throws Exception {
+ return handleUpdate(null);
+ }
- public UpdateAPI(UpdateRequestHandler updateRequestHandler) {
- this.updateRequestHandler = updateRequestHandler;
+ @Override
+ @PermissionName(UPDATE_PERM)
+ public UpdateResponse updateJson(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ 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 UpdateResponse updateXml(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ return handleUpdate(null);
}
- @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 UpdateResponse updateCsv(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ 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 UpdateResponse updateJavabin(
+ Boolean commit,
+ Integer commitWithin,
+ Boolean overwrite,
+ Boolean softCommit,
+ Boolean versions,
+ InputStream requestBody) {
+ return handleUpdate(UpdateRequestHandler.BIN_PATH);
}
- @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);
+ private UpdateResponse handleUpdate(String pathOverride) {
+ final UpdateResponse response =
instantiateJerseyResponse(UpdateResponse.class);
+ if (pathOverride != null) {
+ solrQueryRequest.getContext().put(PATH, pathOverride);
+ }
+ // The distributed update processor writes replication metadata into the
legacy response
+ // header while handling the request. Initialize it for the handler, then
leave serialization
+ // to the typed Jersey response so only one responseHeader is returned to
the client.
+ SolrCore.preDecorateResponse(solrQueryRequest, solrQueryResponse);
+ try {
+ updateRequestHandler.handleRequest(solrQueryRequest, solrQueryResponse);
+ } finally {
+ solrQueryResponse.getValues().remove("responseHeader");
Review Comment:
This drops the whole legacy header, not just the duplicate status/QTime. Two
processors put payload in there:
- `DistributedZkUpdateProcessor:1391` -> `rf` (achieved replication factor)
- `TolerantUpdateProcessor:125,276` -> `errors` / `maxErrors`
So an update with `maxErrors=10` where 3 docs fail now returns 200 with
`adds` and no `errors` list -- the client can't tell anything failed. Suggest
copying `rf`/`errors`/`maxErrors` into `UpdateResponse` before removing the
header.
##########
solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-update-handlers.adoc:
##########
@@ -507,10 +565,26 @@ CSV formatted update requests may be sent to Solr's
`/update` handler using `Con
A sample CSV file is provided at `example/exampledocs/books.csv` that you can
use to add some documents to the Solr "techproducts" example:
+[tabs#csv-file-request]
+======
+V1 API::
++
+====
[source,bash]
----
-curl 'http://localhost:8983/solr/my_collection/update?commit=true'
--data-binary @example/exampledocs/books.csv -H 'Content-type:application/csv'
+curl 'http://localhost:8983/solr/my_collection/update?commit=true' -H
'Content-type:application/csv' -d @example/exampledocs/books.csv
Review Comment:
This was `--data-binary @...` on main. `curl -d` strips newlines, which
destroys CSV record boundaries -- pasting this mangles the load. Same at 584,
and at 797/806 for the tab-separated example.
##########
solr/core/src/test/org/apache/solr/handler/admin/api/UpdateAPITest.java:
##########
@@ -0,0 +1,371 @@
+/*
+ * 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 java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import org.apache.solr.SolrTestCase;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.client.solrj.request.GenericV2SolrRequest;
+import org.apache.solr.client.solrj.request.JavaBinUpdateRequestCodec;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.client.solrj.response.JavaBinResponseParser;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.client.solrj.response.XMLResponseParser;
+import org.apache.solr.client.solrj.response.json.JsonMapResponseParser;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.EnvUtils;
+import org.apache.solr.common.util.NamedList;
+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 SolrTestCase {
+
+ @ClassRule public static SolrJettyTestRule solrTestRule = new
SolrJettyTestRule();
+
+ private static final String CORE_NAME = "update-api-test";
+ private static final String CUSTOM_JSON_CORE_NAME =
"custom-json-update-api-test";
+
+ @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();
+ // can't use techproducts config because it enables srcField, which is
incompatible with nested
+ // split=/exams requests
+ solrTestRule
+ .newCollection(CUSTOM_JSON_CORE_NAME)
+ .withConfigSet(ExternalPaths.DEFAULT_CONFIGSET)
+ .create();
+ }
+
+ @Test
+ public void testV1AndV2GenericUpdateParityAcrossFormats() throws Exception {
+ final SolrClient client = solrTestRule.getSolrClient(CORE_NAME);
+
+ for (UpdateFormat format : UpdateFormat.values()) {
+ final String v1Id = "parity-v1-" +
format.name().toLowerCase(Locale.ROOT);
+ final String v2Id = "parity-v2-" +
format.name().toLowerCase(Locale.ROOT);
+
+ final NamedList<Object> v1Response = sendV1Update(client, format, v1Id);
+ final NamedList<Object> v2Response = sendV2Update(client, format, v2Id);
+
+ assertLegacySuccessfulAdd(format, v1Id, v1Response);
+ assertTypedSuccessfulAdd(format, v2Id, v2Response);
+ assertIndexed(client, v1Id);
+ assertIndexed(client, v2Id);
+ }
+ }
+
+ @Test
+ public void testV1AndV2CustomJsonTransformParity() throws Exception {
+ final SolrClient client =
solrTestRule.getSolrClient(CUSTOM_JSON_CORE_NAME);
+ final String payload =
"{\"exams\":[{\"id\":\"custom-json-v1\",\"name\":\"V1 document\"}]}";
+
+ final ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("split", "/exams");
+ params.add("f", "id:/exams/id");
+ params.add("f", "name_s:/exams/name");
+ params.set("commit", true);
+
+ final GenericSolrRequest v1Request =
+ new GenericSolrRequest(SolrRequest.METHOD.POST, "/update/json/docs",
params);
+ v1Request.setRequiresCollection(true);
+ v1Request.setContentWriter(
+ new RequestWriter.StringPayloadContentWriter(payload,
"application/json"));
+ client.request(v1Request, CUSTOM_JSON_CORE_NAME);
+
+ final String v2Payload = payload.replace("custom-json-v1",
"custom-json-v2");
+ final GenericV2SolrRequest v2Request =
+ new GenericV2SolrRequest(
+ SolrRequest.METHOD.POST, "/cores/" + CUSTOM_JSON_CORE_NAME +
"/update/json", params);
+ v2Request.setContentWriter(
+ new RequestWriter.StringPayloadContentWriter(v2Payload,
"application/json"));
+ client.request(v2Request);
+
+ assertIndexedField(client, CUSTOM_JSON_CORE_NAME, "custom-json-v1",
"name_s", "V1 document");
+ assertIndexedField(client, CUSTOM_JSON_CORE_NAME, "custom-json-v2",
"name_s", "V1 document");
+ }
+
+ @Test
+ public void testUpdateJsonViaV2Api() throws Exception {
+ final SolrClient client = solrTestRule.getSolrClient(CORE_NAME);
+
+ // POST via the V2 /update/json endpoint (also rewrites to
/update/json/docs)
+ final GenericV2SolrRequest addReq =
+ new GenericV2SolrRequest(SolrRequest.METHOD.POST, "/cores/" +
CORE_NAME + "/update/json");
+ addReq.setContentWriter(
+ new RequestWriter.StringPayloadContentWriter(
+ "[{\"id\":\"v2updatejson1\",\"title\":\"V2 update/json test\"}]",
"application/json"));
+ client.request(addReq);
+
+ // Commit via standard SolrJ commit (v2 /update is docs-only and does not
support commands)
Review Comment:
Stale -- `/update` is no longer docs-only, it negotiates on Content-Type.
Same comment at :162.
##########
changelog/unreleased/SOLR-18457-migrate-update-api-to-jax-rs.yml:
##########
@@ -0,0 +1,7 @@
+title: Migrate v2 update endpoints to JAX-RS and rename the Javabin endpoint
to /update/javabin. Migrate Admin UI to using v2 endpoints, fixing UI issues
in the document upload screen. Document screen only works with /upload
endpoint, no longer can specify a custom endpoint.
Review Comment:
Typo: "only works with /upload endpoint" -> `/update`. Also worth splitting
this into a couple of sentences, and adding the techproducts schema change --
that part is user-visible.
##########
solr/webapp/src/test/org/apache/solr/webapp/AdminUiDocumentsScreenTest.java:
##########
@@ -44,6 +44,9 @@ public void testDocumentsScreenForm() {
assertTrue("Doc type dropdown should offer JSON, got " + types,
types.contains("JSON"));
assertTrue("Doc type dropdown should offer XML, got " + types,
types.contains("XML"));
assertTrue("Doc type dropdown should offer CSV, got " + types,
types.contains("CSV"));
+ assertTrue(
+ "Doc type dropdown should offer raw JSON commands, got " + types,
+ types.contains("Solr Command (raw JSON)"));
Review Comment:
These assert the dropdown only. All four document types were rewired to
different v2 methods, but just JSON is actually submitted -- the XML and CSV
submit paths have no coverage, which is where the `wt` problem in
`documents.js:105` lives.
##########
solr/api/src/java/org/apache/solr/client/api/endpoint/UpdateApi.java:
##########
@@ -0,0 +1,211 @@
+/*
+ * 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.api.endpoint;
+
+import static
org.apache.solr.client.api.util.Constants.GENERIC_ENTITY_PROPERTY;
+import static org.apache.solr.client.api.util.Constants.INDEX_PATH_PREFIX;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.extensions.Extension;
+import io.swagger.v3.oas.annotations.extensions.ExtensionProperty;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.QueryParam;
+import java.io.InputStream;
+import org.apache.solr.client.api.model.UpdateResponse;
+import org.apache.solr.client.api.util.StoreApiParameters;
+
+/** V2 API definitions for indexing documents via the update handler. */
+@Path(INDEX_PATH_PREFIX + "/update")
+public interface UpdateApi {
+
+ @POST
Review Comment:
Strict `@Consumes` means JAX-RS returns 415 for anything else before the
handler runs, so `assume.content.type` (`UpdateRequestHandler:78`) no longer
works on v2 -- the old `@EndPoint` version accepted any type and let the
handler resolve it. Worth deciding deliberately rather than losing it as a side
effect.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java:
##########
@@ -17,52 +17,168 @@
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 java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.client.api.endpoint.UpdateApi;
+import org.apache.solr.client.api.model.UpdateResponse;
+import org.apache.solr.client.api.model.VersionedDocument;
+import org.apache.solr.client.api.model.VersionedQuery;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.NamedList;
+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
Review Comment:
Stale after the content-type change: `update()` calls `handleUpdate(null)`,
so only `/update/json` is rewritten to `/update/json/docs`. `/update` does
content-type negotiation.
##########
solr/api/src/java/org/apache/solr/client/api/endpoint/UpdateApi.java:
##########
@@ -0,0 +1,211 @@
+/*
+ * 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.api.endpoint;
+
+import static
org.apache.solr.client.api.util.Constants.GENERIC_ENTITY_PROPERTY;
+import static org.apache.solr.client.api.util.Constants.INDEX_PATH_PREFIX;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.extensions.Extension;
+import io.swagger.v3.oas.annotations.extensions.ExtensionProperty;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.QueryParam;
+import java.io.InputStream;
+import org.apache.solr.client.api.model.UpdateResponse;
+import org.apache.solr.client.api.util.StoreApiParameters;
+
+/** V2 API definitions for indexing documents via the update handler. */
+@Path(INDEX_PATH_PREFIX + "/update")
+public interface UpdateApi {
+
+ @POST
+ @Consumes({
+ "application/json",
+ "text/json",
+ "application/xml",
+ "text/xml",
+ "application/csv",
+ "text/csv",
+ "application/javabin",
+ "application/cbor"
+ })
+ @StoreApiParameters
+ @Operation(
+ summary = "Send updates using any supported content type",
+ tags = {"update"})
+ UpdateResponse update(
+ @Parameter(description = "Commit the update immediately")
@QueryParam("commit")
+ Boolean commit,
+ @Parameter(description = "Commit the update within this many
milliseconds")
+ @QueryParam("commitWithin")
+ Integer commitWithin,
+ @Parameter(description = "Overwrite documents with the same unique key")
+ @QueryParam("overwrite")
+ Boolean overwrite,
+ @Parameter(description = "Perform a soft commit")
@QueryParam("softCommit")
+ Boolean softCommit,
+ @Parameter(description = "Include assigned document versions in the
response")
+ @QueryParam("versions")
+ Boolean versions,
+ @Parameter(required = true)
+ @RequestBody(
+ required = true,
Review Comment:
The generated SolrJ **Java** client can't call this -- `new
UpdateApi.Update(IndexType.CORE, coll, stream)` returns 415, same on
`/update/json`.
The Java template hardcodes `getContentType()` to
`application/octet-stream`, which none of the `@Consumes` above accept. The JS
generator emits the real consumes list, which is why the Admin UI works and
this went unnoticed. Since `/update` picks its loader *by* Content-Type, the
caller needs to control the header.
##########
solr/api/src/java/org/apache/solr/client/api/endpoint/UpdateApi.java:
##########
@@ -0,0 +1,211 @@
+/*
+ * 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.api.endpoint;
+
+import static
org.apache.solr.client.api.util.Constants.GENERIC_ENTITY_PROPERTY;
+import static org.apache.solr.client.api.util.Constants.INDEX_PATH_PREFIX;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.extensions.Extension;
+import io.swagger.v3.oas.annotations.extensions.ExtensionProperty;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.QueryParam;
+import java.io.InputStream;
+import org.apache.solr.client.api.model.UpdateResponse;
+import org.apache.solr.client.api.util.StoreApiParameters;
+
+/** V2 API definitions for indexing documents via the update handler. */
+@Path(INDEX_PATH_PREFIX + "/update")
+public interface UpdateApi {
+
+ @POST
+ @Consumes({
+ "application/json",
+ "text/json",
+ "application/xml",
+ "text/xml",
+ "application/csv",
+ "text/csv",
+ "application/javabin",
+ "application/cbor"
Review Comment:
v1 has a `/update/cbor` convenience path and `/update` consumes CBOR here,
but there's no v2 CBOR path alongside `/json`, `/xml`, `/csv`, `/javabin`.
Deliberate?
##########
solr/solr-ref-guide/modules/indexing-guide/pages/partial-document-updates.adoc:
##########
@@ -85,40 +85,67 @@ If the following document exists in our collection:
[source,json]
----
-{"id":"mydoc",
- "price":10,
- "popularity":42,
- "categories":["kids"],
- "sub_categories":["under_5","under_10"],
- "promo_ids":["a123x"],
- "tags":["free_to_try","buy_now","clearance","on_sale"]
+{"id":"SOLR1000",
+ "name":"Solr, the Enterprise Search Server",
+ "manu":"Apache Software Foundation",
+ "cat":["software","search"],
+ "features":["Advanced Full-Text Search Capabilities using Lucene",
+ "Optimized for High Volume Web Traffic"],
+ "price":0.0,
+ "popularity":10
}
----
And we apply the following update command:
-[source,json]
+[tabs#atomic-update-request]
+======
+V1 API::
++
+====
+[source,bash]
----
-{"id":"mydoc",
- "price":{"set":99},
- "popularity":{"inc":-7},
- "categories":{"add":["toys","games"]},
- "sub_categories":{"add-distinct":"under_10"},
- "promo_ids":{"remove":"a123x"},
- "tags":{"remove":["free_to_try","on_sale"]}
-}
+curl 'http://localhost:8983/solr/techproducts/update' \
+ -H 'Content-Type: application/json' \
+ -d '[
+ {"id":"SOLR1000",
+ "price":{"set":9.99},
+ "popularity":{"inc":5},
+ "cat":{"add":"enterprise"},
+ "features":{"remove":"Optimized for High Volume Web Traffic"}}
+]'
----
+====
+
+V2 API::
Review Comment:
Worth an explicit warning on this page: the same payload sent to
`/update/json` returns 200 and **replaces** the document -- the operator map is
indexed as literal content and the other fields are lost. Only the array form
on `/update` applies atomically. Verified against this branch.
##########
solr/server/solr/configsets/sample_techproducts_configs/conf/managed-schema.xml:
##########
@@ -121,6 +121,12 @@
-->
<field name="_root_" type="string" indexed="true" stored="false" />
+ <!-- records the path to each nested document and enables nested-document
reconstruction -->
+ <field name="_nest_path_" type="_nest_path_" />
Review Comment:
This is a comment by Claude that I believe is corret, but question is how
much attentino we should keep giving to the anonymous children docs now that we
have named children?
----
Adding these to the *shipped* configset enables
`NestedUpdateProcessorFactory` for everyone using it, which breaks anonymous
child documents. The PR shows the cost: it has to delete them again in
`SolrExampleTests:117` and `IndexingNestedDocuments:76`. Users calling
`addChildDocument()` against `sample_techproducts_configs` will start getting
errors after upgrading. Could this land as its own PR?
##########
solr/server/solr/configsets/sample_techproducts_configs/conf/managed-schema.xml:
##########
@@ -249,9 +255,6 @@
<copyField source="manu" dest="manu_exact"/>
<copyField source="name" dest="name_exact"/>
- <!-- Copy the price into a currency enabled field (default USD) -->
- <copyField source="price" dest="price_c"/>
Review Comment:
Removing this means techproducts docs no longer get a currency value, and
two pages still document one:
- `getting-started/pages/tutorial-techproducts.adoc:266` --
`"price_c":"7.99,USD"` and `"price_c____l_ns":799`
- `query-guide/pages/result-clustering.adoc:154`
Either restore the copyField and pick another field for the in-place-update
example, or update both pages.
##########
solr/webapp/web/js/angular/controllers/documents.js:
##########
@@ -64,74 +74,57 @@ solrAdminApp.controller('DocumentsController',
};
$scope.submit = function () {
- var contentType = "";
- var postData = "";
- var params = {};
- var doingFileUpload = false;
-
- if ($scope.handler[0] == '/') {
- params.handler = $scope.handler.substring(1);
- } else {
- params.handler = 'update';
- params.qt = $scope.handler;
+ if ($scope.type == "upload") {
+ FileUpload.upload({
+ core: $routeParams.core,
+ handler: "update",
+ commitWithin: $scope.commitWithin,
+ overwrite: $scope.overwrite,
+ wt: "json",
+ raw: $scope.literalParams
+ }, $scope.fileUpload, function (data) {
+ $scope.responseStatus = "success";
+ $scope.response = JSON.stringify(data, null, ' ');
+ }, function (data) {
+ $scope.responseStatus = "failure";
+ $scope.response = JSON.stringify(data, null, ' ');
+ });
+ return;
}
- params.commitWithin = $scope.commitWithin;
- params.overwrite = $scope.overwrite;
- params.core = $routeParams.core;
- params.wt = "json";
-
+ var postData;
+ var updateMethod;
if ($scope.type == "json" || $scope.type == "wizard") {
postData = "[" + $scope.document + "]";
- contentType = "json";
- } else if ($scope.type == "csv") {
+ updateMethod = UpdateV2.update;
+ } else if ($scope.type == "solr-json") {
postData = $scope.document;
- contentType = "csv";
+ updateMethod = UpdateV2.update;
} else if ($scope.type == "xml") {
postData = "<add>" + $scope.document + "</add>";
- contentType = "xml";
- } else if ($scope.type == "upload") {
- doingFileUpload = true;
- params.raw = $scope.literalParams;
- } else if ($scope.type == "solr") {
+ updateMethod = UpdateV2.updateXml;
Review Comment:
Another finding by claude that I have not tried to validate live, but line
82 with `wt=json` is indeed removed, so could be valid
----
The old code always sent `wt=json`; this doesn't. `XMLLoader.getDefaultWT()`
returns `xml`, `setDefaultWT` injects it, and `MediaTypeOverridingFilter`
prefers `wt` over `Accept` -- so the response comes back `application/xml` and
the generated JS client can't deserialize it. Indexing succeeds but the screen
shows `null`. Passing `wt: "json"` in `updateOptions` should do it.
--
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]
