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 83110aae22dbcd6f5f32acbefb2da134198b344b Author: Gary Gregory <[email protected]> AuthorDate: Tue Feb 9 09:48:44 2021 -0500 [VFS-257] [FTP] Add support for MDTM to get more accurate last modified times. --- .../vfs2/provider/ftp/FTPClientWrapper.java | 53 ++++++++++++++++------ .../commons/vfs2/provider/ftp/FtpClient.java | 32 +++++++++++++ .../commons/vfs2/provider/ftp/FtpFileObject.java | 28 ++++++++++-- .../provider/ftp/FtpFileSystemConfigBuilder.java | 24 ++++++++++ .../org/apache/commons/vfs2/AbstractTestSuite.java | 2 +- .../org/apache/commons/vfs2/LastModifiedTests.java | 4 -- .../provider/ftp/FtpMdtmOnLastModifiedTests.java | 3 -- .../provider/ftp/FtpProviderMdtmOffTestCase.java | 2 +- .../provider/ftp/FtpProviderMdtmOnTestCase.java | 6 +-- .../vfs2/provider/ftp/FtpProviderTestCase.java | 36 +++++++++++---- src/changes/changes.xml | 3 ++ 11 files changed, 152 insertions(+), 41 deletions(-) diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java index 687fed7..909996f 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FTPClientWrapper.java @@ -19,6 +19,7 @@ package org.apache.commons.vfs2.provider.ftp; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.time.Instant; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -46,7 +47,7 @@ public class FTPClientWrapper implements FtpClient { private final GenericFileName root; protected FTPClientWrapper(final GenericFileName root, final FileSystemOptions fileSystemOptions) - throws FileSystemException { + throws FileSystemException { this.root = root; this.fileSystemOptions = fileSystemOptions; getFtpClient(); // fail-fast @@ -101,13 +102,13 @@ public class FTPClientWrapper implements FtpClient { } protected FTPClient createClient(final GenericFileName rootName, final UserAuthenticationData authData) - throws FileSystemException { + throws FileSystemException { return FtpClientFactory.createConnection(rootName.getHostName(), rootName.getPort(), - UserAuthenticatorUtils.getData(authData, UserAuthenticationData.USERNAME, - UserAuthenticatorUtils.toChar(rootName.getUserName())), - UserAuthenticatorUtils.getData(authData, UserAuthenticationData.PASSWORD, - UserAuthenticatorUtils.toChar(rootName.getPassword())), - rootName.getPath(), getFileSystemOptions()); + UserAuthenticatorUtils.getData(authData, UserAuthenticationData.USERNAME, + UserAuthenticatorUtils.toChar(rootName.getUserName())), + UserAuthenticatorUtils.getData(authData, UserAuthenticationData.PASSWORD, + UserAuthenticatorUtils.toChar(rootName.getPassword())), + rootName.getPath(), getFileSystemOptions()); } @Override @@ -163,6 +164,19 @@ public class FTPClientWrapper implements FtpClient { return root; } + /** + * {@inheritDoc} + */ + @Override + public boolean hasFeature(final String feature) throws IOException { + try { + return getFtpClient().hasFeature(feature); + } catch (final IOException ex) { + disconnect(); + return getFtpClient().hasFeature(feature); + } + } + @Override public boolean isConnected() throws FileSystemException { return ftpClient != null && ftpClient.isConnected(); @@ -180,12 +194,10 @@ public class FTPClientWrapper implements FtpClient { } private FTPFile[] listFilesInDirectory(final String relPath) throws IOException { - FTPFile[] files; - // VFS-307: no check if we can simply list the files, this might fail if there are spaces in the path - files = getFtpClient().listFiles(relPath); + FTPFile[] ftpFiles = getFtpClient().listFiles(relPath); if (FTPReply.isPositiveCompletion(getFtpClient().getReplyCode())) { - return files; + return ftpFiles; } // VFS-307: now try the hard way by cd'ing into the directory, list and cd back @@ -199,13 +211,13 @@ public class FTPClientWrapper implements FtpClient { } } - files = getFtpClient().listFiles(); + ftpFiles = getFtpClient().listFiles(); if (relPath != null && !getFtpClient().changeWorkingDirectory(workingDirectory)) { throw new FileSystemException("vfs.provider.ftp.wrapper/change-work-directory-back.error", - workingDirectory); + workingDirectory); } - return files; + return ftpFiles; } @Override @@ -218,6 +230,19 @@ public class FTPClientWrapper implements FtpClient { } } + /** + * {@inheritDoc} + */ + @Override + public Instant mdtmInstant(final String relPath) throws IOException { + try { + return getFtpClient().mdtmFile(relPath).getTimestamp().toInstant(); + } catch (final IOException ex) { + disconnect(); + return getFtpClient().mdtmFile(relPath).getTimestamp().toInstant(); + } + } + @Override public boolean removeDirectory(final String relPath) throws IOException { try { diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java index c6e5885..7bcabd5 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpClient.java @@ -19,6 +19,7 @@ package org.apache.commons.vfs2.provider.ftp; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.time.Instant; import org.apache.commons.net.ftp.FTPFile; import org.apache.commons.net.ftp.FTPReply; @@ -45,12 +46,43 @@ public interface FtpClient { String getReplyString() throws IOException; + /** + * Queries the server for a supported feature. + * + * @param feature the name of the feature, converted to upper case. + * @return {@code true} if the feature is present, {@code false} if the feature is not present or the FTP command + * failed. + * + * @throws IOException on error + * @since 2.8.0 + */ + boolean hasFeature(final String feature) throws IOException; + boolean isConnected() throws FileSystemException; + // This interface should not leak Apache Commons NET types like FTPFile FTPFile[] listFiles(String relPath) throws IOException; boolean makeDirectory(String relPath) throws IOException; + /** + * Sends the MDTM command to get a file's date and time information after file transfer. It is typically more + * accurate than the {@code "LIST"} command response. Time values are always represented in UTC (GMT), and in the + * Gregorian calendar regardless of what calendar may have been in use at the date and time the file was last + * modified. + * <p> + * NOTE: not all remote FTP servers support {@code MDTM}. + * </p> + * + * @param relPath The relative path of the file object to execute {@code MDTM} command against + * @return new {@code Instant} object containing the {@code MDTM} timestamp. + * @throws IOException If the underlying FTP client encountered an error. + * @since 2.8.0 + */ + default Instant mdtmInstant(final String relPath) throws IOException { + return null; + } + boolean removeDirectory(String relPath) throws IOException; boolean rename(String oldName, String newName) throws IOException; diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java index e7fc69c..1c78ef3 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileObject.java @@ -20,10 +20,12 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.time.Instant; import java.util.Calendar; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.TimeZone; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; @@ -130,6 +132,7 @@ public class FtpFileObject extends AbstractFileObject<FtpFileSystem> { private static final FTPFile UNKNOWN = new FTPFile(); private static final Log log = LogFactory.getLog(FtpFileObject.class); + private volatile boolean mdtmSet; private final String relPath; // Cached info private volatile FTPFile ftpFile; @@ -301,7 +304,7 @@ public class FtpFileObject extends AbstractFileObject<FtpFileSystem> { } /** - * get the last modified time on an ftp file + * Get the last modified time on an ftp file * * @see org.apache.commons.vfs2.provider.AbstractFileObject#doGetLastModifiedTime() */ @@ -531,9 +534,26 @@ public class FtpFileObject extends AbstractFileObject<FtpFileSystem> { return relPath; } - private long getTimestampMillis() { - final Calendar timestamp = this.ftpFile != null ? this.ftpFile.getTimestamp() : null; - return timestamp == null ? DEFAULT_TIMESTAMP : timestamp.getTime().getTime(); + /** + * ftpFile is not null. + */ + @SuppressWarnings("resource") // abstractFileSystem is managed in the superclass. + private long getTimestampMillis() throws IOException { + final FtpFileSystem abstractFileSystem = getAbstractFileSystem(); + final Boolean mdtmLastModifiedTime = FtpFileSystemConfigBuilder.getInstance() + .getMdtmLastModifiedTime(abstractFileSystem.getFileSystemOptions()); + if (mdtmLastModifiedTime != null && mdtmLastModifiedTime.booleanValue()) { + final FtpClient client = abstractFileSystem.getClient(); + if (!mdtmSet && client.hasFeature("MDTM")) { + final Instant mdtmInstant = client.mdtmInstant(relPath); + final Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("GMT")); + final long epochMilli = mdtmInstant.toEpochMilli(); + calendar.setTimeInMillis(epochMilli); + ftpFile.setTimestamp(calendar); + mdtmSet = true; + } + } + return ftpFile.getTimestamp().getTime().getTime(); } /** diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileSystemConfigBuilder.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileSystemConfigBuilder.java index 21aecdf..27c9469 100644 --- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileSystemConfigBuilder.java +++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/ftp/FtpFileSystemConfigBuilder.java @@ -24,6 +24,7 @@ import java.util.List; import org.apache.commons.net.ftp.FTPReply; import org.apache.commons.net.ftp.parser.FTPFileEntryParserFactory; +import org.apache.commons.vfs2.FileContent; import org.apache.commons.vfs2.FileSystem; import org.apache.commons.vfs2.FileSystemConfigBuilder; import org.apache.commons.vfs2.FileSystemOptions; @@ -56,6 +57,7 @@ public class FtpFileSystemConfigBuilder extends FileSystemConfigBuilder { private static final String CONTROL_KEEP_ALIVE_REPLY_TIMEOUT = _PREFIX + ".CONTROL_KEEP_ALIVE_REPLY_TIMEOUT"; private static final String USER_DIR_IS_ROOT = _PREFIX + ".USER_DIR_IS_ROOT"; private static final String TRANSFER_ABORTED_OK_REPLY_CODES = _PREFIX + ".TRANSFER_ABORTED_OK_REPLY_CODES"; + private static final String MDTM_LAST_MODIFED_TIME = _PREFIX + ".MDTM_LAST_MODIFED_TIME"; /** * Gets the singleton instance. @@ -189,6 +191,17 @@ public class FtpFileSystemConfigBuilder extends FileSystemConfigBuilder { } /** + * Gets the option to use FTP MDTM for {@link FileContent#getLastModifiedTime()}. + * + * @param opts The FileSystemOptions. + * @return true if MDTM should be used. + * @since 2.8.0 + */ + public Boolean getMdtmLastModifiedTime(final FileSystemOptions opts) { + return getBoolean(opts, MDTM_LAST_MODIFED_TIME); + } + + /** * @param opts The FileSystemOptions. * @return true if passive mode is set. * @see #setPassiveMode @@ -413,6 +426,17 @@ public class FtpFileSystemConfigBuilder extends FileSystemConfigBuilder { } /** + * Sets the option to use FTP MDTM for {@link FileContent#getLastModifiedTime()}. + * + * @param opts The FileSystemOptions. + * @param mdtm true if MDTM should be used. + * @since 2.8.0 + */ + public void setMdtmLastModifiedTime(final FileSystemOptions opts, final boolean mdtm) { + setParam(opts, MDTM_LAST_MODIFED_TIME, toBooleanObject(mdtm)); + } + + /** * Enter into passive mode. * * @param opts The FileSystemOptions. diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractTestSuite.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractTestSuite.java index 5e698df..68d1831 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractTestSuite.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/AbstractTestSuite.java @@ -42,7 +42,7 @@ import junit.framework.TestSuite; * The suite of tests for a file system. */ public abstract class AbstractTestSuite extends TestSetup { - + public static final String WRITE_TESTS_FOLDER = "write-tests"; public static final String READ_TESTS_FOLDER = "read-tests"; diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/LastModifiedTests.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/LastModifiedTests.java index 64f8532..65a22f0 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/LastModifiedTests.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/LastModifiedTests.java @@ -18,10 +18,6 @@ package org.apache.commons.vfs2; import static org.junit.Assert.assertNotEquals; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.nio.file.attribute.FileTime; import java.time.Duration; import java.time.Instant; import java.util.Date; diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpMdtmOnLastModifiedTests.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpMdtmOnLastModifiedTests.java index a228379..0e5f0e1 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpMdtmOnLastModifiedTests.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpMdtmOnLastModifiedTests.java @@ -17,8 +17,6 @@ package org.apache.commons.vfs2.provider.ftp; -import static org.junit.Assert.assertNotEquals; - import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; @@ -41,7 +39,6 @@ public class FtpMdtmOnLastModifiedTests extends LastModifiedTests { final FileObject fileObject = readFolder.resolveFile(fileName); final long lastModifiedTimeMillis = fileObject.getContent().getLastModifiedTime(); // now try to match - final long lastModTimeAccuracyMillis = (long) readFolder.getFileSystem().getLastModTimeAccuracy(); final FileTime lastModifiedTime = Files .getLastModifiedTime(Paths.get(getTestDirectory(), AbstractTestSuite.READ_TESTS_FOLDER, fileName)); assertEqualMillis("getLastModified on File", lastModifiedTime.toMillis(), lastModifiedTimeMillis); diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOffTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOffTestCase.java index e61fbd1..c560428 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOffTestCase.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOffTestCase.java @@ -34,7 +34,7 @@ import junit.framework.Test; public class FtpProviderMdtmOffTestCase extends FtpProviderTestCase { public static Test suite() throws Exception { - return suite(new FtpProviderMdtmOffTestCase(), FtpMdtmOffLastModifiedTests.class); + return suite(new FtpProviderTestCase(false), FtpMdtmOffLastModifiedTests.class); } /** diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOnTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOnTestCase.java index e768c93..4858f24 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOnTestCase.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderMdtmOnTestCase.java @@ -16,17 +16,15 @@ */ package org.apache.commons.vfs2.provider.ftp; -import org.junit.Ignore; - import junit.framework.Test; -@Ignore public class FtpProviderMdtmOnTestCase extends FtpProviderTestCase { /** * MDTM is supported by default for underlying Apache MINA FTP server. */ public static Test suite() throws Exception { - return suite(new FtpProviderTestCase(), FtpMdtmOnLastModifiedTests.class); + return suite(new FtpProviderTestCase(true), FtpMdtmOnLastModifiedTests.class); } + } diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java index 0361006..f29367c 100644 --- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java +++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/ftp/FtpProviderTestCase.java @@ -44,6 +44,7 @@ import junit.framework.Test; * Tests for FTP file systems. */ public class FtpProviderTestCase extends AbstractProviderTestConfig { + private static int SocketPort; /** @@ -172,6 +173,16 @@ public class FtpProviderTestCase extends AbstractProviderTestConfig { } } + private final boolean mdtmLastModifiedTime; + + public FtpProviderTestCase() { + this(false); + } + + public FtpProviderTestCase(boolean mdtmLastModifiedTime) { + this.mdtmLastModifiedTime = mdtmLastModifiedTime; + } + /** * Returns the base folder for tests. You can override the DEFAULT_URI by using the system property name defined by * TEST_URI. @@ -182,17 +193,10 @@ public class FtpProviderTestCase extends AbstractProviderTestConfig { if (uri == null) { uri = ConnectionUri; } - final FileSystemOptions opts = new FileSystemOptions(); + final FileSystemOptions options = new FileSystemOptions(); final FtpFileSystemConfigBuilder builder = FtpFileSystemConfigBuilder.getInstance(); - builder.setUserDirIsRoot(opts, getUserDirIsRoot()); - builder.setPassiveMode(opts, true); - // FtpFileType.BINARY is the default - builder.setFileType(opts, FtpFileType.BINARY); - builder.setConnectTimeout(opts, 10000); - builder.setControlEncoding(opts, "UTF-8"); - builder.setControlKeepAliveReplyTimeout(opts, Duration.ofSeconds(35)); - builder.setControlKeepAliveTimeout(opts, Duration.ofSeconds(30)); - return manager.resolveFile(uri, opts); + init(builder, options); + return manager.resolveFile(uri, options); } /** @@ -226,6 +230,18 @@ public class FtpProviderTestCase extends AbstractProviderTestConfig { return false; } + protected void init(final FtpFileSystemConfigBuilder builder, final FileSystemOptions options) { + builder.setUserDirIsRoot(options, getUserDirIsRoot()); + builder.setPassiveMode(options, true); + // FtpFileType.BINARY is the default + builder.setFileType(options, FtpFileType.BINARY); + builder.setConnectTimeout(options, 10000); + builder.setControlEncoding(options, "UTF-8"); + builder.setControlKeepAliveReplyTimeout(options, Duration.ofSeconds(35)); + builder.setControlKeepAliveTimeout(options, Duration.ofSeconds(30)); + builder.setMdtmLastModifiedTime(options, mdtmLastModifiedTime); + } + /** * Prepares the file system manager. */ diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d22da4b..ca7c4e0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -82,6 +82,9 @@ The <action> type attribute can be add,update,fix,remove. <action dev="ggregory" due-to="Gary Gregory, Michael Graham" type="add"> FTP tests can be configured with a custom FTP server command factory. </action> + <action issue="VFS-257" dev="ggregory" due-to="Andrew Franklin, Michael Graham, Gary Gregory" type="add"> + [FTP] Add support for MDTM to get more accurate last modified times. + </action> <!-- UPDATES --> <action dev="ggregory" due-to="PeterAlfredLee" type="update"> Modify some code use for-each loop and stream API #142.
