SOLR-16903: Make java.io.File a forbidden API #3321

mlbiscoc · 2025-04-11T23:00:43Z

https://issues.apache.org/jira/browse/SOLR-16903

Description

Add java.io.File as a default forbidden API

Solution

Remove all usages of File, either toFile() or old classes

Tests

Updated tests using File

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide

dsmiley

Thanks for this!
I see you also got in some needless getAbsolutePath removals as well.

dsmiley · 2025-04-12T19:09:47Z

solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPIZkFailure.java

-    tmpConfigDir.toFile().deleteOnExit();
+    PathUtils.deleteOnExit(tmpConfigDir);


createTempDir self cleans up per test. It's javadocs say so.

dsmiley · 2025-04-12T19:10:00Z

solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java

@@ -1565,7 +1565,7 @@ public void testDeleteErrors() throws Exception {
    final SolrClient solrClient = getHttpSolrClient(baseUrl);
    final Path configDir = getFile("solr").resolve("configsets/configset-2/conf");
    final Path tmpConfigDir = createTempDir();
-    tmpConfigDir.toFile().deleteOnExit();
+    PathUtils.deleteOnExit(tmpConfigDir);


createTempDir self cleans up per test. It's javadocs say so.

dsmiley · 2025-04-12T19:10:06Z

solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java

@@ -205,7 +205,7 @@ private void setupBaseConfigSet(String baseConfigSetName, Map<String, String> ol
      throws Exception {
    final Path configDir = getFile("solr").resolve("configsets/configset-2/conf");
    final Path tmpConfigDir = createTempDir();
-    tmpConfigDir.toFile().deleteOnExit();
+    PathUtils.deleteOnExit(tmpConfigDir);


createTempDir self cleans up per test. It's javadocs say so.

dsmiley · 2025-04-12T19:11:52Z

solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java

@@ -105,8 +106,7 @@ public static void beforeClass() throws Exception {
    wrongPemFilePath = JWT_TEST_PATH().resolve("security").resolve("jwt_plugin_idp_wrongcert.pem");

    Path tempDir = Files.createTempDirectory(JWTAuthPluginIntegrationTest.class.getSimpleName());


can we call createTempDir instead?

dsmiley · 2025-04-12T19:12:18Z

solr/modules/llm/src/test/org/apache/solr/llm/TestLlmBase.java

@@ -60,9 +60,8 @@ protected static void setupTest(
  protected static void initFolders(boolean isPersistent) throws Exception {
    tmpSolrHome = createTempDir();
    tmpConfDir = tmpSolrHome.resolve(CONF_DIR);
-    tmpConfDir.toFile().deleteOnExit();
+    PathUtils.deleteOnExit(tmpConfDir);


createTempDir self cleans up per test. It's javadocs say so.

dsmiley · 2025-04-12T19:12:42Z

solr/modules/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java

@@ -136,7 +136,7 @@ protected static void setupTestInit(String solrconfig, String schema, boolean is
      throws Exception {
    tmpSolrHome = createTempDir();
    tmpConfDir = tmpSolrHome.resolve(CONF_DIR);
-    tmpConfDir.toFile().deleteOnExit();
+    PathUtils.deleteOnExit(tmpConfDir);


createTempDir self cleans up per test. It's javadocs say so.

dsmiley · 2025-04-12T19:13:59Z

solr/modules/s3-repository/src/test/org/apache/solr/s3/S3BackupRepositoryTest.java

In this suite, can we just use createTempDir?

dsmiley · 2025-04-12T19:15:32Z

solr/modules/s3-repository/src/test/org/apache/solr/s3/S3IndexInputTest.java

mlbiscoc · 2025-04-15T13:29:52Z

Moved to createTempDir. Also think moving to createTempDir fixed that error that bubbled up from the previous precommit run.

dsmiley

Thanks!

I think this closes out SOLR-16903. Looking in CHANGES.txt, there are 2 mentions. The first is:

* SOLR-16903: Update CLI tools and Solr Core to use java.nio.file.Path instead of java.io.File (Andrey Bozhko, Matthew Biscocho)

I propose updating this one to simply be "Replace all java.io.File usages to java.nio.file.Path (NIO)" listing you & @AndreyBozhko

I also see:

* SOLR-17548: Switch all public Java APIs from File to Path. (Matthew Biscocho via Eric Pugh)

which is implied by the total switch from File -> Path. At least move this to be adjacent but you could also simply combine to the same bullet, listing both JIRAs (this happens sometimes).

Thanks to you both!

mlbiscoc · 2025-04-15T19:17:05Z

JWTAuthPluginIntegrationTest > classMethod FAILED
    java.nio.file.ProviderMismatchException: mismatch, expected: FilterPath, got: class sun.nio.fs.UnixPath

Looks like we might be running into that filterPath issue I ran into previously. Also looks like there is precommit issues I am not seeing locally.... I'll need to fix this before merging. Will update the CHANGES.txt combining the change log and list @AndreyBozhko

dsmiley · 2025-04-15T19:50:32Z

solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java

-    Path tempDir = Files.createTempDirectory(JWTAuthPluginIntegrationTest.class.getSimpleName());
-    tempDir.toFile().deleteOnExit();
-
+    Path tempDir = createTempDir(JWTAuthPluginIntegrationTest.class.getSimpleName());


createTempDir already incorporates the test class name

mlbiscoc · 2025-04-16T13:25:23Z

solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginIntegrationTest.java

-    tempDir.toFile().deleteOnExit();
-
+    Path tempDir = FilterPath.unwrap(createTempDir());
    Path modifiedP12Cert = tempDir.resolve(p12Cert.getFileName());


Need to unwrap to resolve the path

mlbiscoc · 2025-04-16T13:26:05Z

solr/CHANGES.txt

-* SOLR-17548: Switch all public Java APIs from File to Path. (Matthew Biscocho via Eric Pugh)
-


Going to just remove this because its implied from the change above anyways.

mlbiscoc added 6 commits April 8, 2025 17:29

Make file a forbidden api

af661b8

Throw a better exception

eb1b33e

Create temporary junit files correctly

2d26952

Delete on exit

0239ec3

More migration

c62e677

Revert un-related change

ff52137

mlbiscoc marked this pull request as ready for review April 11, 2025 23:00

github-actions bot added module:jwt-auth module:ltr module:s3-repository tool:build tests cat:cloud cat:index cat:cli cat:security cat:packagemanager labels Apr 11, 2025

Delete temp directories on exit

512136e

dsmiley reviewed Apr 12, 2025

View reviewed changes

mlbiscoc added 2 commits April 15, 2025 09:21

Use #createTempDir over deleteOnExit

193acd2

createTempDir

8628d8a

dsmiley approved these changes Apr 15, 2025

View reviewed changes

dsmiley reviewed Apr 15, 2025

View reviewed changes

FilterPath failures and CHANGES.txt

16b7f41

mlbiscoc commented Apr 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SOLR-16903: Make java.io.File a forbidden API #3321

SOLR-16903: Make java.io.File a forbidden API #3321

mlbiscoc commented Apr 11, 2025

dsmiley left a comment

dsmiley Apr 12, 2025

dsmiley Apr 12, 2025

dsmiley Apr 12, 2025

dsmiley Apr 12, 2025

dsmiley Apr 12, 2025

dsmiley Apr 12, 2025

dsmiley Apr 12, 2025

dsmiley Apr 12, 2025

mlbiscoc commented Apr 15, 2025

dsmiley left a comment

mlbiscoc commented Apr 15, 2025

dsmiley Apr 15, 2025

mlbiscoc Apr 16, 2025

mlbiscoc Apr 16, 2025

		tmpConfigDir.toFile().deleteOnExit();
		PathUtils.deleteOnExit(tmpConfigDir);

		@@ -105,8 +106,7 @@ public static void beforeClass() throws Exception {
		wrongPemFilePath = JWT_TEST_PATH().resolve("security").resolve("jwt_plugin_idp_wrongcert.pem");

		Path tempDir = Files.createTempDirectory(JWTAuthPluginIntegrationTest.class.getSimpleName());

		* SOLR-17548: Switch all public Java APIs from File to Path. (Matthew Biscocho via Eric Pugh)

SOLR-16903: Make java.io.File a forbidden API #3321

Are you sure you want to change the base?

SOLR-16903: Make java.io.File a forbidden API #3321

Conversation

mlbiscoc commented Apr 11, 2025

Description

Solution

Tests

Checklist

dsmiley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlbiscoc commented Apr 15, 2025

dsmiley left a comment

Choose a reason for hiding this comment

mlbiscoc commented Apr 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment