Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-17611 SolrJ User-Agent, pass Solr version #2927

Merged
merged 9 commits into from
Jan 16, 2025
Merged

SOLR-17611 SolrJ User-Agent, pass Solr version #2927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9b2718c
SOLR-17611 SolrJ User-Agent, pass Solr version
dsmiley Dec 28, 2024
39beed4
CUSC too
dsmiley Dec 28, 2024
a854c08
Use SolrVersion.LATEST_STRING
dsmiley Jan 13, 2025
2d03c71
Merge branch 'refs/heads/main' into userAgent
dsmiley Jan 13, 2025
ad2df59
HttpSolrCall.getUserAgentSolrVersion
dsmiley Jan 13, 2025
2e97dbe
Fix BasicHttpSolrClientTest
dsmiley Jan 13, 2025
6f843a4
Merge branch 'refs/heads/main' into userAgent
dsmiley Jan 13, 2025
6af9747
CHANGES.txt
dsmiley Jan 13, 2025
51e6410
javadocs
dsmiley Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ Other Changes

* SOLR-17589: Prevent error log entry on solr server due to initial HEAD request from HttpJdkSolrClient. (Paul Blanchaert via James Dyer)

* SOLR-17611: SolrJ's User-Agent header now uses the version of SolrJ. There's a corresponding
HttpSolrCall.getUserAgentSolrVersion to parse it. (David Smiley)

================== 9.8.0 ==================
New Features
---------------------
Expand Down
19 changes: 19 additions & 0 deletions solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.apache.http.entity.InputStreamEntity;
import org.apache.solr.api.ApiBag;
import org.apache.solr.api.V2HttpCall;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
import org.apache.solr.client.solrj.impl.HttpClientUtil;
import org.apache.solr.common.SolrException;
Expand Down Expand Up @@ -1170,6 +1171,24 @@ protected Object _getHandler() {
return handler;
}

/**
* Gets the client (user-agent) SolrJ version, or null if isn't SolrJ. Note that older SolrJ
* clients prior to 9.9 present themselves as 1.0 or 2.0.
*/
public SolrVersion getUserAgentSolrVersion() {
String header = req.getHeader("User-Agent");
if (header == null || !header.startsWith("Solr")) {
return null;
}
try {
return SolrVersion.valueOf(header.substring(header.lastIndexOf(' ') + 1));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this gets an older client it will return 1.0 or 2.0, which can be somewhat misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah; I thought it would have no consequence so I did nothing about it. At least by returning a SolrVersion instead of null, we communicate indirectly that it's at least SolrJ. I could add a javadoc comment on 1.0 and 2.0 being used prior to 9.8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this to the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW that method has no callers; isn't tested. I did manually test it. Obviously I intend to have it be used soon.

} catch (Exception e) {
// unexpected but let's not freak out
assert false : e.toString();
return null;
}
}

