diff --git a/.github/workflows/main-ci.yml b/.github/workflows/main-ci.yml new file mode 100644 index 0000000..4fcd238 --- /dev/null +++ b/.github/workflows/main-ci.yml @@ -0,0 +1,21 @@ +name: Java CI with Maven + +on: + push: + branches: [ "main" ] + pull_request: + branches: [ "main" ] + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + cache: maven + - name: Maven build&tests + run: mvn -ntp clean verify --file pom.xml \ No newline at end of file diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 53e7228..0000000 --- a/.travis.yml +++ /dev/null @@ -1,10 +0,0 @@ -language: java - -jdk: - - oraclejdk8 - -cache: - directories: - - "$HOME/.m2/repository" - -dist: trusty diff --git a/pom.xml b/pom.xml index f844d7f..9e36d13 100644 --- a/pom.xml +++ b/pom.xml @@ -5,13 +5,13 @@ com.coveo spillway - 2.2.0-SNAPSHOT + 3.0.0 jar ${project.artifactId} Simple, flexible, distributable rate limiting for your application - http://github.com/coveo/spillway + https://github.com/coveooss/spillway @@ -24,25 +24,25 @@ Guillaume Simard Coveo - http://github.com/coveo + https://github.com/coveo Emile Fugulin Coveo - http://github.com/coveo + https://github.com/coveo Pierre-Alexandre St-Jean http://github.com/pastjean Coveo - http://github.com/coveo + https://github.com/coveo - scm:git:git@github.com:coveo/spillway.git - scm:git:git@github.com:coveo/spillway.git - http://github.com/coveo/spillway + scm:git:git@github.com:coveooss/spillway.git + scm:git:git@github.com:coveooss/spillway.git + https://github.com/coveooss/spillway @@ -59,49 +59,96 @@ UTF-8 - 1.8 - 1.8 + 17 + + 3.17.0 + + 5.0.2 + + 5.14.2 + + 5.11.3 + + 2.0.12 + + 1.4.4 + + 1.4.3 + + + + org.mockito + mockito-bom + ${mockito.version} + pom + import + + + org.junit + junit-bom + ${junit.version} + pom + import + + + + org.slf4j slf4j-api - 1.7.30 + ${slf4j.version} org.apache.commons commons-lang3 - 3.10 + ${commons-lang3.version} redis.clients jedis - 4.2.1 + ${jedis.version} - junit - junit - 4.12 + org.slf4j + slf4j-simple + ${slf4j.version} + test + + + org.junit.jupiter + junit-jupiter-engine + test + + + org.mockito + mockito-core test org.mockito - mockito-all - 1.10.19 + mockito-junit-jupiter test com.google.truth truth - 0.28 + ${truth.version} test + + + junit + junit + + - com.github.kstyrc + com.github.codemonstur embedded-redis - 0.6 + ${embedded-redis.version} test @@ -110,26 +157,32 @@ org.apache.maven.plugins - maven-source-plugin - 3.0.0 + maven-compiler-plugin + 3.13.0 + full + true + lines,vars,source + false - - - attach-sources - - jar - - - org.apache.maven.plugins - maven-javadoc-plugin - 2.10.3 + maven-surefire-plugin + 3.5.1 + + false + + + + org.apache.maven.plugins + maven-source-plugin + 3.3.1 + + - attach-javadocs + attach-sources jar @@ -183,6 +236,19 @@ release + + org.apache.maven.plugins + maven-javadoc-plugin + 3.10.1 + + + attach-javadocs + + jar + + + + org.sonatype.plugins nexus-staging-maven-plugin @@ -197,7 +263,7 @@ org.apache.maven.plugins maven-gpg-plugin - 1.6 + 3.2.7 sign-artifacts @@ -205,6 +271,12 @@ sign + + + --pinentry-mode + loopback + + diff --git a/src/main/java/com/coveo/spillway/storage/RedisStorage.java b/src/main/java/com/coveo/spillway/storage/RedisStorage.java index 2ef0589..2ace2ed 100644 --- a/src/main/java/com/coveo/spillway/storage/RedisStorage.java +++ b/src/main/java/com/coveo/spillway/storage/RedisStorage.java @@ -114,7 +114,7 @@ public Map addAndGet(Collection requests) { pipeline.sync(); } catch (Throwable e) { - logger.error("An exception occured while publishing limits to Redis.", e); + logger.error("An exception occurred while publishing limits to Redis.", e); } } diff --git a/src/test/java/com/coveo/spillway/SpillwayTest.java b/src/test/java/com/coveo/spillway/SpillwayTest.java index c22ca27..8972d59 100644 --- a/src/test/java/com/coveo/spillway/SpillwayTest.java +++ b/src/test/java/com/coveo/spillway/SpillwayTest.java @@ -10,13 +10,12 @@ import com.coveo.spillway.limit.override.LimitOverrideBuilder; import com.coveo.spillway.storage.InMemoryStorage; import com.coveo.spillway.storage.LimitUsageStorage; -import com.coveo.spillway.storage.utils.AddAndGetRequest; import com.coveo.spillway.trigger.LimitTriggerCallback; import com.coveo.spillway.trigger.ValueThresholdTrigger; import com.google.common.collect.ImmutableMap; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import java.time.Clock; @@ -26,16 +25,15 @@ import java.util.Optional; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyListOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.Matchers.eq; public class SpillwayTest { @@ -47,27 +45,10 @@ public class SpillwayTest { private static final String JOHN = "john"; private static final String A_LIMIT_NAME = "perUser"; - private class User { + private record User(String name, String ip) {} - private String name; - private String ip; - - User(String name, String ip) { - this.name = name; - this.ip = ip; - } - - String getName() { - return name; - } - - String getIp() { - return ip; - } - } - - private User john = new User(JOHN, "127.0.0.1"); - private User gina = new User("gina", "127.0.0.1"); + private final User john = new User(JOHN, "127.0.0.1"); + private final User gina = new User("gina", "127.0.0.1"); private LimitUsageStorage mockedStorage; private LimitUsageStorage inMemoryStorage; @@ -76,7 +57,7 @@ String getIp() { private SpillwayFactory inMemoryFactory; private SpillwayFactory mockedFactory; - @Before + @BeforeEach public void setup() { clock = mock(Clock.class); inMemoryStorage = new InMemoryStorage(); @@ -89,9 +70,9 @@ public void setup() { } @Test - public void simpleLimit() throws Exception { + public void simpleLimit() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); assertThat(spillway.tryCall(john)).isTrue(); @@ -99,21 +80,23 @@ public void simpleLimit() throws Exception { assertThat(spillway.tryCall(john)).isFalse(); // Third tryCall fails } - @Test(expected = SpillwayLimitsWithSameNameException.class) - public void multipleLimitsWithOverlap() throws Exception { + @Test + public void multipleLimitsWithOverlap() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(5).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(5).per(Duration.ofHours(1)).build(); Limit limit2 = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofSeconds(1)).build(); - inMemoryFactory.enforce("testResource", limit1, limit2); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofSeconds(1)).build(); + + assertThrows( + SpillwayLimitsWithSameNameException.class, + () -> inMemoryFactory.enforce("testResource", limit1, limit2)); } @Test - public void multipleLimitsNoOverlap() throws Exception { - Limit ipLimit = - LimitBuilder.of("perIp", User::getIp).to(1).per(Duration.ofHours(1)).build(); + public void multipleLimitsNoOverlap() { + Limit ipLimit = LimitBuilder.of("perIp", User::ip).to(1).per(Duration.ofHours(1)).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", ipLimit, userLimit); assertThat(spillway.tryCall(john)).isTrue(); @@ -121,9 +104,9 @@ public void multipleLimitsNoOverlap() throws Exception { } @Test - public void multipleResourcesEachHaveTheirOwnLimit() throws Exception { + public void multipleResourcesEachHaveTheirOwnLimit() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); Spillway spillway1 = inMemoryFactory.enforce("testResource1", userLimit); Spillway spillway2 = inMemoryFactory.enforce("testResource2", userLimit); @@ -134,19 +117,19 @@ public void multipleResourcesEachHaveTheirOwnLimit() throws Exception { } @Test - public void canUseDefaultPropertyExtractor() throws Exception { + public void canUseDefaultPropertyExtractor() { Limit userLimit = LimitBuilder.of("perUser").to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); - assertThat(spillway.tryCall(john.getName())).isTrue(); - assertThat(spillway.tryCall(john.getName())).isFalse(); + assertThat(spillway.tryCall(john.name())).isTrue(); + assertThat(spillway.tryCall(john.name())).isFalse(); } @Test - public void canBeNotifiedWhenLimitIsExceeded() throws Exception { + public void canBeNotifiedWhenLimitIsExceeded() { LimitTriggerCallback callback = mock(LimitTriggerCallback.class); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callback) @@ -162,7 +145,7 @@ public void canBeNotifiedWhenLimitIsExceeded() throws Exception { @Test public void callThrowsAnExceptionWhichContainsAllTheDetails() throws Exception { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); spillway.call(john); @@ -179,9 +162,8 @@ public void callThrowsAnExceptionWhichContainsAllTheDetails() throws Exception { @Test public void callThrowsForMultipleBreachedLimits() throws Exception { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); - Limit ipLimit = - LimitBuilder.of("perIp", User::getIp).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); + Limit ipLimit = LimitBuilder.of("perIp", User::ip).to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit, ipLimit); @@ -198,9 +180,9 @@ public void callThrowsForMultipleBreachedLimits() throws Exception { } @Test - public void bucketChangesWhenTimePasses() throws Exception { + public void bucketChangesWhenTimePasses() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofSeconds(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofSeconds(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); assertThat(spillway.tryCall(john)).isTrue(); @@ -213,9 +195,9 @@ public void bucketChangesWhenTimePasses() throws Exception { } @Test - public void capacityNotIncrementedIfLimitTriggered() throws Exception { + public void capacityNotIncrementedIfLimitTriggered() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(0).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(0).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); assertThat(spillway.tryCall(john)).isFalse(); @@ -228,7 +210,7 @@ public void capacityNotIncrementedIfLimitTriggered() throws Exception { @Test public void canGetCurrentLimitStatus() throws Exception { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); spillway.call(john); @@ -243,7 +225,7 @@ public void canGetCurrentLimitStatus() throws Exception { limitStatuses .keySet() .stream() - .filter(key -> key.getProperty().equals(john.getName())) + .filter(key -> key.getProperty().equals(john.name())) .findFirst(); assertThat(johnKey.isPresent()).isTrue(); assertThat(limitStatuses.get(johnKey.get())).isEqualTo(2); @@ -252,33 +234,33 @@ public void canGetCurrentLimitStatus() throws Exception { limitStatuses .keySet() .stream() - .filter(key -> key.getProperty().equals(gina.getName())) + .filter(key -> key.getProperty().equals(gina.name())) .findFirst(); assertThat(ginaKey.isPresent()).isTrue(); assertThat(limitStatuses.get(ginaKey.get())).isEqualTo(1); } @Test - public void ifCallbackThrowsWeIgnoreThatCallbackAndContinue() throws Exception { + public void ifCallbackThrowsWeIgnoreThatCallbackAndContinue() { LimitTriggerCallback callbackThatIsOkay = mock(LimitTriggerCallback.class); LimitTriggerCallback callbackThatThrows = mock(LimitTriggerCallback.class); doThrow(RuntimeException.class) .when(callbackThatThrows) .trigger(any(LimitDefinition.class), any(Object.class)); Limit ipLimit1 = - LimitBuilder.of("perIp1", User::getIp) + LimitBuilder.of("perIp1", User::ip) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callbackThatThrows) .build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callbackThatIsOkay) .build(); Limit ipLimit2 = - LimitBuilder.of("perIp2", User::getIp) + LimitBuilder.of("perIp2", User::ip) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callbackThatThrows) @@ -295,9 +277,9 @@ public void ifCallbackThrowsWeIgnoreThatCallbackAndContinue() throws Exception { } @Test - public void canExceedLimitByDoingALargeIncrement() throws Exception { + public void canExceedLimitByDoingALargeIncrement() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); try { @@ -310,9 +292,9 @@ public void canExceedLimitByDoingALargeIncrement() throws Exception { } @Test - public void costCanBeALargeNumber() throws Exception { + public void costCanBeALargeNumber() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(4).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(4).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); assertThat(spillway.tryCall(john, 4)).isTrue(); @@ -320,12 +302,12 @@ public void costCanBeALargeNumber() throws Exception { } @Test - public void canAddLimitTriggers() throws Exception { + public void canAddLimitTriggers() { LimitTriggerCallback callback = mock(LimitTriggerCallback.class); ValueThresholdTrigger trigger = new ValueThresholdTrigger(5, callback); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(15) .per(Duration.ofHours(1)) .withLimitTrigger(trigger) @@ -342,8 +324,8 @@ public void canAddLimitTriggers() throws Exception { } @Test - public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() throws Exception { - when(mockedStorage.addAndGet(anyListOf(AddAndGetRequest.class))) + public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() { + when(mockedStorage.addAndGet(anyCollection())) .thenReturn( ImmutableMap.of( mock(LimitKey.class), 1, mock(LimitKey.class), 2, mock(LimitKey.class), 3)); @@ -351,7 +333,7 @@ public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() throws Ex LimitTriggerCallback callback = mock(LimitTriggerCallback.class); ValueThresholdTrigger trigger = new ValueThresholdTrigger(5, callback); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(15) .per(Duration.ofHours(1)) .withLimitTrigger(trigger) @@ -364,11 +346,11 @@ public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() throws Ex } @Test - public void canAddCapacityLimitOverride() throws Exception { + public void canAddCapacityLimitOverride() { LimitOverride override = LimitOverrideBuilder.of(JOHN).to(A_CAPACITY).per(A_DURATION).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(A_HIGHER_CAPACITY) .per(A_DURATION) .withLimitOverride(override) @@ -379,12 +361,12 @@ public void canAddCapacityLimitOverride() throws Exception { } @Test - public void canAddExpirationLimitOverride() throws Exception { + public void canAddExpirationLimitOverride() { LimitOverride override = LimitOverrideBuilder.of(JOHN).to(A_CAPACITY).per(A_SHORT_DURATION).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(A_CAPACITY) .per(A_DURATION) .withLimitOverride(override) @@ -401,7 +383,7 @@ public void canAddExpirationLimitOverride() throws Exception { } @Test - public void canAddTriggersLimitOverride() throws Exception { + public void canAddTriggersLimitOverride() { ArgumentCaptor definitionCaptor = ArgumentCaptor.forClass(LimitDefinition.class); @@ -416,7 +398,7 @@ public void canAddTriggersLimitOverride() throws Exception { .build(); Limit userLimit = - LimitBuilder.of(A_LIMIT_NAME, User::getName) + LimitBuilder.of(A_LIMIT_NAME, User::name) .to(A_CAPACITY) .per(A_DURATION) .withExceededCallback(limitCallback) @@ -435,13 +417,13 @@ public void canAddTriggersLimitOverride() throws Exception { } @Test - public void testAddMultipleLimitOverridesForSameProperty() throws Exception { + public void testAddMultipleLimitOverridesForSameProperty() { LimitOverride override = LimitOverrideBuilder.of(JOHN).to(A_CAPACITY).per(A_DURATION).build(); LimitOverride anotherOverride = LimitOverrideBuilder.of(JOHN).to(A_SMALLER_CAPACITY).per(A_DURATION).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(A_HIGHER_CAPACITY) .per(A_DURATION) .withLimitOverride(override) @@ -452,37 +434,37 @@ public void testAddMultipleLimitOverridesForSameProperty() throws Exception { assertThat(spillway.tryCall(john, A_SMALLER_CAPACITY + 10)).isFalse(); } - @Test(expected = IllegalArgumentException.class) - public void positiveCostOnlyCall() throws Exception { + @Test + public void positiveCostOnlyCall() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); - spillway.call(john, 0); + assertThrows(IllegalArgumentException.class, () -> spillway.call(john, 0)); } - @Test(expected = IllegalArgumentException.class) - public void positiveCostOnlyTryCall() throws Exception { + @Test + public void positiveCostOnlyTryCall() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); - spillway.tryCall(john, 0); + assertThrows(IllegalArgumentException.class, () -> spillway.tryCall(john, 0)); } - @Test(expected = IllegalArgumentException.class) - public void positiveCostOnlyCheckLimit() throws Exception { + @Test + public void positiveCostOnlyCheckLimit() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); - spillway.checkLimit(john, 0); + assertThrows(IllegalArgumentException.class, () -> spillway.checkLimit(john, 0)); } @Test public void checkLimit() throws Exception { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(3).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(3).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); assertThat(spillway.checkLimit(john)).isTrue(); @@ -494,9 +476,9 @@ public void checkLimit() throws Exception { } @Test - public void checkLimitDoesNotAffectStorage() throws Exception { + public void checkLimitDoesNotAffectStorage() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); spillway.checkLimit(john); @@ -506,17 +488,17 @@ public void checkLimitDoesNotAffectStorage() throws Exception { currentCounts .keySet() .stream() - .filter(key -> key.getProperty().equals(john.getName())) + .filter(key -> key.getProperty().equals(john.name())) .findFirst(); assertThat(johnKey.isPresent()).isTrue(); assertThat(currentCounts.get(johnKey.get())).isEqualTo(0); } @Test - public void checkLimitDoesNotFireTriggers() throws Exception { + public void checkLimitDoesNotFireTriggers() { LimitTriggerCallback callback = mock(LimitTriggerCallback.class); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callback) diff --git a/src/test/java/com/coveo/spillway/SpillwayTryUpdateAndVerifyLimitTest.java b/src/test/java/com/coveo/spillway/SpillwayTryUpdateAndVerifyLimitTest.java index 12fab7b..6d85915 100644 --- a/src/test/java/com/coveo/spillway/SpillwayTryUpdateAndVerifyLimitTest.java +++ b/src/test/java/com/coveo/spillway/SpillwayTryUpdateAndVerifyLimitTest.java @@ -1,10 +1,9 @@ package com.coveo.spillway; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyListOf; -import static org.mockito.Matchers.eq; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -22,7 +21,6 @@ import com.coveo.spillway.limit.override.LimitOverrideBuilder; import com.coveo.spillway.storage.InMemoryStorage; import com.coveo.spillway.storage.LimitUsageStorage; -import com.coveo.spillway.storage.utils.AddAndGetRequest; import com.coveo.spillway.trigger.LimitTriggerCallback; import com.coveo.spillway.trigger.ValueThresholdTrigger; import com.google.common.collect.ImmutableMap; @@ -32,9 +30,10 @@ import java.util.Map; import java.util.Optional; import java.util.stream.IntStream; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; + +import com.google.common.truth.Truth; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; public class SpillwayTryUpdateAndVerifyLimitTest { @@ -47,27 +46,10 @@ public class SpillwayTryUpdateAndVerifyLimitTest { private static final String JOHN = "john"; private static final String A_LIMIT_NAME = "perUser"; - private class User { - - private String name; - private String ip; + private record User(String name, String ip) {} - User(String name, String ip) { - this.name = name; - this.ip = ip; - } - - String getName() { - return name; - } - - String getIp() { - return ip; - } - } - - private User john = new User(JOHN, "127.0.0.1"); - private User gina = new User("gina", "127.0.0.1"); + private final User john = new User(JOHN, "127.0.0.1"); + private final User gina = new User("gina", "127.0.0.1"); private LimitUsageStorage mockedStorage; private LimitUsageStorage inMemoryStorage; @@ -76,7 +58,7 @@ String getIp() { private SpillwayFactory inMemoryFactory; private SpillwayFactory mockedFactory; - @Before + @BeforeEach public void setup() { clock = mock(Clock.class); inMemoryStorage = new InMemoryStorage(); @@ -89,9 +71,9 @@ public void setup() { } @Test - public void simpleLimit() throws Exception { + public void simpleLimit() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); assertThat(spillway.tryUpdateAndVerifyLimit(john)).isTrue(); @@ -100,21 +82,23 @@ public void simpleLimit() throws Exception { .isFalse(); // Third tryUpdateAndVerifyLimit fails } - @Test(expected = SpillwayLimitsWithSameNameException.class) - public void multipleLimitsWithOverlap() throws Exception { + @Test + public void multipleLimitsWithOverlap() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(5).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(5).per(Duration.ofHours(1)).build(); Limit limit2 = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofSeconds(1)).build(); - inMemoryFactory.enforce("testResource", limit1, limit2); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofSeconds(1)).build(); + + assertThrows( + SpillwayLimitsWithSameNameException.class, + () -> inMemoryFactory.enforce("testResource", limit1, limit2)); } @Test - public void multipleLimitsNoOverlap() throws Exception { - Limit ipLimit = - LimitBuilder.of("perIp", User::getIp).to(1).per(Duration.ofHours(1)).build(); + public void multipleLimitsNoOverlap() { + Limit ipLimit = LimitBuilder.of("perIp", User::ip).to(1).per(Duration.ofHours(1)).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", ipLimit, userLimit); assertThat(spillway.tryUpdateAndVerifyLimit(john)).isTrue(); @@ -122,9 +106,9 @@ public void multipleLimitsNoOverlap() throws Exception { } @Test - public void multipleResourcesEachHaveTheirOwnLimit() throws Exception { + public void multipleResourcesEachHaveTheirOwnLimit() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); Spillway spillway1 = inMemoryFactory.enforce("testResource1", userLimit); Spillway spillway2 = inMemoryFactory.enforce("testResource2", userLimit); @@ -135,19 +119,19 @@ public void multipleResourcesEachHaveTheirOwnLimit() throws Exception { } @Test - public void canUseDefaultPropertyExtractor() throws Exception { + public void canUseDefaultPropertyExtractor() { Limit userLimit = LimitBuilder.of("perUser").to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); - assertThat(spillway.tryUpdateAndVerifyLimit(john.getName())).isTrue(); - assertThat(spillway.tryUpdateAndVerifyLimit(john.getName())).isFalse(); + assertThat(spillway.tryUpdateAndVerifyLimit(john.name())).isTrue(); + assertThat(spillway.tryUpdateAndVerifyLimit(john.name())).isFalse(); } @Test - public void canBeNotifiedWhenLimitIsExceeded() throws Exception { + public void canBeNotifiedWhenLimitIsExceeded() { LimitTriggerCallback callback = mock(LimitTriggerCallback.class); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callback) @@ -163,7 +147,7 @@ public void canBeNotifiedWhenLimitIsExceeded() throws Exception { @Test public void callThrowsAnExceptionWhichContainsAllTheDetails() throws Exception { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); spillway.updateAndVerifyLimit(john); @@ -180,9 +164,8 @@ public void callThrowsAnExceptionWhichContainsAllTheDetails() throws Exception { @Test public void callThrowsForMultipleBreachedLimits() throws Exception { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofHours(1)).build(); - Limit ipLimit = - LimitBuilder.of("perIp", User::getIp).to(1).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofHours(1)).build(); + Limit ipLimit = LimitBuilder.of("perIp", User::ip).to(1).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit, ipLimit); @@ -199,9 +182,9 @@ public void callThrowsForMultipleBreachedLimits() throws Exception { } @Test - public void bucketChangesWhenTimePasses() throws Exception { + public void bucketChangesWhenTimePasses() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(1).per(Duration.ofSeconds(1)).build(); + LimitBuilder.of("perUser", User::name).to(1).per(Duration.ofSeconds(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); assertThat(spillway.tryUpdateAndVerifyLimit(john)).isTrue(); @@ -214,9 +197,9 @@ public void bucketChangesWhenTimePasses() throws Exception { } @Test - public void capacityNotIncrementedIfLimitTriggered() throws Exception { + public void capacityNotIncrementedIfLimitTriggered() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(0).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(0).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); assertThat(spillway.tryUpdateAndVerifyLimit(john)).isFalse(); @@ -229,7 +212,7 @@ public void capacityNotIncrementedIfLimitTriggered() throws Exception { @Test public void canGetCurrentLimitStatus() throws Exception { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); spillway.updateAndVerifyLimit(john); @@ -244,7 +227,7 @@ public void canGetCurrentLimitStatus() throws Exception { limitStatuses .keySet() .stream() - .filter(key -> key.getProperty().equals(john.getName())) + .filter(key -> key.getProperty().equals(john.name())) .findFirst(); assertThat(johnKey.isPresent()).isTrue(); assertThat(limitStatuses.get(johnKey.get())).isEqualTo(2); @@ -253,33 +236,33 @@ public void canGetCurrentLimitStatus() throws Exception { limitStatuses .keySet() .stream() - .filter(key -> key.getProperty().equals(gina.getName())) + .filter(key -> key.getProperty().equals(gina.name())) .findFirst(); assertThat(ginaKey.isPresent()).isTrue(); assertThat(limitStatuses.get(ginaKey.get())).isEqualTo(1); } @Test - public void ifCallbackThrowsWeIgnoreThatCallbackAndContinue() throws Exception { + public void ifCallbackThrowsWeIgnoreThatCallbackAndContinue() { LimitTriggerCallback callbackThatIsOkay = mock(LimitTriggerCallback.class); LimitTriggerCallback callbackThatThrows = mock(LimitTriggerCallback.class); doThrow(RuntimeException.class) .when(callbackThatThrows) .trigger(any(LimitDefinition.class), any(Object.class)); Limit ipLimit1 = - LimitBuilder.of("perIp1", User::getIp) + LimitBuilder.of("perIp1", User::ip) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callbackThatThrows) .build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callbackThatIsOkay) .build(); Limit ipLimit2 = - LimitBuilder.of("perIp2", User::getIp) + LimitBuilder.of("perIp2", User::ip) .to(1) .per(Duration.ofHours(1)) .withExceededCallback(callbackThatThrows) @@ -296,9 +279,9 @@ public void ifCallbackThrowsWeIgnoreThatCallbackAndContinue() throws Exception { } @Test - public void canExceedLimitByDoingALargeIncrement() throws Exception { + public void canExceedLimitByDoingALargeIncrement() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); try { @@ -311,9 +294,9 @@ public void canExceedLimitByDoingALargeIncrement() throws Exception { } @Test - public void costCanBeALargeNumber() throws Exception { + public void costCanBeALargeNumber() { Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(4).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(4).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); assertThat(spillway.tryUpdateAndVerifyLimit(john, 4)).isTrue(); @@ -321,12 +304,12 @@ public void costCanBeALargeNumber() throws Exception { } @Test - public void canAddLimitTriggers() throws Exception { + public void canAddLimitTriggers() { LimitTriggerCallback callback = mock(LimitTriggerCallback.class); ValueThresholdTrigger trigger = new ValueThresholdTrigger(5, callback); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(15) .per(Duration.ofHours(1)) .withLimitTrigger(trigger) @@ -343,8 +326,8 @@ public void canAddLimitTriggers() throws Exception { } @Test - public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() throws Exception { - when(mockedStorage.addAndGetWithLimit(anyListOf(AddAndGetRequest.class))) + public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() { + when(mockedStorage.addAndGetWithLimit(anyCollection())) .thenReturn( ImmutableMap.of( mock(LimitKey.class), 1, mock(LimitKey.class), 2, mock(LimitKey.class), 3)); @@ -352,7 +335,7 @@ public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() throws Ex LimitTriggerCallback callback = mock(LimitTriggerCallback.class); ValueThresholdTrigger trigger = new ValueThresholdTrigger(5, callback); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(15) .per(Duration.ofHours(1)) .withLimitTrigger(trigger) @@ -365,11 +348,11 @@ public void triggersAreIgnoreIfTheStorageReturnsAnIncoherentResponse() throws Ex } @Test - public void canAddCapacityLimitOverride() throws Exception { + public void canAddCapacityLimitOverride() { LimitOverride override = LimitOverrideBuilder.of(JOHN).to(A_CAPACITY).per(A_DURATION).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(A_HIGHER_CAPACITY) .per(A_DURATION) .withLimitOverride(override) @@ -380,12 +363,12 @@ public void canAddCapacityLimitOverride() throws Exception { } @Test - public void canAddExpirationLimitOverride() throws Exception { + public void canAddExpirationLimitOverride() { LimitOverride override = LimitOverrideBuilder.of(JOHN).to(A_CAPACITY).per(A_SHORT_DURATION).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(A_CAPACITY) .per(A_DURATION) .withLimitOverride(override) @@ -402,7 +385,7 @@ public void canAddExpirationLimitOverride() throws Exception { } @Test - public void canAddTriggersLimitOverride() throws Exception { + public void canAddTriggersLimitOverride() { ArgumentCaptor definitionCaptor = ArgumentCaptor.forClass(LimitDefinition.class); @@ -417,7 +400,7 @@ public void canAddTriggersLimitOverride() throws Exception { .build(); Limit userLimit = - LimitBuilder.of(A_LIMIT_NAME, User::getName) + LimitBuilder.of(A_LIMIT_NAME, User::name) .to(A_CAPACITY) .per(A_DURATION) .withExceededCallback(limitCallback) @@ -436,13 +419,13 @@ public void canAddTriggersLimitOverride() throws Exception { } @Test - public void testAddMultipleLimitOverridesForSameProperty() throws Exception { + public void testAddMultipleLimitOverridesForSameProperty() { LimitOverride override = LimitOverrideBuilder.of(JOHN).to(A_CAPACITY).per(A_DURATION).build(); LimitOverride anotherOverride = LimitOverrideBuilder.of(JOHN).to(A_SMALLER_CAPACITY).per(A_DURATION).build(); Limit userLimit = - LimitBuilder.of("perUser", User::getName) + LimitBuilder.of("perUser", User::name) .to(A_HIGHER_CAPACITY) .per(A_DURATION) .withLimitOverride(override) @@ -453,28 +436,28 @@ public void testAddMultipleLimitOverridesForSameProperty() throws Exception { assertThat(spillway.tryUpdateAndVerifyLimit(john, A_SMALLER_CAPACITY + 10)).isFalse(); } - @Test(expected = IllegalArgumentException.class) + @Test public void positiveCostOnlyCall() throws Exception { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); - spillway.call(john, 0); + assertThrows(IllegalArgumentException.class, () -> spillway.call(john, 0)); } - @Test(expected = IllegalArgumentException.class) - public void positiveCostOnlyTryCall() throws Exception { + @Test + public void positiveCostOnlyTryCall() { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(2).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(2).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); - spillway.tryUpdateAndVerifyLimit(john, 0); + assertThrows(IllegalArgumentException.class, () -> spillway.tryUpdateAndVerifyLimit(john, 0)); } @Test public void checkLimit() throws Exception { Limit limit1 = - LimitBuilder.of("perUser", User::getName).to(3).per(Duration.ofHours(1)).build(); + LimitBuilder.of("perUser", User::name).to(3).per(Duration.ofHours(1)).build(); Spillway spillway = inMemoryFactory.enforce("testResource", limit1); assertThat(spillway.checkLimit(john)).isTrue(); @@ -486,21 +469,21 @@ public void checkLimit() throws Exception { } @Test - public void tryUpdateAndVerifyLimitMultiThread() throws Exception { + public void tryUpdateAndVerifyLimitMultiThread() { IntStream.range(0, 10) .forEach( i -> { InMemoryStorage inMemoryStorage = new InMemoryStorage(); SpillwayFactory inMemoryFactory = new SpillwayFactory(inMemoryStorage); Limit userLimit = - LimitBuilder.of("perUser", User::getName).to(100).per(A_DURATION).build(); + LimitBuilder.of("perUser", User::name).to(100).per(A_DURATION).build(); Spillway spillway = inMemoryFactory.enforce("testResource", userLimit); Optional limitReached = IntStream.range(0, 101) .parallel() .mapToObj(j -> spillway.tryUpdateAndVerifyLimit(john)) .reduce(Boolean::logicalAnd); - Assert.assertFalse(limitReached.orElse(true)); + assertThat(limitReached).hasValue(false); }); } } diff --git a/src/test/java/com/coveo/spillway/functional/SpillwayFunctionalTests.java b/src/test/java/com/coveo/spillway/functional/SpillwayFunctionalTests.java index 3db163e..d8182b0 100644 --- a/src/test/java/com/coveo/spillway/functional/SpillwayFunctionalTests.java +++ b/src/test/java/com/coveo/spillway/functional/SpillwayFunctionalTests.java @@ -14,11 +14,7 @@ import java.util.stream.IntStream; import org.apache.commons.lang3.tuple.Pair; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Ignore; -import org.junit.Test; +import org.junit.jupiter.api.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,14 +27,13 @@ import com.coveo.spillway.storage.AsyncLimitUsageStorage; import com.coveo.spillway.storage.InMemoryStorage; import com.coveo.spillway.storage.RedisStorage; -import com.coveo.spillway.storage.RedisStorageTest; import com.google.common.base.Stopwatch; import com.google.common.collect.Range; import redis.clients.jedis.JedisPool; import redis.embedded.RedisServer; -@Ignore("Functional tests, remove ignore to run them") +@Disabled("Functional tests, remove ignore to run them") public class SpillwayFunctionalTests { private static final String RESOURCE1 = "someResource"; @@ -59,8 +54,7 @@ public class SpillwayFunctionalTests { private static JedisPool jedis; private static RedisStorage storage; - @SuppressWarnings("resource") - @BeforeClass + @BeforeAll public static void startRedis() throws IOException { try { redisServer = new RedisServer(6389); @@ -73,12 +67,12 @@ public static void startRedis() throws IOException { storage = RedisStorage.builder().withJedisPool(new JedisPool("localhost", 6389)).build(); } - @AfterClass - public static void stopRedis() { + @AfterAll + public static void stopRedis() throws IOException { redisServer.stop(); } - @Before + @BeforeEach public void setup() { jedis.getResource().flushDB(); inMemoryFactory = new SpillwayFactory(new InMemoryStorage()); @@ -106,7 +100,7 @@ public void oneMillionConcurrentRequestsWith100Threads() throws Exception { threadPool.shutdown(); threadPool.awaitTermination(1, TimeUnit.MINUTES); - assertThat(counter.get()).isIn(inErrorMargin(ONE_MILLION, MARGIN_OF_ERROR)); + assertThat(counter.get()).isIn(inErrorMargin(MARGIN_OF_ERROR)); } @Test @@ -240,26 +234,16 @@ public void asyncBatchStorageTest() throws Exception { Map currentCounters = asyncStorage.getCurrentLimitCounters(); Map cacheCounters = asyncStorage.debugCacheLimitCounters(); - currentCounters - .entrySet() - .forEach( - entry -> { - assertThat(entry.getValue()).isEqualTo(numberOfCalls); - }); - - cacheCounters - .entrySet() - .forEach( - entry -> { - assertThat(entry.getValue()).isEqualTo(2 * numberOfCalls); - }); + currentCounters.forEach((key, value) -> assertThat(value).isEqualTo(numberOfCalls)); + + cacheCounters.forEach((key, value) -> assertThat(value).isEqualTo(2 * numberOfCalls)); } - private Range inErrorMargin(Integer expectedValue, double marginOfError) { + private Range inErrorMargin(double marginOfError) { marginOfError += 1; - Integer lowerBound = (int) (expectedValue / marginOfError); - Integer upperBound = (int) (expectedValue * marginOfError); + Integer lowerBound = (int) (SpillwayFunctionalTests.ONE_MILLION / marginOfError); + Integer upperBound = (int) (SpillwayFunctionalTests.ONE_MILLION * marginOfError); return Range.closed(lowerBound, upperBound); } diff --git a/src/test/java/com/coveo/spillway/limit/LimitDefinitionTest.java b/src/test/java/com/coveo/spillway/limit/LimitDefinitionTest.java index 2111379..128b206 100644 --- a/src/test/java/com/coveo/spillway/limit/LimitDefinitionTest.java +++ b/src/test/java/com/coveo/spillway/limit/LimitDefinitionTest.java @@ -22,9 +22,8 @@ */ package com.coveo.spillway.limit; -import org.junit.Test; - import com.coveo.spillway.limit.LimitDefinition; +import org.junit.jupiter.api.Test; import java.time.Duration; diff --git a/src/test/java/com/coveo/spillway/limit/LimitTest.java b/src/test/java/com/coveo/spillway/limit/LimitTest.java index 04dc176..8c05295 100644 --- a/src/test/java/com/coveo/spillway/limit/LimitTest.java +++ b/src/test/java/com/coveo/spillway/limit/LimitTest.java @@ -22,10 +22,9 @@ */ package com.coveo.spillway.limit; -import org.junit.Test; - import com.coveo.spillway.limit.Limit; import com.coveo.spillway.limit.LimitDefinition; +import org.junit.jupiter.api.Test; import java.time.Duration; import java.util.ArrayList; diff --git a/src/test/java/com/coveo/spillway/storage/AsyncBatchLimitUsageStorageTest.java b/src/test/java/com/coveo/spillway/storage/AsyncBatchLimitUsageStorageTest.java index 494f15c..30eeb19 100644 --- a/src/test/java/com/coveo/spillway/storage/AsyncBatchLimitUsageStorageTest.java +++ b/src/test/java/com/coveo/spillway/storage/AsyncBatchLimitUsageStorageTest.java @@ -1,7 +1,6 @@ package com.coveo.spillway.storage; import static com.google.common.truth.Truth.*; -import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import java.time.Duration; @@ -14,21 +13,21 @@ import java.util.Map.Entry; import org.apache.commons.lang3.tuple.ImmutablePair; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; import com.coveo.spillway.limit.LimitKey; import com.coveo.spillway.storage.utils.AddAndGetRequest; import com.coveo.spillway.storage.utils.CacheSynchronization; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class AsyncBatchLimitUsageStorageTest { private static final String RESOURCE = "TheResource"; private static final String PROPERTY = "TheProperty"; @@ -47,19 +46,19 @@ public class AsyncBatchLimitUsageStorageTest { private AsyncBatchLimitUsageStorage asyncBatchLimitUsageStorage; - @Before + @BeforeEach public void setup() { cacheSpy = Mockito.spy(new InMemoryStorage()); cacheSynchronizationSpy = Mockito.spy(new CacheSynchronization(cacheSpy, storageMock)); } - @After + @AfterEach public void tearDown() throws Exception { asyncBatchLimitUsageStorage.close(); } @Test - public void testAddWithCacheForcefullyInitialized() throws Exception { + public void testAddWithCacheForcefullyInitialized() { asyncBatchLimitUsageStorage = new AsyncBatchLimitUsageStorage( storageMock, @@ -88,7 +87,7 @@ public void testAddWithCacheForcefullyInitialized() throws Exception { } @Test - public void testAddWithCacheNotForcefullyInitialized() throws Exception { + public void testAddWithCacheNotForcefullyInitialized() { asyncBatchLimitUsageStorage = new AsyncBatchLimitUsageStorage( storageMock, @@ -125,8 +124,7 @@ public void testSynchronizeIsNotAffectingProcess() throws Exception { .then( invocation -> { Thread.sleep(MOCKED_STORAGE_SLEEP); - return ImmutablePair.of( - LimitKey.fromRequest(invocation.getArgumentAt(0, AddAndGetRequest.class)), 100); + return ImmutablePair.of(LimitKey.fromRequest(invocation.getArgument(0)), 100); }); asyncBatchLimitUsageStorage = diff --git a/src/test/java/com/coveo/spillway/storage/AsyncLimitUsageStorageTest.java b/src/test/java/com/coveo/spillway/storage/AsyncLimitUsageStorageTest.java index 2bc1675..dc9bc41 100644 --- a/src/test/java/com/coveo/spillway/storage/AsyncLimitUsageStorageTest.java +++ b/src/test/java/com/coveo/spillway/storage/AsyncLimitUsageStorageTest.java @@ -23,12 +23,12 @@ package com.coveo.spillway.storage; import com.coveo.spillway.limit.LimitKey; -import com.coveo.spillway.storage.AsyncLimitUsageStorage; import com.coveo.spillway.storage.utils.AddAndGetRequest; import com.google.common.collect.ImmutableMap; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,8 +36,7 @@ import java.time.Instant; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Matchers.anyCollectionOf; -import static org.mockito.Mockito.mock; +import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.Mockito.when; public class AsyncLimitUsageStorageTest { @@ -53,8 +52,7 @@ public class AsyncLimitUsageStorageTest { private static final int MOCKED_STORAGE_SLEEP = 100; private AsyncLimitUsageStorage asyncStorage; - private LimitUsageStorage mockedStorage; - private AddAndGetRequest request = + private final AddAndGetRequest request = new AddAndGetRequest.Builder() .withResource(RESOURCE) .withProperty(PROPERTY) @@ -65,10 +63,10 @@ public class AsyncLimitUsageStorageTest { .withExpiration(EXPIRATION) .build(); - @Before + @BeforeEach public void setup() { - mockedStorage = mock(LimitUsageStorage.class); - when(mockedStorage.addAndGet(anyCollectionOf(AddAndGetRequest.class))) + LimitUsageStorage mockedStorage = Mockito.mock(LimitUsageStorage.class); + when(mockedStorage.addAndGet(anyCollection())) .then( invocation -> { Thread.sleep(MOCKED_STORAGE_SLEEP); diff --git a/src/test/java/com/coveo/spillway/storage/InMemoryStorageTest.java b/src/test/java/com/coveo/spillway/storage/InMemoryStorageTest.java index 481f16a..ce8bbbd 100644 --- a/src/test/java/com/coveo/spillway/storage/InMemoryStorageTest.java +++ b/src/test/java/com/coveo/spillway/storage/InMemoryStorageTest.java @@ -22,14 +22,14 @@ */ package com.coveo.spillway.storage; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; import com.coveo.spillway.limit.LimitKey; +import org.mockito.junit.jupiter.MockitoExtension; import java.time.Clock; import java.time.Duration; @@ -39,7 +39,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.*; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class InMemoryStorageTest { private static final String RESOURCE1 = "someResource"; @@ -55,7 +55,7 @@ public class InMemoryStorageTest { @InjectMocks private InMemoryStorage storage; - @Before + @BeforeEach public void setup() { when(clock.instant()).thenReturn(Instant.now()); } diff --git a/src/test/java/com/coveo/spillway/storage/RedisStorageTest.java b/src/test/java/com/coveo/spillway/storage/RedisStorageTest.java index 8ca94d9..0282b25 100644 --- a/src/test/java/com/coveo/spillway/storage/RedisStorageTest.java +++ b/src/test/java/com/coveo/spillway/storage/RedisStorageTest.java @@ -29,13 +29,8 @@ import java.time.Duration; import java.time.Instant; import java.util.Map; -import java.util.stream.Collectors; -import java.util.stream.Stream; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; +import org.junit.jupiter.api.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.coveo.spillway.limit.LimitKey; @@ -50,6 +45,7 @@ * The behavior of the RedisStorage class is tightly coupled with the behavior * of redis so it makes sense imho. */ +//@Disabled("Functional tests, remove ignore to run them") public class RedisStorageTest { private static final String RESOURCE1 = "someResource"; @@ -63,6 +59,7 @@ public class RedisStorageTest { private static final String KEY3 = "yetAnotherKey"; private static final Duration EXPIRATION = Duration.ofHours(1); private static final Instant TIMESTAMP = Instant.now(); + private static final int REDIS_PORT = 7893; private static final Logger logger = LoggerFactory.getLogger(RedisStorageTest.class); @@ -70,26 +67,25 @@ public class RedisStorageTest { private static JedisPool jedis; private static RedisStorage storage; - @SuppressWarnings("resource") - @BeforeClass + @BeforeAll public static void startRedis() throws IOException { try { - redisServer = new RedisServer(6389); + redisServer = new RedisServer(REDIS_PORT); } catch (IOException e) { - logger.error("Failed to start Redis server. Is port 6389 available?"); + logger.error("Failed to start Redis server. Is port {} available?", REDIS_PORT); throw e; } redisServer.start(); - jedis = new JedisPool("localhost", 6389); - storage = RedisStorage.builder().withJedisPool(new JedisPool("localhost", 6389)).build(); + jedis = new JedisPool("localhost", REDIS_PORT); + storage = RedisStorage.builder().withJedisPool(new JedisPool("localhost", REDIS_PORT)).build(); } - @AfterClass - public static void stopRedis() { + @AfterAll + public static void stopRedis() throws IOException { redisServer.stop(); } - @Before + @BeforeEach public void flushDataInRedis() { try (Jedis resource = jedis.getResource()) { resource.flushDB(); @@ -197,7 +193,7 @@ public void canAddLargeValues() { } @Test - public void canAddLargeValuesToExisitingCounters() { + public void canAddLargeValuesToExistingCounters() { storage.incrementAndGet(RESOURCE1, LIMIT1, PROPERTY1, true, EXPIRATION, TIMESTAMP); int result = storage.addAndGet(RESOURCE1, LIMIT1, PROPERTY1, true, EXPIRATION, TIMESTAMP, 5).getValue(); @@ -210,8 +206,13 @@ public void testBackwardCompatibilityWithPreviousKeys() { // Versions pre 2.0.0-alpha.3 are not storing expiration try (Jedis resource = jedis.getResource()) { resource.set( - Stream.of(RedisStorage.DEFAULT_PREFIX, RESOURCE1, LIMIT1, PROPERTY1, TIMESTAMP.toString()) - .collect(Collectors.joining(RedisStorage.KEY_SEPARATOR)), + String.join( + RedisStorage.KEY_SEPARATOR, + RedisStorage.DEFAULT_PREFIX, + RESOURCE1, + LIMIT1, + PROPERTY1, + TIMESTAMP.toString()), "1"); } diff --git a/src/test/java/com/coveo/spillway/storage/utils/CacheSynchronizationTest.java b/src/test/java/com/coveo/spillway/storage/utils/CacheSynchronizationTest.java index a0c0648..d98aa7a 100644 --- a/src/test/java/com/coveo/spillway/storage/utils/CacheSynchronizationTest.java +++ b/src/test/java/com/coveo/spillway/storage/utils/CacheSynchronizationTest.java @@ -1,7 +1,6 @@ package com.coveo.spillway.storage.utils; import static com.google.common.truth.Truth.*; -import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; import java.time.Duration; @@ -12,20 +11,20 @@ import java.util.function.Consumer; import java.util.stream.Collectors; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; import com.coveo.spillway.limit.LimitKey; import com.coveo.spillway.storage.InMemoryStorage; import com.coveo.spillway.storage.LimitUsageStorage; import com.google.common.collect.ImmutableMap; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class CacheSynchronizationTest { private static final String RESOURCE = "TheResource"; private static final String LIMIT = "TheLimit"; @@ -42,7 +41,7 @@ public class CacheSynchronizationTest { private CacheSynchronization cacheSynchronization; - @Before + @BeforeEach public void setup() { cacheSynchronization = new CacheSynchronization(inMemoryStorageMock, limitUsageStorageMock); } @@ -93,8 +92,7 @@ private void givenLimitUsageStorageHasValues(Map counters) { private void givenInMemoryCacheHasValues(Map counters) { doAnswer( invocation -> { - Consumer> consumer = - (Consumer>) invocation.getArgumentAt(0, Consumer.class); + Consumer> consumer = invocation.getArgument(0); for (Entry entry : counters diff --git a/src/test/java/com/coveo/spillway/trigger/AbstractLimitTriggerTest.java b/src/test/java/com/coveo/spillway/trigger/AbstractLimitTriggerTest.java index c19186b..99f76fc 100644 --- a/src/test/java/com/coveo/spillway/trigger/AbstractLimitTriggerTest.java +++ b/src/test/java/com/coveo/spillway/trigger/AbstractLimitTriggerTest.java @@ -1,21 +1,21 @@ package com.coveo.spillway.trigger; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.*; import java.time.Duration; import java.time.Instant; -import org.junit.Before; -import org.junit.Test; - import com.coveo.spillway.limit.LimitDefinition; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; public class AbstractLimitTriggerTest { private static final LimitDefinition LIMIT_DEFINITION = new LimitDefinition("testLimit", 100, Duration.ofMinutes(1)); - private class SimpleThresholdTrigger extends AbstractLimitTrigger { + private static class SimpleThresholdTrigger extends AbstractLimitTrigger { public SimpleThresholdTrigger(LimitTriggerCallback callback) { super(callback); @@ -31,7 +31,7 @@ protected boolean triggered( private LimitTriggerCallback callback; private AbstractLimitTrigger abstractLimitTrigger; - @Before + @BeforeEach public void setup() { callback = mock(LimitTriggerCallback.class); abstractLimitTrigger = new SimpleThresholdTrigger(callback); @@ -44,7 +44,7 @@ public void testCallbackNotTriggeredTwiceInSameBucket() { abstractLimitTrigger.callbackIfRequired(null, 1, now.plusSeconds(10), 1, LIMIT_DEFINITION); abstractLimitTrigger.callbackIfRequired(null, 1, now.plusSeconds(20), 1, LIMIT_DEFINITION); - verify(callback).trigger(any(LimitDefinition.class), any(Object.class)); + verify(callback).trigger(any(LimitDefinition.class), isNull()); } @Test @@ -53,7 +53,7 @@ public void testCallbackTriggeredWhenBucketChange() { abstractLimitTrigger.callbackIfRequired(null, 1, now, 1, LIMIT_DEFINITION); abstractLimitTrigger.callbackIfRequired(null, 1, now.plusSeconds(70), 1, LIMIT_DEFINITION); - verify(callback, times(2)).trigger(any(LimitDefinition.class), any(Object.class)); + verify(callback, times(2)).trigger(any(LimitDefinition.class), isNull()); } private Instant givenABucketStartingInstant() { diff --git a/src/test/java/com/coveo/spillway/trigger/PercentageThresholdTriggerTest.java b/src/test/java/com/coveo/spillway/trigger/PercentageThresholdTriggerTest.java index 17272a6..d6693f0 100644 --- a/src/test/java/com/coveo/spillway/trigger/PercentageThresholdTriggerTest.java +++ b/src/test/java/com/coveo/spillway/trigger/PercentageThresholdTriggerTest.java @@ -22,39 +22,39 @@ */ package com.coveo.spillway.trigger; -import org.junit.Before; -import org.junit.Test; - import com.coveo.spillway.limit.LimitDefinition; -import com.coveo.spillway.trigger.LimitTriggerCallback; -import com.coveo.spillway.trigger.PercentageThresholdTrigger; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import java.time.Duration; import static com.google.common.truth.Truth.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; public class PercentageThresholdTriggerTest { private LimitTriggerCallback callback; - private LimitDefinition limitDef = new LimitDefinition("testLimit", 100, Duration.ofDays(1)); + private final LimitDefinition limitDef = + new LimitDefinition("testLimit", 100, Duration.ofDays(1)); private PercentageThresholdTrigger trigger; - @Before + @BeforeEach public void setup() { callback = mock(LimitTriggerCallback.class); // Will trigger at 50% of the limit trigger = new PercentageThresholdTrigger(0.5, callback); } - @Test(expected = IllegalArgumentException.class) + @Test public void negativeThresholdThrows() { - new PercentageThresholdTrigger(-1, callback); + assertThrows( + IllegalArgumentException.class, () -> new PercentageThresholdTrigger(-1, callback)); } - @Test(expected = IllegalArgumentException.class) + @Test public void largerThanOneThresholdThrows() { - new PercentageThresholdTrigger(2, callback); + assertThrows(IllegalArgumentException.class, () -> new PercentageThresholdTrigger(2, callback)); } @Test diff --git a/src/test/java/com/coveo/spillway/trigger/ValueThresholdTriggerTest.java b/src/test/java/com/coveo/spillway/trigger/ValueThresholdTriggerTest.java index 1c5731e..3ad51dd 100644 --- a/src/test/java/com/coveo/spillway/trigger/ValueThresholdTriggerTest.java +++ b/src/test/java/com/coveo/spillway/trigger/ValueThresholdTriggerTest.java @@ -22,12 +22,9 @@ */ package com.coveo.spillway.trigger; -import org.junit.Before; -import org.junit.Test; - import com.coveo.spillway.limit.LimitDefinition; -import com.coveo.spillway.trigger.LimitTriggerCallback; -import com.coveo.spillway.trigger.ValueThresholdTrigger; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import java.time.Duration; @@ -35,13 +32,13 @@ import static org.mockito.Mockito.mock; public class ValueThresholdTriggerTest { - private LimitTriggerCallback callback; - private LimitDefinition limitDef = new LimitDefinition("testLimit", 100, Duration.ofDays(1)); + private final LimitDefinition limitDef = + new LimitDefinition("testLimit", 100, Duration.ofDays(1)); private ValueThresholdTrigger trigger; - @Before + @BeforeEach public void setup() { - callback = mock(LimitTriggerCallback.class); + LimitTriggerCallback callback = mock(LimitTriggerCallback.class); // Will trigger at 50% of the limit trigger = new ValueThresholdTrigger(50, callback); }