You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2026/04/05 14:12:34 UTC
(commons-vfs) branch master updated (5f773632e -> 3d72c9aac)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-vfs.git
from 5f773632e [VFS] Fix refresh() not clearing cached state when file was never attached (#758)
new f6d991cc4 Fix refresh() not clearing cached state when file was never attached #758
new 3d72c9aac Fix refresh() not clearing cached state when file was never attached #758
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../commons/vfs2/provider/sftp/SftpFileObject.java | 28 +++++------
.../provider/ftp/FtpRefreshCachedStateTest.java | 57 +++++++++-------------
.../provider/sftp/SftpRefreshCachedStateTest.java | 34 +++++--------
src/changes/changes.xml | 1 +
4 files changed, 48 insertions(+), 72 deletions(-)
(commons-vfs) 02/02: Fix refresh() not clearing cached state when file was never attached #758
Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-vfs.git
commit 3d72c9aac57ffa2a170693f355565797a9172782
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sun Apr 5 10:12:27 2026 -0400
Fix refresh() not clearing cached state when file was never
attached #758
- Use longer lines
- Sort members
- Reduce vertical whitespace
---
.../commons/vfs2/provider/sftp/SftpFileObject.java | 28 +++++------
.../provider/ftp/FtpRefreshCachedStateTest.java | 57 +++++++++-------------
.../provider/sftp/SftpRefreshCachedStateTest.java | 34 +++++--------
3 files changed, 47 insertions(+), 72 deletions(-)
diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java
index 230eb87e4..678d11258 100644
--- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/sftp/SftpFileObject.java
@@ -147,21 +147,6 @@ public class SftpFileObject extends AbstractFileObject<SftpFileSystem> {
attrs = null;
}
- /**
- * {@inheritDoc}
- * <p>
- * Clears the cached {@code attrs} regardless of the {@code attached} state.
- * The {@code attrs} field can be populated without going through {@code attach()}
- * (e.g. via {@code setStat()} in {@code doListChildrenResolved()}), so
- * {@code doDetach()} alone may not clear it if the file was never attached.
- * </p>
- */
- @Override
- public synchronized void refresh() throws FileSystemException {
- super.refresh();
- attrs = null;
- }
-
/**
* Returns the size of the file content (in bytes).
*/
@@ -511,6 +496,19 @@ public class SftpFileObject extends AbstractFileObject<SftpFileSystem> {
getAbstractFileSystem().putChannel(channel);
}
+ /**
+ * {@inheritDoc}
+ * <p>
+ * Clears the cached {@code attrs} regardless of the {@code attached} state. The {@code attrs} field can be populated without going through {@code attach()}
+ * (e.g. via {@code setStat()} in {@code doListChildrenResolved()}), so {@code doDetach()} alone may not clear it if the file was never attached.
+ * </p>
+ */
+ @Override
+ public synchronized void refresh() throws FileSystemException {
+ super.refresh();
+ attrs = null;
+ }
+
/**
* Sets attrs from listChildrenResolved
*/
diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpRefreshCachedStateTest.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpRefreshCachedStateTest.java
index dea70bfa3..147ff155e 100644
--- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpRefreshCachedStateTest.java
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpRefreshCachedStateTest.java
@@ -14,6 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
package org.apache.commons.vfs2.provider.ftp;
import java.io.File;
@@ -29,18 +30,25 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
/**
- * Tests that {@link org.apache.commons.vfs2.provider.AbstractFileObject#refresh()} correctly clears cached state
- * even when the file was never attached.
+ * Tests that {@link org.apache.commons.vfs2.provider.AbstractFileObject#refresh()} correctly clears cached state even when the file was never attached.
* <p>
- * Regression test for the bug in {@code AbstractFileObject.detach()} where the {@code if (attached)} guard
- * prevented clearing cached fields ({@code type}, {@code parent}, {@code children}) on objects that were never
- * attached. Provider-specific cached fields like {@code FtpFileObject.childMap} can be populated without going
- * through {@code attach()} (e.g. via {@code getChildFile()} → {@code doGetChildren()}), so {@code refresh()}
- * must clear cached state regardless of the {@code attached} flag.
+ * Regression test for the bug in {@code AbstractFileObject.detach()} where the {@code if (attached)} guard prevented clearing cached fields ({@code type},
+ * {@code parent}, {@code children}) on objects that were never attached. Provider-specific cached fields like {@code FtpFileObject.childMap} can be populated
+ * without going through {@code attach()} (e.g. via {@code getChildFile()} → {@code doGetChildren()}), so {@code refresh()} must clear cached state regardless
+ * of the {@code attached} flag.
* </p>
*/
public class FtpRefreshCachedStateTest {
+ private static FileSystemOptions createOptions() {
+ final FileSystemOptions options = new FileSystemOptions();
+ final FtpFileSystemConfigBuilder builder = FtpFileSystemConfigBuilder.getInstance();
+ builder.setUserDirIsRoot(options, true);
+ builder.setPassiveMode(options, true);
+ builder.setConnectTimeout(options, Duration.ofSeconds(10));
+ return options;
+ }
+
@BeforeEach
public void setUp() throws Exception {
FtpProviderTest.setUpClass(VfsTestUtils.getTestDirectory(), null, null);
@@ -51,27 +59,16 @@ public class FtpRefreshCachedStateTest {
FtpProviderTest.tearDownClass();
}
- private static FileSystemOptions createOptions() {
- final FileSystemOptions options = new FileSystemOptions();
- final FtpFileSystemConfigBuilder builder = FtpFileSystemConfigBuilder.getInstance();
- builder.setUserDirIsRoot(options, true);
- builder.setPassiveMode(options, true);
- builder.setConnectTimeout(options, Duration.ofSeconds(10));
- return options;
- }
-
/**
- * Tests that {@code exists()} returns {@code false} for a file that was deleted from the FTP server,
- * even when the parent's cached {@code childMap} was populated without {@code attach()}.
+ * Tests that {@code exists()} returns {@code false} for a file that was deleted from the FTP server, even when the parent's cached {@code childMap} was
+ * populated without {@code attach()}.
* <p>
- * The scenario: a file exists, its parent's {@code childMap} is populated (via {@code getChildFile()}
- * during the first {@code exists()} call), the file is then deleted on the server, and the parent's
- * {@code refresh()} must clear the stale {@code childMap} so the next {@code exists()} returns
- * {@code false}.
+ * The scenario: a file exists, its parent's {@code childMap} is populated (via {@code getChildFile()} during the first {@code exists()} call), the file is
+ * then deleted on the server, and the parent's {@code refresh()} must clear the stale {@code childMap} so the next {@code exists()} returns {@code false}.
* </p>
* <p>
- * Before the fix, {@code refresh()} → {@code detach()} skipped cache clearing because the parent was
- * never explicitly attached ({@code attached == false}), leaving stale data in {@code childMap}.
+ * Before the fix, {@code refresh()} → {@code detach()} skipped cache clearing because the parent was never explicitly attached ({@code attached == false}),
+ * leaving stale data in {@code childMap}.
* </p>
*/
@Test
@@ -80,32 +77,24 @@ public class FtpRefreshCachedStateTest {
final File ftpHome = new File(VfsTestUtils.getTestDirectory());
final File tempFile = new File(ftpHome, "refresh-test-file.txt");
tempFile.createNewFile();
-
try (final DefaultFileSystemManager manager = new DefaultFileSystemManager()) {
manager.addProvider("ftp", new FtpFileProvider());
manager.init();
-
final FileSystemOptions options = createOptions();
- final FileObject file = manager.resolveFile(
- FtpProviderTest.getConnectionUri() + "/refresh-test-file.txt", options);
-
+ final FileObject file = manager.resolveFile(FtpProviderTest.getConnectionUri() + "/refresh-test-file.txt", options);
// Verify the file exists. This populates the parent's childMap
// via setFTPFile() → getParent().getChildFile() → doGetChildren().
Assertions.assertTrue(file.exists(), "File should exist on the FTP server");
-
// Delete the file directly on the filesystem.
Assertions.assertTrue(tempFile.delete(), "Temp file should be deleted");
-
// Refresh the parent to clear its cached childMap, then refresh the file.
// Before the fix, the parent's refresh() skipped clearing childMap because
// the parent was never attached (attached == false).
final FileObject parent = file.getParent();
parent.refresh();
file.refresh();
-
// exists() must return false now that the file is deleted.
- Assertions.assertFalse(file.exists(),
- "exists() must return false after file is deleted and parent is refreshed");
+ Assertions.assertFalse(file.exists(), "exists() must return false after file is deleted and parent is refreshed");
} finally {
tempFile.delete();
}
diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpRefreshCachedStateTest.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpRefreshCachedStateTest.java
index 9a4da3bf5..34b69b2e3 100644
--- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpRefreshCachedStateTest.java
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/sftp/SftpRefreshCachedStateTest.java
@@ -14,6 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
package org.apache.commons.vfs2.provider.sftp;
import java.io.File;
@@ -28,13 +29,11 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
/**
- * Tests that {@link SftpFileObject#refresh()} correctly clears cached {@code attrs}
- * even when the file was never attached.
+ * Tests that {@link SftpFileObject#refresh()} correctly clears cached {@code attrs} even when the file was never attached.
* <p>
- * Regression test for the bug where {@code attrs} can be populated without going through
- * {@code attach()} (via {@code setStat()} in {@code doListChildrenResolved()}). Without
- * the fix, {@code refresh()} skips clearing {@code attrs} because {@code attached == false},
- * causing {@code exists()} to return stale results.
+ * Regression test for the bug where {@code attrs} can be populated without going through {@code attach()} (via {@code setStat()} in
+ * {@code doListChildrenResolved()}). Without the fix, {@code refresh()} skips clearing {@code attrs} because {@code attached == false}, causing
+ * {@code exists()} to return stale results.
* </p>
*/
public class SftpRefreshCachedStateTest {
@@ -50,14 +49,12 @@ public class SftpRefreshCachedStateTest {
}
/**
- * Tests that {@code exists()} returns {@code false} for a file that was deleted,
- * when the child's {@code attrs} was populated via the parent's listing without
- * {@code attach()}.
+ * Tests that {@code exists()} returns {@code false} for a file that was deleted, when the child's {@code attrs} was populated via the parent's listing
+ * without {@code attach()}.
* <p>
- * The key is that we do NOT call {@code exists()} or {@code getType()} on the child
- * before deleting — only the parent's {@code getChildren()} populates the child's
- * {@code attrs} via {@code setStat()} (without triggering {@code attach()}).
- * Then after deleting the file, {@code refresh()} must clear the stale {@code attrs}.
+ * The key is that we do NOT call {@code exists()} or {@code getType()} on the child before deleting — only the parent's {@code getChildren()} populates the
+ * child's {@code attrs} via {@code setStat()} (without triggering {@code attach()}). Then after deleting the file, {@code refresh()} must clear the stale
+ * {@code attrs}.
* </p>
*/
@Test
@@ -65,22 +62,17 @@ public class SftpRefreshCachedStateTest {
final File testDir = VfsTestUtils.getTestDirectoryFile();
final File tempFile = new File(testDir, "sftp-refresh-test-file.txt");
tempFile.createNewFile();
-
try (final DefaultFileSystemManager manager = new DefaultFileSystemManager()) {
manager.addProvider("sftp", new SftpFileProvider());
manager.init();
-
final FileSystemOptions options = new FileSystemOptions();
final SftpFileSystemConfigBuilder builder = SftpFileSystemConfigBuilder.getInstance();
builder.setStrictHostKeyChecking(options, "no");
-
final String uri = SftpTestServerHelper.getConnectionUri();
-
// List the parent's children. doListChildrenResolved() calls setStat()
// on each child, populating attrs WITHOUT calling attach().
final FileObject parent = manager.resolveFile(uri, options);
final FileObject[] children = parent.getChildren();
-
// Find our file among the children — its attrs is set but attached=false.
FileObject file = null;
for (final FileObject child : children) {
@@ -90,18 +82,14 @@ public class SftpRefreshCachedStateTest {
}
}
Assertions.assertNotNull(file, "File should be found in parent listing");
-
// Delete the file directly on the filesystem.
Assertions.assertTrue(tempFile.delete(), "Temp file should be deleted");
-
// Refresh the child. Before the fix, refresh() skipped clearing attrs
// because attached == false (attrs was set via setStat, not attach).
file.refresh();
-
// exists() must return false. Before the fix, doGetType() found the stale
// attrs (non-null), skipped statSelf(), and returned FILE instead of IMAGINARY.
- Assertions.assertFalse(file.exists(),
- "exists() must return false after file is deleted and refreshed");
+ Assertions.assertFalse(file.exists(), "exists() must return false after file is deleted and refreshed");
} finally {
tempFile.delete();
}
(commons-vfs) 01/02: Fix refresh() not clearing cached state when file was never attached #758
Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-vfs.git
commit f6d991cc4fd4f93aba8699a151a486c564d8ff08
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sun Apr 5 09:59:32 2026 -0400
Fix refresh() not clearing cached state when file was never
attached #758
---
src/changes/changes.xml | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 7ae84793f..5fef01541 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -63,6 +63,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Vaishnavi Kumbhar, Gary Gregory" issue="VFS-861">Http5FileProvider Basic authentication fails: UserAuthenticationData.setData(Type, char[]) should clone its array input.</action>
<action type="fix" dev="ggregory" due-to="Vaishnavi Kumbhar, Gary Gregory" issue="VFS-861">Http5FileProvider Basic authentication fails: UserAuthenticationData.setData(Type, char[]) should clone its array input.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory, Ilan Goldfeld">Fix FtpFileObject.exists() returning true for root-level folders after connection drop #757.</action>
+ <action type="fix" dev="ggregory" due-to="Gary Gregory, Ilan Goldfeld">Fix refresh() not clearing cached state when file was never attached #758.</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary Gregory">Add org.apache.commons.vfs2.provider.ftp.FTPClientWrapper.sendOptions(String, String).</action>
<action type="add" dev="ggregory" due-to="Gary Gregory">Add FtpFileSystemConfigBuilder.getControlEncodingCharset(FileSystemOptions) and deprecate getControlEncoding(FileSystemOptions).</action>