private AuthorizationContext getAuthCtx() {

String resource = getPath();
Expand Down
2 changes: 1 addition & 1 deletion solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public void writeTo(OutputStream out) throws IOException {
method.setConfig(requestConfigBuilder.build());

method.setEntity(template);
method.addHeader("User-Agent", HttpSolrClient.AGENT);
method.addHeader("User-Agent", HttpSolrClient.USER_AGENT);
method.addHeader("Content-Type", contentType);

response =
Expand Down
8 changes: 5 additions & 3 deletions solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrClientFunction;
Expand Down Expand Up @@ -106,11 +107,12 @@
* </ul>
*/
public class Http2SolrClient extends HttpSolrClientBase {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
public static final String REQ_PRINCIPAL_KEY = "solr-req-principal";
private static final String USER_AGENT =
"Solr[" + MethodHandles.lookup().lookupClass().getName() + "] " + SolrVersion.LATEST_STRING;

private static volatile SSLConfig defaultSSLConfig;
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private static final String AGENT = "Solr[" + Http2SolrClient.class.getName() + "] 2.0";

private final HttpClient httpClient;

Expand Down Expand Up @@ -251,7 +253,7 @@ private HttpClient createHttpClient(Builder builder) {
httpClient.setFollowRedirects(false);
httpClient.setMaxRequestsQueuedPerDestination(
asyncTracker.getMaxRequestsQueuedPerDestination());
httpClient.setUserAgentField(new HttpField(HttpHeader.USER_AGENT, AGENT));
httpClient.setUserAgentField(new HttpField(HttpHeader.USER_AGENT, USER_AGENT));
httpClient.setIdleTimeout(idleTimeoutMillis);

if (builder.cookieStore != null) {
Expand Down
3 changes: 2 additions & 1 deletion solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.net.ssl.SSLContext;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
Expand All @@ -73,7 +74,7 @@ public class HttpJdkSolrClient extends HttpSolrClientBase {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

private static final String USER_AGENT =
"Solr[" + MethodHandles.lookup().lookupClass().getName() + "] 1.0";
"Solr[" + MethodHandles.lookup().lookupClass().getName() + "] " + SolrVersion.LATEST_STRING;
madrob marked this conversation as resolved.
Show resolved Hide resolved

protected HttpClient httpClient;

Expand Down
12 changes: 4 additions & 8 deletions solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import org.apache.http.entity.mime.content.StringBody;
import org.apache.http.message.BasicHeader;
import org.apache.http.message.BasicNameValuePair;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrRequest;
Expand All @@ -85,8 +86,6 @@
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SolrNamedThreadFactory;
import org.apache.solr.common.util.Utils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;

/**
Expand All @@ -98,15 +97,12 @@
public class HttpSolrClient extends BaseHttpSolrClient {

private static final Charset FALLBACK_CHARSET = StandardCharsets.UTF_8;
private static final String DEFAULT_PATH = "/select";
private static final long serialVersionUID = -946812319974801896L;

protected static final Set<Integer> UNMATCHED_ACCEPTED_ERROR_CODES = Collections.singleton(429);

/** User-Agent String. */
public static final String AGENT = "Solr[" + HttpSolrClient.class.getName() + "] 1.0";

private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
static final String USER_AGENT =
"Solr[" + MethodHandles.lookup().lookupClass().getName() + "] " + SolrVersion.LATEST_STRING;

static final Class<HttpSolrClient> cacheKey = HttpSolrClient.class;

Expand Down Expand Up @@ -542,7 +538,7 @@ protected NamedList<Object> executeMethod(
final ResponseParser processor,
final boolean isV2Api)
throws SolrServerException {
method.addHeader("User-Agent", AGENT);
method.addHeader("User-Agent", USER_AGENT);

org.apache.http.client.config.RequestConfig.Builder requestConfigBuilder =
HttpClientUtil.createDefaultRequestConfigBuilder();
Expand Down
40 changes: 21 additions & 19 deletions solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.apache.http.protocol.HttpContext;
import org.apache.solr.SolrJettyTestBase;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrRequest;
Expand All @@ -79,6 +80,7 @@
public class BasicHttpSolrClientTest extends SolrJettyTestBase {

private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private static final String UA_VERSION = SolrVersion.LATEST_STRING;

public static class RedirectServlet extends HttpServlet {
@Override
Expand Down Expand Up @@ -260,7 +262,7 @@ public void testQuery() throws Exception {
assertEquals("get", DebugServlet.lastMethod);
// agent
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
// default wt
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
Expand All @@ -271,7 +273,7 @@ public void testQuery() throws Exception {
client.getParser().getVersion(), DebugServlet.parameters.get(CommonParams.VERSION)[0]);
// agent
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
// keepalive
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
Expand All @@ -288,7 +290,7 @@ public void testQuery() throws Exception {

assertEquals("post", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("javabin", DebugServlet.parameters.get(CommonParams.WT)[0]);
Expand All @@ -298,7 +300,7 @@ public void testQuery() throws Exception {
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
assertEquals(
Expand All @@ -312,7 +314,7 @@ public void testQuery() throws Exception {

assertEquals("put", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("javabin", DebugServlet.parameters.get(CommonParams.WT)[0]);
Expand All @@ -322,7 +324,7 @@ public void testQuery() throws Exception {
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
assertEquals(
Expand All @@ -342,7 +344,7 @@ public void testQuery() throws Exception {

assertEquals("get", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
Expand All @@ -352,7 +354,7 @@ public void testQuery() throws Exception {
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));

Expand All @@ -363,7 +365,7 @@ public void testQuery() throws Exception {

assertEquals("post", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
Expand All @@ -373,7 +375,7 @@ public void testQuery() throws Exception {
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
assertEquals(
Expand All @@ -386,7 +388,7 @@ public void testQuery() throws Exception {

assertEquals("put", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
Expand All @@ -396,7 +398,7 @@ public void testQuery() throws Exception {
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
assertEquals(
Expand All @@ -420,7 +422,7 @@ public void testDelete() throws Exception {
assertEquals("post", DebugServlet.lastMethod);
// agent
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
// default wt
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
Expand All @@ -431,7 +433,7 @@ public void testDelete() throws Exception {
client.getParser().getVersion(), DebugServlet.parameters.get(CommonParams.VERSION)[0]);
// agent
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
// keepalive
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
Expand All @@ -451,15 +453,15 @@ public void testDelete() throws Exception {

assertEquals("post", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(
client.getParser().getVersion(), DebugServlet.parameters.get(CommonParams.VERSION)[0]);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
}
Expand Down Expand Up @@ -493,7 +495,7 @@ public void testUpdate() throws Exception {
assertEquals("post", DebugServlet.lastMethod);
// agent
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
// default wt
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
Expand Down Expand Up @@ -524,7 +526,7 @@ public void testUpdate() throws Exception {

assertEquals("post", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
Expand All @@ -551,7 +553,7 @@ public void testUpdate() throws Exception {

assertEquals("post", DebugServlet.lastMethod);
assertEquals(
"Solr[" + HttpSolrClient.class.getName() + "] 1.0",
"Solr[" + HttpSolrClient.class.getName() + "] " + UA_VERSION,
DebugServlet.headers.get("User-Agent"));
assertEquals(1, DebugServlet.parameters.get(CommonParams.WT).length);
assertEquals("javabin", DebugServlet.parameters.get(CommonParams.WT)[0]);
Expand Down
3 changes: 2 additions & 1 deletion solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.concurrent.TimeUnit;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
Expand All @@ -44,7 +45,7 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase {

@Override
protected String expectedUserAgent() {
return "Solr[" + Http2SolrClient.class.getName() + "] 2.0";
return "Solr[" + Http2SolrClient.class.getName() + "] " + SolrVersion.LATEST_STRING;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.apache.lucene.util.NamedThreadFactory;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.ResponseParser;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
Expand Down Expand Up @@ -532,7 +533,7 @@ private void assertNoHeadRequestWithSsl(HttpJdkSolrClient client) {

@Override
protected String expectedUserAgent() {
return "Solr[" + HttpJdkSolrClient.class.getName() + "] 1.0";
return "Solr[" + HttpJdkSolrClient.class.getName() + "] " + SolrVersion.LATEST_STRING;
}

@Override
Expand Down
Loading