Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
github-actions[bot] commented on PR #4181: URL: https://github.com/apache/solr/pull/4181#issuecomment-4367568283 This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! -- 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]
Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
epugh commented on PR #4181:
URL: https://github.com/apache/solr/pull/4181#issuecomment-3994742364
> Locallhy I am getting wrapping in the configSets list with breaks
`test_zk.bats` assert!:
>
> ```
> # "configSets":["_default",
> # "techproducts2"],
> # "responseHeader":{
> # "status":0,
> # "QTime":27}}
> ```
In digging more, it was the change to the json mapper stuff that was causing
this side effect. I just merged a redo that gets us closer, thoguh parts I
still don't love.
--
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]
Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
epugh commented on PR #4181:
URL: https://github.com/apache/solr/pull/4181#issuecomment-3993846856
Locallhy I am getting wrapping in the configSets list with breaks
`test_zk.bats` assert!:
```
# "configSets":["_default",
# "techproducts2"],
# "responseHeader":{
# "status":0,
# "QTime":27}}
```
--
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]
Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
epugh commented on code in PR #4181:
URL: https://github.com/apache/solr/pull/4181#discussion_r2879247754
##
solr/core/src/java/org/apache/solr/handler/admin/api/RealTimeGetAPI.java:
##
@@ -30,19 +34,25 @@
* This API (GET /v2/collections/collectionName/get) is analogous to the v1
* /solr/collectionName/get API.
*/
-public class RealTimeGetAPI {
+public class RealTimeGetAPI extends JerseyResource implements RealTimeGetApi {
private final RealTimeGetHandler rtgHandler;
+ private final SolrQueryRequest solrQueryRequest;
+ private final SolrQueryResponse solrQueryResponse;
- public RealTimeGetAPI(RealTimeGetHandler rtgHandler) {
-this.rtgHandler = rtgHandler;
+ @Inject
+ public RealTimeGetAPI(
+ SolrCore solrCore, SolrQueryRequest solrQueryRequest, SolrQueryResponse
solrQueryResponse) {
+this.rtgHandler = (RealTimeGetHandler) solrCore.getRequestHandler("/get");
+this.solrQueryRequest = solrQueryRequest;
+this.solrQueryResponse = solrQueryResponse;
}
- @EndPoint(
- path = {"/get"},
- method = GET,
- permission = PermissionNameProvider.Name.READ_PERM)
- public void getDocuments(SolrQueryRequest req, SolrQueryResponse rsp) throws
Exception {
-rtgHandler.handleRequestBody(req, rsp);
+ @Override
+ @PermissionName(PermissionNameProvider.Name.READ_PERM)
+ public FlexibleSolrJerseyResponse getDocuments(String id, List ids)
throws Exception {
+final var response =
instantiateJerseyResponse(FlexibleSolrJerseyResponse.class);
+rtgHandler.handleRequestBody(solrQueryRequest, solrQueryResponse);
+return response;
Review Comment:
added some docs!
--
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]
Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
epugh commented on code in PR #4181:
URL: https://github.com/apache/solr/pull/4181#discussion_r2879160354
##
solr/core/src/java/org/apache/solr/jersey/JerseyApplications.java:
##
@@ -44,6 +44,7 @@ public CoreContainerApp() {
// Request and response serialization/deserialization
// TODO: could these be singletons to save per-request object creations?
register(JacksonJsonProvider.class, 1);
+ register(MessageBodyWriters.JsonMessageBodyWriter.class, 5);
Review Comment:
Changing this to
```
register(MessageBodyWriters.JsonMessageBodyWriter.class, 1);
register(JacksonJsonProvider.class, 5);
```
caused test failures... so I'm not going to move forward with this change...
--
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]
Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
epugh commented on code in PR #4181:
URL: https://github.com/apache/solr/pull/4181#discussion_r2879160354
##
solr/core/src/java/org/apache/solr/jersey/JerseyApplications.java:
##
@@ -44,6 +44,7 @@ public CoreContainerApp() {
// Request and response serialization/deserialization
// TODO: could these be singletons to save per-request object creations?
register(JacksonJsonProvider.class, 1);
+ register(MessageBodyWriters.JsonMessageBodyWriter.class, 5);
Review Comment:
Changing this to ```
register(MessageBodyWriters.JsonMessageBodyWriter.class, 1);
register(JacksonJsonProvider.class, 5);
```
caused test failures... so I'm not going to move forward with this change...
--
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]
Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
epugh commented on code in PR #4181:
URL: https://github.com/apache/solr/pull/4181#discussion_r2878619719
##
solr/core/src/java/org/apache/solr/jersey/JerseyApplications.java:
##
@@ -44,6 +44,7 @@ public CoreContainerApp() {
// Request and response serialization/deserialization
// TODO: could these be singletons to save per-request object creations?
register(JacksonJsonProvider.class, 1);
+ register(MessageBodyWriters.JsonMessageBodyWriter.class, 5);
Review Comment:
@gerlowskija I would love your eyes on this
--
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]
Re: [PR] Migrate RealTimeGetAPI from @EndPoint to JAX-RS [solr]
Copilot commented on code in PR #4181:
URL: https://github.com/apache/solr/pull/4181#discussion_r2878120805
##
solr/core/src/java/org/apache/solr/jersey/JerseyApplications.java:
##
@@ -44,6 +44,7 @@ public CoreContainerApp() {
// Request and response serialization/deserialization
// TODO: could these be singletons to save per-request object creations?
register(JacksonJsonProvider.class, 1);
+ register(MessageBodyWriters.JsonMessageBodyWriter.class, 5);
Review Comment:
This migration relies on `JsonMessageBodyWriter` being selected over
Jersey’s `JacksonJsonProvider` for `application/json` so dynamic fields written
into `SolrQueryResponse` are preserved. Please ensure the registration
rank/priority here actually causes `JsonMessageBodyWriter` to win provider
selection for JSON; if not, responses will be serialized by Jackson directly
and the issue this PR is fixing will regress. A tangible fix is to adjust the
registration rank (or add an explicit `@Priority`) so the Solr JSON writer is
preferred for `application/json`.
```suggestion
register(JacksonJsonProvider.class, 5);
register(MessageBodyWriters.JsonMessageBodyWriter.class, 1);
```
##
solr/core/src/test/org/apache/solr/handler/admin/api/RealTimeGetAPITest.java:
##
@@ -0,0 +1,101 @@
+/*
+ * 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.util.List;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.api.model.FlexibleSolrJerseyResponse;
+import org.apache.solr.client.api.model.IndexType;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.request.DocumentsApi;
+import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.response.V2Response;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.util.SolrJettyTestRule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+/** Integration test for the {@link RealTimeGetAPI} JAX-RS endpoint. */
+public class RealTimeGetAPITest extends SolrTestCaseJ4 {
+
+ private static final String COLLECTION = "rtgTestCollection";
+
+ @ClassRule public static SolrJettyTestRule solrTestRule = new
SolrJettyTestRule();
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+System.setProperty(
+ALLOW_PATHS_SYSPROP,
configset("cloud-minimal").getParent().toAbsolutePath().toString());
+solrTestRule.startSolr(createTempDir());
Review Comment:
This test mutates a global system property (`ALLOW_PATHS_SYSPROP`) but
doesn’t restore the previous value afterward, which can leak into other tests
in the same JVM. Capture the previous value before setting it and restore/clear
it in an `@AfterClass` method to prevent cross-test contamination.
##
solr/core/src/java/org/apache/solr/handler/admin/api/RealTimeGetAPI.java:
##
@@ -30,19 +34,25 @@
* This API (GET /v2/collections/collectionName/get) is analogous to the v1
* /solr/collectionName/get API.
*/
-public class RealTimeGetAPI {
+public class RealTimeGetAPI extends JerseyResource implements RealTimeGetApi {
private final RealTimeGetHandler rtgHandler;
+ private final SolrQueryRequest solrQueryRequest;
+ private final SolrQueryResponse solrQueryResponse;
- public RealTimeGetAPI(RealTimeGetHandler rtgHandler) {
-this.rtgHandler = rtgHandler;
+ @Inject
+ public RealTimeGetAPI(
+ SolrCore solrCore, SolrQueryRequest solrQueryRequest, SolrQueryResponse
solrQueryResponse) {
+this.rtgHandler = (RealTimeGetHandler) solrCore.getRequestHandler("/get");
+this.solrQueryRequest = solrQueryRequest;
+this.solrQueryResponse = solrQueryResponse;
}
- @EndPoint(
- path = {"/get"},
- method = GET,
- permission = PermissionNameProvider.Name.READ_PERM)
- public void getDocuments(SolrQueryRequest req, SolrQueryResponse rsp) throws
Exception {
-rtgHandler.handleRequestBody(req, rsp);
+ @Override
+ @PermissionName(PermissionNameProvider.Name.READ_PERM)
+ public FlexibleSol
