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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
