Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Storage): Adding subpath strategy to the List operation #3775

Merged
merged 3 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ public extension StorageListRequest {
@available(*, deprecated, message: "Use `path` in Storage API instead of `Options`")
public let path: String?

/// The strategy to use when listing contents from subpaths. Defaults to [`.include`](x-source-tag://SubpathStrategy.include)
///
/// - Tag: StorageListRequestOptions.subpathStrategy
public let subpathStrategy: SubpathStrategy

/// Number between 1 and 1,000 that indicates the limit of how many entries to fetch when
/// retreiving file lists from the server.
///
Expand Down Expand Up @@ -94,15 +99,47 @@ public extension StorageListRequest {
public init(accessLevel: StorageAccessLevel = .guest,
targetIdentityId: String? = nil,
path: String? = nil,
subpathStrategy: SubpathStrategy = .include,
pageSize: UInt = 1000,
nextToken: String? = nil,
pluginOptions: Any? = nil) {
self.accessLevel = accessLevel
self.targetIdentityId = targetIdentityId
self.path = path
self.subpathStrategy = subpathStrategy
self.pageSize = pageSize
self.nextToken = nextToken
self.pluginOptions = pluginOptions
}
}
}

public extension StorageListRequest.Options {
/// Represents the strategy used when listing contents from subpaths relative to the given path.
///
/// - Tag: StorageListRequestOptions.SubpathStrategy
enum SubpathStrategy {
/// Items from nested subpaths are included in the results
///
/// - Tag: SubpathStrategy.include
case include

/// Items from nested subpaths are not included in the results. Their subpaths are instead grouped under [`StorageListResult.excludedSubpaths`](StorageListResult.excludedSubpaths).
///
/// - Parameter delimitedBy: The delimiter used to determine subpaths. Defaults to `"/"`
///
/// - SeeAlso: [`StorageListResult.excludedSubpaths`](x-source-tag://StorageListResult.excludedSubpaths)
///
/// - Tag: SubpathStrategy.excludeWithDelimiter
case exclude(delimitedBy: String = "/")

/// Items from nested subpaths are not included in the results. Their subpaths are instead grouped under [`StorageListResult.excludedSubpaths`](StorageListResult.excludedSubpaths).
///
/// - SeeAlso: [`StorageListResult.excludedSubpaths`](x-source-tag://StorageListResult.excludedSubpaths)
///
/// - Tag: SubpathStrategy.exclude
public static var exclude: SubpathStrategy {
return .exclude()
}
}
}
14 changes: 13 additions & 1 deletion Amplify/Categories/Storage/Result/StorageListResult.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@ public struct StorageListResult {
/// [StorageCategoryBehavior.list](x-source-tag://StorageCategoryBehavior.list).
///
/// - Tag: StorageListResult.init
public init(items: [Item], nextToken: String? = nil) {
public init(
items: [Item],
excludedSubpaths: [String] = [],
nextToken: String? = nil
) {
self.items = items
self.excludedSubpaths = excludedSubpaths
self.nextToken = nextToken
}

Expand All @@ -27,6 +32,13 @@ public struct StorageListResult {
/// - Tag: StorageListResult.items
public var items: [Item]


/// Array of excluded subpaths in the Result.
/// This field is only populated when [`StorageListRequest.Options.subpathStrategy`](x-source-tag://StorageListRequestOptions.subpathStragegy) is set to [`.exclude()`](x-source-tag://SubpathStrategy.exclude).
///
/// - Tag: StorageListResult.excludedSubpaths
public var excludedSubpaths: [String]

/// Opaque string indicating the page offset at which to resume a listing. This value is usually copied to
/// [StorageListRequestOptions.nextToken](x-source-tag://StorageListRequestOptions.nextToken).
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ extension AWSS3StorageService {
}
let input = ListObjectsV2Input(bucket: bucket,
continuationToken: options.nextToken,
delimiter: nil,
delimiter: options.subpathStrategy.delimiter,
maxKeys: Int(options.pageSize),
prefix: finalPrefix,
startAfter: nil)
Expand All @@ -41,7 +41,20 @@ extension AWSS3StorageService {
let items = try contents.map {
try StorageListResult.Item(s3Object: $0, prefix: prefix)
}
return StorageListResult(items: items, nextToken: response.nextContinuationToken)

let commonPrefixes = response.commonPrefixes ?? []
let excludedSubpaths: [String] = commonPrefixes.compactMap {
guard let commonPrefix = $0.prefix else {
return nil
}
return String(commonPrefix.dropFirst(prefix.count))
}

return StorageListResult(
items: items,
excludedSubpaths: excludedSubpaths,
nextToken: response.nextContinuationToken
)
} catch let error as StorageErrorConvertible {
throw error.storageError
} catch {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

import Amplify

extension StorageListRequest.Options.SubpathStrategy {
/// The delimiter for this strategy
var delimiter: String? {
switch self {
case .exclude(let delimiter):
return delimiter
case .include:
return nil
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class AWSS3StorageListObjectsTask: StorageListObjectsTask, DefaultLogger {
}
let input = ListObjectsV2Input(bucket: storageBehaviour.bucket,
continuationToken: request.options.nextToken,
delimiter: nil,
delimiter: request.options.subpathStrategy.delimiter,
maxKeys: Int(request.options.pageSize),
prefix: path,
startAfter: nil)
Expand All @@ -57,9 +57,15 @@ class AWSS3StorageListObjectsTask: StorageListObjectsTask, DefaultLogger {
return StorageListResult.Item(
path: path,
eTag: s3Object.eTag,
lastModified: s3Object.lastModified)
lastModified: s3Object.lastModified
)
}
return StorageListResult(items: items, nextToken: response.nextContinuationToken)
let commonPrefixes = response.commonPrefixes ?? []
return StorageListResult(
items: items,
excludedSubpaths: commonPrefixes.compactMap { $0.prefix },
nextToken: response.nextContinuationToken
)
} catch let error as StorageErrorConvertible {
throw error.storageError
} catch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,22 @@ class AWSS3StoragePluginAsyncBehaviorTests: XCTestCase {
XCTAssertEqual(1, storageService.interactions.count)
}

/// - Given: A plugin configured with a mocked service
/// - When: The list API is invoked with subpathStrategy set to .exclude
/// - Then: The list of excluded subpaths and the list of items should be populated
func testPluginListWithCommonPrefixesAsync() async throws {
storageService.listHandler = { (_, _) in
return .init(
items: [.init(path: "path")],
excludedSubpaths: ["subpath1", "subpath2"]
)
}
let output = try await storagePlugin.list(options: .init(subpathStrategy: .exclude))
XCTAssertEqual(1, output.items.count, String(describing: output))
XCTAssertEqual("path", output.items.first?.path)
XCTAssertEqual(2, output.excludedSubpaths.count)
XCTAssertEqual("subpath1", output.excludedSubpaths[0])
XCTAssertEqual("subpath2", output.excludedSubpaths[1])
XCTAssertEqual(1, storageService.interactions.count)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase {
storageBehaviour: serviceMock)
let value = try await task.value
XCTAssertEqual(value.items.count, 2)
XCTAssertTrue(value.excludedSubpaths.isEmpty)
XCTAssertEqual(value.nextToken, "continuationToken")
XCTAssertEqual(value.items[0].eTag, "tag")
XCTAssertEqual(value.items[0].key, "key")
Expand Down Expand Up @@ -130,4 +131,81 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase {
XCTAssertEqual(field, "path", "Field in error should be `path`")
}
}

/// - Given: A configured Storage List Objects Task with mocked service
/// - When: AWSS3StorageListObjectsTask value is invoked with subpathStrategy set to .exclude
/// - Then: The delimiter should be set, the list of excluded subpaths and the list of items should be populated
func testListObjectsTask_withSubpathStrategyExclude_shouldSucceed() async throws {
let serviceMock = MockAWSS3StorageService()
let client = serviceMock.client as! MockS3Client
client.listObjectsV2Handler = { input in
XCTAssertNotNil(input.delimiter, "Expected delimiter to be set")
return .init(
commonPrefixes: [
.init(prefix: "path/subpath1/"),
.init(prefix: "path/subpath2/")
],
contents: [
.init(eTag: "tag", key: "path/result", lastModified: Date())
],
nextContinuationToken: "continuationToken"
)
}

let request = StorageListRequest(
path: StringStoragePath.fromString("path/"),
options: .init(
subpathStrategy: .exclude
)
)
let task = AWSS3StorageListObjectsTask(
request,
storageConfiguration: AWSS3StoragePluginConfiguration(),
storageBehaviour: serviceMock
)
let value = try await task.value
XCTAssertEqual(value.items.count, 1)
XCTAssertEqual(value.items[0].eTag, "tag")
XCTAssertEqual(value.items[0].path, "path/result")
XCTAssertNotNil(value.items[0].lastModified)
XCTAssertEqual(value.excludedSubpaths.count, 2)
XCTAssertEqual(value.excludedSubpaths[0], "path/subpath1/")
XCTAssertEqual(value.excludedSubpaths[1], "path/subpath2/")
XCTAssertEqual(value.nextToken, "continuationToken")
}

/// - Given: A configured Storage List Objects Task with mocked service
/// - When: AWSS3StorageListObjectsTask value is invoked with subpathStrategy set to .include
/// - Then: The delimiter should not be set, the list of excluded subpaths should be empty and the list of items should be populated
func testListObjectsTask_withSubpathStrategyInclude_shouldSucceed() async throws {
let serviceMock = MockAWSS3StorageService()
let client = serviceMock.client as! MockS3Client
client.listObjectsV2Handler = { input in
XCTAssertNil(input.delimiter, "Expected delimiter to be nil")
return .init(
contents: [
.init(eTag: "tag", key: "path", lastModified: Date()),
],
nextContinuationToken: "continuationToken"
)
}

let request = StorageListRequest(
path: StringStoragePath.fromString("path"),
options: .init(
subpathStrategy: .include
)
)
let task = AWSS3StorageListObjectsTask(
request,
storageConfiguration: AWSS3StoragePluginConfiguration(),
storageBehaviour: serviceMock)
let value = try await task.value
XCTAssertEqual(value.items.count, 1)
XCTAssertEqual(value.items[0].eTag, "tag")
XCTAssertEqual(value.items[0].path, "path")
XCTAssertNotNil(value.items[0].lastModified)
XCTAssertTrue(value.excludedSubpaths.isEmpty)
XCTAssertEqual(value.nextToken, "continuationToken")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,67 @@ class AWSS3StoragePluginListObjectsIntegrationTests: AWSS3StoragePluginTestBase
}
}

/// Given: Multiple objects uploaded to a public path
/// When: `Amplify.Storage.list` is invoked with `subpathStrategy: .exclude`
/// Then: The API should execute successfully and list objects for the given path, without including contens from its subpaths
func testList_withSubpathStrategyExclude_shouldExcludeSubpaths() async throws {
let path = UUID().uuidString
let data = Data(path.utf8)
let uniqueStringPath = "public/\(path)"

// Upload data
_ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "/test1"), data: data, options: nil).value
_ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "/test2"), data: data, options: nil).value
_ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "/subpath1/test"), data: data, options: nil).value
_ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "/subpath2/test"), data: data, options: nil).value

