Skip to content

Commit ee443af

Browse files
committed
Make the AttributeCachingPath on demand and remove all init logic
For copyCachedAttributesTo() only copy attributes if they were previously cached or we force copy attributes via forceCopyAttributes = true Add additional tests to cover copyCachedAttributesTo() functionality changes A large portion of this commit (15b3086, cd5ddb5, 5513234) was adapted from work by @mbayerPK
1 parent 8e1d377 commit ee443af

8 files changed

Lines changed: 673 additions & 220 deletions

File tree

detekt.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,7 @@ style:
144144
- 'org.junit.BeforeClass'
145145
- 'org.junit.After'
146146
- 'org.junit.AfterClass'
147-
- 'org.junit.jupiter.api.Assertions'
148-
forbiddenPatterns: '(?:junit\.framework|org\.junit\.jupiter\.api\.Assertions)\.(?!assertThrows).+'
147+
forbiddenPatterns: '(?:junit\.framework|org\.junit\.jupiter\.api\.Assertions)\.assertThrows.+'
149148
ForbiddenComment:
150149
comments:
151150
- reason: 'Forbidden FIXME todo marker in comment, please fix the problem.'

file-attribute-caching/src/main/kotlin/com/pkware/filesystem/attributecaching/AttributeCachingFileSystem.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public class AttributeCachingFileSystem(
2828

2929
override fun getPath(first: String, vararg more: String?): Path {
3030
val delegate = super.getPath(first, *more)
31-
return AttributeCachingPath(this, delegate, true)
31+
return AttributeCachingPath(this, delegate)
3232
}
3333

3434
/**
@@ -39,7 +39,7 @@ public class AttributeCachingFileSystem(
3939
* @return A [Path] that now has [AttributeCachingPath] properties, or the original [path] object.
4040
*/
4141
public fun convertToCachingPath(path: Path): Path = if (path !is AttributeCachingPath) {
42-
AttributeCachingPath(this, path, true)
42+
AttributeCachingPath(this, path)
4343
} else {
4444
path
4545
}

file-attribute-caching/src/main/kotlin/com/pkware/filesystem/attributecaching/AttributeCachingFileSystemProvider.kt

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ internal class AttributeCachingFileSystemProvider : FileSystemProvider() {
119119
it != StandardCopyOption.COPY_ATTRIBUTES
120120
}.toTypedArray()
121121

122-
source.copyCachedAttributesTo(target)
122+
source.copyCachedAttributesTo(target, true)
123123
Files.copy(actualSourcePath, actualTargetPath, *newOptions)
124124
} else {
125125
// If the StandardCopyOption.COPY_ATTRIBUTES option is not selected, there is no need to cache the
@@ -159,7 +159,7 @@ internal class AttributeCachingFileSystemProvider : FileSystemProvider() {
159159
it != StandardCopyOption.COPY_ATTRIBUTES
160160
}.toTypedArray()
161161

162-
source.copyCachedAttributesTo(target)
162+
source.copyCachedAttributesTo(target, true)
163163
Files.move(actualSourcePath, actualTargetPath, *newOptions)
164164
} else {
165165
// If the StandardCopyOption.COPY_ATTRIBUTES option is not selected, there is no need to cache the
@@ -344,7 +344,7 @@ internal class AttributeCachingFileSystemProvider : FileSystemProvider() {
344344
* @param path The [Path] to set the given [attribute] on. It must be a [AttributeCachingPath] otherwise an
345345
* [IOException] will be thrown.
346346
* @param attribute The attribute name to set and associate with the [path].
347-
* @param value The value of the [attribute] to set.
347+
* @param value The value of the [attribute] to set, can be `null`.
348348
* @param options The [LinkOption]s indicating how symbolic links are handled.
349349
* @throws IOException If an IO error occurs.
350350
* @throws UnsupportedOperationException If the attribute view for the given [attribute] name is not available.
@@ -362,9 +362,6 @@ internal class AttributeCachingFileSystemProvider : FileSystemProvider() {
362362
return
363363
}
364364

365-
val delegatePath = path.delegate
366-
val delegateProvider = delegatePath.fileSystem.provider()
367-
368365
// Then set our cache
369366
// Need to make sure that we only supply class names to path.setAttributeByName
370367
// cannot set single attribute in the cache
@@ -377,11 +374,11 @@ internal class AttributeCachingFileSystemProvider : FileSystemProvider() {
377374

378375
// Even if we have a single attribute only we should get the entire attribute view class for that single
379376
// attribute to properly set the cache.
380-
val fileAttributeView: FileAttributeView? = delegateProvider.getFileAttributeView(
381-
delegatePath,
377+
val fileAttributeView: FileAttributeView? = provider.getFileAttributeView(
378+
path.delegate,
382379
getAttributeViewJavaClassType(attributeCacheKey),
383380
)
384-
path.setAttributeByName(attributeCacheKey, fileAttributeView)
381+
path.setAttributeByNameUsingView(attributeCacheKey, fileAttributeView)
385382
}
386383

387384
/**

file-attribute-caching/src/main/kotlin/com/pkware/filesystem/attributecaching/AttributeCachingPath.kt

Lines changed: 70 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import java.nio.file.FileSystem
66
import java.nio.file.LinkOption
77
import java.nio.file.NoSuchFileException
88
import java.nio.file.Path
9+
import java.nio.file.attribute.AclEntry
910
import java.nio.file.attribute.AclFileAttributeView
1011
import java.nio.file.attribute.BasicFileAttributeView
1112
import java.nio.file.attribute.BasicFileAttributes
@@ -14,6 +15,7 @@ import java.nio.file.attribute.DosFileAttributes
1415
import java.nio.file.attribute.FileAttributeView
1516
import java.nio.file.attribute.PosixFileAttributeView
1617
import java.nio.file.attribute.PosixFileAttributes
18+
import java.nio.file.attribute.UserPrincipal
1719
import java.nio.file.spi.FileSystemProvider
1820

1921
/**
@@ -37,88 +39,71 @@ public const val CACHE_KEY_POSIX: String = "posix:*"
3739
public const val CACHE_KEY_ACL: String = "acl:*"
3840

3941
/**
40-
* A [Path] instance that supports caching of [BasicFileAttributes] and other classes that extend it such as
41-
* [DosFileAttributes] and [PosixFileAttributes].
42+
* A [Path] instance that supports caching of [BasicFileAttributes] along with other classes such as
43+
* [DosFileAttributes], [PosixFileAttributes], ACL (access control list) [UserPrincipal] owner, and ACL
44+
* (access control list) [AclEntry].
4245
*
4346
* @param fileSystem the [FileSystem] associated with this [AttributeCachingPath] instance.
4447
* @param delegate the [Path] to forward calls to if needed.
45-
* @param initializeCache `true` to initialize the cached attribute fields of this [AttributeCachingPath] instance.
46-
* The default is `false`.
4748
*/
4849
internal class AttributeCachingPath(
4950
private val fileSystem: FileSystem,
5051
internal val delegate: Path,
51-
private val initializeCache: Boolean = false,
5252
) : ForwardingPath(delegate) {
5353

54-
/**
55-
* Status variable to tell outside callers if this path has been initialized or not.
56-
*/
57-
private var isInitialized = false
58-
5954
private val delegateSupportedFileAttributeViews = delegate.fileSystem.supportedFileAttributeViews()
6055

61-
private var basicAttributes by LazyAttribute {
56+
/**
57+
* On demand [LazyAttribute] field for [BasicFileAttributes].
58+
*/
59+
val cachedBasicAttributes = LazyAttribute {
6260
delegate.fileSystem.provider().readAttributes(delegate, BasicFileAttributes::class.java)
6361
}
6462

65-
private var dosAttributes by LazyAttribute {
63+
/**
64+
* On demand [LazyAttribute] field for [DosFileAttributes].
65+
*/
66+
val cachedDosAttributes = LazyAttribute {
6667
if (delegateSupportedFileAttributeViews.contains("dos")) {
6768
delegate.fileSystem.provider().readAttributes(delegate, DosFileAttributes::class.java)
6869
} else {
6970
null
7071
}
7172
}
7273

73-
private var posixAttributes by LazyAttribute {
74+
/**
75+
* On demand [LazyAttribute] field for [PosixFileAttributes].
76+
*/
77+
val cachedPosixAttributes = LazyAttribute {
7478
if (delegateSupportedFileAttributeViews.contains("posix")) {
7579
delegate.fileSystem.provider().readAttributes(delegate, PosixFileAttributes::class.java)
7680
} else {
7781
null
7882
}
7983
}
8084

81-
private var accessControlListOwner by LazyAttribute {
85+
/**
86+
* On demand [LazyAttribute] field for the ACL (access control list) [UserPrincipal] owner.
87+
*/
88+
val cachedAccessControlListOwner = LazyAttribute {
8289
if (delegateSupportedFileAttributeViews.contains("acl")) {
8390
delegate.fileSystem.provider().getFileAttributeView(delegate, AclFileAttributeView::class.java).owner
8491
} else {
8592
null
8693
}
8794
}
8895

89-
private var accessControlListEntries by LazyAttribute {
96+
/**
97+
* On demand [LazyAttribute] field for the ACL (access control list) [AclEntry]s.
98+
*/
99+
val cachedAccessControlListEntries = LazyAttribute {
90100
if (delegateSupportedFileAttributeViews.contains("acl")) {
91101
delegate.fileSystem.provider().getFileAttributeView(delegate, AclFileAttributeView::class.java).acl
92102
} else {
93103
null
94104
}
95105
}
96106

97-
init {
98-
if (initializeCache && !isInitialized) {
99-
try {
100-
// Force all attributes to be initialized
101-
basicAttributes
102-
dosAttributes
103-
posixAttributes
104-
accessControlListOwner
105-
accessControlListEntries
106-
isInitialized = true
107-
} catch (expected: NoSuchFileException) {
108-
// Swallow NoSuchFileExceptions and skip cache checks on files that do not exist or are not regular
109-
// files.
110-
// Checking these attributes directly does incur OTHER_IOPS penalties which we want to avoid.
111-
}
112-
}
113-
}
114-
115-
/**
116-
* Gets the status of cache initialization for this [AttributeCachingPath].
117-
*
118-
* @return `true` if the cache fields have been initialized, `false` otherwise.
119-
*/
120-
fun isCachedInitialized(): Boolean = isInitialized
121-
122107
override fun getFileSystem(): FileSystem = fileSystem
123108

124109
override fun getRoot(): Path? = if (delegate.root != null && delegate.root !is AttributeCachingPath) {
@@ -158,7 +143,7 @@ internal class AttributeCachingPath(
158143

159144
override fun getName(index: Int): Path {
160145
val nameDelegate = delegate.getName(index)
161-
// Dont need to copy cache here because root cannot be returned, only element closest to root is returned.
146+
// Don't need to copy cache here because root cannot be returned, only element closest to root is returned.
162147
return AttributeCachingPath(fileSystem, nameDelegate)
163148
}
164149

@@ -229,50 +214,70 @@ internal class AttributeCachingPath(
229214
* @param name The name of the attribute to cache.
230215
* @param value The [FileAttributeView] to cache from.
231216
*/
232-
fun <A : FileAttributeView> setAttributeByName(name: String, value: A?) {
217+
fun <A : FileAttributeView> setAttributeByNameUsingView(name: String, value: A?) {
233218
// This check is to ensure that we are only storing attribute classes and not specific attributes.
234219
// Remove basic: from our attribute name if present as basicFileAttributes can be accessed without that
235220
// qualifier
236221
when (name.substringAfter("basic:")) {
237222
CACHE_KEY_BASIC -> {
238223
val basicFileAttributeView = value as? BasicFileAttributeView
239-
basicAttributes = basicFileAttributeView?.readAttributes()
224+
cachedBasicAttributes.value = basicFileAttributeView?.readAttributes()
240225
}
241226
CACHE_KEY_DOS -> {
242227
val dosFileAttributeView = value as? DosFileAttributeView
243-
dosAttributes = dosFileAttributeView?.readAttributes()
228+
cachedDosAttributes.value = dosFileAttributeView?.readAttributes()
244229
}
245230
CACHE_KEY_POSIX -> {
246231
val posixFileAttributeView = value as? PosixFileAttributeView
247-
posixAttributes = posixFileAttributeView?.readAttributes()
232+
cachedPosixAttributes.value = posixFileAttributeView?.readAttributes()
248233
}
249234
CACHE_KEY_ACL -> {
250235
val aclFileAttributeView = value as? AclFileAttributeView
251-
accessControlListOwner = aclFileAttributeView?.owner
252-
accessControlListEntries = aclFileAttributeView?.acl
236+
cachedAccessControlListOwner.value = aclFileAttributeView?.owner
237+
cachedAccessControlListEntries.value = aclFileAttributeView?.acl
253238
}
254239
}
255240
}
256241

257242
/**
258243
* Copies this [AttributeCachingPath]s values to the [target].
259244
*
245+
* This function only copies attribute views supported by __both__ filesystems.
246+
*
260247
* @param target The [AttributeCachingPath] to copy cached attributes to.
248+
* @param forceCopyAndInitTarget 'true' to indicate attributes should be copied from this
249+
* [AttributeCachingPath] regardless if they are `null` or not. Accessing a `null` attribute will force a lookup
250+
* and incur an OTHER_IOPS penalty. Default is `false` - only copy attributes if they have been previously cached.
261251
* @throws IOException If something goes wrong with the underlying calls with obtaining this
262252
* [AttributeCachingPath]'s attributes to copy.
263253
* @throws UnsupportedOperationException If something goes wrong with the underlying calls with obtaining this
264254
* [AttributeCachingPath]'s attributes to copy.
265255
*/
266256
@Throws(IOException::class, UnsupportedOperationException::class)
267-
fun copyCachedAttributesTo(target: AttributeCachingPath) {
257+
fun copyCachedAttributesTo(target: AttributeCachingPath, forceCopyAndInitTarget: Boolean = false) {
268258
try {
269-
// Can set null values here but that's okay.
270-
target.basicAttributes = basicAttributes
271-
target.dosAttributes = dosAttributes
272-
target.posixAttributes = posixAttributes
273-
target.accessControlListOwner = accessControlListOwner
274-
target.accessControlListEntries = accessControlListEntries
275-
target.isInitialized = isInitialized
259+
val commonAttributeViews = delegate.fileSystem.supportedFileAttributeViews().intersect(
260+
target.delegate.fileSystem.supportedFileAttributeViews(),
261+
)
262+
263+
// Copy only the attribute views shared by both filesystems.
264+
for (view in commonAttributeViews) {
265+
when (view) {
266+
"basic" -> target.cachedBasicAttributes.copyValue(cachedBasicAttributes, forceCopyAndInitTarget)
267+
"dos" -> target.cachedDosAttributes.copyValue(cachedDosAttributes, forceCopyAndInitTarget)
268+
"posix" -> target.cachedPosixAttributes.copyValue(cachedPosixAttributes, forceCopyAndInitTarget)
269+
"acl" -> {
270+
target.cachedAccessControlListOwner.copyValue(
271+
cachedAccessControlListOwner,
272+
forceCopyAndInitTarget,
273+
)
274+
target.cachedAccessControlListEntries.copyValue(
275+
cachedAccessControlListEntries,
276+
forceCopyAndInitTarget,
277+
)
278+
}
279+
}
280+
}
276281
} catch (expected: NoSuchFileException) {
277282
// Swallow NoSuchFileExceptions and skip cache checks on files that do not exist or are not regular files.
278283
// Checking these attributes directly does incur OTHER_IOPS penalties which we want to avoid.
@@ -293,9 +298,9 @@ internal class AttributeCachingPath(
293298
@Suppress("UNCHECKED_CAST")
294299
@Throws(IOException::class, UnsupportedOperationException::class)
295300
fun <A : BasicFileAttributes?> getAllAttributesMatchingClass(type: Class<A>): A? = when (type) {
296-
BasicFileAttributes::class.java -> basicAttributes as A
297-
DosFileAttributes::class.java -> dosAttributes as A
298-
PosixFileAttributes::class.java -> posixAttributes as A
301+
BasicFileAttributes::class.java -> cachedBasicAttributes.value as A
302+
DosFileAttributes::class.java -> cachedDosAttributes.value as A
303+
PosixFileAttributes::class.java -> cachedPosixAttributes.value as A
299304
else -> null
300305
}
301306

@@ -322,21 +327,23 @@ internal class AttributeCachingPath(
322327
// get our acl attributes and translate them to MutableMap<String, Any>?
323328
// Acl file attributes do not extend BasicFileAttributes and are their own separate entity.
324329
try {
325-
attributeMap["owner"] = requireNotNull(accessControlListOwner)
326-
attributeMap["acl"] = requireNotNull(accessControlListEntries)
330+
attributeMap["owner"] = requireNotNull(cachedAccessControlListOwner.value)
331+
attributeMap["acl"] = requireNotNull(cachedAccessControlListEntries.value)
327332
} catch (expected: IllegalArgumentException) {
328333
return null
329334
}
335+
// We do not catch NoSuchFileException here and throw it up the chain instead
330336
} else {
331337
// get our attribute class from the cache, should be BasicFileAttributes, DosFileAttributes, or
332338
// PosixFileAttributes.
333339
val attributeClass = if (checkedNames.startsWith("dos")) {
334-
dosAttributes
340+
cachedDosAttributes.value
335341
} else if (checkedNames.startsWith("posix")) {
336-
posixAttributes
342+
cachedPosixAttributes.value
337343
} else {
338-
basicAttributes
344+
cachedBasicAttributes.value
339345
}
346+
// We do not catch NoSuchFileException here and throw it up the chain instead
340347

341348
if (attributeClass == null) return null
342349

0 commit comments

Comments
 (0)