Skip to content
Merged
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-S3TransferManager-ad9d4c6.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "S3 Transfer Manager",
"contributor": "jencymaryjoseph",
"description": "DownloadFilter type incompatability methods overriden from extended interface"
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package software.amazon.awssdk.transfer.s3.config;

import java.util.Objects;
import java.util.function.Predicate;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.services.s3.model.S3Object;
Expand All @@ -39,8 +40,47 @@ public interface DownloadFilter extends Predicate<S3Object> {
boolean test(S3Object s3Object);

/**
* A {@link DownloadFilter} that downloads all non-folder objects. A folder is a 0-byte object created when a customer
* uses S3 console to create a folder, and it always ends with "/".
* Returns a composed filter that represents the logical AND of this filter and another.
* The composed filter returns true only if both this filter and the other filter return true.
* @param other a predicate that will be logically-ANDed with this
* predicate
* @return a composed filter that represents the logical AND of this filter and the other filter
* @throws NullPointerException if other is null
*/
@Override
default DownloadFilter and(Predicate<? super S3Object> other) {
Objects.requireNonNull(other, "Other predicate cannot be null");
return s3Object -> test(s3Object) && other.test(s3Object);
}

/**
* Returns a composed filter that represents the logical OR of this filter and another.
* The composed filter returns true if either this filter or the other filter returns true.
* @param other a predicate that will be logically-ORed with this
* predicate
* @return a composed filter that represents the logical OR of this filter and the other filter
* @throws NullPointerException if other is null
*/
@Override
default DownloadFilter or(Predicate<? super S3Object> other) {
Objects.requireNonNull(other, "Other predicate cannot be null");
return s3Object -> test(s3Object) || other.test(s3Object);
Copy link
Contributor

@joviegas joviegas Jun 2, 2025

Choose a reason for hiding this comment

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

We can add a null check on other and test case when user passes null in args

Copy link
Contributor Author

@jencymaryjoseph jencymaryjoseph Jun 2, 2025

Choose a reason for hiding this comment

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

Thanks @joviegas. Added null checks using Objects.requireNonNull() for both and and or methods.
Also added Unit test for NullPointerException.

}

/**
* Returns a filter that represents the logical negation of this predicate.
* The returned filter returns true when this filter returns false, and vice versa.
* @return a filter that represents the logical negation of this filter
* predicate
*/
@Override
default DownloadFilter negate() {
return s3Object -> !test(s3Object);
}

/**
* A {@link DownloadFilter} that downloads all non-folder objects. A folder is a 0-byte object created when a customer uses S3
* console to create a folder, and it always ends with "/".
*
* <p>
* This is the default behavior if no filter is provided.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@
package software.amazon.awssdk.transfer.s3.config;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.function.Function;
import java.util.stream.Stream;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -38,4 +41,168 @@ public static Stream<Arguments> s3Objects() {
void allObjectsFilter_shouldWork(S3Object s3Object, boolean result) {
assertThat(DownloadFilter.allObjects().test(s3Object)).isEqualTo(result);
}

private static Stream<Arguments> filterOperationTestCases() {
Function<S3Object, DownloadFilter> folder1OrFolder3Filter = s3Object -> {
DownloadFilter folder1 = obj -> obj.key().startsWith("folder1");
DownloadFilter folder3 = obj -> obj.key().startsWith("folder3");
return folder1.or(folder3);
};

Function<S3Object, DownloadFilter> txtAndLargeSizeFilter = s3Object -> {
DownloadFilter txtFilter = obj -> obj.key().endsWith(".txt");
DownloadFilter sizeFilter = obj -> obj.size() > 1000L;
return txtFilter.and(sizeFilter);
};

Function<S3Object, DownloadFilter> notFolder1Filter = s3Object -> {
DownloadFilter folder1 = obj -> obj.key().startsWith("folder1");
return folder1.negate();
};

Function<S3Object, DownloadFilter> notLargeSizeFilter = s3Object -> {
DownloadFilter largeSize = obj -> obj.size() > 1000L;
return largeSize.negate();
};

Function<S3Object, DownloadFilter> complexFilter = s3Object -> {
DownloadFilter folder1 = obj -> obj.key().startsWith("folder1");
DownloadFilter folder3 = obj -> obj.key().startsWith("folder3");
DownloadFilter sizeFilter = obj -> obj.size() > 1000L;
return folder1.or(folder3).and(sizeFilter);
};
Function<S3Object, DownloadFilter> nullParameterFilter = s3Object -> {
DownloadFilter baseFilter = obj -> obj.key().startsWith("folder1");
return s -> {
assertThrows(NullPointerException.class,
() -> baseFilter.or(null),
"or() should throw NullPointerException when other is null");
assertThrows(NullPointerException.class,
() -> baseFilter.and(null),
"and() should throw NullPointerException when other is null");
return true; // Return value doesn't matter as we're testing for exceptions
};
};


return Stream.of(
// OR operation tests
Arguments.of(
"OR: folder1/test.txt matches (folder1 OR folder3)",
S3Object.builder().key("folder1/test.txt").size(2000L).build(),
folder1OrFolder3Filter,
true
),
Arguments.of(
"OR: folder3/test.txt matches (folder1 OR folder3)",
S3Object.builder().key("folder3/test.txt").size(2000L).build(),
folder1OrFolder3Filter,
true
),
Arguments.of(
"OR: folder2/test.txt does not match (folder1 OR folder3)",
S3Object.builder().key("folder2/test.txt").size(2000L).build(),
folder1OrFolder3Filter,
false
),

// AND operation tests
Arguments.of(
"AND: large .txt file matches (.txt AND size > 1000)",
S3Object.builder().key("folder1/test.txt").size(2000L).build(),
txtAndLargeSizeFilter,
true
),
Arguments.of(
"AND: small .txt file does not match (.txt AND size > 1000)",
S3Object.builder().key("folder1/test.txt").size(500L).build(),
txtAndLargeSizeFilter,
false
),
Arguments.of(
"AND: large .pdf file does not match (.txt AND size > 1000)",
S3Object.builder().key("folder1/test.pdf").size(2000L).build(),
txtAndLargeSizeFilter,
false
),

// NEGATE operation tests
Arguments.of(
"NEGATE: folder1 file does not match NOT(folder1)",
S3Object.builder().key("folder1/test.txt").size(1000L).build(),
notFolder1Filter,
false
),
Arguments.of(
"NEGATE: folder2 file matches NOT(folder1)",
S3Object.builder().key("folder2/test.txt").size(1000L).build(),
notFolder1Filter,
true
),
Arguments.of(
"NEGATE: large file does not match NOT(size > 1000)",
S3Object.builder().key("test.txt").size(2000L).build(),
notLargeSizeFilter,
false
),
Arguments.of(
"NEGATE: small file matches NOT(size > 1000)",
S3Object.builder().key("test.txt").size(500L).build(),
notLargeSizeFilter,
true
),

// Complex chained operations
Arguments.of(
"COMPLEX: large file in folder1 matches ((folder1 OR folder3) AND size > 1000)",
S3Object.builder().key("folder1/test.txt").size(2000L).build(),
complexFilter,
true
),
Arguments.of(
"COMPLEX: small file in folder1 does not match ((folder1 OR folder3) AND size > 1000)",
S3Object.builder().key("folder1/test.txt").size(500L).build(),
complexFilter,
false
),
Arguments.of(
"COMPLEX: large file in folder2 does not match ((folder1 OR folder3) AND size > 1000)",
S3Object.builder().key("folder2/test.txt").size(2000L).build(),
complexFilter,
false
),
Arguments.of(
"COMPLEX: large file in folder3 matches ((folder1 OR folder3) AND size > 1000)",
S3Object.builder().key("folder3/test.txt").size(2000L).build(),
complexFilter,
true
),
// NullPointerException
Arguments.of(
"NULL: or/and with null parameter should throw NullPointerException",
S3Object.builder().key("folder1/test.txt").size(1000L).build(),
nullParameterFilter,
true
)

);
}

@ParameterizedTest
@MethodSource("filterOperationTestCases")
@DisplayName("Test DownloadFilter operations (AND, OR, NEGATE)")
void testFilterOperations(String scenario, S3Object s3Object,
Function<S3Object, DownloadFilter> filterFactory,
boolean expectedResult) {
// Given
DownloadFilter filter = filterFactory.apply(s3Object);

// When
boolean actualResult = filter.test(s3Object);

// Then
assertThat(actualResult)
.as(scenario)
.isEqualTo(expectedResult);
}
}