Skip to content

Commit 371b0ef

Browse files
zoewanggzoewangg
committed
Add validation in S3ClientConfiguration
1 parent 3a3d998 commit 371b0ef

File tree

5 files changed

+231
-8
lines changed

5 files changed

+231
-8
lines changed

‎pom.xml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,14 @@
479479
</goals>
480480
</execution>
481481
</executions>
482+
<configuration>
483+
<excludes>
484+
<exclude>software/amazon/awssdk/modulepath/tests/**/*.class</exclude>
485+
<exclude>software/amazon/awssdk/nativeimagetest/**/*.class</exclude>
486+
<exclude>software/amazon/awssdk/testutils/service/**/*.class</exclude>
487+
<exclude>**/*Benchmark.class</exclude>
488+
</excludes>
489+
</configuration>
482490
</plugin>
483491

484492
<plugin>

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

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@
1515

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

18+
import java.util.Objects;
1819
import java.util.Optional;
1920
import software.amazon.awssdk.annotations.SdkPreviewApi;
2021
import software.amazon.awssdk.annotations.SdkPublicApi;
2122
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
2223
import software.amazon.awssdk.regions.Region;
24+
import software.amazon.awssdk.utils.Validate;
2325
import software.amazon.awssdk.utils.builder.CopyableBuilder;
2426
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;
2527

@@ -33,16 +35,17 @@
3335
public final class S3ClientConfiguration implements ToCopyableBuilder<S3ClientConfiguration.Builder, S3ClientConfiguration> {
3436
private final AwsCredentialsProvider credentialsProvider;
3537
private final Region region;
36-
private final Long partSizeBytes;
38+
private final Long minimumPartSizeInBytes;
3739
private final Double targetThroughputInGbps;
3840
private final Integer maxConcurrency;
3941

4042
private S3ClientConfiguration(DefaultBuilder builder) {
4143
this.credentialsProvider = builder.credentialsProvider;
4244
this.region = builder.region;
43-
this.partSizeBytes = builder.partSizeBytes;
44-
this.targetThroughputInGbps = builder.targetThroughputInGbps;
45-
this.maxConcurrency = builder.maxConcurrency;
45+
this.minimumPartSizeInBytes = Validate.isPositiveOrNull(builder.minimumPartSizeInBytes, "minimumPartSizeInBytes");
46+
this.targetThroughputInGbps = Validate.isPositiveOrNull(builder.targetThroughputInGbps, "targetThroughputInGbps");
47+
this.maxConcurrency = Validate.isPositiveOrNull(builder.maxConcurrency,
48+
"maxConcurrency");
4649
}
4750

4851
/**
@@ -63,7 +66,7 @@ public Optional<Region> region() {
6366
* @return the optional minimum part size for transfer parts.
6467
*/
6568
public Optional<Long> minimumPartSizeInBytes() {
66-
return Optional.ofNullable(partSizeBytes);
69+
return Optional.ofNullable(minimumPartSizeInBytes);
6770
}
6871

