Migrate V2 /update endpoints to using JAX-RS#4177
Migrate V2 /update endpoints to using JAX-RS#4177epugh wants to merge 16 commits intoapache:mainfrom
Conversation
- Add UpdateApi JAX-RS interface in solr/api module with @path, @post, @operation annotations for /update, /update/json, /update/xml, /update/csv, /update/bin endpoints - Rewrite UpdateAPI as a JerseyResource implementing UpdateApi; delegates to UpdateRequestHandler; adds UpdateRequestHandlerConfig inner class (APIConfigProvider.APIConfig) for injection - Update V2UpdateRequestHandler to implement APIConfigProvider and use getJerseyResources() instead of getApis() - Replace mock-based V2UpdateAPIMappingTest with UpdateAPITest integration test using SolrJettyTestRule Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Resolves failing test: NodeHealthAPITest2.testLegacyMode_WithoutMaxGenerationLagReturnsOk The test verifies that the V2 GET /api/node/health endpoint returns status=OK in legacy (standalone, non-ZooKeeper) mode when maxGenerationLag is not specified in the request. This exercises the null-maxGenerationLag code path in HealthCheckHandler.healthCheckLegacyMode() which immediately returns OK without checking replication lag. Test uses SolrJettyTestRule with no ZooKeeper to run in legacy mode. Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…n V2Request update/json
The V2Request("/c/{collection}/update/json") call was broken after the UpdateAPI
migration to JAX-RS: the JAX-RS path regex "cores|collections" doesn't match
the "/c/" alias. Replace with the proper SolrJ cloudClient.add() (which goes
through the v1 /update path) and update span name/collection assertions to match.
Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Normalize /c/ → /collections/ in ContainerRequestUtils.getRequestUri() before
Jersey sees the URI, fixing JAX-RS path matching for all endpoints that use
INDEX_PATH_PREFIX (update, select, schema, etc.) with the /c/ shorthand.
The tracing span name correctly preserves /c/{collection}/... since it is
computed from V2HttpCall.pathSegments which use the original request path.
Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…o UpdateRequestHandler DistributedZkUpdateProcessor.handleReplicationFactor() calls rsp.getResponseHeader().add(...), which requires the response header to be initialized by SolrCore.preDecorateResponse(). In the JAX-RS path, this was never called (only the old Api path calls it in executeCoreRequest). Add pre/postDecorateResponse calls around the updateRequestHandler.handleRequest() call in UpdateAPI.handleUpdate(), mirroring what SolrCore.execute() does for v1. Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds Ref Guide documentation for Solr’s V2 update endpoints and (in the same change set) migrates the V2 /update implementation to a JAX-RS resource, including URI normalization to support the /c/ collection shorthand in Jersey routing.
Changes:
- Added a new Ref Guide page documenting V2 update endpoints and their behavior vs V1 update handlers.
- Updated indexing guide navigation to include and cross-reference the new V2 update documentation.
- Switched V2 update handling to a Jersey resource + added
/c/→/collections/URI normalization, along with updated tests and a new OpenAPI endpoint interface.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-v2-apis.adoc | New V2 update API documentation and V1↔V2 comparison. |
| solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-update-handlers.adoc | Cross-reference note + page-children update for new V2 page. |
| solr/solr-ref-guide/modules/indexing-guide/indexing-nav.adoc | Adds the new V2 page to the indexing sidebar nav. |
| solr/core/src/java/org/apache/solr/jersey/container/ContainerRequestUtils.java | Normalizes /c/ collection shorthand for Jersey path matching. |
| solr/core/src/java/org/apache/solr/handler/admin/api/UpdateAPI.java | New Jersey-based V2 update resource delegating to UpdateRequestHandler. |
| solr/core/src/java/org/apache/solr/handler/V2UpdateRequestHandler.java | Registers UpdateAPI as a Jersey resource and provides config injection. |
| solr/api/src/java/org/apache/solr/client/api/endpoint/UpdateApi.java | New OpenAPI/JAX-RS endpoint interface for v2 update paths. |
| solr/core/src/test/org/apache/solr/handler/V2UpdateAPIMappingTest.java | Removes old unit test for v2→v1 update path rewriting. |
| solr/core/src/test/org/apache/solr/handler.admin.api/UpdateAPITest.java | Adds integration tests covering V2 update endpoints. |
Comments suppressed due to low confidence (1)
solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-v2-apis.adoc:136
- Capitalization of “Javabin” here (and in the table / comparison section) is inconsistent with the rest of the ref guide, which uses “JavaBin”. Please standardize on “JavaBin” (while keeping the MIME type
application/javabinas-is).
== Javabin Document Indexing
The v2 `/update/bin` endpoint accepts documents in Javabin format, which is the native binary format used by SolrJ.
This endpoint is primarily intended for use by SolrJ clients.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
solr/core/src/java/org/apache/solr/jersey/container/ContainerRequestUtils.java
Show resolved
Hide resolved
solr/solr-ref-guide/modules/indexing-guide/pages/indexing-with-v2-apis.adoc
Outdated
Show resolved
Hide resolved
| SolrCore.preDecorateResponse(solrQueryRequest, solrQueryResponse); | ||
| updateRequestHandler.handleRequest(solrQueryRequest, solrQueryResponse); | ||
| SolrCore.postDecorateResponse(updateRequestHandler, solrQueryRequest, solrQueryResponse); |
There was a problem hiding this comment.
handleUpdate calls SolrCore.postDecorateResponse(...), which adds a responseHeader to solrQueryResponse. The Jersey layer then serializes the returned SolrJerseyResponse by squashing it into the same SolrQueryResponse (also including its own responseHeader), which can result in duplicate responseHeader entries in the output. Consider removing the explicit pre/post decoration here (to align with other Jersey resources) or otherwise ensure only one responseHeader is serialized (e.g., clear the existing header before returning, or use a squash-without-header approach for this endpoint).
| SolrCore.preDecorateResponse(solrQueryRequest, solrQueryResponse); | |
| updateRequestHandler.handleRequest(solrQueryRequest, solrQueryResponse); | |
| SolrCore.postDecorateResponse(updateRequestHandler, solrQueryRequest, solrQueryResponse); | |
| // Do not call SolrCore.preDecorateResponse/postDecorateResponse here: | |
| // Jersey will handle response decoration (including responseHeader) to avoid duplicates. | |
| updateRequestHandler.handleRequest(solrQueryRequest, solrQueryResponse); |
There was a problem hiding this comment.
one of the tests was failing with out this...
| @POST | ||
| @StoreApiParameters | ||
| @Operation( | ||
| summary = "Add, delete, or update documents using any supported content type", | ||
| tags = {"update"}) | ||
| SolrJerseyResponse update() throws Exception; | ||
|
|
||
| @POST | ||
| @Path("/json") | ||
| @StoreApiParameters | ||
| @Operation( | ||
| summary = "Add, delete, or update documents in JSON format", | ||
| tags = {"update"}) | ||
| SolrJerseyResponse updateJson() throws Exception; |
There was a problem hiding this comment.
The OpenAPI summaries for /update and /update/json currently say they can “Add, delete, or update” and (for /update) accept “any supported content type”. In this PR’s implementation these endpoints are rewritten to /update/json/docs (docs-only JSON loader) and do not support JSON update commands in the request body, nor non-JSON bodies. Please update the summaries to match actual behavior so generated docs/clients don’t advertise unsupported operations.
| @POST | ||
| @Path("/csv") | ||
| @StoreApiParameters | ||
| @Operation( | ||
| summary = "Add, delete, or update documents in CSV format", | ||
| tags = {"update"}) | ||
| SolrJerseyResponse updateCsv() throws Exception; | ||
|
|
||
| @POST | ||
| @Path("/bin") | ||
| @StoreApiParameters | ||
| @Operation( | ||
| summary = "Add, delete, or update documents in JavaBin format", | ||
| tags = {"update"}) | ||
| SolrJerseyResponse updateBin() throws Exception; |
There was a problem hiding this comment.
Similarly, the summaries for /update/csv and /update/bin say “Add, delete, or update documents…”, but these loaders are document-ingest only (no delete/commit commands in-body). Please tighten the wording (e.g., “index documents”) to avoid implying full update-command support for these content types.
…-v2-apis.adoc Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| 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) |
There was a problem hiding this comment.
While we CAN do it this way, can't we also do it with the solrj generaed code?
| client.commit(CORE_NAME); | ||
|
|
||
| // Verify the document was indexed | ||
| final ModifiableSolrParams queryParams = new ModifiableSolrParams(); |
There was a problem hiding this comment.
is there a more concise way of doing this?
| SolrCore.preDecorateResponse(solrQueryRequest, solrQueryResponse); | ||
| updateRequestHandler.handleRequest(solrQueryRequest, solrQueryResponse); | ||
| SolrCore.postDecorateResponse(updateRequestHandler, solrQueryRequest, solrQueryResponse); |
There was a problem hiding this comment.
one of the tests was failing with out this...
solr/core/src/java/org/apache/solr/jersey/container/ContainerRequestUtils.java
Show resolved
Hide resolved
|
|
||
| == Javabin Document Indexing | ||
|
|
||
| The v2 `/update/bin` endpoint accepts documents in Javabin format, which is the native binary format used by SolrJ. |
There was a problem hiding this comment.
should this be /update/javabin ?
This PR migrates the V2
/updateendpoints to using JAX-RS.This did require adding the
/c/path support to our JAX-RS support, so that is buried in here. We may rip it out or rename it, there is a ticket https://issues.apache.org/jira/browse/SOLR-11013 out there that refers to this.The V2 JAX-RS update API has meaningful behavioral differences from V1 update handlers that aren't documented anywhere in the Ref Guide. So I added a new documentation page that is specific to V2, unlike our previous patterns of having both v1 and v2 documented next to each other.
New:
indexing-with-v2-apis.adocDocuments the V2 update endpoints with emphasis on the critical behavioral difference:
/updateand/update/jsonin V2 are document-only — they rewrite internally to/update/json/docsand do not support the JSON update command syntax (commit/delete/optimize in the request body). Full comparison:/solr/{collection}/update/api/collections/{collection}/update/updateor/update/json/docs/updateor/update/json/update/updateor/update/json/update(via Content-Type)/update/xml/update/csv/update/csv/update(via Content-Type)/update/binSomeday, I wonder if we do want to re-instate the multiple commands support like we have in v1, but that would be expanding on what the existing v2
/updatedoes. I think we would need to do that to retire the v1 version.