Migrate Schema Designer to JAX-RS Apis. Fix some bugs.#4203
Migrate Schema Designer to JAX-RS Apis. Fix some bugs.#4203epugh wants to merge 46 commits intoapache:mainfrom
Conversation
Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…mments Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…y docs guard) Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…ader, sanitize filename Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…/docs map for JS rendering Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the Schema Designer v2 API implementation to a Jersey-based resource + OpenAPI-defined interface, updates the admin UI download URL accordingly, and includes a small improvement to JSON-lines parsing to avoid unchecked-cast warnings.
Changes:
- Migrate
SchemaDesignerAPIfrom the legacy@EndPointstyle to aJerseyResourceimplementing a newSchemaDesignerApiinterface, and update tests to call the new method signatures. - Adjust the Schema Designer UI download endpoint URL to the new
/api/schema-designer/download?configSet=...form. - Narrow unchecked-cast suppression in
DefaultSampleDocumentsLoadervia a helper method usinginstanceof Map<?, ?>.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| solr/webapp/web/js/angular/controllers/schema-designer.js | Updates Schema Designer download URL to new endpoint shape. |
| solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerAPI.java | Reworks tests to call new Jersey-style API methods and response handling. |
| solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerSettings.java | Changes settings class visibility to public. |
| solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConstants.java | Removes unused constants related to old request-param handling. |
| solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java | Minor cleanup/modernization and removal of unused helper logic. |
| solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java | Major migration to JerseyResource + SchemaDesignerApi, plus download/query response changes. |
| solr/core/src/java/org/apache/solr/handler/designer/SampleDocuments.java | Uses Stream.toList() instead of Collectors.toList(). |
| solr/core/src/java/org/apache/solr/handler/designer/DefaultSchemaSuggester.java | Adjusts field-prop guessing API/signature usage. |
| solr/core/src/java/org/apache/solr/handler/designer/DefaultSampleDocumentsLoader.java | Introduces parseStringToJson() helper and narrows unchecked-cast suppression. |
| solr/core/src/java/org/apache/solr/core/CoreContainer.java | Registers Schema Designer as a Jersey resource class. |
| solr/api/src/java/org/apache/solr/client/api/endpoint/SchemaDesignerApi.java | Adds new OpenAPI/JAX-RS interface defining Schema Designer endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if (errorsDuringIndexing != null) { | ||
| Map<String, Object> response = new HashMap<>(); | ||
| rsp.setException( | ||
| new SolrException( | ||
| SolrException.ErrorCode.BAD_REQUEST, | ||
| "Failed to re-index sample documents after schema updated.")); | ||
| response.put(ERROR_DETAILS, errorsDuringIndexing); | ||
| rsp.getValues().addAll(response); | ||
| return; | ||
| throw new SolrException( | ||
| SolrException.ErrorCode.BAD_REQUEST, | ||
| "Failed to re-index sample documents after schema updated."); | ||
| } |
There was a problem hiding this comment.
query() used to return an error response containing errorDetails when re-indexing failed, which the Schema Designer UI surfaces (see its errorHandler). This new implementation throws a SolrException without attaching errorDetails, so clients will lose the per-doc failure details. Consider instantiating a FlexibleSolrJerseyResponse early and setting an errorDetails top-level property before throwing, or otherwise preserve the prior error payload structure.
| protected void requireSchemaVersion(Integer schemaVersion) { | ||
| if (schemaVersion == null) { | ||
| throw new SolrException( | ||
| SolrException.ErrorCode.BAD_REQUEST, | ||
| SCHEMA_VERSION_PARAM + " is a required parameter for the " + req.getPath() + " endpoint"); | ||
| SolrException.ErrorCode.BAD_REQUEST, SCHEMA_VERSION_PARAM + " is a required parameter!"); | ||
| } | ||
| return schemaVersion; | ||
| } |
There was a problem hiding this comment.
requireSchemaVersion() now only rejects null, but previously the API treated -1 as “missing” (since getInt(..., -1) was used) and rejected it. As written, a client can pass schemaVersion=-1 and bypass this validation; it would be safer to also reject negative values to keep the same contract.
…sentinel contract) Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…emadesignerapi-to-v2-annotations
… github.com:epugh/solr into copilot/migrate-schemadesignerapi-to-v2-annotations
|
Discussing with @gerlowskija this ticket, and he brings up the "Shouldn't this be more restFul"? Looking at it, |
…ner API Agent-Logs-Url: https://github.com/epugh/solr/sessions/3e11a7a9-caca-4333-8a6b-bbcd5a2cd862 Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Agent-Logs-Url: https://github.com/epugh/solr/sessions/8f08e90a-243e-43eb-9aaf-070c2e9ed092 Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…playName query param Agent-Logs-Url: https://github.com/epugh/solr/sessions/eff3e08e-97a3-4e83-832c-74cc41e9058d Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Replace all FlexibleSolrJerseyResponse usages with the specific typed response POJOs (SchemaDesignerResponse, SchemaDesignerInfoResponse, SchemaDesignerCollectionsResponse, SchemaDesignerSchemaDiffResponse) returned by the SchemaDesigner API methods. Also fix SchemaDesigner.setSchemaObjectField() to handle the add-field and add-field-type action names used by the Schema API request JSON, so that response.field is populated for add-field requests and response.fieldType for add-field-type requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…r API Agent-Logs-Url: https://github.com/epugh/solr/sessions/507f5cc3-13ae-4824-a6c1-ef4f98052d35 Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…i-to-v2-annotations' into copilot/migrate-schemadesignerapi-to-v2-annotations # Conflicts: # solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
Agent-Logs-Url: https://github.com/epugh/solr/sessions/b74e99f0-20cb-45c6-afa8-b2acdd295385 Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…s $resource in JS Agent-Logs-Url: https://github.com/epugh/solr/sessions/adec0806-852d-4a34-a0fb-aef713d8bf76 Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
… tests Agent-Logs-Url: https://github.com/epugh/solr/sessions/10ac2a78-7ae6-44d5-858d-e74dd03593ea Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…adesignerapi-to-v2-annotations Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…ddenInConfigSets on ConfigSetService) Agent-Logs-Url: https://github.com/epugh/solr/sessions/8fcb8001-f311-4bcb-bf46-522ebd7a9d05 Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…e download+getFile to SchemaDesignerApi Agent-Logs-Url: https://github.com/epugh/solr/sessions/07fed0d8-5175-46b5-bff4-7111da9bb8c3 Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…emadesignerapi-to-v2-annotations
… DownloadConfigSet.zipConfigSet() Agent-Logs-Url: https://github.com/epugh/solr/sessions/fd2b245f-2e34-45e1-96fe-a5047e48206b Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
…ownload endpoint directly Agent-Logs-Url: https://github.com/epugh/solr/sessions/4f648089-c8e3-4fcd-9ac5-c55c3e9307df Co-authored-by: epugh <22395+epugh@users.noreply.github.com>
|
We took a diversion to update ConfigsetsAPI with an idea that we would reuse those APIs in the schema designer. Unfortunantly the only reusable API was teh "download a zipped configset". It turns out the other SchemaDesignerAPI endpoints that look like they could be replace with a ConfigsetsAPI call can't be because of Schema Designer specific business logic. I'll paste below two examples: listConfigs vs listConfigSets — cannot replacelistConfigSets returns ListConfigsetsResponse { List configSets } — a flat list of all raw configset names, including ._designer_films. listConfigs does designer-specific logic the JS UI depends on: Filters out system/excluded configsets and raw .designer* entries from the top-level list updateFileContents vs uploadConfigSetFile — cannot replaceuploadConfigSetFile (PUT /configsets/{configSetName}/{filePath}) is a general-purpose file upload — it writes bytes to a configset and returns a plain SolrJerseyResponse. updateFileContents does three critical designer-specific things that uploadConfigSetFile cannot do: solrconfig.xml pre-validation: Before saving, it loads and parses the XML using InMemoryResourceLoader / SolrConfig.readFromResourceLoader to catch errors before corrupting the configset. The error is returned in the response body (updateFileError + fileContent fields) without touching ZK. |




Summary
Migrate the SchemaDesigner to JAX-RS. Fixed a bug in the analyze feature that wasn't working in
main.Changes
JAX-RS adoption, RESTful style routing, and code review. Had to change up the routes in the JavaScript to match.
The runtime behaviour is unchanged.