Skip to content

Commit 5a2db7b

Browse files
LokowandtgLokowandtg
committed
fix review findings
1 parent 7728153 commit 5a2db7b

File tree

7 files changed

+184
-70
lines changed

7 files changed

+184
-70
lines changed

‎integration-test/pom.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,12 @@
7373
<dependency>
7474
<groupId>io.github.resilience4j</groupId>
7575
<artifactId>resilience4j-ratelimiter</artifactId>
76-
<version>1.7.0</version>
76+
<version>${resilience4j.version}</version>
7777
</dependency>
7878
<dependency>
7979
<groupId>io.github.resilience4j</groupId>
8080
<artifactId>resilience4j-reactor</artifactId>
81-
<version>1.7.0</version>
81+
<version>${resilience4j.version}</version>
8282
</dependency>
8383
<dependency>
8484
<groupId>org.cloudfoundry</groupId>

‎integration-test/src/test/java/org/cloudfoundry/IntegrationTestConfiguration.java‎

Lines changed: 17 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,6 @@
7373
import org.cloudfoundry.uaa.groups.ListGroupsRequest;
7474
import org.cloudfoundry.uaa.groups.ListGroupsResponse;
7575
import org.cloudfoundry.uaa.groups.MemberType;
76-
import org.cloudfoundry.uaa.ratelimit.Current;
77-
import org.cloudfoundry.uaa.ratelimit.RatelimitRequest;
78-
import org.cloudfoundry.uaa.ratelimit.RatelimitResponse;
7976
import org.cloudfoundry.uaa.users.CreateUserRequest;
8077
import org.cloudfoundry.uaa.users.CreateUserResponse;
8178
import org.cloudfoundry.uaa.users.Email;
@@ -199,17 +196,21 @@ UaaClient adminUaaClient(
199196
ConnectionContext connectionContext,
200197
@Value("${test.admin.clientId}") String clientId,
201198
@Value("${test.admin.clientSecret}") String clientSecret,
202-
int uaaLimiterMapping) {
203-
return new ThrottlingUaaClient(
199+
@Value("${uaa.api.request.limit:#{null}}") Integer environmentRequestLimit) {
200+
ReactorUaaClient unthrottledClient =
204201
ReactorUaaClient.builder()
205202
.connectionContext(connectionContext)
206203
.tokenProvider(
207204
ClientCredentialsGrantTokenProvider.builder()
208205
.clientId(clientId)
209206
.clientSecret(clientSecret)
210207
.build())
211-
.build(),
212-
uaaLimiterMapping);
208+
.build();
209+
if (environmentRequestLimit == null) {
210+
return unthrottledClient;
211+
} else {
212+
return new ThrottlingUaaClient(unthrottledClient, environmentRequestLimit);
213+
}
213214
}
214215

215216
@Bean(initMethod = "block")
@@ -249,7 +250,6 @@ String clientSecret(NameFactory nameFactory) {
249250
}
250251

