Merge pull request #46 from marcosbarbero/spring-cloud-zuul-ratelimit-45

Author: marcosbarbero

.travis.yml

@@ -18,6 +18,7 @@ cache:
 notifications:
   email:
   - marcos.hgb@gmail.com
+  - lchayoun@gmail.com
 env:
   global:
     secure: B5QVZ6SWzgQmlKmStQCHg0RcQPVbkti2Z3s5vi1gR0Oef6KYxBpkzP87yES08OvHG7Ha6XfJ28feML8URtTcdhBtkr4aKMGy9R+VvY9i7hykPLGnuejip89oTt4k8SlWkqmvzx4G6qB1YbooVdAbvaqARlS+t1RjSfOJU5kIBMyyrBshxPlavfzDp55tUcBrfbDEbQmdnRY84gXng+gz+ceJTzg/vxNPyKCcG5Ap/2qW0lH+ZRcJTUgtmNZZIpiw56YD3T/ltc3wVIHEgFzCGUiZAaRtHJW3PGComDKd9Z3JPaZO1nuJlD999Kok4I9IEZjpu7g1mXlaioYsAwholJTsugoinEZ6JChaeT/92ZWY1Ua0vT5YJjz0QCGzlhJ12I751QFk1Lam9hwlhZ7LafDi1p8vhQuN1I5NOulgnoXwZboZ9/2jbNp63o1nq773jULJiAsMXY+Pv8SLCNKzVANyEK9ECYKG3DSxuVLIpX/j1cYWf/UH5FxQ5aiDYnZpBVblWfsQxPuyVjHnIyaAvw4LsTq4DfRG8JMqjjGr0IUGV6Xa/6/hCfx5ojvWFQ17YJeGHJMGAfPwvsTPhLTNl6Wm1Iw6Lt3N9KJ6VaGnJDuza/mJuTrTAdGhZ28bcevNq03vW3bFenLh0EC8zamBBA9eNuqqDak6Q1OzV3z6dRo=

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/RateLimitAutoConfiguration.java