6972
/**
@@ -85,6 +88,42 @@ public Builder toBuilder() {
8588
return new DefaultBuilder(this);
8689
}
8790

91+
@Override
92+
public boolean equals(Object o) {
93+
if (this == o) {
94+
return true;
95+
}
96+
if (o == null || getClass() != o.getClass()) {
97+
return false;
98+
}
99+
100+
S3ClientConfiguration that = (S3ClientConfiguration) o;
101+
102+
if (!Objects.equals(credentialsProvider, that.credentialsProvider)) {
103+
return false;
104+
}
105+
if (!Objects.equals(region, that.region)) {
106+
return false;
107+
}
108+
if (!Objects.equals(minimumPartSizeInBytes, that.minimumPartSizeInBytes)) {
109+
return false;
110+
}
111+
if (!Objects.equals(targetThroughputInGbps, that.targetThroughputInGbps)) {
112+
return false;
113+
}
114+
return Objects.equals(maxConcurrency, that.maxConcurrency);
115+
}
116+
117+
@Override
118+
public int hashCode() {
119+
int result = credentialsProvider != null ? credentialsProvider.hashCode() : 0;
120+
result = 31 * result + (region != null ? region.hashCode() : 0);
121+
result = 31 * result + (minimumPartSizeInBytes != null ? minimumPartSizeInBytes.hashCode() : 0);
122+
result = 31 * result + (targetThroughputInGbps != null ? targetThroughputInGbps.hashCode() : 0);
123+
result = 31 * result + (maxConcurrency != null ? maxConcurrency.hashCode() : 0);
124+
return result;
125+
}
126+
88127
public static Builder builder() {
89128
return new DefaultBuilder();
90129
}
@@ -152,7 +191,7 @@ public interface Builder extends CopyableBuilder<Builder, S3ClientConfiguration>
152191
private static final class DefaultBuilder implements Builder {
153192
private AwsCredentialsProvider credentialsProvider;
154193
private Region region;
155-
private Long partSizeBytes;
194+
private Long minimumPartSizeInBytes;
156195
private Double targetThroughputInGbps;
157196
private Integer maxConcurrency;
158197

@@ -162,7 +201,7 @@ private DefaultBuilder() {
162201
private DefaultBuilder(S3ClientConfiguration configuration) {
163202
this.credentialsProvider = configuration.credentialsProvider;
164203
this.region = configuration.region;
165-
this.partSizeBytes = configuration.partSizeBytes;
204+
this.minimumPartSizeInBytes = configuration.minimumPartSizeInBytes;
166205
this.targetThroughputInGbps = configuration.targetThroughputInGbps;
167206
this.maxConcurrency = configuration.maxConcurrency;
168207
}
@@ -181,7 +220,7 @@ public Builder region(Region region) {
181220

182221
@Override
183222
public Builder minimumPartSizeInBytes(Long partSizeBytes) {
184-
this.partSizeBytes = partSizeBytes;
223+
this.minimumPartSizeInBytes = partSizeBytes;
185224
return this;
186225
}
187226

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

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.transfer.s3;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
20+
import static software.amazon.awssdk.transfer.s3.SizeConstant.MB;
21+
22+
import org.junit.Test;
23+
import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider;
24+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
25+
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
26+
import software.amazon.awssdk.regions.Region;
27+
28+
public class S3ClientConfigurationTest {
29+
30+
@Test
31+
public void nonPositiveMinimumPartSizeInBytes_shouldThrowException() {
32+
assertThatThrownBy(() -> S3ClientConfiguration.builder()
33+
.minimumPartSizeInBytes(-10L)
34+
.build())
35+
.hasMessageContaining("must be positive");
36+
assertThatThrownBy(() -> S3ClientConfiguration.builder()
37+
.minimumPartSizeInBytes(0L)
38+
.build())
39+
.hasMessageContaining("must be positive");
40+
41+
}
42+
43+
@Test
44+
public void nonPositiveTargetThroughput_shouldThrowException() {
45+
assertThatThrownBy(() -> S3ClientConfiguration.builder()
46+
.targetThroughputInGbps(-10.0)
47+
.build())
48+
.hasMessageContaining("must be positive");
49+
assertThatThrownBy(() -> S3ClientConfiguration.builder()
50+
.targetThroughputInGbps(0.0)
51+
.build())
52+
.hasMessageContaining("must be positive");
53+
}
54+
55+
@Test
56+
public void nonPositiveMaxConcurrency_shouldThrowException() {
57+
assertThatThrownBy(() -> S3ClientConfiguration.builder()
58+
.maxConcurrency(-10)
59+
.build())
60+
.hasMessageContaining("must be positive");
61+
assertThatThrownBy(() -> S3ClientConfiguration.builder()
62+
.maxConcurrency(0)
63+
.build())
64+
.hasMessageContaining("must be positive");
65+
}
66+
67+
@Test
68+
public void build_allProperties() {
69+
AwsCredentialsProvider credentials = () -> AwsBasicCredentials.create("test"
70+
, "test");
71+
S3ClientConfiguration configuration = S3ClientConfiguration.builder()
72+
.credentialsProvider(credentials)
73+
.maxConcurrency(100)
74+
.targetThroughputInGbps(10.0)
75+
.region(Region.US_WEST_2)
76+
.minimumPartSizeInBytes(5 * MB)
77+
.build();
78+
79+
assertThat(configuration.credentialsProvider()).contains(credentials);
80+
assertThat(configuration.maxConcurrency()).contains(100);
81+
assertThat(configuration.region()).contains(Region.US_WEST_2);
82+
assertThat(configuration.targetThroughputInGbps()).contains(10.0);
83+
assertThat(configuration.minimumPartSizeInBytes()).contains(5 * MB);
84+
}
85+
86+
@Test
87+
public void build_emptyBuilder() {
88+
S3ClientConfiguration configuration = S3ClientConfiguration.builder()
89+
.build();
90+
91+
assertThat(configuration.credentialsProvider()).isEmpty();
92+
assertThat(configuration.maxConcurrency()).isEmpty();
93+
assertThat(configuration.region()).isEmpty();
94+
assertThat(configuration.targetThroughputInGbps()).isEmpty();
95+
assertThat(configuration.minimumPartSizeInBytes()).isEmpty();
96+
}
97+
98+
@Test
99+
public void equalsHashCode() {
100+
AwsCredentialsProvider credentials = () -> AwsBasicCredentials.create("test"
101+
, "test");
102+
S3ClientConfiguration configuration1 = S3ClientConfiguration.builder()
103+
.credentialsProvider(credentials)
104+
.maxConcurrency(100)
105+
.targetThroughputInGbps(10.0)
106+
.region(Region.US_WEST_2)
107+
.minimumPartSizeInBytes(5 * MB)
108+
.build();
109+
110+
S3ClientConfiguration configuration2 = S3ClientConfiguration.builder()
111+
.credentialsProvider(credentials)
112+
.maxConcurrency(100)
113+
.targetThroughputInGbps(10.0)
114+
.region(Region.US_WEST_2)
115+
.minimumPartSizeInBytes(5 * MB)
116+
.build();
117+
118+
S3ClientConfiguration configuration3 = configuration1.toBuilder()
119+
.credentialsProvider(AnonymousCredentialsProvider.create())
120+
.maxConcurrency(50)
121+
.targetThroughputInGbps(1.0)
122+
.build();
123+
124+
assertThat(configuration1).isEqualTo(configuration2);
125+
assertThat(configuration1.hashCode()).isEqualTo(configuration2.hashCode());
126+
assertThat(configuration1).isNotEqualTo(configuration3);
127+
assertThat(configuration1.hashCode()).isNotEqualTo(configuration3.hashCode());
128+
}
129+
}

‎utils/src/main/java/software/amazon/awssdk/utils/Validate.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,13 @@ public static long isPositive(long num, String fieldName) {
619619
return num;
620620
}
621621

622+
public static double isPositive(double num, String fieldName) {
623+
if (num <= 0) {
624+
throw new IllegalArgumentException(String.format("%s must be positive", fieldName));
625+
}
626+
return num;
627+
}
628+
622629
public static int isNotNegative(int num, String fieldName) {
623630

624631
if (num < 0) {
@@ -685,6 +692,21 @@ public static Integer isPositiveOrNull(Integer num, String fieldName) {
685692
return isPositive(num, fieldName);
686693
}
687694

695+
/**
696+
* Asserts that the given boxed double is positive (non-negative and non-zero) or null.
697+
*
698+
* @param num Boxed double to validate
699+
* @param fieldName Field name to display in exception message if not positive.
700+
* @return Duration if double or null.
701+
*/
702+
public static Double isPositiveOrNull(Double num, String fieldName) {
703+
if (num == null) {
704+
return null;
705+
}
706+
707+
return isPositive(num, fieldName);
708+
}
709+
688710
/**
689711
* Asserts that the given boxed long is positive (non-negative and non-zero) or null.
690712
*

‎utils/src/test/java/software/amazon/awssdk/utils/ValidateTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,31 @@ public void isPositiveOrNullLong_negative_throws() {
578578
Validate.isPositiveOrNull(-1L, "foo");
579579
}
580580

581+
@Test
582+
public void isPositiveOrNullDouble_null_returnsNull() {
583+
assertNull(Validate.isPositiveOrNull((Double) null, "foo"));
584+
}
585+
586+
@Test
587+
public void isPositiveOrNullDouble_positive_returnsInteger() {
588+
Double num = 100.0;
589+
assertEquals(num, Validate.isPositiveOrNull(num, "foo"));
590+
}
591+
592+
@Test
593+
public void isPositiveOrNullDouble_zero_throws() {
594+
expected.expect(IllegalArgumentException.class);
595+
expected.expectMessage("foo");
596+
Validate.isPositiveOrNull(0.0, "foo");
597+
}
598+
599+
@Test
600+
public void isPositiveOrNullDouble_negative_throws() {
601+
expected.expect(IllegalArgumentException.class);
602+
expected.expectMessage("foo");
603+
Validate.isPositiveOrNull(-1.0, "foo");
604+
}
605+
581606
@Test
582607
public void isNull_notNull_shouldThrow() {
583608
expected.expect(IllegalArgumentException.class);

0 commit comments

Comments
 (0)