251252
@Bean
252-
@DependsOn("uaaRatelimit")
253253
CloudFoundryCleaner cloudFoundryCleaner(
254254
@Qualifier("admin") CloudFoundryClient cloudFoundryClient,
255255
NameFactory nameFactory,
@@ -326,54 +326,6 @@ DefaultConnectionContext connectionContext(
326326
return connectionContext.build();
327327
}
328328

329-
@Bean
330-
public Integer uaaLimiterMapping(
331-
@Value("${uaa.api.request.limit:#{null}}") Integer environmentRequestLimit) {
332-
return environmentRequestLimit;
333-
}
334-
335-
@Bean
336-
Boolean uaaRatelimit(
337-
ConnectionContext connectionContext, @Qualifier("admin") UaaClient uaaClient) {
338-
return uaaClient
339-
.rateLimit()
340-
.getRatelimit(RatelimitRequest.builder().build())
341-
.map(response -> getServerRatelimit(response))
342-
.timeout(Duration.ofSeconds(5))
343-
.onErrorResume(
344-
ex -> {
345-
logger.error(
346-
"Warning: could not fetch UAA rate limit, using default"
347-
+ " "
348-
+ 0
349-
+ ". Cause: "
350-
+ ex);
351-
return Mono.just(false);
352-
})
353-
.block();
354-
}
355-
356-
private Boolean getServerRatelimit(RatelimitResponse response) {
357-
Current curr = response.getCurrentData();
358-
if (!"ACTIVE".equals(curr.getStatus())) {
359-
logger.debug(
360-
"UaaRatelimitInitializer server ratelimit is not 'ACTIVE', but "
361-
+ curr.getStatus()
362-
+ ". Ignoring server value for ratelimit.");
363-
return false;
364-
}
365-
Integer result = curr.getLimiterMappings();
366-
logger.info(
367-
"Server uses uaa rate limiting. There are "
368-
+ result
369-
+ " mappings declared in file "
370-
+ response.getFromSource());
371-
logger.info(
372-
"If you encounter 429 return codes, configure uaa rate limiting or set variable"
373-
+ " 'UAA_API_REQUEST_LIMIT' to a save value.");
374-
return true;
375-
}
376-
377329
@Bean
378330
DopplerClient dopplerClient(ConnectionContext connectionContext, TokenProvider tokenProvider) {
379331
return ReactorDopplerClient.builder()
@@ -701,13 +653,17 @@ PasswordGrantTokenProvider tokenProvider(
701653
UaaClient uaaClient(
702654
ConnectionContext connectionContext,
703655
TokenProvider tokenProvider,
704-
int uaaLimiterMapping) {
705-
return new ThrottlingUaaClient(
656+
@Value("${uaa.api.request.limit:#{null}}") Integer environmentRequestLimit) {
657+
ReactorUaaClient unthrottledClient =
706658
ReactorUaaClient.builder()
707659
.connectionContext(connectionContext)
708660
.tokenProvider(tokenProvider)
709-
.build(),
710-
uaaLimiterMapping);
661+
.build();
662+
if (environmentRequestLimit == null) {
663+
return unthrottledClient;
664+
} else {
665+
return new ThrottlingUaaClient(unthrottledClient, environmentRequestLimit);
666+
}
711667
}
712668

713669
@Bean(initMethod = "block")
@@ -790,7 +746,7 @@ String username(NameFactory nameFactory) {
790746
return nameFactory.getUserName();
791747
}
792748

793-
private static final class FailingDeserializationProblemHandler
749+
public static final class FailingDeserializationProblemHandler
794750
extends DeserializationProblemHandler {
795751

796752
@Override

‎integration-test/src/test/java/org/cloudfoundry/ThrottlingUaaClient.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,22 +69,29 @@
6969
public class ThrottlingUaaClient implements UaaClient {
7070

7171
private final UaaClient delegate;
72-
72+
private final int maxRequestsPerSecond;
7373
private final RateLimiter rateLimiter;
7474
private final ThrottledUsers users;
7575
private Groups groups;
7676

77-
public ThrottlingUaaClient(ReactorUaaClient delegate, int uaaLimit) {
77+
/**
78+
* An {@link UaaClient} implementation that throttles calls to the UAA
79+
* {@code /Groups} and {@code /Users} endpoints. It uses a single "bucket"
80+
* for throttling requests to both endpoints.
81+
*
82+
* @see <a href="https://resilience4j.readme.io/docs/ratelimiter">resilience4j docs</a>
83+
*/
84+
public ThrottlingUaaClient(ReactorUaaClient delegate, int maxRequestsPerSecond) {
7885
// uaaLimit is calls per second. We need the milliseconds for one call because
7986
// resilience4j uses sliced timeslots, while the uaa server uses a sliding window.
80-
int timeBasePerRequest = 1000 / uaaLimit;
87+
int clockSkewMillis = 20; // 20ms clock skew is a save value for ~5 requests per second.
88+
int rateLimitRefreshPeriodMillis = (1000 / maxRequestsPerSecond) + clockSkewMillis;
8189
this.delegate = delegate;
90+
this.maxRequestsPerSecond = maxRequestsPerSecond;
8291
RateLimiterConfig config =
8392
RateLimiterConfig.custom()
8493
.limitForPeriod(1)
85-
.limitRefreshPeriod(
86-
Duration.ofMillis(
87-
timeBasePerRequest + 20)) // 20 ms to handle clock skew.
94+
.limitRefreshPeriod(Duration.ofMillis(rateLimitRefreshPeriodMillis))
8895
.timeoutDuration(Duration.ofSeconds(10))
8996
.build();
9097
this.rateLimiter = RateLimiter.of("uaa", config);
@@ -144,6 +151,10 @@ public Ratelimit rateLimit() {
144151
return this.delegate.rateLimit();
145152
}
146153

154+
public int getMaxRequestsPerSecond() {
155+
return maxRequestsPerSecond;
156+
}
157+
147158
public class ThrottledUsers implements Users {
148159

149160
private final Users usersDelegate;

‎integration-test/src/test/java/org/cloudfoundry/uaa/RatelimitTestConfiguration.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package org.cloudfoundry.uaa;
2+
3+
import java.time.Duration;
4+
import org.cloudfoundry.IntegrationTestConfiguration.FailingDeserializationProblemHandler;
5+
import org.cloudfoundry.ThrottlingUaaClient;
6+
import org.cloudfoundry.reactor.ConnectionContext;
7+
import org.cloudfoundry.reactor.DefaultConnectionContext;
8+
import org.cloudfoundry.reactor.ProxyConfiguration;
9+
import org.cloudfoundry.reactor.tokenprovider.ClientCredentialsGrantTokenProvider;
10+
import org.cloudfoundry.reactor.uaa.ReactorUaaClient;
11+
import org.springframework.beans.factory.annotation.Value;
12+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
13+
import org.springframework.context.annotation.Bean;
14+
import org.springframework.context.annotation.Configuration;
15+
import org.springframework.util.StringUtils;
16+
17+
@Configuration
18+
@EnableAutoConfiguration
19+
public class RatelimitTestConfiguration {
20+
21+
@Bean
22+
UaaClient adminUaaClient(
23+
ConnectionContext connectionContext,
24+
@Value("${test.admin.clientId}") String clientId,
25+
@Value("${test.admin.clientSecret}") String clientSecret,
26+
@Value("${uaa.api.request.limit:0}") int uaaLimit) {
27+
ReactorUaaClient client =
28+
ReactorUaaClient.builder()
29+
.connectionContext(connectionContext)
30+
.tokenProvider(
31+
ClientCredentialsGrantTokenProvider.builder()
32+
.clientId(clientId)
33+
.clientSecret(clientSecret)
34+
.build())
35+
.build();
36+
if (uaaLimit > 0) {
37+
return new ThrottlingUaaClient(client, uaaLimit);
38+
}
39+
return client;
40+
}
41+
42+
@Bean
43+
DefaultConnectionContext connectionContext(
44+
@Value("${test.apiHost}") String apiHost,
45+
@Value("${test.proxy.host:}") String proxyHost,
46+
@Value("${test.proxy.password:}") String proxyPassword,
47+
@Value("${test.proxy.port:8080}") Integer proxyPort,
48+
@Value("${test.proxy.username:}") String proxyUsername,
49+
@Value("${test.skipSslValidation:false}") Boolean skipSslValidation) {
50+
51+
DefaultConnectionContext.Builder connectionContext =
52+
DefaultConnectionContext.builder()
53+
.apiHost(apiHost)
54+
.problemHandler(
55+
new FailingDeserializationProblemHandler()) // Test-only problem
56+
// handler
57+
.skipSslValidation(skipSslValidation)
58+
.sslHandshakeTimeout(Duration.ofSeconds(30));
59+
60+
if (StringUtils.hasText(proxyHost)) {
61+
ProxyConfiguration.Builder proxyConfiguration =
62+
ProxyConfiguration.builder().host(proxyHost).port(proxyPort);
63+
64+
if (StringUtils.hasText(proxyUsername)) {
65+
proxyConfiguration.password(proxyPassword).username(proxyUsername);
66+
}
67+
68+
connectionContext.proxyConfiguration(proxyConfiguration.build());
69+
}
70+
71+
return connectionContext.build();
72+
}
73+
}

‎integration-test/src/test/java/org/cloudfoundry/uaa/UaaRatelimitTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package org.cloudfoundry.uaa;
2+
3+
import java.time.Duration;
4+
import org.cloudfoundry.ThrottlingUaaClient;
5+
import org.cloudfoundry.uaa.ratelimit.Current;
6+
import org.cloudfoundry.uaa.ratelimit.RatelimitRequest;
7+
import org.cloudfoundry.uaa.ratelimit.RatelimitResponse;
8+
import org.junit.jupiter.api.Test;
9+
import org.slf4j.Logger;
10+
import org.slf4j.LoggerFactory;
11+
import org.springframework.beans.factory.annotation.Autowired;
12+
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
13+
import reactor.core.publisher.Mono;
14+
import reactor.test.StepVerifier;
15+
16+
@SpringJUnitConfig(classes = RatelimitTestConfiguration.class)
17+
public class UaaRatelimitTest {
18+
private static final Logger LOGGER = LoggerFactory.getLogger(UaaRatelimitTest.class);
19+
20+
@Autowired private UaaClient adminUaaClient;
21+
22+
@Test
23+
public void getRatelimit() {
24+
int envRatelimit;
25+
if (adminUaaClient instanceof ThrottlingUaaClient) {
26+
ThrottlingUaaClient throttlingClient = (ThrottlingUaaClient) adminUaaClient;
27+
envRatelimit = throttlingClient.getMaxRequestsPerSecond();
28+
} else {
29+
envRatelimit = 0;
30+
}
31+
Mono<Boolean> tmp =
32+
adminUaaClient
33+
.rateLimit()
34+
.getRatelimit(RatelimitRequest.builder().build())
35+
.map(response -> getServerRatelimit(response, envRatelimit))
36+
.timeout(Duration.ofSeconds(5))
37+
.onErrorResume(
38+
ex -> {
39+
LOGGER.error(
40+
"Warning: could not fetch UAA rate limit, using default"
41+
+ " "
42+
+ 0
43+
+ ". Cause: "
44+
+ ex);
45+
return Mono.just(false);
46+
});
47+
StepVerifier.create(tmp.materialize()).expectNextCount(1).verifyComplete();
48+
}
49+
50+
private Boolean getServerRatelimit(RatelimitResponse response, int maxRequestsPerSecond) {
51+
Current curr = response.getCurrentData();
52+
if (!"ACTIVE".equals(curr.getStatus())) {
53+
LOGGER.debug(
54+
"UaaRatelimitInitializer server ratelimit is not 'ACTIVE', but "
55+
+ curr.getStatus()
56+
+ ". Ignoring server value for ratelimit.");
57+
return false;
58+
}
59+
Integer result = curr.getLimiterMappings();
60+
LOGGER.info(
61+
"Server uses uaa rate limiting. There are "
62+
+ result
63+
+ " mappings declared in file "
64+
+ response.getFromSource());
65+
if (maxRequestsPerSecond == 0) {
66+
LOGGER.warn(
67+
"Ratelimiting is not set locally, set variable 'UAA_API_REQUEST_LIMIT' to a"
68+
+ " save value or you might experience http 429 responses.");
69+
}
70+
return true;
71+
}
72+
}

‎integration-test/src/test/resources/logback-test.xml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
<logger name="cloudfoundry-client.compatibility" level="${CLIENT_LOGGING_LEVEL:-INFO}"/>
2727
<logger name="cloudfoundry-client.delay" level="${CLIENT_LOGGING_LEVEL:-INFO}"/>
2828
<logger name="cloudfoundry-client.operations" level="${CLIENT_LOGGING_LEVEL:-INFO}"/>
29-
<logger name="cloudfoundry-client.request" level="DEBUG"/>
29+
<logger name="cloudfoundry-client.request" level="${CLIENT_LOGGING_LEVEL:-INFO}"/>
3030
<logger name="cloudfoundry-client.resource-matching" level="${CLIENT_LOGGING_LEVEL:-INFO}"/>
3131
<logger name="cloudfoundry-client.response" level="${CLIENT_LOGGING_LEVEL:-INFO}"/>
3232
<logger name="cloudfoundry-client.test" level="${TEST_LOGGING_LEVEL:-INFO}"/>
@@ -36,6 +36,7 @@
3636
<logger name="example" level="DEBUG"/>
3737
<logger name="reactor.netty" level="INFO"/>
3838
<logger name="stream" level="INFO"/>
39+
<logger name="org.cloudfoundry.uaa.UaaRatelimitTest" level="INFO"/>
3940

4041
<root level="WARN">
4142
<appender-ref ref="STDOUT"/>

‎pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
<wire.plugin.version>3.0.2</wire.plugin.version>
7575
<wire.suffix></wire.suffix>
7676
<spotless.version>2.44.4</spotless.version>
77+
<resilience4j.version>1.7.0</resilience4j.version>
7778
</properties>
7879

7980
<dependencyManagement>

0 commit comments

Comments
 (0)