@@ -27,6 +27,8 @@
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.properties.RateLimitProperties;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.properties.RateLimitProperties.Policy;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.ConsulRateLimiter;
+import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.DefaultRateLimiterErrorHandler;
+import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.RateLimiterErrorHandler;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.InMemoryRateLimiter;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.RedisRateLimiter;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.springdata.JpaRateLimiter;
@@ -64,6 +66,12 @@ public class RateLimitAutoConfiguration {
 
     private final UrlPathHelper urlPathHelper = new UrlPathHelper();
 
+    @Bean
+    @ConditionalOnMissingBean(RateLimiterErrorHandler.class)
+    public RateLimiterErrorHandler rateLimiterErrorHandler() {
+        return new DefaultRateLimiterErrorHandler();
+    }
+
     @Bean
     public ZuulFilter rateLimiterPreFilter(final RateLimiter rateLimiter,
                                            final RateLimitProperties rateLimitProperties,
@@ -99,8 +107,9 @@ public StringRedisTemplate redisTemplate(final RedisConnectionFactory connection
         }
 
         @Bean
-        public RateLimiter redisRateLimiter(@Qualifier("rateLimiterRedisTemplate") final RedisTemplate redisTemplate) {
-            return new RedisRateLimiter(redisTemplate);
+        public RateLimiter redisRateLimiter(RateLimiterErrorHandler rateLimiterErrorHandler,
+            @Qualifier("rateLimiterRedisTemplate") final RedisTemplate redisTemplate) {
+            return new RedisRateLimiter(rateLimiterErrorHandler, redisTemplate);
         }
     }
 
@@ -110,8 +119,9 @@ public RateLimiter redisRateLimiter(@Qualifier("rateLimiterRedisTemplate") final
     public static class ConsulConfiguration {
 
         @Bean
-        public RateLimiter consultRateLimiter(final ConsulClient consulClient, final ObjectMapper objectMapper) {
-            return new ConsulRateLimiter(consulClient, objectMapper);
+        public RateLimiter consultRateLimiter(final RateLimiterErrorHandler rateLimiterErrorHandler,
+            final ConsulClient consulClient, final ObjectMapper objectMapper) {
+            return new ConsulRateLimiter(rateLimiterErrorHandler, consulClient, objectMapper);
         }
 
     }
@@ -123,8 +133,9 @@ public RateLimiter consultRateLimiter(final ConsulClient consulClient, final Obj
     public static class SpringDataConfiguration {
 
         @Bean
-        public RateLimiter springDataRateLimiter(final RateLimiterRepository rateLimiterRepository) {
-            return new JpaRateLimiter(rateLimiterRepository);
+        public RateLimiter springDataRateLimiter(final RateLimiterErrorHandler rateLimiterErrorHandler,
+            final RateLimiterRepository rateLimiterRepository) {
+            return new JpaRateLimiter(rateLimiterErrorHandler, rateLimiterRepository);
         }
 
     }
@@ -134,8 +145,8 @@ public RateLimiter springDataRateLimiter(final RateLimiterRepository rateLimiter
     public static class InMemoryConfiguration {
 
         @Bean
-        public RateLimiter inMemoryRateLimiter() {
-            return new InMemoryRateLimiter();
+        public RateLimiter inMemoryRateLimiter(final RateLimiterErrorHandler rateLimiterErrorHandler) {
+            return new InMemoryRateLimiter(rateLimiterErrorHandler);
         }
     }
 

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/AbstractRateLimiter.java

@@ -16,13 +16,14 @@
 
 package com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository;
 
+import static java.util.concurrent.TimeUnit.SECONDS;
+
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.Rate;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.RateLimiter;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.properties.RateLimitProperties.Policy;
-
 import java.util.Date;
-
-import static java.util.concurrent.TimeUnit.SECONDS;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
 
 /**
  * Abstract implementation for {@link RateLimiter}.
@@ -31,22 +32,34 @@
  * @author Marcos Barbero
  * @since 2017-08-28
  */
+@Slf4j
+@RequiredArgsConstructor
 public abstract class AbstractRateLimiter implements RateLimiter {
 
-    protected abstract Rate getRate(String key);
+    private final RateLimiterErrorHandler rateLimiterErrorHandler;
 
+    protected abstract Rate getRate(String key);
     protected abstract void saveRate(Rate rate);
 
     @Override
     public synchronized Rate consume(final Policy policy, final String key, final Long requestTime) {
         Rate rate = this.create(policy, key);
         updateRate(policy, rate, requestTime);
-        saveRate(rate);
+        try {
+            saveRate(rate);
+        } catch (RuntimeException e) {
+            rateLimiterErrorHandler.handleSaveError(key, e);
+        }
         return rate;
     }
 
     private Rate create(final Policy policy, final String key) {
-        Rate rate = this.getRate(key);
+        Rate rate = null;
+        try {
+            rate = this.getRate(key);
+        } catch (RuntimeException e) {
+            rateLimiterErrorHandler.handleFetchError(key, e);
+        }
 
         if (!isExpired(rate)) {
             return rate;

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/ConsulRateLimiter.java

@@ -16,19 +16,16 @@
 
 package com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository;
 
+import static org.springframework.util.StringUtils.hasText;
+
 import com.ecwid.consul.v1.ConsulClient;
 import com.ecwid.consul.v1.kv.model.GetValue;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.Rate;
-
 import java.io.IOException;
-
-import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 
-import static org.springframework.util.StringUtils.hasText;
-
 /**
  * Consul rate limiter configuration.
  *
@@ -37,12 +34,18 @@
  * @since 2017-08-15
  */
 @Slf4j
-@RequiredArgsConstructor
 public class ConsulRateLimiter extends AbstractRateLimiter {
 
     private final ConsulClient consulClient;
     private final ObjectMapper objectMapper;
 
+    public ConsulRateLimiter(RateLimiterErrorHandler rateLimiterErrorHandler,
+        ConsulClient consulClient, ObjectMapper objectMapper) {
+        super(rateLimiterErrorHandler);
+        this.consulClient = consulClient;
+        this.objectMapper = objectMapper;
+    }
+
     @Override
     protected Rate getRate(String key) {
         Rate rate = null;

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/DefaultRateLimiterErrorHandler.java

@@ -0,0 +1,22 @@
+package com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository;
+
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+public class DefaultRateLimiterErrorHandler implements RateLimiterErrorHandler {
+
+    @Override
+    public void handleSaveError(String key, Exception e) {
+        log.error("Failed saving rate for " + key + ", returning unsaved rate", e);
+    }
+
+    @Override
+    public void handleFetchError(String key, Exception e) {
+        log.error("Failed retrieving rate for " + key + ", will create new rate", e);
+    }
+
+    @Override
+    public void handleError(String msg, Exception e) {
+        log.error(msg, e);
+    }
+}

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/InMemoryRateLimiter.java

@@ -32,6 +32,10 @@ public class InMemoryRateLimiter extends AbstractRateLimiter {
 
     private Map<String, Rate> repository = new ConcurrentHashMap<>();
 
+    public InMemoryRateLimiter(RateLimiterErrorHandler rateLimiterErrorHandler) {
+        super(rateLimiterErrorHandler);
+    }
+
     @Override
     protected Rate getRate(String key) {
         return this.repository.get(key);

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/RateLimiterErrorHandler.java

@@ -0,0 +1,8 @@
+package com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository;
+
+public interface RateLimiterErrorHandler {
+
+    void handleSaveError(String key, Exception e);
+    void handleFetchError(String key, Exception e);
+    void handleError(String msg, Exception e);
+}

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/RedisRateLimiter.java

@@ -16,26 +16,27 @@
 
 package com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository;
 
+import static java.util.concurrent.TimeUnit.SECONDS;
+
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.Rate;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.RateLimiter;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.properties.RateLimitProperties.Policy;
-
-import org.springframework.data.redis.core.RedisTemplate;
-
 import lombok.RequiredArgsConstructor;
-
-import static java.util.concurrent.TimeUnit.SECONDS;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.data.redis.core.RedisTemplate;
 
 /**
  * @author Marcos Barbero
  * @author Liel Chayoun
  */
+@Slf4j
 @RequiredArgsConstructor
 @SuppressWarnings("unchecked")
 public class RedisRateLimiter implements RateLimiter {
 
     private static final String QUOTA_SUFFIX = "-quota";
 
+    private final RateLimiterErrorHandler rateLimiterErrorHandler;
     private final RedisTemplate redisTemplate;
 
     @Override
@@ -55,7 +56,13 @@ private void calcRemainingLimit(Long limit, Long refreshInterval,
         if (limit != null) {
             handleExpiration(key, refreshInterval, rate);
             long usage = requestTime == null ? 1L : 0L;
-            Long current = this.redisTemplate.boundValueOps(key).increment(usage);
+            Long current = 0L;
+            try {
+                current = this.redisTemplate.boundValueOps(key).increment(usage);
+            } catch (RuntimeException e) {
+                String msg = "Failed retrieving rate for " + key + ", will return limit";
+                rateLimiterErrorHandler.handleError(msg, e);
+            }
             rate.setRemaining(Math.max(-1, limit - current));
         }
     }
@@ -66,17 +73,29 @@ private void calcRemainingQuota(Long quota, Long refreshInterval,
             String quotaKey = key + QUOTA_SUFFIX;
             handleExpiration(quotaKey, refreshInterval, rate);
             Long usage = requestTime != null ? requestTime : 0L;
-            Long current = this.redisTemplate.boundValueOps(quotaKey).increment(usage);
+            Long current = 0L;
+            try {
+                current = this.redisTemplate.boundValueOps(quotaKey).increment(usage);
+            } catch (RuntimeException e) {
+                String msg = "Failed retrieving rate for " + quotaKey + ", will return quota limit";
+                rateLimiterErrorHandler.handleError(msg, e);
+            }
             rate.setRemainingQuota(Math.max(-1, quota - current));
         }
     }
 
     private void handleExpiration(String key, Long refreshInterval, Rate rate) {
-        Long expire = this.redisTemplate.getExpire(key);
-        if (expire == null || expire == -1) {
-            this.redisTemplate.expire(key, refreshInterval, SECONDS);
-            expire = refreshInterval;
+        Long expire = null;
+        try {
+            expire = this.redisTemplate.getExpire(key);
+            if (expire == null || expire == -1) {
+                this.redisTemplate.expire(key, refreshInterval, SECONDS);
+                expire = refreshInterval;
+            }
+        } catch (RuntimeException e) {
+            String msg = "Failed retrieving expiration for " + key + ", will reset now";
+            rateLimiterErrorHandler.handleError(msg, e);
         }
-        rate.setReset(SECONDS.toMillis(expire));
+        rate.setReset(SECONDS.toMillis(expire == null ? 0L : expire));
     }
 }

spring-cloud-zuul-ratelimit-core/src/main/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/springdata/JpaRateLimiter.java

@@ -18,8 +18,7 @@
 
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.Rate;
 import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.AbstractRateLimiter;
-
-import lombok.RequiredArgsConstructor;
+import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository.RateLimiterErrorHandler;
 
 /**
  * In memory rate limiter configuration for dev environment.
@@ -28,11 +27,16 @@
  * @author Liel Chayoun
  * @since 2017-06-23
  */
-@RequiredArgsConstructor
 public class JpaRateLimiter extends AbstractRateLimiter {
 
     private final RateLimiterRepository repository;
 
+    public JpaRateLimiter(RateLimiterErrorHandler rateLimiterErrorHandler,
+        RateLimiterRepository repository) {
+        super(rateLimiterErrorHandler);
+        this.repository = repository;
+    }
+
     @Override
     protected Rate getRate(String key) {
         return this.repository.findOne(key);

spring-cloud-zuul-ratelimit-core/src/test/java/com/marcosbarbero/cloud/autoconfigure/zuul/ratelimit/config/repository/AbstractRateLimiterTest.java

@@ -0,0 +1,53 @@
+package com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.repository;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.matches;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.marcosbarbero.cloud.autoconfigure.zuul.ratelimit.config.properties.RateLimitProperties.Policy;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class AbstractRateLimiterTest {
+
+    private AbstractRateLimiter abstractRateLimiter;
+    private RateLimiterErrorHandler rateLimiterErrorHandler;
+
+    @Before
+    public void setUp() throws Exception {
+        abstractRateLimiter = Mockito.mock(AbstractRateLimiter.class, Mockito.CALLS_REAL_METHODS);
+        Field rateLimiterErrorHandler = AbstractRateLimiter.class.getDeclaredField("rateLimiterErrorHandler");
+        boolean accessible = rateLimiterErrorHandler.isAccessible();
+        rateLimiterErrorHandler.setAccessible(true);
+        Field modifiersField = Field.class.getDeclaredField("modifiers");
+        modifiersField.setAccessible(true);
+        modifiersField.setInt(rateLimiterErrorHandler, rateLimiterErrorHandler.getModifiers() & ~Modifier.FINAL);
+        this.rateLimiterErrorHandler = Mockito.mock(RateLimiterErrorHandler.class);
+        rateLimiterErrorHandler.set(abstractRateLimiter, this.rateLimiterErrorHandler);
+        rateLimiterErrorHandler.setAccessible(accessible);
+        modifiersField.setInt(rateLimiterErrorHandler, rateLimiterErrorHandler.getModifiers() & Modifier.FINAL);
+    }
+
+    @Test
+    public void testConsumeGetRateException() {
+        when(abstractRateLimiter.getRate(any())).thenThrow(new RuntimeException());
+        Policy policy = new Policy();
+        policy.setLimit(100L);
+        abstractRateLimiter.consume(policy, "key", 0L);
+        verify(rateLimiterErrorHandler).handleFetchError(matches("key"), any());
+    }
+
+    @Test
+    public void testConsumeSaveRateException() {
+        doThrow(new RuntimeException()).when(abstractRateLimiter).saveRate(any());
+        Policy policy = new Policy();
+        policy.setLimit(100L);
+        abstractRateLimiter.consume(policy, "key", 0L);
+        verify(rateLimiterErrorHandler).handleSaveError(matches("key"), any());
+    }
+}