gerlowskija commented on code in PR #1649:
URL: https://github.com/apache/solr/pull/1649#discussion_r1207021591
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SchemaInfoAPI.java:
##########
@@ -31,18 +38,29 @@
* <p>This API (GET /v2/collections/collectionName/schema) is analogous to the
v1
* /solr/collectionName/schema API.
*/
-public class SchemaInfoAPI {
- private final SchemaHandler schemaHandler;
+public class SchemaInfoAPI extends JerseyResource {
- public SchemaInfoAPI(SchemaHandler schemaHandler) {
- this.schemaHandler = schemaHandler;
+ private SolrCore solrCore;
+
+ @Inject
+ public SchemaInfoAPI(SolrCore solrCore) {
+ this.solrCore = solrCore;
+ }
+
+ @GET
+ @Path("/schema")
+ @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_ATOM_XML,
BINARY_CONTENT_TYPE_V2})
Review Comment:
[Q] Did you mean to do "ATOM_XML" here, or did you intend
`MediaType.APPLICATION_XML`?
##########
solr/core/src/java/org/apache/solr/handler/SchemaHandler.java:
##########
@@ -313,10 +323,6 @@ public void inform(SolrCore core) {
public Collection<Api> getApis() {
Review Comment:
[-1] Typically, JAX-RS APIs can be picked up via classpath-scanning at
startup time, but for a few reasons we've opted not to do that in Solr.
(Mostly bc scanning slows down Solr startup, but also in part because of how
each core in Solr can load custom jars, APIs etc.)
So you're right to remove this line here, but we actually still need a
similar line down in the `getJerseyResources` method of this class.
I've fixed this in a commit I'm about to push to your branch, but wanted to
tag you here as an FYI @bszabo97
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SchemaSimilarityAPI.java:
##########
@@ -31,18 +38,30 @@
* <p>This API (GET /v2/collections/collectionName/schema/similarity) is
analogous to the v1
* /solr/collectionName/schema/similarity API.
*/
-public class SchemaSimilarityAPI {
- private final SchemaHandler schemaHandler;
+public class SchemaSimilarityAPI extends JerseyResource {
- public SchemaSimilarityAPI(SchemaHandler schemaHandler) {
- this.schemaHandler = schemaHandler;
+ private SolrCore solrCore;
+
+ @Inject
+ public SchemaSimilarityAPI(SolrCore solrCore) {
+ this.solrCore = solrCore;
+ }
+
+ @GET
+ @Path("/schema/similarity")
+ @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_ATOM_XML,
BINARY_CONTENT_TYPE_V2})
+ @PermissionName(PermissionNameProvider.Name.SCHEMA_READ_PERM)
+ public SchemaSimilarityResponse getSchemaSimilarity() {
+ SchemaSimilarityResponse response =
instantiateJerseyResponse(SchemaSimilarityResponse.class);
+
+ response.similarity =
+
solrCore.getLatestSchema().getSimilarityFactory().getNamedPropertyValues();
+
+ return response;
}
- @EndPoint(
- path = {"/schema/similarity"},
- method = GET,
- permission = PermissionNameProvider.Name.SCHEMA_READ_PERM)
- public void getSchemaSimilarity(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
- schemaHandler.handleRequestBody(req, rsp);
+ public static class SchemaSimilarityResponse extends SolrJerseyResponse {
+ @JsonProperty("similarity")
+ SimpleOrderedMap<Object> similarity;
Review Comment:
[-1] This field needs to be 'public', or else the Jackson serialization has
trouble detecting it and adding it to the response 😬
(I've fixed this in a commit I'll upload shortly, just mentioning it here as
an FYI)
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SchemaInfoAPI.java:
##########
@@ -31,18 +38,29 @@
* <p>This API (GET /v2/collections/collectionName/schema) is analogous to the
v1
* /solr/collectionName/schema API.
*/
-public class SchemaInfoAPI {
- private final SchemaHandler schemaHandler;
+public class SchemaInfoAPI extends JerseyResource {
- public SchemaInfoAPI(SchemaHandler schemaHandler) {
- this.schemaHandler = schemaHandler;
+ private SolrCore solrCore;
+
+ @Inject
+ public SchemaInfoAPI(SolrCore solrCore) {
+ this.solrCore = solrCore;
+ }
+
+ @GET
+ @Path("/schema")
Review Comment:
[-1] Oops, this looked good to me at first glance as well, but I think under
the JAX-RS framework these annotations need to include the full path, that is:
`@Path("/collections/{collectionName}/schema")`.
I've fixed this in a commit I'm about to push up, but gonna tag you here as
an FYI @bszabo97
--
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]