let result = try await Amplify.Storage.list(
path: .fromString("\(uniqueStringPath)/"),
options: .init(
subpathStrategy: .exclude
)
)

// Validate result
XCTAssertEqual(result.items.count, 2)
XCTAssertTrue(result.items.contains(where: { $0.path.hasPrefix("\(uniqueStringPath)/test") }), "Unexpected item")
XCTAssertEqual(result.excludedSubpaths.count, 2)
XCTAssertTrue(result.excludedSubpaths.contains(where: { $0.hasPrefix("\(uniqueStringPath)/subpath") }), "Unexpected excluded subpath")

// Clean up
_ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "/test1"))
_ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "/test2"))
_ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "/subpath1/test"))
_ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "/subpath2/test"))
}

/// Given: Multiple objects uploaded to a public path
/// When: `Amplify.Storage.list` is invoked with `subpathStrategy: .exclude(delimitedBy:)`
/// Then: The API should execute successfully and list objects for the given path, without including contents from any subpath that is determined by the given delimiter
func testList_withSubpathStrategyExclude_andCustomDelimiter_shouldExcludeSubpaths() async throws {
let path = UUID().uuidString
let data = Data(path.utf8)
let uniqueStringPath = "public/\(path)"

// Upload data
_ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "-test"), data: data, options: nil).value
_ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "-subpath-test"), data: data, options: nil).value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when the subpath is --subpath-test ?

Copy link
Member Author

@sebaland sebaland Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, the excluded subpath will be public/[UUID]--


From the S3 perspective, here's how it works. You have these objects:

public/[UUID]-test
public/[UUID]--subpath-test

You filter everything that starts with the prefix public/[UUID]-, and both objects still match:

public/[UUID]-test
public/[UUID]--subpath-test 

But since you set the delimiter to -, any object that contains it after the given prefix is gets rolled up until that character. So public/[UUID]--subpath-test is rolled up to the public/[UUID]-- common prefix, because the - character is the very next character after the prefix.
This value is then reported as an excludedSubpath by Amplify.


let result = try await Amplify.Storage.list(
path: .fromString("\(uniqueStringPath)-"),
options: .init(
subpathStrategy: .exclude(delimitedBy: "-")
)
)

// Validate result
XCTAssertEqual(result.items.count, 1)
XCTAssertEqual(result.items.first?.path, "\(uniqueStringPath)-test")
XCTAssertEqual(result.excludedSubpaths.count, 1)
XCTAssertEqual(result.excludedSubpaths.first, "\(uniqueStringPath)-subpath-")

// Clean up
_ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "-test"))
_ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "-subpath-test"))
}
}
Loading