You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "risdenk (via GitHub)" <gi...@apache.org> on 2023/03/21 21:47:12 UTC
[GitHub] [solr] risdenk opened a new pull request, #1478: WIP Commons io to jdk isms
risdenk opened a new pull request, #1478:
URL: https://github.com/apache/solr/pull/1478
https://issues.apache.org/jira/browse/SOLR-XXXXX
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1478: SOLR-16716: Replace commons-io usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1478:
URL: https://github.com/apache/solr/pull/1478#discussion_r1145640212
##########
solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java:
##########
@@ -93,14 +93,14 @@ private Properties makeCoreProperties(
}
private void addCoreWithProps(Properties stockProps, File propFile) throws Exception {
- if (!propFile.getParentFile().exists()) propFile.getParentFile().mkdirs();
- Writer out = new OutputStreamWriter(new FileOutputStream(propFile), StandardCharsets.UTF_8);
- try {
+ if (!propFile.getParentFile().exists()) {
+ propFile.getParentFile().mkdirs();
+ }
+ try (Writer out =
+ new OutputStreamWriter(new FileOutputStream(propFile), StandardCharsets.UTF_8)) {
Review Comment:
Addressed in 7079308294cb7f4d4c468dbdc890afd2a6d67d2e
##########
solr/core/src/test/org/apache/solr/TestSolrCoreProperties.java:
##########
@@ -42,45 +39,46 @@ public class TestSolrCoreProperties extends SolrJettyTestBase {
@BeforeClass
public static void beforeTest() throws Exception {
- File homeDir = createTempDir().toFile();
+ Path homeDir = createTempDir();
- File collDir = new File(homeDir, "collection1");
- File dataDir = new File(collDir, "data");
- File confDir = new File(collDir, "conf");
+ Path collDir = homeDir.resolve("collection1");
+ Path dataDir = collDir.resolve("data");
+ Path confDir = collDir.resolve("conf");
- homeDir.mkdirs();
- collDir.mkdirs();
- dataDir.mkdirs();
- confDir.mkdirs();
+ Files.createDirectories(homeDir);
+ Files.createDirectories(collDir);
+ Files.createDirectories(dataDir);
+ Files.createDirectories(confDir);
- FileUtils.copyFile(
- new File(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), new File(homeDir, "solr.xml"));
+ Files.copy(Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), homeDir.resolve("solr.xml"));
String src_dir = TEST_HOME() + "/collection1/conf";
- FileUtils.copyFile(new File(src_dir, "schema-tiny.xml"), new File(confDir, "schema.xml"));
- FileUtils.copyFile(
- new File(src_dir, "solrconfig-coreproperties.xml"), new File(confDir, "solrconfig.xml"));
- FileUtils.copyFile(
- new File(src_dir, "solrconfig.snippet.randomindexconfig.xml"),
- new File(confDir, "solrconfig.snippet.randomindexconfig.xml"));
+ Files.copy(Path.of(src_dir, "schema-tiny.xml"), confDir.resolve("schema.xml"));
+ Files.copy(
+ Path.of(src_dir, "solrconfig-coreproperties.xml"), confDir.resolve("solrconfig.xml"));
+ Files.copy(
+ Path.of(src_dir, "solrconfig.snippet.randomindexconfig.xml"),
+ confDir.resolve("solrconfig.snippet.randomindexconfig.xml"));
Properties p = new Properties();
p.setProperty("foo.foo1", "f1");
p.setProperty("foo.foo2", "f2");
- Writer fos =
+ try (Writer fos =
new OutputStreamWriter(
- new FileOutputStream(new File(confDir, "solrcore.properties")), StandardCharsets.UTF_8);
Review Comment:
o neat didn't know `Files.newBufferedWriter` existed
##########
solr/core/src/test/org/apache/solr/TestSolrCoreProperties.java:
##########
@@ -42,45 +39,46 @@ public class TestSolrCoreProperties extends SolrJettyTestBase {
@BeforeClass
public static void beforeTest() throws Exception {
- File homeDir = createTempDir().toFile();
+ Path homeDir = createTempDir();
- File collDir = new File(homeDir, "collection1");
- File dataDir = new File(collDir, "data");
- File confDir = new File(collDir, "conf");
+ Path collDir = homeDir.resolve("collection1");
+ Path dataDir = collDir.resolve("data");
+ Path confDir = collDir.resolve("conf");
- homeDir.mkdirs();
- collDir.mkdirs();
- dataDir.mkdirs();
- confDir.mkdirs();
+ Files.createDirectories(homeDir);
+ Files.createDirectories(collDir);
+ Files.createDirectories(dataDir);
+ Files.createDirectories(confDir);
- FileUtils.copyFile(
- new File(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), new File(homeDir, "solr.xml"));
+ Files.copy(Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), homeDir.resolve("solr.xml"));
String src_dir = TEST_HOME() + "/collection1/conf";
- FileUtils.copyFile(new File(src_dir, "schema-tiny.xml"), new File(confDir, "schema.xml"));
- FileUtils.copyFile(
- new File(src_dir, "solrconfig-coreproperties.xml"), new File(confDir, "solrconfig.xml"));
- FileUtils.copyFile(
- new File(src_dir, "solrconfig.snippet.randomindexconfig.xml"),
- new File(confDir, "solrconfig.snippet.randomindexconfig.xml"));
+ Files.copy(Path.of(src_dir, "schema-tiny.xml"), confDir.resolve("schema.xml"));
+ Files.copy(
+ Path.of(src_dir, "solrconfig-coreproperties.xml"), confDir.resolve("solrconfig.xml"));
+ Files.copy(
+ Path.of(src_dir, "solrconfig.snippet.randomindexconfig.xml"),
+ confDir.resolve("solrconfig.snippet.randomindexconfig.xml"));
Properties p = new Properties();
p.setProperty("foo.foo1", "f1");
p.setProperty("foo.foo2", "f2");
- Writer fos =
+ try (Writer fos =
new OutputStreamWriter(
- new FileOutputStream(new File(confDir, "solrcore.properties")), StandardCharsets.UTF_8);
Review Comment:
Addressed in 7079308294cb7f4d4c468dbdc890afd2a6d67d2e
##########
solr/core/src/java/org/apache/solr/cloud/ZkCLI.java:
##########
@@ -424,30 +419,27 @@ public static void main(String[] args)
System.exit(1);
}
- String path = arglist.get(0).toString();
- InputStream is = new FileInputStream(arglist.get(1).toString());
- byte[] data = IOUtils.toByteArray(is);
+ String path = arglist.get(0);
+ byte[] data;
+ try (InputStream inputStream = Files.newInputStream(Path.of(arglist.get(1)))) {
+ data = inputStream.readAllBytes();
+ }
Review Comment:
Addressed in 7079308294cb7f4d4c468dbdc890afd2a6d67d2e
##########
solr/core/src/test/org/apache/solr/handler/admin/CoreAdminCreateDiscoverTest.java:
##########
@@ -112,32 +112,32 @@ public void testCreateSavesSysProps() throws Exception {
// verify props are in persisted file
Properties props = new Properties();
- File propFile =
- new File(solrHomeDirectory, coreSysProps + "/" + CorePropertiesLocator.PROPERTIES_FILENAME);
- FileInputStream is = new FileInputStream(propFile);
- try {
+ Path propFile =
+ solrHomeDirectory
+ .toPath()
+ .resolve(coreSysProps)
+ .resolve(CorePropertiesLocator.PROPERTIES_FILENAME);
+ try (InputStream is = Files.newInputStream(propFile)) {
Review Comment:
Addressed in 7079308294cb7f4d4c468dbdc890afd2a6d67d2e
##########
solr/core/src/test/org/apache/solr/handler/admin/CoreAdminCreateDiscoverTest.java:
##########
@@ -290,32 +290,32 @@ public void testCreateSavesRegProps() throws Exception {
// verify props are in persisted file
Properties props = new Properties();
- File propFile =
- new File(solrHomeDirectory, coreNormal + "/" + CorePropertiesLocator.PROPERTIES_FILENAME);
- FileInputStream is = new FileInputStream(propFile);
- try {
+ Path propFile =
+ solrHomeDirectory
+ .toPath()
+ .resolve(coreNormal)
+ .resolve(CorePropertiesLocator.PROPERTIES_FILENAME);
+ try (InputStream is = Files.newInputStream(propFile)) {
Review Comment:
Addressed in 7079308294cb7f4d4c468dbdc890afd2a6d67d2e
##########
solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java:
##########
@@ -70,12 +70,9 @@ private static NamedList<Object> getConfigSetProperties(
throws IOException {
byte[] oldPropsData = configSetService.downloadFileFromConfig(configName, propertyPath);
if (oldPropsData != null) {
- InputStreamReader reader =
- new InputStreamReader(new ByteArrayInputStream(oldPropsData), StandardCharsets.UTF_8);
- try {
+ try (InputStreamReader reader =
Review Comment:
will keep it in mind for the future
##########
solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java:
##########
@@ -338,7 +337,7 @@ public void testDeleteInstanceDir() throws Exception {
CoreAdminRequest.renameCore("corerename", "brand_new_core_name", client);
Properties props = new Properties();
try (InputStreamReader is =
- new InputStreamReader(new FileInputStream(renamePropFile), StandardCharsets.UTF_8)) {
+ new InputStreamReader(Files.newInputStream(renamePropFile), StandardCharsets.UTF_8)) {
Review Comment:
Addressed in 7079308294cb7f4d4c468dbdc890afd2a6d67d2e
##########
solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java:
##########
@@ -214,15 +213,11 @@ public void testPutFile() throws Exception {
String fromZk =
new String(zkClient.getData("/solr.xml", null, null, true), StandardCharsets.UTF_8);
- File locFile = new File(SOLR_HOME + File.separator + "solr-stress-new.xml");
- InputStream is = new FileInputStream(locFile);
- String fromLoc;
- try {
- fromLoc = new String(IOUtils.toByteArray(is), StandardCharsets.UTF_8);
- } finally {
- IOUtils.closeQuietly(is);
+ Path locFile = Path.of(SOLR_HOME, "solr-stress-new.xml");
+ try (InputStream is = Files.newInputStream(locFile)) {
+ String fromLoc = new String(is.readAllBytes(), StandardCharsets.UTF_8);
+ assertEquals("Should get back what we put in ZK", fromZk, fromLoc);
}
Review Comment:
Addressed in 7079308294cb7f4d4c468dbdc890afd2a6d67d2e
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1478: WIP Commons io to jdk isms
Posted by "sonatype-lift[bot] (via GitHub)" <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1478:
URL: https://github.com/apache/solr/pull/1478#discussion_r1144054955
##########
solr/test-framework/src/java/org/apache/solr/handler/BackupRestoreUtils.java:
##########
@@ -110,13 +109,9 @@ public static void runReplicationHandlerCommand(
}
static void executeHttpRequest(String requestUrl) throws IOException {
- InputStream stream = null;
- try {
- URL url = new URL(requestUrl);
- stream = url.openStream();
- stream.close();
- } finally {
- IOUtils.closeQuietly(stream);
+ URL url = new URL(requestUrl);
+ try (InputStream stream = url.openStream()) {
Review Comment:
<picture><img alt="11% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/11/display.svg"></picture>
<b>*[URLCONNECTION_SSRF_FD](https://find-sec-bugs.github.io/bugs.htm#URLCONNECTION_SSRF_FD):</b>* This web server request could be used by an attacker to expose internal services and filesystem.
ââ <b>2 similar findings have been found in this PR</b>
<details><summary>đ Expand here to view all instances of this finding</summary><br/>
<div align=\"center\">
| **File Path** | **Line Number** |
| ------------- | ------------- |
| solr/test-framework/src/java/org/apache/solr/handler/TestRestoreCoreUtil.java | [39](https://github.com/apache/solr/blob/b663aa6a6d6831adfd9aa934c8a16bd5b3fba833/solr/test-framework/src/java/org/apache/solr/handler/TestRestoreCoreUtil.java#L39) |
| solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java | [132](https://github.com/apache/solr/blob/b663aa6a6d6831adfd9aa934c8a16bd5b3fba833/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java#L132) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GW327X6R622KG1RZC6PKD5CN?t=FindSecBugs|URLCONNECTION_SSRF_FD" target="_blank">Visit the Lift Web Console</a> to find more details in your report.</p></div></details>
---
<details><summary>âšī¸ Expand to see all <b>@sonatype-lift</b> commands</summary>
You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
| **Command** | **Usage** |
| ------------- | ------------- |
| `@sonatype-lift ignore` | Leave out the above finding from this PR |
| `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
| `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
**Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
<sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
---
<b>Help us improve LIFT! (<i>Sonatype LiftBot external survey</i>)</b>
Was this a good recommendation for you? <sub><small>Answering this survey will not impact your Lift settings.</small></sub>
[â[đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=448158436&lift_comment_rating=1)â] - [â[đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=448158436&lift_comment_rating=2)â] - [â[đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=448158436&lift_comment_rating=3)â] - [â[đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=448158436&lift_comment_rating=4)â] - [â[đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=448158436&lift_comment_rating=5)â]
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk merged pull request #1478: SOLR-16716: Replace commons-io usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk merged PR #1478:
URL: https://github.com/apache/solr/pull/1478
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1478: SOLR-16716: Replace commons-io usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1478:
URL: https://github.com/apache/solr/pull/1478#discussion_r1145654825
##########
solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java:
##########
@@ -251,7 +251,7 @@ public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOExcepti
.setWritableDocFields(resolver)
.marshal(rsp.getValues(), out);
- try (InputStream in = out.toInputStream()) {
+ try (ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray())) {
Review Comment:
Take a look at f03649c15ace75d8e3ce99960a9412dc386a7501 and see what you think
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on a diff in pull request #1478: SOLR-16716: Replace commons-io usages with pure Java
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #1478:
URL: https://github.com/apache/solr/pull/1478#discussion_r1145439820
##########
solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java:
##########
@@ -93,14 +93,14 @@ private Properties makeCoreProperties(
}
private void addCoreWithProps(Properties stockProps, File propFile) throws Exception {
- if (!propFile.getParentFile().exists()) propFile.getParentFile().mkdirs();
- Writer out = new OutputStreamWriter(new FileOutputStream(propFile), StandardCharsets.UTF_8);
- try {
+ if (!propFile.getParentFile().exists()) {
+ propFile.getParentFile().mkdirs();
+ }
+ try (Writer out =
+ new OutputStreamWriter(new FileOutputStream(propFile), StandardCharsets.UTF_8)) {
Review Comment:
Could use
`var out = Files.newBufferedWriter(propFile.toPath())`
##########
solr/core/src/test/org/apache/solr/TestSolrCoreProperties.java:
##########
@@ -42,45 +39,46 @@ public class TestSolrCoreProperties extends SolrJettyTestBase {
@BeforeClass
public static void beforeTest() throws Exception {
- File homeDir = createTempDir().toFile();
+ Path homeDir = createTempDir();
- File collDir = new File(homeDir, "collection1");
- File dataDir = new File(collDir, "data");
- File confDir = new File(collDir, "conf");
+ Path collDir = homeDir.resolve("collection1");
+ Path dataDir = collDir.resolve("data");
+ Path confDir = collDir.resolve("conf");
- homeDir.mkdirs();
- collDir.mkdirs();
- dataDir.mkdirs();
- confDir.mkdirs();
+ Files.createDirectories(homeDir);
+ Files.createDirectories(collDir);
+ Files.createDirectories(dataDir);
+ Files.createDirectories(confDir);
- FileUtils.copyFile(
- new File(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), new File(homeDir, "solr.xml"));
+ Files.copy(Path.of(SolrTestCaseJ4.TEST_HOME(), "solr.xml"), homeDir.resolve("solr.xml"));
String src_dir = TEST_HOME() + "/collection1/conf";
- FileUtils.copyFile(new File(src_dir, "schema-tiny.xml"), new File(confDir, "schema.xml"));
- FileUtils.copyFile(
- new File(src_dir, "solrconfig-coreproperties.xml"), new File(confDir, "solrconfig.xml"));
- FileUtils.copyFile(
- new File(src_dir, "solrconfig.snippet.randomindexconfig.xml"),
- new File(confDir, "solrconfig.snippet.randomindexconfig.xml"));
+ Files.copy(Path.of(src_dir, "schema-tiny.xml"), confDir.resolve("schema.xml"));
+ Files.copy(
+ Path.of(src_dir, "solrconfig-coreproperties.xml"), confDir.resolve("solrconfig.xml"));
+ Files.copy(
+ Path.of(src_dir, "solrconfig.snippet.randomindexconfig.xml"),
+ confDir.resolve("solrconfig.snippet.randomindexconfig.xml"));
Properties p = new Properties();
p.setProperty("foo.foo1", "f1");
p.setProperty("foo.foo2", "f2");
- Writer fos =
+ try (Writer fos =
new OutputStreamWriter(
- new FileOutputStream(new File(confDir, "solrcore.properties")), StandardCharsets.UTF_8);
Review Comment:
or just use `Files.newBufferedWriter` so you don't mention OutputStreamWriter
##########
solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java:
##########
@@ -196,14 +197,16 @@ public static NamedList<Object> getParsedResponse(SolrQueryRequest req, SolrQuer
}
Resolver resolver = new Resolver(req, rsp.getReturnFields());
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- try (JavaBinCodec jbc = new JavaBinCodec(resolver)) {
- jbc.setWritableDocFields(resolver).marshal(rsp.getValues(), out);
- }
+ try (ByteArrayOutputStream out = new ByteArrayOutputStream()) {
+ try (JavaBinCodec jbc = new JavaBinCodec(resolver)) {
+ jbc.setWritableDocFields(resolver).marshal(rsp.getValues(), out);
+ }
- InputStream in = out.toInputStream();
Review Comment:
again, consider BAOS to avoid the extra byte array & copy
##########
solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java:
##########
@@ -70,12 +70,9 @@ private static NamedList<Object> getConfigSetProperties(
throws IOException {
byte[] oldPropsData = configSetService.downloadFileFromConfig(configName, propertyPath);
if (oldPropsData != null) {
- InputStreamReader reader =
- new InputStreamReader(new ByteArrayInputStream(oldPropsData), StandardCharsets.UTF_8);
- try {
+ try (InputStreamReader reader =
Review Comment:
As you change code, use of "var" in lines like this would use probably one less line yet still be quite clear.
Do or not as you wish.
##########
solr/core/src/java/org/apache/solr/cloud/ZkCLI.java:
##########
@@ -424,30 +419,27 @@ public static void main(String[] args)
System.exit(1);
}
- String path = arglist.get(0).toString();
- InputStream is = new FileInputStream(arglist.get(1).toString());
- byte[] data = IOUtils.toByteArray(is);
+ String path = arglist.get(0);
+ byte[] data;
+ try (InputStream inputStream = Files.newInputStream(Path.of(arglist.get(1)))) {
+ data = inputStream.readAllBytes();
+ }
Review Comment:
Instead use `Files.readAllBytes(path)`
##########
solr/core/src/test/org/apache/solr/handler/admin/CoreAdminCreateDiscoverTest.java:
##########
@@ -290,32 +290,32 @@ public void testCreateSavesRegProps() throws Exception {
// verify props are in persisted file
Properties props = new Properties();
- File propFile =
- new File(solrHomeDirectory, coreNormal + "/" + CorePropertiesLocator.PROPERTIES_FILENAME);
- FileInputStream is = new FileInputStream(propFile);
- try {
+ Path propFile =
+ solrHomeDirectory
+ .toPath()
+ .resolve(coreNormal)
+ .resolve(CorePropertiesLocator.PROPERTIES_FILENAME);
+ try (InputStream is = Files.newInputStream(propFile)) {
Review Comment:
`Files.newBufferedReader`
##########
solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java:
##########
@@ -251,7 +251,7 @@ public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOExcepti
.setWritableDocFields(resolver)
.marshal(rsp.getValues(), out);
- try (InputStream in = out.toInputStream()) {
+ try (ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray())) {
Review Comment:
This is less efficient as it produces an extra byte array (and copies to it) than commons-io. There's a hack around it though... see `org.apache.solr.client.solrj.impl.BinaryRequestWriter.BAOS` which shows that the original buffer can be accessible. Maybe you could improve on that utility and use it here?
##########
solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java:
##########
@@ -338,7 +337,7 @@ public void testDeleteInstanceDir() throws Exception {
CoreAdminRequest.renameCore("corerename", "brand_new_core_name", client);
Properties props = new Properties();
try (InputStreamReader is =
- new InputStreamReader(new FileInputStream(renamePropFile), StandardCharsets.UTF_8)) {
+ new InputStreamReader(Files.newInputStream(renamePropFile), StandardCharsets.UTF_8)) {
Review Comment:
`var reader = Files.newBufferedReader(renamePropFile.toPath())`
##########
solr/core/src/test/org/apache/solr/handler/admin/CoreAdminCreateDiscoverTest.java:
##########
@@ -112,32 +112,32 @@ public void testCreateSavesSysProps() throws Exception {
// verify props are in persisted file
Properties props = new Properties();
- File propFile =
- new File(solrHomeDirectory, coreSysProps + "/" + CorePropertiesLocator.PROPERTIES_FILENAME);
- FileInputStream is = new FileInputStream(propFile);
- try {
+ Path propFile =
+ solrHomeDirectory
+ .toPath()
+ .resolve(coreSysProps)
+ .resolve(CorePropertiesLocator.PROPERTIES_FILENAME);
+ try (InputStream is = Files.newInputStream(propFile)) {
Review Comment:
`Files.newBufferedReader`
##########
solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java:
##########
@@ -214,15 +213,11 @@ public void testPutFile() throws Exception {
String fromZk =
new String(zkClient.getData("/solr.xml", null, null, true), StandardCharsets.UTF_8);
- File locFile = new File(SOLR_HOME + File.separator + "solr-stress-new.xml");
- InputStream is = new FileInputStream(locFile);
- String fromLoc;
- try {
- fromLoc = new String(IOUtils.toByteArray(is), StandardCharsets.UTF_8);
- } finally {
- IOUtils.closeQuietly(is);
+ Path locFile = Path.of(SOLR_HOME, "solr-stress-new.xml");
+ try (InputStream is = Files.newInputStream(locFile)) {
+ String fromLoc = new String(is.readAllBytes(), StandardCharsets.UTF_8);
+ assertEquals("Should get back what we put in ZK", fromZk, fromLoc);
}
Review Comment:
Instead use `Files.readString(path)`
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1478: SOLR-16716: Replace commons-io usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1478:
URL: https://github.com/apache/solr/pull/1478#discussion_r1146253848
##########
solr/core/src/java/org/apache/solr/response/BinaryResponseWriter.java:
##########
@@ -196,14 +197,16 @@ public static NamedList<Object> getParsedResponse(SolrQueryRequest req, SolrQuer
}
Resolver resolver = new Resolver(req, rsp.getReturnFields());
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- try (JavaBinCodec jbc = new JavaBinCodec(resolver)) {
- jbc.setWritableDocFields(resolver).marshal(rsp.getValues(), out);
- }
+ try (ByteArrayOutputStream out = new ByteArrayOutputStream()) {
+ try (JavaBinCodec jbc = new JavaBinCodec(resolver)) {
+ jbc.setWritableDocFields(resolver).marshal(rsp.getValues(), out);
+ }
- InputStream in = out.toInputStream();
Review Comment:
Resolved in c6cf531 by @dsmiley
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #1478: SOLR-16716: Replace commons-io usages with pure Java
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1478:
URL: https://github.com/apache/solr/pull/1478#discussion_r1146253545
##########
solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java:
##########
@@ -251,7 +251,7 @@ public void writeResults(ResultContext ctx, JavaBinCodec codec) throws IOExcepti
.setWritableDocFields(resolver)
.marshal(rsp.getValues(), out);
- try (InputStream in = out.toInputStream()) {
+ try (ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray())) {
Review Comment:
Thanks @dsmiley I like the changes you made in c6cf531
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org