Skip to content

Commit 1f431c6

Browse files
netominetomi
committed
chore: cleanup log message for rate limit, add support for a ipAddressFunction, remove obsolete cache bean
1 parent 9fe38bc commit 1f431c6

7 files changed

Lines changed: 76 additions & 31 deletions

File tree

‎server/src/main/java/org/eclipse/openvsx/ratelimit/IdentityService.java‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,46 @@
1212
*****************************************************************************/
1313
package org.eclipse.openvsx.ratelimit;
1414

15+
import com.giffing.bucket4j.spring.boot.starter.context.ExpressionParams;
1516
import jakarta.servlet.http.HttpServletRequest;
1617
import org.eclipse.openvsx.ratelimit.config.RateLimitConfig;
18+
import org.eclipse.openvsx.ratelimit.config.RateLimitProperties;
19+
import org.slf4j.Logger;
20+
import org.slf4j.LoggerFactory;
21+
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
1722
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
23+
import org.springframework.context.expression.BeanFactoryResolver;
24+
import org.springframework.expression.ExpressionParser;
25+
import org.springframework.expression.spel.support.StandardEvaluationContext;
1826
import org.springframework.stereotype.Service;
1927

28+
import java.util.Map;
29+
2030
@Service
2131
@ConditionalOnBean(RateLimitConfig.class)
2232
public class IdentityService {
2333

34+
private final Logger logger = LoggerFactory.getLogger(IdentityService.class);
35+
36+
private final ExpressionParser expressionParser;
37+
private final ConfigurableBeanFactory beanFactory;
38+
2439
private final TierService tierService;
2540
private final CustomerService customerService;
41+
private final RateLimitProperties rateLimitProperties;
2642

27-
public IdentityService(TierService tierService, CustomerService customerService) {
43+
public IdentityService(
44+
ExpressionParser expressionParser,
45+
ConfigurableBeanFactory beanFactory,
46+
TierService tierService,
47+
CustomerService customerService,
48+
RateLimitProperties rateLimitProperties
49+
) {
50+
this.expressionParser = expressionParser;
51+
this.beanFactory = beanFactory;
2852
this.tierService = tierService;
2953
this.customerService = customerService;
54+
this.rateLimitProperties = rateLimitProperties;
3055
}
3156

3257
public ResolvedIdentity resolveIdentity(HttpServletRequest request) {
@@ -69,10 +94,19 @@ public ResolvedIdentity resolveIdentity(HttpServletRequest request) {
6994
}
7095

7196
private String getIPAddress(HttpServletRequest request) {
72-
// TODO: make this configurable rather than hardcode,
73-
// if the server is run without proxy, someone
74-
// could fake the X-Forwarded-For header
75-
var forwardedFor = request.getHeader("X-Forwarded-For");
76-
return forwardedFor != null ? forwardedFor : request.getRemoteAddr();
97+
var ipAddressFunction = rateLimitProperties.getIpAddressFunction();
98+
var params = new ExpressionParams<>(request);
99+
var context = getContext(params.getParams());
100+
var expr = expressionParser.parseExpression(ipAddressFunction);
101+
String result = expr.getValue(context, params.getRootObject(), String.class);
102+
logger.trace("GetIPAddress -> result:{};expression:{}", result, ipAddressFunction);
103+
return result;
104+
}
105+
106+
private StandardEvaluationContext getContext(Map<String, Object> params) {
107+
var context = new StandardEvaluationContext();
108+
params.forEach(context::setVariable);
109+
context.setBeanResolver(new BeanFactoryResolver(beanFactory));
110+
return context;
77111
}
78112
}

‎server/src/main/java/org/eclipse/openvsx/ratelimit/ResolvedIdentity.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public boolean isCustomer() {
3333
if (isCustomer()) {
3434
return customer;
3535
} else {
36-
throw new RuntimeException("no customer associated with identity");
36+
throw new RuntimeException("No customer associated with this identity.");
3737
}
3838
}
3939
}

‎server/src/main/java/org/eclipse/openvsx/ratelimit/UsageStatsService.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public void incrementUsage(Customer customer) {
5252
var key = customer.getId();
5353
var window = getCurrentUsageWindow();
5454
var old = jedisCluster.hincrBy(USAGE_DATA_KEY, key + ":" + window, 1);
55-
logger.debug("usage count for {}: {}", customer.getName(), old + 1);
55+
logger.debug("Usage count for {}: {}", customer.getName(), old + 1);
5656
}
5757

5858
public void persistUsageStats() {
@@ -68,7 +68,7 @@ public void persistUsageStats() {
6868
var key = result.getKey();
6969
var value = result.getValue();
7070

71-
logger.debug("usage stats: {} - {}", key, value);
71+
logger.debug("Usage stats: {} - {}", key, value);
7272

7373
var component = key.split(":");
7474
var customerId = Long.parseLong(component[0]);
@@ -77,7 +77,7 @@ public void persistUsageStats() {
7777
if (window < currentWindow) {
7878
var customer = customerService.getCustomerById(customerId);
7979
if (customer.isEmpty()) {
80-
logger.warn("failed to find customer with id {}", customerId);
80+
logger.warn("Failed to find customer with id {}", customerId);
8181
} else {
8282
UsageStats stats = new UsageStats();
8383

‎server/src/main/java/org/eclipse/openvsx/ratelimit/config/RateLimitConfig.java‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232
import org.springframework.cache.caffeine.CaffeineCacheManager;
3333
import org.springframework.context.annotation.Bean;
3434
import org.springframework.context.annotation.Configuration;
35+
import org.springframework.expression.ExpressionParser;
36+
import org.springframework.expression.spel.SpelCompilerMode;
37+
import org.springframework.expression.spel.SpelParserConfiguration;
38+
import org.springframework.expression.spel.standard.SpelExpressionParser;
3539
import redis.clients.jedis.DefaultJedisClientConfig;
3640
import redis.clients.jedis.HostAndPort;
3741
import redis.clients.jedis.JedisCluster;
@@ -48,6 +52,15 @@ public class RateLimitConfig {
4852

4953
private final Logger logger = LoggerFactory.getLogger(RateLimitConfig.class);
5054

55+
@Bean
56+
@ConditionalOnMissingBean(ExpressionParser.class)
57+
public ExpressionParser expressionParser() {
58+
SpelParserConfiguration config = new SpelParserConfiguration(
59+
SpelCompilerMode.IMMEDIATE,
60+
this.getClass().getClassLoader());
61+
return new SpelExpressionParser(config);
62+
}
63+
5164
@Bean
5265
public JedisCluster jedisCluster(RedisProperties properties) {
5366
logger.info("Configure jedis-cluster rate-limiting cache");
@@ -70,8 +83,8 @@ public JedisCluster jedisCluster(RedisProperties properties) {
7083

7184
@Bean
7285
public Cache<Object, Object> customerCache(
73-
@Value("${ovsx.caching.customer.tti:PT1H}") Duration timeToIdle,
74-
@Value("${ovsx.caching.customer.max-size:10000}") long maxSize
86+
@Value("${ovsx.caching.customer.tti:P1D}") Duration timeToIdle,
87+
@Value("${ovsx.caching.customer.max-size:100}") long maxSize
7588
) {
7689
return Caffeine.newBuilder()
7790
.expireAfterAccess(timeToIdle)
@@ -83,21 +96,8 @@ public Cache<Object, Object> customerCache(
8396

8497
@Bean
8598
public Cache<Object, Object> tierCache(
86-
@Value("${ovsx.caching.tier.tti:PT1H}") Duration timeToIdle,
87-
@Value("${ovsx.caching.tier.max-size:10000}") long maxSize
88-
) {
89-
return Caffeine.newBuilder()
90-
.expireAfterAccess(timeToIdle)
91-
.maximumSize(maxSize)
92-
.scheduler(Scheduler.systemScheduler())
93-
.recordStats()
94-
.build();
95-
}
96-
97-
@Bean
98-
public Cache<Object, Object> bucketCache(
99-
@Value("${ovsx.caching.bucket.tti:PT1H}") Duration timeToIdle,
100-
@Value("${ovsx.caching.bucket.max-size:10000}") long maxSize
99+
@Value("${ovsx.caching.tier.tti:P1D}") Duration timeToIdle,
100+
@Value("${ovsx.caching.tier.max-size:20}") long maxSize
101101
) {
102102
return Caffeine.newBuilder()
103103
.expireAfterAccess(timeToIdle)

‎server/src/main/java/org/eclipse/openvsx/ratelimit/config/RateLimitProperties.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public class RateLimitProperties {
3333
@NotNull
3434
private Boolean enabled = false;
3535

36+
@NotBlank
37+
private String ipAddressFunction = "getRemoteAddr()";
38+
3639
@Valid
3740
private UsageStatsProperties usageStats = new UsageStatsProperties();
3841

@@ -58,6 +61,14 @@ public void setEnabled(boolean enabled) {
5861
this.enabled = enabled;
5962
}
6063

64+
public String getIpAddressFunction() {
65+
return ipAddressFunction;
66+
}
67+
68+
public void setIpAddressFunction(String ipAddressFunction) {
69+
this.ipAddressFunction = ipAddressFunction;
70+
}
71+
6172
public UsageStatsProperties getUsageStats() {
6273
return usageStats;
6374
}

‎server/src/main/java/org/eclipse/openvsx/ratelimit/filter/RateLimitServletFilter.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ protected void doFilterInternal(
6666
@NonNull FilterChain chain
6767
) throws ServletException, IOException {
6868
var identity = identityService.resolveIdentity(request);
69-
logger.debug("rate limit filter: {}: {}", request.getRequestURI(), identity.ipAddress());
69+
logger.debug("Rate limit filter: {}: {}", request.getRequestURI(), identity.ipAddress());
7070

7171
if (identity.isCustomer()) {
7272
var customer = identity.getCustomer();
@@ -84,7 +84,7 @@ protected void doFilterInternal(
8484
response.setHeader(HEADER_RATE_LIMIT_LIMIT, Long.toString(bucketPair.availableTokens()));
8585

8686
ConsumptionProbe probe = bucket.tryConsumeAndReturnRemaining(1);
87-
logger.debug("remainingTokens: {}", probe.getRemainingTokens());
87+
logger.debug("Remaining tokens for {}: {}", identity.cacheKey(), probe.getRemainingTokens());
8888
if (probe.isConsumed()) {
8989
response.setHeader(HEADER_RATE_LIMIT_REMAINING, Long.toString(probe.getRemainingTokens()));
9090
chain.doFilter(request, response);

‎server/src/main/java/org/eclipse/openvsx/ratelimit/jobs/CollectUsageStatsHandler.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ public CollectUsageStatsHandler(Optional<UsageStatsService> usageStatsService) {
3636
@Job(name = "Collect usage stats")
3737
public void run(HandlerJobRequest<?> jobRequest) throws Exception {
3838
if (usageStatsService != null) {
39-
logger.debug(">> start collecting usage data");
39+
logger.debug(">> Start collecting usage data");
4040
usageStatsService.persistUsageStats();
41-
logger.debug("<< finished collecting usage data");
41+
logger.debug("<< Finished collecting usage data");
4242
}
4343
}
4444
}

0 commit comments

Comments
 (0)