You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/03/07 15:24:58 UTC

[GitHub] [solr] madrob commented on a change in pull request #729: SOLR-16074: Use PathUtils.deleteDirectory(path) to try to avoid commons-io IO-751

madrob commented on a change in pull request #729:
URL: https://github.com/apache/solr/pull/729#discussion_r820719971



##########
File path: solr/core/src/test/org/apache/solr/rest/schema/analysis/TestManagedSynonymFilterFactory.java
##########
@@ -36,20 +37,20 @@
 
 public class TestManagedSynonymFilterFactory extends RestTestBase {
 
-  private static File tmpSolrHome;
+  private static Path tmpSolrHome;
 
   /** Setup to make the schema mutable */
   @Before
   public void before() throws Exception {
-    tmpSolrHome = createTempDir().toFile();
-    FileUtils.copyDirectory(new File(TEST_HOME()), tmpSolrHome.getAbsoluteFile());
+    tmpSolrHome = createTempDir();
+    PathUtils.copyDirectory(Paths.get(TEST_HOME()), tmpSolrHome.toAbsolutePath());

Review comment:
       `TEST_PATH()`

##########
File path: solr/modules/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java
##########
@@ -134,40 +135,48 @@ public static ManagedModelStore getManagedModelStore() {
 
   protected static void setupTestInit(String solrconfig, String schema, boolean isPersistent)
       throws Exception {
-    tmpSolrHome = createTempDir().toFile();
-    tmpConfDir = new File(tmpSolrHome, CONF_DIR);
-    tmpConfDir.deleteOnExit();
-    FileUtils.copyDirectory(new File(TEST_HOME()), tmpSolrHome.getAbsoluteFile());
+    tmpSolrHome = createTempDir();
+    tmpConfDir = tmpSolrHome.resolve(CONF_DIR);
+    tmpConfDir.toFile().deleteOnExit();
+    PathUtils.copyDirectory(Paths.get(TEST_HOME()), tmpSolrHome.toAbsolutePath());
 
-    final File fstore = new File(tmpConfDir, FEATURE_FILE_NAME);
-    final File mstore = new File(tmpConfDir, MODEL_FILE_NAME);
+    final Path fstore = tmpConfDir.resolve(FEATURE_FILE_NAME);
+    final Path mstore = tmpConfDir.resolve(MODEL_FILE_NAME);
 
     if (isPersistent) {
       fstorefile = fstore;
       mstorefile = mstore;
     }
 
-    if (fstore.exists()) {
+    if (Files.exists(fstore)) {
       if (log.isInfoEnabled()) {
-        log.info("remove feature store config file in {}", fstore.getAbsolutePath());
+        log.info("remove feature store config file in {}", fstore.toAbsolutePath());
       }
-      Files.delete(fstore.toPath());
+      Files.delete(fstore);
     }
-    if (mstore.exists()) {
+    if (Files.exists(mstore)) {
       if (log.isInfoEnabled()) {
-        log.info("remove model store config file in {}", mstore.getAbsolutePath());
+        log.info("remove model store config file in {}", mstore.toAbsolutePath());
       }
-      Files.delete(mstore.toPath());
+      Files.delete(mstore);
     }
     if (!solrconfig.equals("solrconfig.xml")) {
-      FileUtils.copyFile(
-          new File(tmpSolrHome.getAbsolutePath() + "/collection1/conf/" + solrconfig),
-          new File(tmpSolrHome.getAbsolutePath() + "/collection1/conf/solrconfig.xml"));
+      Files.copy(
+          tmpSolrHome.toAbsolutePath().resolve("collection1").resolve("conf").resolve(solrconfig),

Review comment:
       might as well `resolve(CONF_DIR)` here?

##########
File path: solr/modules/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java
##########
@@ -134,40 +135,48 @@ public static ManagedModelStore getManagedModelStore() {
 
   protected static void setupTestInit(String solrconfig, String schema, boolean isPersistent)
       throws Exception {
-    tmpSolrHome = createTempDir().toFile();
-    tmpConfDir = new File(tmpSolrHome, CONF_DIR);
-    tmpConfDir.deleteOnExit();
-    FileUtils.copyDirectory(new File(TEST_HOME()), tmpSolrHome.getAbsoluteFile());
+    tmpSolrHome = createTempDir();
+    tmpConfDir = tmpSolrHome.resolve(CONF_DIR);
+    tmpConfDir.toFile().deleteOnExit();
+    PathUtils.copyDirectory(Paths.get(TEST_HOME()), tmpSolrHome.toAbsolutePath());
 
-    final File fstore = new File(tmpConfDir, FEATURE_FILE_NAME);
-    final File mstore = new File(tmpConfDir, MODEL_FILE_NAME);
+    final Path fstore = tmpConfDir.resolve(FEATURE_FILE_NAME);
+    final Path mstore = tmpConfDir.resolve(MODEL_FILE_NAME);
 
     if (isPersistent) {
       fstorefile = fstore;
       mstorefile = mstore;
     }
 
-    if (fstore.exists()) {
+    if (Files.exists(fstore)) {
       if (log.isInfoEnabled()) {
-        log.info("remove feature store config file in {}", fstore.getAbsolutePath());
+        log.info("remove feature store config file in {}", fstore.toAbsolutePath());
       }
-      Files.delete(fstore.toPath());
+      Files.delete(fstore);
     }
-    if (mstore.exists()) {
+    if (Files.exists(mstore)) {
       if (log.isInfoEnabled()) {
-        log.info("remove model store config file in {}", mstore.getAbsolutePath());
+        log.info("remove model store config file in {}", mstore.toAbsolutePath());
       }
-      Files.delete(mstore.toPath());
+      Files.delete(mstore);
     }
     if (!solrconfig.equals("solrconfig.xml")) {
-      FileUtils.copyFile(
-          new File(tmpSolrHome.getAbsolutePath() + "/collection1/conf/" + solrconfig),
-          new File(tmpSolrHome.getAbsolutePath() + "/collection1/conf/solrconfig.xml"));
+      Files.copy(

Review comment:
       We don't actually need absolute paths here... right? They are supposed to still resolve to the same path, so shouldn't change the behavior for the copy?

##########
File path: solr/modules/ltr/src/test/org/apache/solr/ltr/model/TestAdapterModel.java
##########
@@ -62,21 +62,22 @@ public void setup() throws Exception {
         "popularity", FieldValueFeature.class.getName(), "test", "{\"field\":\"popularity\"}");
 
     scoreValue = random().nextFloat();
-    final File scoreValueFile = new File(tmpConfDir, "scoreValue.txt");
+    final Path scoreValueFile = tmpConfDir.resolve("scoreValue.txt");
     try (BufferedWriter writer =
         new BufferedWriter(
-            new OutputStreamWriter(new FileOutputStream(scoreValueFile), StandardCharsets.UTF_8))) {
+            new OutputStreamWriter(
+                Files.newOutputStream(scoreValueFile), StandardCharsets.UTF_8))) {
       writer.write(Float.toString(scoreValue));
     }
-    scoreValueFile.deleteOnExit();
+    scoreValueFile.toFile().deleteOnExit();

Review comment:
       I suspect we don't need this because the `tmpConfDir` should clean itself up after the test, but we don't have to address that here/now.

##########
File path: solr/core/src/test/org/apache/solr/rest/schema/analysis/TestManagedSynonymFilterFactory.java
##########
@@ -36,20 +36,20 @@
 
 public class TestManagedSynonymFilterFactory extends RestTestBase {
 
-  private static File tmpSolrHome;
+  private static Path tmpSolrHome;
 
   /** Setup to make the schema mutable */
   @Before
   public void before() throws Exception {
-    tmpSolrHome = createTempDir().toFile();
-    FileUtils.copyDirectory(new File(TEST_HOME()), tmpSolrHome.getAbsoluteFile());
+    tmpSolrHome = createTempDir();
+    PathUtils.copyDirectory(Path.of(TEST_HOME()), tmpSolrHome.toAbsolutePath());

Review comment:
       `TEST_PATH()`

##########
File path: solr/modules/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java
##########
@@ -134,40 +134,48 @@ public static ManagedModelStore getManagedModelStore() {
 
   protected static void setupTestInit(String solrconfig, String schema, boolean isPersistent)
       throws Exception {
-    tmpSolrHome = createTempDir().toFile();
-    tmpConfDir = new File(tmpSolrHome, CONF_DIR);
-    tmpConfDir.deleteOnExit();
-    FileUtils.copyDirectory(new File(TEST_HOME()), tmpSolrHome.getAbsoluteFile());
+    tmpSolrHome = createTempDir();
+    tmpConfDir = tmpSolrHome.resolve(CONF_DIR);
+    tmpConfDir.toFile().deleteOnExit();
+    PathUtils.copyDirectory(Path.of(TEST_HOME()), tmpSolrHome.toAbsolutePath());

Review comment:
       `TEST_PATH()`

##########
File path: solr/modules/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java
##########
@@ -134,40 +135,48 @@ public static ManagedModelStore getManagedModelStore() {
 
   protected static void setupTestInit(String solrconfig, String schema, boolean isPersistent)
       throws Exception {
-    tmpSolrHome = createTempDir().toFile();
-    tmpConfDir = new File(tmpSolrHome, CONF_DIR);
-    tmpConfDir.deleteOnExit();
-    FileUtils.copyDirectory(new File(TEST_HOME()), tmpSolrHome.getAbsoluteFile());
+    tmpSolrHome = createTempDir();
+    tmpConfDir = tmpSolrHome.resolve(CONF_DIR);
+    tmpConfDir.toFile().deleteOnExit();
+    PathUtils.copyDirectory(Paths.get(TEST_HOME()), tmpSolrHome.toAbsolutePath());

Review comment:
       `TEST_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