garydgregory commented on code in PR #759:
URL: https://github.com/apache/commons-vfs/pull/759#discussion_r3036914322
##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java:
##########
@@ -58,6 +58,32 @@ public interface FtpClient {
*/
boolean completePendingCommand() throws IOException;
+ /**
+ * Changes the current working directory of the FTP session.
+ *
+ * @param relPath The pathname of the directory to change to.
+ * @return true if successfully completed, false if not.
+ * @throws IOException If an I/O error occurs.
+ * @since 2.10.1
Review Comment:
The next feature version will be 2.11.0 as you can see in the POM. How did
you get to 2.10.1? Is this AI generated?
##########
commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java:
##########
@@ -58,6 +58,32 @@ public interface FtpClient {
*/
boolean completePendingCommand() throws IOException;
+ /**
+ * Changes the current working directory of the FTP session.
+ *
+ * @param relPath The pathname of the directory to change to.
+ * @return true if successfully completed, false if not.
+ * @throws IOException If an I/O error occurs.
+ * @since 2.10.1
+ */
+ default boolean changeDirectory(String relPath) throws IOException {
+ return false;
+ }
+
+ /**
+ * Tests whether a remote path is an existing directory by attempting to
CWD into it.
+ * Implementations must save and restore the working directory to avoid
side effects
+ * on subsequent operations that use relative paths.
+ *
+ * @param relPath The pathname to test.
+ * @return true if the path is an existing directory, false otherwise.
+ * @throws IOException If an I/O error occurs.
+ * @since 2.10.1
Review Comment:
The next feature version will be 2.11.0 as you can see in the POM. How did
you get to 2.10.1? Is this AI generated?
##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpCwdOptimizationTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * 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;
+import java.io.FileWriter;
+import java.time.Duration;
+
+import org.apache.commons.vfs2.FileObject;
+import org.apache.commons.vfs2.FileSystemOptions;
+import org.apache.commons.vfs2.FileType;
+import org.apache.commons.vfs2.VfsTestUtils;
+import org.apache.commons.vfs2.impl.DefaultFileSystemManager;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests that {@link FtpFileObject} uses CWD to check directory existence
before
+ * falling back to the expensive parent LIST command.
+ * <p>
+ * Verifies that:
+ * <ul>
+ * <li>Directories are correctly detected via CWD (no parent LIST
needed)</li>
+ * <li>Files are correctly detected via parent LIST fallback</li>
+ * <li>Non-existent paths return {@link FileType#IMAGINARY}</li>
+ * <li>Working directory is preserved after all operations</li>
+ * <li>Directory timestamps are lazily fetched via parent LIST when
requested</li>
+ * </ul>
+ */
+public class FtpCwdOptimizationTest {
+
+ private File testDir;
+ private File nestedDir;
+ private File fileInRoot;
+ private File fileInNested;
+
+ @BeforeEach
+ public void setUp() throws Exception {
+ FtpProviderTest.setUpClass(VfsTestUtils.getTestDirectory(), null,
null);
+
+ testDir = new File(VfsTestUtils.getTestDirectory());
+ final File cwdOptDir = new File(testDir, "cwdOptDir");
+ nestedDir = new File(cwdOptDir, "nested");
+ nestedDir.mkdirs();
+ fileInRoot = new File(testDir, "rootfile.txt");
+ fileInNested = new File(cwdOptDir, "file.txt");
+ try (FileWriter w = new FileWriter(fileInRoot)) {
+ w.write("root content");
+ }
+ try (FileWriter w = new FileWriter(fileInNested)) {
+ w.write("nested content");
+ }
+ }
+
+ @AfterEach
+ public void tearDown() {
+ FtpProviderTest.tearDownClass();
+ if (fileInNested != null) fileInNested.delete();
+ if (fileInRoot != null) fileInRoot.delete();
+ if (nestedDir != null) nestedDir.delete();
+ new File(testDir, "cwdOptDir").delete();
+ }
+
+ private static FileSystemOptions createOptions() {
+ final FileSystemOptions options = new FileSystemOptions();
+ final FtpFileSystemConfigBuilder builder =
FtpFileSystemConfigBuilder.getInstance();
+ builder.setPassiveMode(options, true);
+ builder.setConnectTimeout(options, Duration.ofSeconds(10));
+ return options;
+ }
+
+ @Test
+ public void testNestedDirectoryExistsViaOptimization() throws Exception {
+ try (final DefaultFileSystemManager manager = new
DefaultFileSystemManager()) {
+ manager.addProvider("ftp", new FtpFileProvider());
+ manager.init();
+ final FileObject dir = manager.resolveFile(
+ FtpProviderTest.getConnectionUri() + "/cwdOptDir/nested",
createOptions());
+ Assertions.assertTrue(dir.exists(), "Nested directory should
exist");
Review Comment:
Please use static imports but only for JUnit.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
