Skip to content

Commit 26692ee

Browse files
jencymaryjosephjencymaryjoseph
authored
Fix DownloadFilter interface to properly return DownloadFilter type (#6148)
* Fix DownloadFilter interface to properly return DownloadFilter type - Override Predicate default methods to return DownloadFilter - Add comprehensive parameterized tests * Add null checks and improve documentation for DownloadFilter * Add null pointer exception test for DownloadFilter * Improve DownloadFilter documentation and fix indentation * Sonarcube issue fix - Removed the paranthesis around s3object parameter * Consistency in documentation description * additional changes
1 parent aec2ad4 commit 26692ee

File tree

3 files changed

+215
-2
lines changed

3 files changed

+215
-2
lines changed

‎.changes/next-release/bugfix-S3TransferManager-ad9d4c6.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "S3 Transfer Manager",
4+
"contributor": "jencymaryjoseph",
5+
"description": "DownloadFilter type incompatability methods overriden from extended interface"
6+
}

‎services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/config/DownloadFilter.java‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

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

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

4142
/**
42-
* A {@link DownloadFilter} that downloads all non-folder objects. A folder is a 0-byte object created when a customer
43-
* uses S3 console to create a folder, and it always ends with "/".
43+
* Returns a composed filter that represents the logical AND of this filter and another.
44+
* The composed filter returns true only if both this filter and the other filter return true.
45+
* @param other a predicate that will be logically-ANDed with this
46+
* predicate
47+
* @return a composed filter that represents the logical AND of this filter and the other filter
48+
* @throws NullPointerException if other is null
49+
*/
50+
@Override
51+
default DownloadFilter and(Predicate<? super S3Object> other) {
52+
Objects.requireNonNull(other, "Other predicate cannot be null");
53+
return s3Object -> test(s3Object) && other.test(s3Object);
54+
}
55+
56+
/**
57+
* Returns a composed filter that represents the logical OR of this filter and another.
58+
* The composed filter returns true if either this filter or the other filter returns true.
59+
* @param other a predicate that will be logically-ORed with this
60+
* predicate
61+
* @return a composed filter that represents the logical OR of this filter and the other filter
62+
* @throws NullPointerException if other is null
63+
*/
64+
@Override
65+
default DownloadFilter or(Predicate<? super S3Object> other) {
66+
Objects.requireNonNull(other, "Other predicate cannot be null");
67+
return s3Object -> test(s3Object) || other.test(s3Object);
68+
}
69+
70+
/**
71+
* Returns a filter that represents the logical negation of this predicate.
72+
* The returned filter returns true when this filter returns false, and vice versa.
73+
* @return a filter that represents the logical negation of this filter
74+
* predicate
75+
*/
76+
@Override
77+
default DownloadFilter negate() {
78+
return s3Object -> !test(s3Object);
79+
}
80+
81+
/**
82+
* A {@link DownloadFilter} that downloads all non-folder objects. A folder is a 0-byte object created when a customer uses S3
83+
* console to create a folder, and it always ends with "/".
4484
*
4585
* <p>
4686
* This is the default behavior if no filter is provided.

‎services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/config/DownloadFilterTest.java‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
package software.amazon.awssdk.transfer.s3.config;
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
1920

21+
import java.util.function.Function;
2022
import java.util.stream.Stream;
23+
import org.junit.jupiter.api.DisplayName;
2124
import org.junit.jupiter.params.ParameterizedTest;
2225
import org.junit.jupiter.params.provider.Arguments;
2326
import org.junit.jupiter.params.provider.MethodSource;
@@ -38,4 +41,168 @@ public static Stream<Arguments> s3Objects() {
3841
void allObjectsFilter_shouldWork(S3Object s3Object, boolean result) {
3942
assertThat(DownloadFilter.allObjects().test(s3Object)).isEqualTo(result);
4043
}
44+
45+
private static Stream<Arguments> filterOperationTestCases() {
46+
Function<S3Object, DownloadFilter> folder1OrFolder3Filter = s3Object -> {
47+
DownloadFilter folder1 = obj -> obj.key().startsWith("folder1");
48+
DownloadFilter folder3 = obj -> obj.key().startsWith("folder3");
49+
return folder1.or(folder3);
50+
};
51+
52+
Function<S3Object, DownloadFilter> txtAndLargeSizeFilter = s3Object -> {
53+
DownloadFilter txtFilter = obj -> obj.key().endsWith(".txt");
54+
DownloadFilter sizeFilter = obj -> obj.size() > 1000L;
55+
return txtFilter.and(sizeFilter);
56+
};
57+
58+
Function<S3Object, DownloadFilter> notFolder1Filter = s3Object -> {
59+
DownloadFilter folder1 = obj -> obj.key().startsWith("folder1");
60+
return folder1.negate();
61+
};
62+
63+
Function<S3Object, DownloadFilter> notLargeSizeFilter = s3Object -> {
64+
DownloadFilter largeSize = obj -> obj.size() > 1000L;
65+
return largeSize.negate();
66+
};
67+
68+
Function<S3Object, DownloadFilter> complexFilter = s3Object -> {
69+
DownloadFilter folder1 = obj -> obj.key().startsWith("folder1");
70+
DownloadFilter folder3 = obj -> obj.key().startsWith("folder3");
71+
DownloadFilter sizeFilter = obj -> obj.size() > 1000L;
72+
return folder1.or(folder3).and(sizeFilter);
73+
};
74+
Function<S3Object, DownloadFilter> nullParameterFilter = s3Object -> {
75+
DownloadFilter baseFilter = obj -> obj.key().startsWith("folder1");
76+
return s -> {
77+
assertThrows(NullPointerException.class,
78+
() -> baseFilter.or(null),
79+
"or() should throw NullPointerException when other is null");
80+
assertThrows(NullPointerException.class,
81+
() -> baseFilter.and(null),
82+
"and() should throw NullPointerException when other is null");
83+
return true; // Return value doesn't matter as we're testing for exceptions
84+
};
85+
};
86+
87+
88+
return Stream.of(
89+
// OR operation tests
90+
Arguments.of(
91+
"OR: folder1/test.txt matches (folder1 OR folder3)",
92+
S3Object.builder().key("folder1/test.txt").size(2000L).build(),
93+
folder1OrFolder3Filter,
94+
true
95+
),
96+
Arguments.of(
97+
"OR: folder3/test.txt matches (folder1 OR folder3)",
98+
S3Object.builder().key("folder3/test.txt").size(2000L).build(),
99+
folder1OrFolder3Filter,
100+
true
101+
),
102+
Arguments.of(
103+
"OR: folder2/test.txt does not match (folder1 OR folder3)",
104+
S3Object.builder().key("folder2/test.txt").size(2000L).build(),
105+
folder1OrFolder3Filter,
106+
false
107+
),
108+
109+
// AND operation tests
110+
Arguments.of(
111+
"AND: large .txt file matches (.txt AND size > 1000)",
112+
S3Object.builder().key("folder1/test.txt").size(2000L).build(),
113+
txtAndLargeSizeFilter,
114+
true
115+
),
116+
Arguments.of(
117+
"AND: small .txt file does not match (.txt AND size > 1000)",
118+
S3Object.builder().key("folder1/test.txt").size(500L).build(),
119+
txtAndLargeSizeFilter,
120+
false
121+
),
122+
Arguments.of(
123+
"AND: large .pdf file does not match (.txt AND size > 1000)",
124+
S3Object.builder().key("folder1/test.pdf").size(2000L).build(),
125+
txtAndLargeSizeFilter,
126+
false
127+
),
128+
129+
// NEGATE operation tests
130+
Arguments.of(
131+
"NEGATE: folder1 file does not match NOT(folder1)",
132+
S3Object.builder().key("folder1/test.txt").size(1000L).build(),
133+
notFolder1Filter,
134+
false
135+
),
136+
Arguments.of(
137+
"NEGATE: folder2 file matches NOT(folder1)",
138+
S3Object.builder().key("folder2/test.txt").size(1000L).build(),
139+
notFolder1Filter,
140+
true
141+
),
142+
Arguments.of(
143+
"NEGATE: large file does not match NOT(size > 1000)",
144+
S3Object.builder().key("test.txt").size(2000L).build(),
145+
notLargeSizeFilter,
146+
false
147+
),
148+
Arguments.of(
149+
"NEGATE: small file matches NOT(size > 1000)",
150+
S3Object.builder().key("test.txt").size(500L).build(),
151+
notLargeSizeFilter,
152+
true
153+
),
154+
155+
// Complex chained operations
156+
Arguments.of(
157+
"COMPLEX: large file in folder1 matches ((folder1 OR folder3) AND size > 1000)",
158+
S3Object.builder().key("folder1/test.txt").size(2000L).build(),
159+
complexFilter,
160+
true
161+
),
162+
Arguments.of(
163+
"COMPLEX: small file in folder1 does not match ((folder1 OR folder3) AND size > 1000)",
164+
S3Object.builder().key("folder1/test.txt").size(500L).build(),
165+
complexFilter,
166+
false
167+
),
168+
Arguments.of(
169+
"COMPLEX: large file in folder2 does not match ((folder1 OR folder3) AND size > 1000)",
170+
S3Object.builder().key("folder2/test.txt").size(2000L).build(),
171+
complexFilter,
172+
false
173+
),
174+
Arguments.of(
175+
"COMPLEX: large file in folder3 matches ((folder1 OR folder3) AND size > 1000)",
176+
S3Object.builder().key("folder3/test.txt").size(2000L).build(),
177+
complexFilter,
178+
true
179+
),
180+
// NullPointerException
181+
Arguments.of(
182+
"NULL: or/and with null parameter should throw NullPointerException",
183+
S3Object.builder().key("folder1/test.txt").size(1000L).build(),
184+
nullParameterFilter,
185+
true
186+
)
187+
188+
);
189+
}
190+
191+
@ParameterizedTest
192+
@MethodSource("filterOperationTestCases")
193+
@DisplayName("Test DownloadFilter operations (AND, OR, NEGATE)")
194+
void testFilterOperations(String scenario, S3Object s3Object,
195+
Function<S3Object, DownloadFilter> filterFactory,
196+
boolean expectedResult) {
197+
// Given
198+
DownloadFilter filter = filterFactory.apply(s3Object);
199+
200+
// When
201+
boolean actualResult = filter.test(s3Object);
202+
203+
// Then
204+
assertThat(actualResult)
205+
.as(scenario)
206+
.isEqualTo(expectedResult);
207+
}
41208
}

0 commit comments

Comments
 (0)