risdenk commented on code in PR #1158:
URL: https://github.com/apache/solr/pull/1158#discussion_r1011844656
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientBuilder.java:
##########
@@ -46,6 +50,21 @@ public B withResponseParser(ResponseParser responseParser) {
return getThis();
}
+ public B withRequestWriter(RequestWriter requestWriter) {
+ this.requestWriter = requestWriter;
+ return getThis();
+ }
+
+ public B withUseMultiPartPost(Boolean useMultiPartPost) {
+ this.useMultiPartPost = useMultiPartPost;
+ return getThis();
+ }
+
+ public B setFollowRedirects(boolean withFollowRedirects) {
Review Comment:
`withFollowRedirects`?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/TestBatchUpdate.java:
##########
@@ -46,24 +46,28 @@ public static void beforeTest() throws Exception {
@Test
public void testWithXml() throws Exception {
- HttpSolrClient client = (HttpSolrClient) getSolrClient();
- client.setRequestWriter(new RequestWriter());
+ client =
Review Comment:
`client` must be closed in `SolrJettyTestBase`? would be clearer to do
`try-with-resources` for each of these?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTests.java:
##########
@@ -602,58 +602,60 @@ public void testUnicode() throws Exception {
SolrClient client = getSolrClient();
// save the old parser, so we can set it back.
- ResponseParser oldParser = null;
- if (client instanceof HttpSolrClient) {
- HttpSolrClient httpSolrClient = (HttpSolrClient) client;
- oldParser = httpSolrClient.getParser();
- }
-
- try {
- for (int iteration = 0; iteration < numIterations; iteration++) {
- // choose format
- if (client instanceof HttpSolrClient) {
- if (random.nextBoolean()) {
- ((HttpSolrClient) client).setParser(new BinaryResponseParser());
- } else {
- ((HttpSolrClient) client).setParser(new XMLResponseParser());
- }
- }
-
- int numDocs = TestUtil.nextInt(random(), 1, 10 * RANDOM_MULTIPLIER);
-
- // Empty the database...
- client.deleteByQuery("*:*"); // delete everything!
-
- List<SolrInputDocument> docs = new ArrayList<>();
- for (int i = 0; i < numDocs; i++) {
- // Now add something...
- SolrInputDocument doc = new SolrInputDocument();
- doc.addField("id", "" + i);
- doc.addField("unicode_s", randomTestString(30));
- docs.add(doc);
- }
+ // ResponseParser oldParser = null;
+ // if (client instanceof HttpSolrClient) {
+ // HttpSolrClient httpSolrClient = (HttpSolrClient) client;
+ // oldParser = httpSolrClient.getParser();
+ // }
Review Comment:
Guess still figuring out how this tests works/needs to be converted?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java:
##########
@@ -478,6 +484,8 @@ public void testUpdate() throws Exception {
assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
// XML response and writer
+ // Have not been able to move this to the Builder pattern, the below
check for application/xml
+ // always returns as application/javabin when I try this!
Review Comment:
Would be good to dig to why this happens
##########
solr/solrj/src/test/org/apache/solr/client/solrj/impl/BasicHttpSolrClientTest.java:
##########
@@ -341,7 +346,6 @@ public void testQuery() throws Exception {
assertEquals("keep-alive", DebugServlet.headers.get("Connection"));
// XML/POST
- client.setParser(new XMLResponseParser());
Review Comment:
Is there a side effect here about having a new `XMLResponseParser` to be
concerned with?
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClient.java:
##########
@@ -107,8 +107,8 @@ protected ConcurrentUpdateSolrClient(Builder builder) {
.withHttpClient(builder.httpClient)
.withConnectionTimeout(builder.connectionTimeoutMillis)
.withSocketTimeout(builder.socketTimeoutMillis)
+ .setFollowRedirects(false)
Review Comment:
`withFollowRedirects`?
##########
solr/solrj/src/test/org/apache/solr/client/solrj/embedded/SolrExampleJettyTest.java:
##########
@@ -118,72 +111,22 @@ public void testUtf8PerfDegradation() throws Exception {
doc.addField("id", "1");
doc.addField("b_is", IntStream.range(0,
30000).boxed().collect(Collectors.toList()));
- HttpSolrClient client = (HttpSolrClient) getSolrClient();
- client.add(doc);
- client.commit();
- long start = System.nanoTime();
- QueryResponse rsp = client.query(new SolrQuery("*:*"));
- System.out.println("time taken : " + ((System.nanoTime() - start)) / (1000
* 1000));
- assertEquals(1, rsp.getResults().getNumFound());
- }
-
- @Ignore
Review Comment:
when was this marked `@Ignore` would be good to find out why?
--
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]
