fix(gateway-engine): correct `SingletonRateLimitingMap` implementation
ADD
common/config/src/main/java/io/apiman/common/config/ConfigDirectoryFinder.java
+package io.apiman.common.config;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+/**
+ *
+ *
+ * @author Marc Savy {@literal <marc@blackparrotlabs.io>}
+ */
+public class ConfigDirectoryFinder {
+
+ /**
+ * Return the standard Apiman config directory.
+ *
+ * Following precedence is used:
+ * <ul>
+ * <li><code>${apiman.config.dir}</code></li>
+ * <li><code>${jboss.server.config.dir}</code></li>
+ * <li><code>${catalina.home}/conf</code></li>
+ * </ul>
+ */
+ public static Path getConfigDirectory() {
+ // Grand unified conf directory!
+ String confDir = System.getProperty("apiman.config.dir");
+ if (confDir != null) {
+ return Paths.get(confDir);
+ }
+
+ // If that wasn't set, then check to see if we're running in wildfly/eap
+ confDir = System.getProperty("jboss.server.config.dir");
+ if (confDir != null) {
+ return Paths.get(confDir);
+ }
+
+ // If that didn't work, try to locate a tomcat data directory
+ confDir = System.getProperty("catalina.home");
+ if (confDir != null) {
+ return Paths.get(confDir, "conf");
+ }
+ throw new IllegalStateException("No config directory has been set. Please set apiman.config.dir=<data dir>");
+ }
+
+ /**
+ * Return the standard Apiman data directory.
+ *
+ * Following precedence is used:
+ * <ul>
+ * <li><code>${apiman.data.dir}</code></li>
+ * <li><code>${jboss.server.data.dir}</code></li>
+ * <li><code>${catalina.home}/data</code></li>
+ * </ul>
+ */
+ public static Path getDataDirectory() {
+ // Grand unified data directory!
+ String dataDir = System.getProperty("apiman.data.dir");
+ if (dataDir != null) {
+ return Paths.get(dataDir);
+ }
+
+ // If that wasn't set, then check to see if we're running in wildfly/eap
+ dataDir = System.getProperty("jboss.server.data.dir");
+ if (dataDir != null) {
+ return Paths.get(dataDir, "apiman");
+ }
+
+ // If that didn't work, try to locate a tomcat data directory
+ dataDir = System.getProperty("catalina.home");
+ if (dataDir != null) {
+ return Paths.get(dataDir, "conf");
+ }
+ throw new IllegalStateException("No data directory has been set. Please set apiman.data.dir=<data dir>");
+ }
+
+}

MODIFY
common/util/pom.xml
common/util/pom.xml
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
- <scope>provided</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.datatype</groupId>

