From 03cee942dcffecd6b735c13c4d3b51b734239b14 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sat, 14 Dec 2024 12:48:36 +0100 Subject: [PATCH] GH-643: provide interfaces for caching file attributes on paths Remote file systems may have a need to cache file attributes on Path instances. A typical use case is iterating over all files in a directory: the directory listing returns paths, but the underlying remote listing operation may also return file attributes for each entry. This is the case in Sftp, but may also occur for other file systems, for instance a file system wrapping Amazon S3. It would be unfortunate and inefficient if iterating through the paths returned and doing something with the attributes would have to re-fetch the attributes again if they were already available. By implementing WithFileAttributes of its Paths, a file system can associate file attributes with a path instance, and client code can access them. If a file system also makes its paths implement the second interface WithFileAttributeCache, then the SftpSubsystem uses it internally to avoid making repeated remote calls to get file attributes. --- .../client/fs/SftpFileSystemProvider.java | 7 +- .../apache/sshd/sftp/client/fs/SftpPath.java | 9 +-- .../sshd/sftp/client/fs/SftpPathIterator.java | 5 +- .../client/fs/WithFileAttributeCache.java | 71 +++++++++++++++++++ .../sftp/client/fs/WithFileAttributes.java | 35 +++++++++ .../sshd/sftp/client/impl/SftpPathImpl.java | 51 ++----------- .../server/AbstractSftpSubsystemHelper.java | 16 ++--- .../sshd/sftp/server/SftpSubsystem.java | 4 +- 8 files changed, 127 insertions(+), 71 deletions(-) create mode 100644 sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributeCache.java create mode 100644 sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributes.java diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java index 4157e2114..3897d895e 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpFileSystemProvider.java @@ -100,7 +100,6 @@ import org.apache.sshd.sftp.client.SftpErrorDataHandler; import org.apache.sshd.sftp.client.SftpVersionSelector; import org.apache.sshd.sftp.client.extensions.CopyFileExtension; -import org.apache.sshd.sftp.client.impl.SftpPathImpl; import org.apache.sshd.sftp.client.impl.SftpRemotePathChannel; import org.apache.sshd.sftp.common.SftpConstants; import org.apache.sshd.sftp.common.SftpException; @@ -1117,7 +1116,7 @@ public SftpClient.Attributes readRemoteAttributes(SftpPath path, LinkOption... o // SftpPathImpl.withAttributeCache() invocation. So we ensure here that if we are already within a caching // scope, we do use the cached attributes, but if we are not, we clear any possibly cached attributes and // do actually read them from the remote. - return SftpPathImpl.withAttributeCache(path, p -> resolveRemoteFileAttributes(path, options)); + return WithFileAttributeCache.withAttributeCache(path, p -> resolveRemoteFileAttributes(path, options)); } protected SftpClient.Attributes resolveRemoteFileAttributes(SftpPath path, LinkOption... options) throws IOException { @@ -1136,8 +1135,8 @@ protected SftpClient.Attributes resolveRemoteFileAttributes(SftpPath path, LinkO if (log.isTraceEnabled()) { log.trace("resolveRemoteFileAttributes({})[{}]: {}", fs, path, attrs); } - if (path instanceof SftpPathImpl) { - ((SftpPathImpl) path).cacheAttributes(attrs); + if (path instanceof WithFileAttributeCache) { + ((WithFileAttributeCache) path).setAttributes(attrs); } return attrs; } catch (SftpException e) { diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPath.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPath.java index 807c0626a..683c45989 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPath.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPath.java @@ -30,18 +30,13 @@ /** * A {@link java.nio.file.Path} on an {@link SftpFileSystem}. */ -public class SftpPath extends BasePath { +public class SftpPath extends BasePath implements WithFileAttributes { public SftpPath(SftpFileSystem fileSystem, String root, List names) { super(fileSystem, root, names); } - /** - * Retrieves the cached {@link SftpClient.Attributes} of this {@link SftpPath}, if it has any. - * - * @return the cached {@link SftpClient.Attributes} or {@code null} if there are none cached - */ - @SuppressWarnings("javadoc") + @Override public SftpClient.Attributes getAttributes() { // Subclasses may override return null; diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPathIterator.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPathIterator.java index 0d701ac66..76c89fc6e 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPathIterator.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/SftpPathIterator.java @@ -30,7 +30,6 @@ import java.util.Objects; import org.apache.sshd.sftp.client.SftpClient; -import org.apache.sshd.sftp.client.impl.SftpPathImpl; /** * Implements and {@link Iterator} of {@link SftpPath}-s returned by a {@link DirectoryStream#iterator()} method. @@ -116,8 +115,8 @@ protected SftpPath nextEntry(SftpPath root, DirectoryStream.Filter dotdotIgnored = true; } else { SftpPath candidate = root.resolve(entry.getFilename()); - if (candidate instanceof SftpPathImpl) { - ((SftpPathImpl) candidate).setAttributes(entry.getAttributes()); + if (candidate instanceof WithFileAttributeCache) { + ((WithFileAttributeCache) candidate).setAttributes(entry.getAttributes()); } try { if ((selector == null) || selector.accept(candidate)) { diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributeCache.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributeCache.java new file mode 100644 index 000000000..2b89eaccf --- /dev/null +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributeCache.java @@ -0,0 +1,71 @@ +/* + * 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 + * + * http://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.sshd.sftp.client.fs; + +import java.io.IOException; +import java.nio.file.Path; + +import org.apache.sshd.common.util.io.functors.IOFunction; +import org.apache.sshd.sftp.client.SftpClient; + +/** + * A mix-in interface for paths that can carry and cache file attributes of the referenced file. + */ +public interface WithFileAttributeCache extends WithFileAttributes { + + /** + * Sets the attributes. + * + * @param attributes {@link SftpClient.Attributes} to set + */ + void setAttributes(SftpClient.Attributes attributes); + + /** + * Performs the given operation with attribute caching. If {@code SftpClient.Attributes} are fetched by the + * operation, they will be cached and subsequently these cached attributes will be re-used for this {@link SftpPath} + * instance throughout the operation. Calls to {@link #withAttributeCache(IOFunction)} may be nested. The cache is + * cleared at the start and at the end of the outermost invocation. + * + * @param result type of the {@code operation} + * @param operation to perform; may return {@code null} if it has no result + * @return the result of the {@code operation} + * @throws IOException if thrown by the {@code operation} + */ + T withAttributeCache(IOFunction operation) throws IOException; + + /** + * Performs the given operation with attribute caching, if the given {@link Path} implements the + * {@link WithFileAttributeCache} interface, otherwise simply executes the operation. + * + * @param result type of the {@code operation} + * @param path {@link Path} to operate on + * @param operation to perform; may return {@code null} if it has no result + * @return the result of the {@code operation} + * @throws IOException if thrown by the {@code operation} + * + * @see #withAttributeCache(IOFunction) + */ + static T withAttributeCache(Path path, IOFunction operation) throws IOException { + if (path instanceof WithFileAttributeCache) { + return ((WithFileAttributeCache) path).withAttributeCache(operation); + } + return operation.apply(path); + } + +} diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributes.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributes.java new file mode 100644 index 000000000..1c3a578d0 --- /dev/null +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/fs/WithFileAttributes.java @@ -0,0 +1,35 @@ +/* + * 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 + * + * http://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.sshd.sftp.client.fs; + +import org.apache.sshd.sftp.client.SftpClient.Attributes; + +/** + * A mix-in interface for paths that may have file attributes of the file referenced by the path. + */ +public interface WithFileAttributes { + + /** + * Retrieves the {@link Attributes} of this object, if it has any. + * + * @return the {@link Attributes} or {@code null} if there are none + */ + Attributes getAttributes(); + +} diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpPathImpl.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpPathImpl.java index fa27fe20a..0097e52f8 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpPathImpl.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/SftpPathImpl.java @@ -26,11 +26,12 @@ import org.apache.sshd.sftp.client.SftpClient; import org.apache.sshd.sftp.client.fs.SftpFileSystem; import org.apache.sshd.sftp.client.fs.SftpPath; +import org.apache.sshd.sftp.client.fs.WithFileAttributeCache; /** * An {@link SftpPath} that can cache {@code SftpClient.Attributes}. */ -public class SftpPathImpl extends SftpPath { +public class SftpPathImpl extends SftpPath implements WithFileAttributeCache { private SftpClient.Attributes attributes; @@ -77,23 +78,7 @@ protected void cacheAttributes(boolean doCache) { } } - /** - * Sets the cached attributes to the argument if this {@link SftpPath} instance is currently caching attributes. - * Otherwise a no-op. - * - * @param attributes the {@code SftpClient.Attributes} to cache - */ - public void cacheAttributes(SftpClient.Attributes attributes) { - if (cachingLevel > 0) { - setAttributes(attributes); - } - } - - /** - * Unconditionally set the cached attributes, whether or not this instance's attribute cache is enabled. - * - * @param attributes the {@code SftpClient.Attributes} to cache - */ + @Override public void setAttributes(SftpClient.Attributes attributes) { this.attributes = attributes; } @@ -103,17 +88,7 @@ public SftpClient.Attributes getAttributes() { return attributes; } - /** - * Performs the given operation with attribute caching. If {@code SftpClient.Attributes} are fetched by the - * operation, they will be cached and subsequently these cached attributes will be re-used for this {@link SftpPath} - * instance throughout the operation. Calls to {@link #withAttributeCache(IOFunction)} may be nested. The cache is - * cleared at the start and at the end of the outermost invocation. - * - * @param result type of the {@code operation} - * @param operation to perform; may return {@code null} if it has no result - * @return the result of the {@code operation} - * @throws IOException if thrown by the {@code operation} - */ + @Override public T withAttributeCache(IOFunction operation) throws IOException { cacheAttributes(true); try { @@ -123,22 +98,4 @@ public T withAttributeCache(IOFunction operation) throws IOExceptio } } - /** - * Performs the given operation with attribute caching, if the given {@link Path} can cache attributes, otherwise - * simply executes the operation. - * - * @param result type of the {@code operation} - * @param path {@link Path} to operate on - * @param operation to perform; may return {@code null} if it has no result - * @return the result of the {@code operation} - * @throws IOException if thrown by the {@code operation} - * - * @see #withAttributeCache(IOFunction) - */ - public static T withAttributeCache(Path path, IOFunction operation) throws IOException { - if (path instanceof SftpPathImpl) { - return ((SftpPathImpl) path).withAttributeCache(operation); - } - return operation.apply(path); - } } diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java index b25b039f4..f9bd266ec 100755 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java @@ -94,8 +94,8 @@ import org.apache.sshd.sftp.SftpModuleProperties; import org.apache.sshd.sftp.client.SftpClient; import org.apache.sshd.sftp.client.extensions.openssh.OpenSSHLimitsExtensionInfo; -import org.apache.sshd.sftp.client.fs.SftpPath; -import org.apache.sshd.sftp.client.impl.SftpPathImpl; +import org.apache.sshd.sftp.client.fs.WithFileAttributeCache; +import org.apache.sshd.sftp.client.fs.WithFileAttributes; import org.apache.sshd.sftp.common.SftpConstants; import org.apache.sshd.sftp.common.SftpException; import org.apache.sshd.sftp.common.SftpHelper; @@ -1654,7 +1654,7 @@ protected void doMakeDirectory( LinkOption[] options = accessor.resolveFileAccessLinkOptions( this, resolvedPath, SftpConstants.SSH_FXP_MKDIR, "", false); final boolean followLinks = resolvePathResolutionFollowLinks(SftpConstants.SSH_FXP_MKDIR, "", resolvedPath); - SftpPathImpl.withAttributeCache(resolvedPath, p -> { + WithFileAttributeCache.withAttributeCache(resolvedPath, p -> { Boolean symlinkCheck = validateParentExistWithNoSymlinksIfNeverFollowSymlinks(p, !followLinks); if (!Boolean.TRUE.equals(symlinkCheck)) { throw new AccessDeniedException(p.toString(), p.toString(), @@ -1715,7 +1715,7 @@ protected void doRemoveFile(int id, String path) throws IOException { // never resolve links in the final path to remove as we want to remove the symlink, not the target LinkOption[] options = accessor.resolveFileAccessLinkOptions( this, resolvedPath, SftpConstants.SSH_FXP_REMOVE, "", false); - SftpPathImpl.withAttributeCache(resolvedPath, p -> { + WithFileAttributeCache.withAttributeCache(resolvedPath, p -> { Boolean status = checkSymlinkState(p, followLinks, options); if (status == null) { throw signalRemovalPreConditionFailure(id, path, p, @@ -2253,8 +2253,8 @@ protected int doReadDir( } else { Path f = dir.next(); String shortName = getShortName(f); - if (f instanceof SftpPath) { - SftpClient.Attributes attributes = ((SftpPath) f).getAttributes(); + if (f instanceof WithFileAttributes) { + SftpClient.Attributes attributes = ((WithFileAttributes) f).getAttributes(); if (attributes != null) { entries.put(shortName, f); writeDirEntry(session, id, buffer, nb, f, shortName, attributes); @@ -2416,7 +2416,7 @@ protected String getShortName(Path f) throws IOException { protected NavigableMap resolveFileAttributes( Path path, int flags, boolean neverFollowSymLinks, LinkOption... options) throws IOException { - return SftpPathImpl.withAttributeCache(path, file -> { + return WithFileAttributeCache.withAttributeCache(path, file -> { Boolean status = checkSymlinkState(file, neverFollowSymLinks, options); if (status == null) { return handleUnknownStatusFileAttributes(file, flags, options); @@ -2492,7 +2492,7 @@ protected NavigableMap handleUnknownStatusFileAttributes( protected NavigableMap getAttributes(Path path, int flags, LinkOption... options) throws IOException { NavigableMap attrs - = SftpPathImpl.withAttributeCache(path, file -> resolveReportedFileAttributes(file, flags, options)); + = WithFileAttributeCache.withAttributeCache(path, file -> resolveReportedFileAttributes(file, flags, options)); SftpFileSystemAccessor accessor = getFileSystemAccessor(); return accessor.resolveReportedFileAttributes(this, path, flags, attrs, options); } diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java index d8c380061..956b5aba3 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/SftpSubsystem.java @@ -82,7 +82,7 @@ import org.apache.sshd.server.session.ServerSession; import org.apache.sshd.sftp.SftpModuleProperties; import org.apache.sshd.sftp.client.fs.SftpPath; -import org.apache.sshd.sftp.client.impl.SftpPathImpl; +import org.apache.sshd.sftp.client.fs.WithFileAttributeCache; import org.apache.sshd.sftp.common.SftpConstants; import org.apache.sshd.sftp.common.SftpException; import org.apache.sshd.sftp.common.SftpHelper; @@ -782,7 +782,7 @@ protected void doReadDir(Buffer buffer, int id) throws IOException { @Override protected String doOpenDir(int id, String path, Path dir, LinkOption... options) throws IOException { - SftpPathImpl.withAttributeCache(dir, p -> { + WithFileAttributeCache.withAttributeCache(dir, p -> { Boolean status = IoUtils.checkFileExistsAnySymlinks(p, !IoUtils.followLinks(options)); if (status == null) { throw signalOpenFailure(id, path, p, true,