Skip to content

Commit 840a2bf

Browse files
authored
Fix SonarCloud code smells (#3991)
* Fix SonarCloud code smells * Move AdaptiveRetryStrategyResourceConstrainedTest to an integration test This change is to workaround the SonarCloud code smell of the Sleep usage in this test
1 parent 1e4dfc4 commit 840a2bf

File tree

11 files changed

+33
-32
lines changed

11 files changed

+33
-32
lines changed

core/retries-api/src/main/java/software/amazon/awssdk/retries/api/TokenAcquisitionFailedException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*/
2323
@SdkPublicApi
2424
public final class TokenAcquisitionFailedException extends RuntimeException {
25-
private final RetryToken token;
25+
private final transient RetryToken token;
2626

2727
/**
2828
* Exception construction accepting message with no root cause.

core/retries-api/src/test/java/software/amazon/awssdk/retries/api/RetryStrategyBuilderTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
import org.junit.jupiter.params.ParameterizedTest;
2525
import org.junit.jupiter.params.provider.MethodSource;
2626

27-
public class RetryStrategyBuilderTest {
27+
class RetryStrategyBuilderTest {
2828

29-
public static Collection<TestCase> parameters() {
29+
static Collection<TestCase> parameters() {
3030
return Arrays.asList(
3131
new TestCase()
3232
.configure(b -> b.retryOnException(IllegalArgumentException.class))
@@ -105,7 +105,7 @@ public static Collection<TestCase> parameters() {
105105

106106
@ParameterizedTest
107107
@MethodSource("parameters")
108-
public void testCase(TestCase testCase) {
108+
void testCase(TestCase testCase) {
109109
assertThat(testCase.run()).isEqualTo(testCase.expected());
110110
}
111111

core/retries-api/src/test/java/software/amazon/awssdk/retries/api/internal/backoff/ExponentialDelayWithJitterTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,14 @@
2525
import org.junit.jupiter.params.ParameterizedTest;
2626
import org.junit.jupiter.params.provider.MethodSource;
2727

28-
public class ExponentialDelayWithJitterTest {
28+
class ExponentialDelayWithJitterTest {
2929
static final ComputedNextInt MIN_VALUE_RND = new ComputedNextInt(bound -> 0);
3030
static final ComputedNextInt MID_VALUE_RND = new ComputedNextInt(bound -> bound / 2);
3131
static final ComputedNextInt MAX_VALUE_RND = new ComputedNextInt(bound -> bound - 1);
3232
static final Duration BASE_DELAY = Duration.ofMillis(23);
3333
static final Duration MAX_DELAY = Duration.ofSeconds(20);
3434

35-
public static Collection<TestCase> parameters() {
35+
static Collection<TestCase> parameters() {
3636
return Arrays.asList(
3737
// --- Using random that returns: bound - 1
3838
new TestCase()
@@ -126,7 +126,7 @@ public static Collection<TestCase> parameters() {
126126

127127
@ParameterizedTest
128128
@MethodSource("parameters")
129-
public void testCase(TestCase testCase) {
129+
void testCase(TestCase testCase) {
130130
assertThat(testCase.run()).isEqualTo(testCase.expected());
131131
}
132132

core/retries-api/src/test/java/software/amazon/awssdk/retries/api/internal/backoff/FixedDelayWithJitterTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@
2525
import org.junit.jupiter.params.ParameterizedTest;
2626
import org.junit.jupiter.params.provider.MethodSource;
2727

28-
public class FixedDelayWithJitterTest {
28+
class FixedDelayWithJitterTest {
2929
static final ComputedNextInt MIN_VALUE_RND = new ComputedNextInt(bound -> 0);
3030
static final ComputedNextInt MID_VALUE_RND = new ComputedNextInt(bound -> bound / 2);
3131
static final ComputedNextInt MAX_VALUE_RND = new ComputedNextInt(bound -> bound - 1);
3232
static final Duration BASE_DELAY = Duration.ofMillis(23);
3333

34-
public static Collection<TestCase> parameters() {
34+
static Collection<TestCase> parameters() {
3535
return Arrays.asList(
3636
// --- Using random that returns: bound - 1
3737
new TestCase()
@@ -126,7 +126,7 @@ public static Collection<TestCase> parameters() {
126126

127127
@ParameterizedTest
128128
@MethodSource("parameters")
129-
public void testCase(TestCase testCase) {
129+
void testCase(TestCase testCase) {
130130
assertThat(testCase.run()).isEqualTo(testCase.expected());
131131
}
132132

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@
5151
* are made to a very resource-constrained set of resources, and we expect that the delays created by the rate limiter allows the
5252
* perceived availability for the clients to be close to 1.0.
5353
*/
54-
public class AdaptiveRetryStrategyResourceConstrainedTest {
54+
class AdaptiveRetryStrategyResourceConstrainedTest {
5555

5656
@Test
57-
public void seemsToBeCorrectAndThreadSafe() {
57+
void seemsToBeCorrectAndThreadSafe() {
5858
// Arrange the test. We allocate a single thread for each server worker
5959
// and for each client worker.
6060
int serverWorkers = 1;
@@ -93,7 +93,7 @@ public void seemsToBeCorrectAndThreadSafe() {
9393
executor.shutdown();
9494
}
9595

96-
public static List<Client> createClients(Server server, AdaptiveRetryStrategy strategy, int amount, int jobsPerClient) {
96+
private static List<Client> createClients(Server server, AdaptiveRetryStrategy strategy, int amount, int jobsPerClient) {
9797
return IntStream.range(0, amount)
9898
.mapToObj(idx -> createClient(server, strategy, jobsPerClient))
9999
.collect(Collectors.toCollection(() -> new ArrayList<>(amount)));
@@ -103,7 +103,7 @@ private static Client createClient(Server server, AdaptiveRetryStrategy strategy
103103
return new Client(createJobs(jobs), server, strategy);
104104
}
105105

106-
public static List<Job> createJobs(int amount) {
106+
private static List<Job> createJobs(int amount) {
107107
// We use a non-small but fixed size here instead of random ones to have a more predictable workload.
108108
int rows = 256;
109109
int cols = 256 + 128;
@@ -142,13 +142,13 @@ static class Client {
142142
this.strategy = strategy;
143143
}
144144

145-
public void processAllJobs() {
145+
void processAllJobs() {
146146
for (Job job : jobs) {
147147
process(job);
148148
}
149149
}
150150

151-
public void process(Job job) {
151+
void process(Job job) {
152152
// submit job
153153
AcquireInitialTokenResponse response = strategy.acquireInitialToken(AcquireInitialTokenRequest.create("client"));
154154
RetryToken token = response.token();
@@ -222,7 +222,7 @@ void stop() {
222222
CompletableFuture.allOf(workers.toArray(new CompletableFuture[0])).join();
223223
}
224224

225-
public void accept(Job job) {
225+
void accept(Job job) {
226226
if (!jobQueue.offer(job)) {
227227
// No space left in the queue to take this job, throw a ThrottlingException to notify the
228228
// client about it and let him retry at a later time.
@@ -238,6 +238,7 @@ static class ServerWorker implements Runnable {
238238
this.jobQueue = jobQueue;
239239
}
240240

241+
@Override
241242
public void run() {
242243
while (true) {
243244
try {

core/retries/src/main/java/software/amazon/awssdk/retries/internal/AdaptiveRetryStrategyImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public AcquireInitialTokenResponse acquireInitialToken(AcquireInitialTokenReques
8282
@Override
8383
public RefreshRetryTokenResponse refreshRetryToken(RefreshRetryTokenRequest request) {
8484
DefaultRetryToken token = asStandardRetryToken(request.token());
85-
AcquireResponse acquireResponse = requestAcquireCapacity(request, token);
85+
AcquireResponse acquireResponse = requestAcquireCapacity(token);
8686

8787
// Check if we meet the preconditions needed for retrying. These will throw if the expected condition is not meet.
8888
// 1) is retryable?
@@ -291,7 +291,7 @@ static DefaultRetryToken asStandardRetryToken(RetryToken token) {
291291
token.getClass().getName());
292292
}
293293

294-
private AcquireResponse requestAcquireCapacity(RefreshRetryTokenRequest request, DefaultRetryToken token) {
294+
private AcquireResponse requestAcquireCapacity(DefaultRetryToken token) {
295295
TokenBucket tokenBucket = tokenBucketStore.tokenBucketForScope(token.scope());
296296
return tokenBucket.tryAcquire(exceptionCost);
297297
}

core/retries/src/main/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public AcquireInitialTokenResponse acquireInitialToken(AcquireInitialTokenReques
7474
@Override
7575
public RefreshRetryTokenResponse refreshRetryToken(RefreshRetryTokenRequest request) {
7676
DefaultRetryToken token = asStandardRetryToken(request.token());
77-
AcquireResponse acquireResponse = requestAcquireCapacity(request, token);
77+
AcquireResponse acquireResponse = requestAcquireCapacity(token);
7878

7979
// Check if we meet the preconditions needed for retrying. These will throw if the expected condition is not meet.
8080
// 1) is retryable?
@@ -263,7 +263,7 @@ static DefaultRetryToken asStandardRetryToken(RetryToken token) {
263263
token.getClass().getName());
264264
}
265265

266-
private AcquireResponse requestAcquireCapacity(RefreshRetryTokenRequest request, DefaultRetryToken token) {
266+
private AcquireResponse requestAcquireCapacity(DefaultRetryToken token) {
267267
TokenBucket tokenBucket = tokenBucketStore.tokenBucketForScope(token.scope());
268268
if (!circuitBreakerEnabled) {
269269
return tokenBucket.tryAcquire(0);

core/retries/src/test/java/software/amazon/awssdk/retries/internal/AdaptiveRetryStrategyTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,15 @@
4444
// The tests here are the same set of test from the StandardRetryStrategy, both should be passing the same battery of tests.
4545
// Unfortunately It's not possible to create a single parametrized test on RetryStrategy given the different types required to
4646
// configure each strategy.
47-
public class AdaptiveRetryStrategyTest {
47+
class AdaptiveRetryStrategyTest {
4848
static final int TEST_BUCKET_CAPACITY = 100;
4949
static final int TEST_EXCEPTION_COST = 5;
5050
static final IllegalArgumentException IAE = new IllegalArgumentException();
5151
static final RuntimeException RTE = new RuntimeException();
5252

5353
@ParameterizedTest
5454
@MethodSource("parameters")
55-
public void testCase(TestCase testCase) {
55+
void testCase(TestCase testCase) {
5656
testCase.run();
5757
if (testCase.shouldSucceed) {
5858
assertThat(testCase.thrown)
@@ -68,7 +68,7 @@ public void testCase(TestCase testCase) {
6868
assertThat(testCase.token.state()).as(testCase.name).isEqualTo(testCase.expectedState);
6969
}
7070

71-
public static Collection<TestCase> parameters() {
71+
static Collection<TestCase> parameters() {
7272
BackoffStrategy backoff = BackoffStrategy.exponentialDelay(Duration.ofMillis(10), Duration.ofSeconds(25));
7373
return Arrays.asList(
7474
new TestCase("Succeeds when no exceptions are thrown")

core/retries/src/test/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyMiscTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@
2828
* Tests that the circuit breaker remembers its previous state for separated
2929
* requests.
3030
*/
31-
public class StandardRetryStrategyMiscTest {
31+
class StandardRetryStrategyMiscTest {
3232
static final int TEST_EXCEPTION_COST = 5;
3333
static final int TEST_MAX = 50;
3434
static final IllegalArgumentException IAE = new IllegalArgumentException();
3535
static final RuntimeException RTE = new RuntimeException();
3636

3737
@Test
38-
public void circuitBreakerRemembersState() {
38+
void circuitBreakerRemembersState() {
3939
BackoffStrategy backoff = BackoffStrategy.exponentialDelay(Duration.ofMillis(10), Duration.ofSeconds(25));
4040
TestCase testCase = new TestCase("circuit breaker remembers state")
4141
.configure(b -> b.maxAttempts(3))

core/retries/src/test/java/software/amazon/awssdk/retries/internal/StandardRetryStrategyTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@
4040
import software.amazon.awssdk.retries.api.internal.AcquireInitialTokenRequestImpl;
4141
import software.amazon.awssdk.retries.internal.circuitbreaker.TokenBucketStore;
4242

43-
public class StandardRetryStrategyTest {
43+
class StandardRetryStrategyTest {
4444
static final int TEST_BUCKET_CAPACITY = 100;
4545
static final int TEST_EXCEPTION_COST = 5;
4646
static final IllegalArgumentException IAE = new IllegalArgumentException();
4747
static final RuntimeException RTE = new RuntimeException();
4848

4949
@ParameterizedTest
5050
@MethodSource("parameters")
51-
public void testCase(TestCase testCase) {
51+
void testCase(TestCase testCase) {
5252
testCase.run();
5353
if (testCase.shouldSucceed) {
5454
assertThat(testCase.thrown)
@@ -65,7 +65,7 @@ public void testCase(TestCase testCase) {
6565

6666
}
6767

68-
public static Collection<TestCase> parameters() {
68+
static Collection<TestCase> parameters() {
6969
BackoffStrategy backoff = BackoffStrategy.exponentialDelay(Duration.ofMillis(10), Duration.ofSeconds(25));
7070
return Arrays.asList(
7171
new TestCase("Succeeds when no exceptions are thrown")

core/retries/src/test/java/software/amazon/awssdk/retries/internal/ratelimiter/RateLimiterTokenBucketTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import org.junit.jupiter.params.ParameterizedTest;
2525
import org.junit.jupiter.params.provider.MethodSource;
2626

27-
public class RateLimiterTokenBucketTest {
27+
class RateLimiterTokenBucketTest {
2828
private static MutableClock clock = null;
2929
private static RateLimiterTokenBucket tokenBucket = null;
3030
private static final double EPSILON = 0.0001;
@@ -37,7 +37,7 @@ static void setup() {
3737

3838
@ParameterizedTest
3939
@MethodSource("parameters")
40-
public void testCase(TestCase testCase) {
40+
void testCase(TestCase testCase) {
4141
clock.setCurrent(testCase.givenTimestamp);
4242
RateLimiterUpdateResponse res;
4343
tokenBucket.tryAcquire();
@@ -53,7 +53,7 @@ public void testCase(TestCase testCase) {
5353
}
5454

5555

56-
public static Collection<TestCase> parameters() {
56+
static Collection<TestCase> parameters() {
5757
return Arrays.asList(
5858
new TestCase()
5959
.givenSuccessResponse()

0 commit comments

Comments
 (0)