epugh commented on code in PR #4266:
URL: https://github.com/apache/solr/pull/4266#discussion_r3038353881
##########
solr/core/src/java/org/apache/solr/security/SimplePrincipal.java:
##########
Review Comment:
Nice... I gave this a quick stab and it didn't work, so it was on my list
of "someday" ;-)
##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -156,27 +151,27 @@ private void createCollection(
case 1:
log.info("Creating collection with V1 API");
// Sometimes use v1 API
- String url =
+ var jetty1 = cluster.getRandomJetty(random());
+ String url1 =
String.format(
Locale.ROOT,
"%s/admin/collections?action=CREATE&name=%s&collection.configName=%s&numShards=%s&maxShardsPerNode=%s&nrtReplicas=%s&tlogReplicas=%s&pullReplicas=%s",
- cluster.getRandomJetty(random()).getBaseUrl(),
+ jetty1.getBaseUrl(),
collectionName,
"conf",
numShards,
100, // maxShardsPerNode
nrtReplicas,
tlogReplicas,
pullReplicas);
- HttpGet createCollectionGet = new HttpGet(url);
- ((CloudLegacySolrClient) cluster.getSolrClient())
- .getHttpClient()
- .execute(createCollectionGet);
+ var response1 =
jetty1.getSolrClient().getHttpClient().newRequest(url1).send();
+ assertEquals(200, response1.getStatus());
break;
case 2:
log.info("Creating collection with V2 API");
// Sometimes use V2 API
- url = cluster.getRandomJetty(random()).getBaseUrl().toString() +
"/____v2/collections";
+ var jetty2 = cluster.getRandomJetty(random());
Review Comment:
did you mean to start the new format of `jetty1`, `jetty2` etc? unless we
need both at same time, not sure it adds anything to the test? I don't have a
strong opinon, just noticed it.
##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -187,14 +182,17 @@ private void createCollection(
nrtReplicas,
tlogReplicas,
pullReplicas);
- HttpPost createCollectionPost = new HttpPost(url);
- createCollectionPost.setHeader("Content-type", "application/json");
- createCollectionPost.setEntity(new StringEntity(requestBody));
- HttpResponse httpResponse =
- ((CloudLegacySolrClient) cluster.getSolrClient())
+ var response2 =
Review Comment:
This is much nicer, much less clunky plumbing!
##########
solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java:
##########
@@ -345,171 +353,201 @@ public void testCollectionNamesMustBeAbsent() throws
Exception {
ZkStateReader zkStateReader = cluster.getZkStateReader();
zkStateReader.createClusterStateWatchersAndUpdate();
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get =
- new HttpGet(
- baseUrl
- + "/admin/collections?action=CREATEALIAS"
- + "&wt=json"
- + "&name="
- + getTestName()
- + "&collections=collection1meta,collection2meta"
- + "&router.field=evt_dt"
- + "&router.name=time"
- + "&router.start=2018-01-15T00:00:00Z"
- + "&router.interval=%2B30MINUTE"
- + "&create-collection.collection.configName=_default"
- + "&create-collection.numShards=1");
- assertFailure(get, "Collections cannot be specified");
+ final var jetty = cluster.getRandomJetty(random());
+ final String baseUrl = jetty.getBaseUrl().toString();
+ assertFailure(
+ jetty
+ .getSolrClient()
+ .getHttpClient()
+ .newRequest(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=json"
+ + "&name="
+ + getTestName()
+ + "&collections=collection1meta,collection2meta"
+ + "&router.field=evt_dt"
+ + "&router.name=time"
+ + "&router.start=2018-01-15T00:00:00Z"
+ + "&router.interval=%2B30MINUTE"
+ + "&create-collection.collection.configName=_default"
+ + "&create-collection.numShards=1"),
+ "Collections cannot be specified");
}
@Test
public void testAliasNameMustBeValid() throws Exception {
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get =
- new HttpGet(
- baseUrl
- + "/admin/collections?action=CREATEALIAS"
- + "&wt=json"
- + "&name=735741!45"
- + // ! not allowed
- "&router.field=evt_dt"
- + "&router.name=time"
- + "&router.start=2018-01-15T00:00:00Z"
- + "&router.interval=%2B30MINUTE"
- + "&create-collection.collection.configName=_default"
- + "&create-collection.numShards=1");
- assertFailure(get, "Invalid alias");
+ final var jetty = cluster.getRandomJetty(random());
+ final String baseUrl = jetty.getBaseUrl().toString();
+ assertFailure(
+ jetty
+ .getSolrClient()
+ .getHttpClient()
+ .newRequest(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=json"
+ + "&name=735741!45"
+ + // ! not allowed
Review Comment:
this is a nit on hte original test, not your PR, but the comment is a bit
werid, why not just inline it with the `&name` line?
##########
solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java:
##########
@@ -238,49 +228,53 @@ public void testProperties() throws Exception {
public void testModifyPropertiesV2() throws Exception {
final String aliasName = getSaferTestName();
ZkStateReader zkStateReader = createColectionsAndAlias(aliasName);
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseURLV2().toString();
+ final JettySolrRunner runner = cluster.getRandomJetty(random());
+ final String baseUrl = runner.getBaseURLV2().toString();
+ final HttpClient httpClient = runner.getSolrClient().getHttpClient();
String aliasApi = String.format(Locale.ENGLISH, "/aliases/%s/properties",
aliasName);
- HttpPut withoutBody = new HttpPut(baseUrl + aliasApi);
- assertEquals(400,
httpClient.execute(withoutBody).getStatusLine().getStatusCode());
-
- HttpPut update = new HttpPut(baseUrl + aliasApi);
- update.setEntity(
- new StringEntity(
- "{\n"
- + " \"properties\":\n"
- + " {\n"
- + " \"foo\": \"baz\",\n"
- + " \"bar\": \"bam\"\n"
- + " }\n"
- + "}",
- ContentType.APPLICATION_JSON));
- assertSuccess(update);
+ assertEquals(
+ 400, httpClient.newRequest(baseUrl +
aliasApi).method(HttpMethod.PUT).send().getStatus());
+
+ String body =
+ "{\n"
Review Comment:
i had good luck with the `"""` tags to simplify json formatted strings.
Since you are reformatting a lot in this PR, might be worth embracing it for
the JSON?
##########
solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java:
##########
@@ -238,49 +228,53 @@ public void testProperties() throws Exception {
public void testModifyPropertiesV2() throws Exception {
final String aliasName = getSaferTestName();
ZkStateReader zkStateReader = createColectionsAndAlias(aliasName);
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseURLV2().toString();
+ final JettySolrRunner runner = cluster.getRandomJetty(random());
+ final String baseUrl = runner.getBaseURLV2().toString();
+ final HttpClient httpClient = runner.getSolrClient().getHttpClient();
String aliasApi = String.format(Locale.ENGLISH, "/aliases/%s/properties",
aliasName);
- HttpPut withoutBody = new HttpPut(baseUrl + aliasApi);
- assertEquals(400,
httpClient.execute(withoutBody).getStatusLine().getStatusCode());
-
- HttpPut update = new HttpPut(baseUrl + aliasApi);
- update.setEntity(
- new StringEntity(
- "{\n"
- + " \"properties\":\n"
- + " {\n"
- + " \"foo\": \"baz\",\n"
- + " \"bar\": \"bam\"\n"
- + " }\n"
- + "}",
- ContentType.APPLICATION_JSON));
- assertSuccess(update);
+ assertEquals(
+ 400, httpClient.newRequest(baseUrl +
aliasApi).method(HttpMethod.PUT).send().getStatus());
+
+ String body =
+ "{\n"
+ + " \"properties\":\n"
+ + " {\n"
+ + " \"foo\": \"baz\",\n"
+ + " \"bar\": \"bam\"\n"
+ + " }\n"
+ + "}";
+ assertSuccess(
+ httpClient
+ .newRequest(baseUrl + aliasApi)
+ .method(HttpMethod.PUT)
+ .body(new StringRequestContent("application/json", body,
StandardCharsets.UTF_8))
+ .send());
checkFooAndBarMeta(aliasName, zkStateReader, "baz", "bam");
String aliasPropertyApi =
String.format(Locale.ENGLISH, "/aliases/%s/properties/%s", aliasName,
"foo");
- HttpPut updateByProperty = new HttpPut(baseUrl + aliasPropertyApi);
- updateByProperty.setEntity(
- new StringEntity("{ \"value\": \"zab\" }",
ContentType.APPLICATION_JSON));
- assertSuccess(updateByProperty);
+ assertSuccess(
+ httpClient
+ .newRequest(baseUrl + aliasPropertyApi)
+ .method(HttpMethod.PUT)
+ .body(
+ new StringRequestContent(
+ "application/json", "{ \"value\": \"zab\" }",
StandardCharsets.UTF_8))
+ .send());
checkFooAndBarMeta(aliasName, zkStateReader, "zab", "bam");
- HttpDelete deleteByProperty = new HttpDelete(baseUrl + aliasPropertyApi);
- assertSuccess(deleteByProperty);
+ assertSuccess(
+ httpClient.newRequest(baseUrl +
aliasPropertyApi).method(HttpMethod.DELETE).send());
checkFooAndBarMeta(aliasName, zkStateReader, null, "bam");
- HttpPut deleteByEmptyValue = new HttpPut(baseUrl + aliasApi);
- deleteByEmptyValue.setEntity(
- new StringEntity(
- "{\n"
- + " \"properties\":\n"
- + " {\n"
- + " \"bar\": \"\"\n"
- + " }\n"
- + "}",
- ContentType.APPLICATION_JSON));
- assertSuccess(deleteByEmptyValue);
+ body = "{ \"properties\": { \"bar\": \"\" } }";
Review Comment:
i know you can make the json more compact, but the long form makes the
nesting easier to read. maybe just my personal preference.
##########
solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java:
##########
@@ -508,12 +502,10 @@ private ZkStateReader createColectionsAndAlias(String
aliasName)
return zkStateReader;
}
- private void assertSuccess(HttpUriRequest msg) throws IOException {
- try (CloseableHttpResponse response = httpClient.execute(msg)) {
- if (200 != response.getStatusLine().getStatusCode()) {
- System.err.println(EntityUtils.toString(response.getEntity()));
- fail("Unexpected status: " + response.getStatusLine());
- }
+ private void assertSuccess(ContentResponse response) {
Review Comment:
i like the one level less of nesting!
##########
solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java:
##########
@@ -316,34 +310,31 @@ public void testFailOnSingleNode() throws Exception {
public Map<?, ?> callMigrateReplicas(CloudSolrClient cloudClient,
MigrateReplicasRequestBody body)
throws IOException {
- HttpEntityEnclosingRequestBase httpRequest = null;
- HttpEntity entity;
String response = null;
Map<?, ?> r = null;
String uri =
cluster.getJettySolrRunners().get(0).getBaseURLV2().toString()
+ "/cluster/replicas/migrate";
+ HttpClient httpClient =
cluster.getRandomJetty(random()).getSolrClient().getHttpClient();
try {
- httpRequest = new HttpPost(uri);
-
- httpRequest.setEntity(
- new ByteArrayEntity(
- Utils.toJSON(Utils.getReflectWriter(body)),
ContentType.APPLICATION_JSON));
- httpRequest.setHeader("Accept", "application/json");
- entity =
- ((CloudLegacySolrClient)
cloudClient).getHttpClient().execute(httpRequest).getEntity();
- try {
- response = EntityUtils.toString(entity, UTF_8);
- r = (Map<?, ?>) Utils.fromJSONString(response);
- assertNotNull("No response given from MigrateReplicas API", r);
- assertNotNull("No responseHeader given from MigrateReplicas API",
r.get("responseHeader"));
- } catch (JSONParser.ParseException e) {
- log.error("err response: {}", response);
- throw new AssertionError(e);
- }
- } finally {
- httpRequest.releaseConnection();
+ var resp =
+ httpClient
+ .newRequest(uri)
+ .method(HttpMethod.POST)
+ .body(
+ new BytesRequestContent(
+ "application/json",
Utils.toJSON(Utils.getReflectWriter(body))))
+ .send();
+ response = resp.getContentAsString();
+ r = (Map<?, ?>) Utils.fromJSONString(response);
Review Comment:
i don't love `r` as a variable name, and yes it's in the original test.
Sometimes we use `response` to mean the object, and `responseBody` as the body
text...
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -332,25 +321,19 @@ private void verifyProperties(
@Test
public void testUploadErrors() throws Exception {
- final SolrClient solrClient =
-
getHttpSolrClient(cluster.getJettySolrRunners().get(0).getBaseUrl().toString());
-
ByteBuffer emptyData = ByteBuffer.allocate(0);
ignoreException("The configuration name should be provided");
// Checking error when no configuration name is specified in request
Map<?, ?> map =
postDataAndGetResponse(
- cluster.getSolrClient(),
- cluster.getJettySolrRunners().get(0).getBaseUrl().toString()
- + "/admin/configs?action=UPLOAD",
+ cluster.getJettySolrRunners().getFirst(),
Review Comment:
nice to see getFirst() being used. I have seen opportunities to use it in
lots of places, but hadn't done much with it becasue I believe it was a java 21
thing. Could we now do a sweep of all the code and update that usage? And
maybe add `.get(0)` to forbidden apis?
##########
solr/core/src/test/org/apache/solr/cloud/TestMiniSolrCloudClusterSSL.java:
##########
@@ -341,30 +335,34 @@ private static void checkClusterJettys(
if (!sslConfig.isClientAuthMode()) {
// verify simple HTTP(S) client can't do HEAD request for URL with
wrong protocol
- try (CloseableHttpClient client =
getSslAwareClientWithNoClientCerts()) {
+ var httpClient = newSslAwareClientWithNoClientCerts();
+ try {
Review Comment:
ntry with resrouces?
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -374,41 +357,21 @@ public void testUploadErrors() throws Exception {
ignoreException("already exists");
map =
postDataAndGetResponse(
- cluster.getSolrClient(),
- cluster.getJettySolrRunners().get(0).getBaseUrl().toString()
- + "/admin/configs?action=UPLOAD&name=myconf",
+ cluster.getJettySolrRunners().getFirst(),
+ "/admin/configs?action=UPLOAD&name=myconf",
emptyData,
null,
false);
- assertNotNull(map);
unIgnoreException("already exists`");
- statusCode = (long) getObjectByPath(map, Arrays.asList("responseHeader",
"status"));
- assertEquals(400l, statusCode);
+ assertEquals(400, getStatusCode(map));
assertTrue(
"Expected file doesnt exist in zk. It's possibly overwritten",
zkClient.exists("/configs/myconf/firstDummyFile"));
assertTrue(
"Expected file doesnt exist in zk. It's possibly overwritten",
zkClient.exists("/configs/myconf/anotherDummyFile"));
- // Checking error when configuration name contains invalid characters
- for (String invalidName : new String[] {"configset!", "-configset"}) {
- map =
- postDataAndGetResponse(
- cluster.getSolrClient(),
- cluster.getJettySolrRunners().get(0).getBaseUrl().toString()
- + "/admin/configs?action=UPLOAD&name="
- + invalidName,
- emptyData,
- null,
- false);
- assertNotNull(map);
- statusCode = (long) getObjectByPath(map, Arrays.asList("responseHeader",
"status"));
- assertEquals("Expected 400 for invalid configset name: " + invalidName,
400l, statusCode);
- }
-
zkClient.close();
Review Comment:
try with resources opportunity?
##########
solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java:
##########
@@ -345,171 +353,201 @@ public void testCollectionNamesMustBeAbsent() throws
Exception {
ZkStateReader zkStateReader = cluster.getZkStateReader();
zkStateReader.createClusterStateWatchersAndUpdate();
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get =
- new HttpGet(
- baseUrl
- + "/admin/collections?action=CREATEALIAS"
- + "&wt=json"
- + "&name="
- + getTestName()
- + "&collections=collection1meta,collection2meta"
- + "&router.field=evt_dt"
- + "&router.name=time"
- + "&router.start=2018-01-15T00:00:00Z"
- + "&router.interval=%2B30MINUTE"
- + "&create-collection.collection.configName=_default"
- + "&create-collection.numShards=1");
- assertFailure(get, "Collections cannot be specified");
+ final var jetty = cluster.getRandomJetty(random());
+ final String baseUrl = jetty.getBaseUrl().toString();
+ assertFailure(
+ jetty
+ .getSolrClient()
+ .getHttpClient()
+ .newRequest(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=json"
+ + "&name="
+ + getTestName()
+ + "&collections=collection1meta,collection2meta"
+ + "&router.field=evt_dt"
+ + "&router.name=time"
+ + "&router.start=2018-01-15T00:00:00Z"
+ + "&router.interval=%2B30MINUTE"
+ + "&create-collection.collection.configName=_default"
+ + "&create-collection.numShards=1"),
+ "Collections cannot be specified");
}
@Test
public void testAliasNameMustBeValid() throws Exception {
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get =
- new HttpGet(
- baseUrl
- + "/admin/collections?action=CREATEALIAS"
- + "&wt=json"
- + "&name=735741!45"
- + // ! not allowed
- "&router.field=evt_dt"
- + "&router.name=time"
- + "&router.start=2018-01-15T00:00:00Z"
- + "&router.interval=%2B30MINUTE"
- + "&create-collection.collection.configName=_default"
- + "&create-collection.numShards=1");
- assertFailure(get, "Invalid alias");
+ final var jetty = cluster.getRandomJetty(random());
+ final String baseUrl = jetty.getBaseUrl().toString();
+ assertFailure(
+ jetty
+ .getSolrClient()
+ .getHttpClient()
+ .newRequest(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=json"
+ + "&name=735741!45"
+ + // ! not allowed
+ "&router.field=evt_dt"
+ + "&router.name=time"
+ + "&router.start=2018-01-15T00:00:00Z"
+ + "&router.interval=%2B30MINUTE"
+ + "&create-collection.collection.configName=_default"
+ + "&create-collection.numShards=1"),
+ "Invalid alias");
}
@Test
public void testRandomRouterNameFails() throws Exception {
final String aliasName = getSaferTestName();
- final String baseUrl =
cluster.getRandomJetty(random()).getBaseUrl().toString();
- HttpGet get =
- new HttpGet(
- baseUrl
- + "/admin/collections?action=CREATEALIAS"
- + "&wt=json"
- + "&name="
- + aliasName
- + "&router.field=evt_dt"
- + "&router.name=tiafasme"
- + // bad
- "&router.start=2018-01-15T00:00:00Z"
- + "&router.interval=%2B30MINUTE"
- + "&create-collection.collection.configName=_default"
- + "&create-collection.numShards=1");
- assertFailure(get, " is not in supported types, ");
+ final var jetty = cluster.getRandomJetty(random());
+ final String baseUrl = jetty.getBaseUrl().toString();
+ assertFailure(
+ jetty
+ .getSolrClient()
+ .getHttpClient()
+ .newRequest(
+ baseUrl
+ + "/admin/collections?action=CREATEALIAS"
+ + "&wt=json"
+ + "&name="
+ + aliasName
+ + "&router.field=evt_dt"
+ + "&router.name=tiafasme"
+ + // bad
Review Comment:
likewise you hae to learn the pattern is the line after has the comment. I
like the comment, it helps me find the specific attirbute that matters... it's
just odd it's after the line.
##########
solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionsAPIViaSolrCloudCluster.java:
##########
@@ -328,11 +325,12 @@ public void testUserDefinedProperties() throws Exception {
case 1:
// Sometimes use v1 API
- String url =
+ var jetty1 = cluster.getRandomJetty(random());
Review Comment:
same question as before... why `jetty1`? I won't comment on these as I
review if it's a common pattern, but did want to flag it these two times.
##########
solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java:
##########
@@ -976,6 +974,14 @@ public static LinkedHashMapWriter getRespMap(String path,
RestTestHarness restHa
}
}
+ @SuppressWarnings({"rawtypes"})
+ private static LinkedHashMapWriter getRespMap(String baseUrl, String path)
throws Exception {
+ // a hack... if possible pass in an httpClient and also be able to init
RestTestHarness with it
Review Comment:
is this a note to yourself for future work in this PR, or for a future
developer? if it's for a future dev, maybe elaborate a bit more and add that
tag we use? todo or something?
##########
solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java:
##########
@@ -258,7 +256,6 @@ public static void setUpOnce() {
distribStateManagerMock = mock(DistribStateManager.class);
coreContainerMock = mock(CoreContainer.class);
updateShardHandlerMock = mock(UpdateShardHandler.class);
- httpClientMock = mock(HttpClient.class);
Review Comment:
i like seeing less mocks!
##########
solr/core/src/test/org/apache/solr/filestore/TestDistribFileStore.java:
##########
@@ -217,16 +213,17 @@ public static void checkAllNodesForFile(
assertResponseValues(10, new Fetcher(url, jettySolrRunner), expected);
if (verifyContent) {
- try (HttpSolrClient solrClient = (HttpSolrClient)
jettySolrRunner.newClient()) {
- ByteBuffer buf =
- HttpClientUtil.executeGET(
- solrClient.getHttpClient(),
- baseUrl + "/cluster/filestore/files" + path,
- Utils.newBytesConsumer(Integer.MAX_VALUE));
- assertEquals(
-
"d01b51de67ae1680a84a813983b1de3b592fc32f1a22b662fc9057da5953abd1b72476388ba342cad21671cd0b805503c78ab9075ff2f3951fdf75fa16981420",
- DigestUtils.sha512Hex(new ByteBufferInputStream(buf)));
- }
+ var resp =
+ jettySolrRunner
+ .getSolrClient()
+ .getHttpClient()
+ .newRequest(baseUrl + "/cluster/filestore/files" + path)
+ .send();
+ assertEquals(200, resp.getStatus());
+ ByteBuffer buf = ByteBuffer.wrap(resp.getContent());
Review Comment:
reads so much nicer!
##########
solr/core/src/test/org/apache/solr/cloud/TestPullReplica.java:
##########
@@ -140,23 +136,25 @@ public void testCreateDelete() throws Exception {
break;
case 1:
// Sometimes use v1 API
- String url =
+ var jetty1 = cluster.getRandomJetty(random());
+ String url1 =
String.format(
Locale.ROOT,
"%s/admin/collections?action=CREATE&name=%s&collection.configName=%s&numShards=%s&pullReplicas=%s",
- cluster.getRandomJetty(random()).getBaseUrl(),
+ jetty1.getBaseUrl(),
collectionName,
"conf",
2, // numShards
3); // pullReplicas
// These options should all mean the same
- url = url + pickRandom("", "&nrtReplicas=1", "&replicationFactor=1");
- HttpGet createCollectionGet = new HttpGet(url);
- getHttpClient().execute(createCollectionGet);
+ url1 = url1 + pickRandom("", "&nrtReplicas=1",
"&replicationFactor=1");
Review Comment:
ah my old friend, you are back again numbered variable..
##########
solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java:
##########
@@ -131,16 +130,16 @@ public void testProperty() throws Exception {
+ " }";
runConfigCommand(harness, "/config", payload);
- MapWriter m = getRespMap("/config/overlay", harness);
+ MapWriter m = getRespMap(harness, "/config/overlay");
Review Comment:
reads nicer
##########
solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java:
##########
@@ -258,7 +256,6 @@ public static void setUpOnce() {
distribStateManagerMock = mock(DistribStateManager.class);
coreContainerMock = mock(CoreContainer.class);
updateShardHandlerMock = mock(UpdateShardHandler.class);
- httpClientMock = mock(HttpClient.class);
Review Comment:
so I didn't see the use of the mock below, so it was dead code?
##########
solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionReloadTest.java:
##########
@@ -187,14 +182,17 @@ private void createCollection(
nrtReplicas,
tlogReplicas,
pullReplicas);
- HttpPost createCollectionPost = new HttpPost(url);
- createCollectionPost.setHeader("Content-type", "application/json");
- createCollectionPost.setEntity(new StringEntity(requestBody));
- HttpResponse httpResponse =
- ((CloudLegacySolrClient) cluster.getSolrClient())
+ var response2 =
Review Comment:
even thoguh we set all the same things.
##########
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java:
##########
@@ -1500,59 +1438,47 @@ protected CollectionAdminResponse createCollection(
return res;
}
+ /** for Javabin request, JSON response */
public static Map<?, ?> postDataAndGetResponse(
- CloudSolrClient cloudClient, String uri, ByteBuffer bytarr, String
username, boolean usePut)
- throws IOException {
- HttpEntityEnclosingRequestBase httpRequest = null;
- HttpEntity entity;
- String response = null;
- Map<?, ?> m = null;
+ JettySolrRunner jetty, String uri, ByteBuffer javabinBody, String
username, boolean usePut)
+ throws Exception {
+ return postDataAndGetResponse(
+ jetty, uri, "application/octet-stream", javabinBody, username, usePut);
+ }
+ /** for {@code contentType} request, JSON response */
+ public static Map<?, ?> postDataAndGetResponse(
Review Comment:
much nicer
--
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]
