dsmiley commented on code in PR #4038:
URL: https://github.com/apache/solr/pull/4038#discussion_r2670907077
##########
solr/solrj-jetty/src/java/org/apache/solr/client/solrj/jetty/ConcurrentUpdateJettySolrClient.java:
##########
@@ -43,6 +46,40 @@ public class ConcurrentUpdateJettySolrClient extends
ConcurrentUpdateBaseSolrCli
private final HttpJettySolrClient client;
+ /**
+ * Jetty-specific implementation of StreamingResponse that wraps Jetty's
+ * InputStreamResponseListener.
+ */
+ private static class JettyStreamingResponse implements StreamingResponse {
Review Comment:
can we place this private inner class after public ones? Or evan at the end
of the file? It seem so distracting here. At least, the Builder should be
first.
##########
solr/core/src/java/org/apache/solr/update/StreamingSolrClients.java:
##########
@@ -151,7 +151,8 @@ public void handleError(Throwable ex) {
}
@Override
- public void onSuccess(Response resp, InputStream respBody) {
+ public void onSuccess(Object responseMetadata, InputStream respBody) {
+ Response resp = (Response) responseMetadata;
Review Comment:
maybe name that var jettyResp to reinforce this is a Jetty specific thing
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/StreamingResponse.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.solrj.impl;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * Abstraction for streaming HTTP response handling in {@link
ConcurrentUpdateBaseSolrClient}. This
+ * allows the base class to work with different HTTP client implementations
without direct
+ * dependencies on specific client libraries (e.g., Jetty, JDK HttpClient).
+ *
+ * <p>Implementations should wrap the underlying HTTP client's response
mechanism and provide access
+ * to response status and body stream.
+ */
+public interface StreamingResponse extends AutoCloseable {
Review Comment:
Seams like this should be an inner class of CUSC. The namespace is helpful
as clearly showing where its used. Otherwise would seem too generic.
##########
solr/solrj-jetty/build.gradle:
##########
@@ -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.
+ */
+
+plugins {
+ id 'java-library'
+}
+
+description = 'Solrj-Jetty - Jetty-based HTTP clients for SolrJ'
+
+dependencies {
+ // Core dependencies on solr-solrj (without Jetty)
+ api project(':solr:solrj')
+
+ // Jetty dependencies - moved from solr-solrj
+ api libs.eclipse.jetty.http2.client
+ implementation libs.eclipse.jetty.http2.httpclienttransport
+ implementation libs.eclipse.jetty.http
+ implementation libs.eclipse.jetty.client
+ implementation libs.eclipse.jetty.util
+ implementation libs.eclipse.jetty.io
+ runtimeOnly libs.eclipse.jetty.alpnjavaclient
+
+ // Inherited from solr-solrj
+ implementation libs.slf4j.api
+
+ compileOnly libs.stephenc.jcip.annotations
+
+ // Test dependencies
+ testImplementation libs.apache.httpcomponents.httpclient
+ testImplementation libs.apache.httpcomponents.httpcore
+
+ testImplementation project(':solr:test-framework')
+ testImplementation project(':solr:core')
+ testImplementation project(':solr:solrj')
+ // Access to solrj test classes (e.g., HttpSolrClientTestBase)
+ testImplementation project(path: ':solr:solrj', configuration:
'testArtifacts')
+ // Test classes use main classes from this module
+ testImplementation project(':solr:solrj-jetty')
+
+ // ZK dependencies for tests
+ testImplementation project(':solr:solrj-zookeeper')
+ permitTestUnusedDeclared project(':solr:solrj-zookeeper')
+ testImplementation(libs.apache.zookeeper.zookeeper, {
+ exclude group: "org.apache.yetus", module: "audience-annotations"
+ })
+ permitTestUnusedDeclared libs.apache.zookeeper.zookeeper
+ testImplementation libs.apache.zookeeper.jute
+
+ testImplementation libs.apache.lucene.core
+ testImplementation libs.apache.lucene.testframework
+
+ testImplementation libs.carrotsearch.randomizedtesting.runner
+ testImplementation libs.junit.junit
+ testImplementation libs.hamcrest.hamcrest
+
+ testImplementation libs.commonsio.commonsio
+
+ testImplementation libs.jakarta.servlet.api
+
+ testImplementation libs.eclipse.jetty.server
+ testImplementation libs.eclipse.jetty.ee10.servlet
+ testImplementation libs.eclipse.jetty.ee10.webapp
+ testRuntimeOnly(libs.eclipse.jetty.alpnjavaserver, {
+ exclude group: "org.eclipse.jetty.alpn", module: "alpn-api"
+ })
+ testImplementation libs.eclipse.jetty.session
+ testImplementation(libs.mockito.core, {
+ exclude group: "net.bytebuddy", module: "byte-buddy-agent"
+ })
+ testRuntimeOnly(libs.mockito.subclass, {
+ exclude group: "net.bytebuddy", module: "byte-buddy-agent"
+ })
+ testRuntimeOnly(libs.apache.log4j.slf4j2impl, {
+ exclude group: "org.apache.logging.log4j", module: "log4j-api"
+ })
+
+ // Permit unused test dependencies (transitive dependencies)
+ permitTestUnusedDeclared libs.commonsio.commonsio
Review Comment:
Did you actually have to add all of these to make the build happy?
Use of them is each a work-around to a deficiency in the cutterslade plugin
. Sometimes I wonder if we'd be better off without that thing. Maybe sharing
this pittiful situation here with that project may motivate a proper fix there.
--
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]