MODIFY
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/InMemoryRateLimiterComponent.java
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/InMemoryRateLimiterComponent.java
@@ -48,7 +48,6 @@ public class InMemoryRateLimiterComponent implements IRateLimiterComponent {
public void accept(String bucketId, RateBucketPeriod period, long limit, long increment, IAsyncResultHandler<RateLimitResponse> handler) {
RateLimiterBucket bucket;
RateLimitResponse response = new RateLimitResponse();
- int reset = 0;
synchronized (buckets) {
bucket = buckets.get(bucketId);
if (bucket == null) {
@@ -63,9 +62,9 @@ public class InMemoryRateLimiterComponent implements IRateLimiterComponent {
bucket.setCount(bucket.getCount() + increment);
bucket.setLast(System.currentTimeMillis());
}
- reset = (int) (bucket.getResetMillis(period) / 1000L);
- }
+ int reset = (int) (bucket.getResetMillis(period) / 1000L);
response.setReset(reset);
+ }
response.setRemaining(limit - bucket.getCount());
handler.handle(AsyncResultImpl.create(response));
}

MODIFY
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/SingleNodeRateLimiterComponent.java
gateway/engine/core/src/main/java/io/apiman/gateway/engine/impl/SingleNodeRateLimiterComponent.java
package io.apiman.gateway.engine.impl;
+import io.apiman.common.config.ConfigDirectoryFinder;
+import io.apiman.common.logging.ApimanLoggerFactory;
+import io.apiman.common.logging.IApimanLogger;
import io.apiman.gateway.engine.async.AsyncResultImpl;
import io.apiman.gateway.engine.async.IAsyncResultHandler;
import io.apiman.gateway.engine.components.IRateLimiterComponent;
@@ -27,10 +30,16 @@ import java.io.File;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
/**
* An implementation of the rate limiter component that is optimized for use
@@ -39,11 +48,14 @@ import java.util.concurrent.ConcurrentHashMap;
*/
public class SingleNodeRateLimiterComponent implements IRateLimiterComponent {
- private boolean isPersistent;
+ private final IApimanLogger LOGGER = ApimanLoggerFactory.getLogger(SingleNodeRateLimiterComponent.class);
+ private final boolean isPersistent;
+
+ private final Map<String, RateLimiterBucket> buckets = new HashMap<>();
private File savedRates;
- private ConcurrentHashMap<String, RateLimiterBucket> buckets = new ConcurrentHashMap<>();
private long lastSavedOn;
private long lastModifiedOn;
+ private final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
/**
* Constructor.
@@ -59,11 +71,15 @@ public class SingleNodeRateLimiterComponent implements IRateLimiterComponent {
public SingleNodeRateLimiterComponent(Map<String, String> config) {
isPersistent = "true".equals(config.get("persistence.enabled")); //$NON-NLS-1$//$NON-NLS-2$
if (isPersistent) {
- String ratesFilePath = config.get("persistence.file"); //$NON-NLS-1$
- if (ratesFilePath == null) {
- throw new RuntimeException("No 'persistence.file' configured - configuration of " + getClass().getName() + " failed."); //$NON-NLS-1$ //$NON-NLS-2$
+ String ratesPathRaw = config.get("persistence.file"); //$NON-NLS-1$
+ Path ratesFilePath;
+ if (ratesPathRaw == null || ratesPathRaw.isBlank()) {
+ ratesFilePath = ConfigDirectoryFinder.getDataDirectory().resolve("apiman-single-node-rate-limiting.properties");
+ } else {
+ ratesFilePath = Paths.get(ratesPathRaw);
}
- savedRates = new File(ratesFilePath);
+ LOGGER.info("Rate limiting data will be loaded from {0}", ratesFilePath);
+ savedRates = ratesFilePath.toFile();
if (savedRates.isFile()) {
loadBuckets();
}
@@ -77,13 +93,15 @@ public class SingleNodeRateLimiterComponent implements IRateLimiterComponent {
@Override
public void accept(String bucketId, RateBucketPeriod period, long limit, long increment,
IAsyncResultHandler<RateLimitResponse> handler) {
- RateLimiterBucket newBucket = new RateLimiterBucket();
- RateLimiterBucket existingBucket = buckets.putIfAbsent(bucketId, newBucket);
- RateLimiterBucket bucket = existingBucket == null ? newBucket : existingBucket;
-
- boolean rateModified = bucket.resetIfNecessary(period);
-
+ RateLimiterBucket bucket;
RateLimitResponse response = new RateLimitResponse();
+ synchronized (buckets) {
+ bucket = buckets.get(bucketId);
+ if (bucket == null) {
+ bucket = new RateLimiterBucket();
+ buckets.put(bucketId, bucket);
+ }
+ boolean rateModified = bucket.resetIfNecessary(period);
if (bucket.getCount() > limit) {
response.setAccepted(false);
} else {
@@ -92,15 +110,14 @@ public class SingleNodeRateLimiterComponent implements IRateLimiterComponent {
bucket.setLast(System.currentTimeMillis());
rateModified = true;
}
-
+ int reset = (int) (bucket.getResetMillis(period) / 1000L);
+ response.setReset(reset);
if (rateModified) {
lastModifiedOn = System.currentTimeMillis();
}
-
- int reset = (int) (bucket.getResetMillis(period) / 1000L);
- response.setReset(reset);
+ }
response.setRemaining(limit - bucket.getCount());
- handler.handle(AsyncResultImpl.<RateLimitResponse>create(response));
+ handler.handle(AsyncResultImpl.create(response));
}
/**
@@ -111,27 +128,13 @@ public class SingleNodeRateLimiterComponent implements IRateLimiterComponent {
private void startBucketSavingThread(Map<String, String> config) {
String delay = config.get("persistence.delay"); //$NON-NLS-1$
if (delay == null) {
- delay = "30"; //$NON-NLS-1$
+ delay = "5"; //$NON-NLS-1$
}
String period = config.get("persistence.period"); //$NON-NLS-1$
if (period == null) {
- period = "60"; //$NON-NLS-1$
- }
- final long delayL = new Long(delay) * 1000L;
- final long periodL = new Long(period) * 1000L;
-
- Thread thread = new Thread(new Runnable() {
- @Override
- public void run() {
- try { Thread.sleep(delayL); } catch (InterruptedException e) { }
- boolean done = false;
- while (!done) {
- saveBuckets();
- try { Thread.sleep(periodL); } catch (InterruptedException e) { }
- }
+ period = "10"; //$NON-NLS-1$
}
- });
- thread.start();
+ executor.scheduleAtFixedRate(this::saveBuckets, Long.parseLong(delay), Long.parseLong(period), TimeUnit.SECONDS);
}
/**
@@ -139,14 +142,14 @@ public class SingleNodeRateLimiterComponent implements IRateLimiterComponent {
*/
private void saveBuckets() {
if (lastModifiedOn > lastSavedOn) {
- System.out.println("Persisting current rates to: " + savedRates); //$NON-NLS-1$
+ LOGGER.info("Persisting current rates to: {0}", savedRates); //$NON-NLS-1$
Properties props = new Properties();
for (Entry<String, RateLimiterBucket> entry : buckets.entrySet()) {
String value = entry.getValue().getCount() + "|" + entry.getValue().getLast(); //$NON-NLS-1$
props.setProperty(entry.getKey(), value);
}
try (FileWriter writer = new FileWriter(savedRates)) {
- props.store(writer, "All apiman rate limits"); //$NON-NLS-1$
+ props.store(writer, "All Apiman rate limits"); //$NON-NLS-1$
} catch (IOException e) {
e.printStackTrace();
}
@@ -162,18 +165,18 @@ public class SingleNodeRateLimiterComponent implements IRateLimiterComponent {
try (FileReader reader = new FileReader(savedRates)) {
props.load(reader);
} catch (IOException e) {
- throw new RuntimeException(e);
+ throw new UncheckedIOException(e);
}
for (Entry<Object, Object> entry : props.entrySet()) {
String key = entry.getKey().toString();
String value = entry.getValue().toString();
- String [] split = value.split("="); //$NON-NLS-1$
- long count = new Long(split[0]);
- long last = new Long(split[1]);
+ String [] split = value.split("\\|"); //$NON-NLS-1$
+ long count = Long.parseLong(split[0]);
+ long last = Long.parseLong(split[1]);
RateLimiterBucket bucket = new RateLimiterBucket();
bucket.setCount(count);
bucket.setLast(last);
- this.buckets.put(key, bucket);
+ buckets.put(key, bucket);
}
}

fix(gateway-engine): correct `SingletonRateLimitingMap` implementation