diff --git a/README.md b/README.md index 7efb8ef..3629212 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,57 @@ The OpenAPI document is served at `/v3/api-docs` and Swagger UI at `/swagger-ui. annotations. Only `/api/**` is documented. To turn both off (e.g. in production), set `springdoc.api-docs.enabled=false` and `springdoc.swagger-ui.enabled=false`. +## Redis +The application uses Redis (`spring.data.redis.host`/`port`, default `localhost:6379`). + +**Cache** (Spring Cache on Spring Data Redis, values stored as JSON): + +| Cache | What | TTL | +|---|---|---| +| `sdlc-systems` | SDLC systems by id, read on every create, update and SDLC call | `resourceapi.cache.sdlc-systems-ttl` (10m) | +| `external-projects` | `GET /api/v2/projects/{id}/external` answers per SDLC system and external id | `resourceapi.cache.external-projects-ttl` (5m); "the SDLC system doesn't know this project" (stored as a cached null) only `resourceapi.cache.missing-external-projects-ttl` (30s), so a project created there is found soon | + +A background sync always asks the SDLC system and replaces the cached answer with its own. SDLC systems are not changed +through this API; one changed directly in the database is picked up when its entry expires. The cache is an +optimization: if Redis is unreachable, cache reads and writes are logged and skipped (each waits at most the +500 ms Redis timeouts) and requests are served without it. + +**Lock** ([Redisson](https://redisson.org/) `RLock`): at most one `POST /api/v2/projects/{id}/sync` per project +runs at a time, across all instances; a second one gets `409 Conflict` until the first ends. The lock is +released when the background sync ends and expires after `resourceapi.sync.lock-lease-time` (60s) in case +an instance dies mid-sync. If Redis can't be reached, the sync runs without the lock (a duplicate sync only +costs an extra SDLC call; the failed lock request takes about a second). Redisson connects lazily and uses the +same Redis connection settings; its keys are prefixed with `resourceapi.redisson.key-prefix`. Only a +standalone Redis is supported for the lock: with Sentinel or Cluster the application still starts and syncs +run unlocked; TLS uses the JVM's trust store (Boot SSL bundles aren't passed to Redisson). (Redisson's Spring Boot starter isn't used: it targets +Spring Boot 3. Redisson brings a JCache provider, so `spring.cache.type=redis` is set explicitly.) + +**Rate limits** (Redisson `RRateLimiter`, sliding windows shared by all instances: a used permit becomes +available again one period later, which is also what `Retry-After` says): + +| Limit | Default | When exceeded | +|---|---|---| +| Calls to one SDLC system (`resourceapi.rate-limit.sdlc.*`), protecting its API quota; cache hits don't count | 10 per 1s, waiting up to 1s for a permit | `503` + `Retry-After` (our own limit, not the SDLC system's failure); a background sync logs it | +| `POST /api/v2/projects/{id}/sync` per project (`resourceapi.rate-limit.sync.*`); requests refused anyway (unknown project, sync already running) don't count, one refused because no background slot is free (`503`) does | 6 per minute | `429` + `Retry-After` | +| All `/api/**` requests per client IP (`resourceapi.rate-limit.client.*`); behind a reverse proxy the IP comes from `X-Forwarded-For`, trusted only from internal proxy addresses (`server.forward-headers-strategy=native`, see below) | 100 per second | `429` + `Retry-After` | + +A changed rate is applied to the existing limiter when an instance first uses it; unused limiters expire from +Redis after 10 periods (at least a minute), so per-IP limiters don't pile up. Invalid limits (e.g. +`permits=0`) stop the application at startup. If Redis can't be reached, requests are allowed (limiting +protects, losing it shouldn't stop the application), and Redis is not asked again for 10 seconds, so an +outage doesn't add a timeout to every request. + +The per-IP limit only means something if the application sees the client's address. Tomcat takes it from +`X-Forwarded-For` when the request comes from an address matching `server.tomcat.remoteip.internal-proxies` +(by default 10/8, 192.168/16, 172.16/12, 169.254/16, 127/8 and IPv6 local addresses). If the reverse proxy or +load balancer has another address, set that property to match it; otherwise every request appears to come from +the proxy and all clients share one limit. Never match addresses clients can connect from directly, or they +can choose their own key by sending `X-Forwarded-For`. + +**Local development**: `./mvnw spring-boot:run` starts Redis from `compose.yaml` automatically +(`spring-boot-docker-compose`); with `java -jar`, start it yourself with `docker compose up -d` (it listens on +`localhost:6379`). Tests start their own Redis with Testcontainers, so they need Docker. + ## Tracing Every request gets a trace id ([Micrometer Tracing](https://micrometer.io/docs/tracing) with Brave, W3C trace context): diff --git a/compose.yaml b/compose.yaml new file mode 100644 index 0000000..942b34f --- /dev/null +++ b/compose.yaml @@ -0,0 +1,7 @@ +# Local development services. `./mvnw spring-boot:run` starts them automatically +# (spring-boot-docker-compose); or run `docker compose up -d` yourself. +services: + redis: + image: redis:8-alpine + ports: + - "6379:6379" diff --git a/docs/docker.md b/docs/docker.md index 09e4700..0805423 100644 --- a/docs/docker.md +++ b/docs/docker.md @@ -125,9 +125,12 @@ the build. The executable is built for the architecture of the machine (or `--pl ## Configuration -All three images are configured like the application itself, e.g. with environment variables: +All three images are configured like the application itself, e.g. with environment variables. The +application uses Redis (see the README); point it at one with `SPRING_DATA_REDIS_HOST`/`SPRING_DATA_REDIS_PORT` +(without Redis it still works, uncached): ```sh +docker run --rm -p 8080:8080 -e SPRING_DATA_REDIS_HOST=redis --network my-network resourceapi docker run --rm -p 9090:9090 -e SERVER_PORT=9090 resourceapi docker run --rm -p 8080:8080 -e SPRING_HTTP_CLIENTS_READ_TIMEOUT=10s resourceapi:aot ``` diff --git a/pom.xml b/pom.xml index b1c7804..98f9ef3 100644 --- a/pom.xml +++ b/pom.xml @@ -17,6 +17,7 @@ 25 3.2.0 + 4.8.0 3.13.2 0.8.16 @@ -49,6 +50,21 @@ io.micrometer micrometer-tracing-bridge-brave + + + org.springframework.boot + spring-boot-starter-data-redis + + + org.springframework.boot + spring-boot-starter-cache + + + + org.redisson + redisson + ${redisson.version} + org.apache.httpcomponents.client5 @@ -59,6 +75,13 @@ spring-boot-h2console + + + org.springframework.boot + spring-boot-docker-compose + runtime + true + org.springframework.boot spring-boot-devtools @@ -87,6 +110,12 @@ spring-boot-starter-webmvc-test test + + + org.springframework.boot + spring-boot-testcontainers + test + org.wiremock diff --git a/src/main/java/com/company/resourceapi/config/CacheConfig.java b/src/main/java/com/company/resourceapi/config/CacheConfig.java new file mode 100644 index 0000000..8b0533d --- /dev/null +++ b/src/main/java/com/company/resourceapi/config/CacheConfig.java @@ -0,0 +1,79 @@ +package com.company.resourceapi.config; + +import com.company.resourceapi.clients.ExternalProject; +import com.company.resourceapi.entities.SdlcSystem; +import org.springframework.aot.hint.annotation.RegisterReflectionForBinding; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.boot.cache.autoconfigure.RedisCacheManagerBuilderCustomizer; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.cache.annotation.CachingConfigurer; +import org.springframework.cache.annotation.EnableCaching; +import org.springframework.cache.interceptor.CacheErrorHandler; +import org.springframework.cache.interceptor.LoggingCacheErrorHandler; +import org.springframework.cache.support.NullValue; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Role; +import org.springframework.data.redis.serializer.GenericJacksonJsonRedisSerializer; +import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair; +import tools.jackson.databind.jsontype.BasicPolymorphicTypeValidator; + +/** + * Redis-backed Spring Cache. Values are stored as JSON with their type, limited to the classes the + * cached values consist of. A cache that can't be reached is logged and skipped, so Redis being down + * makes requests slower, not fail. + */ +@Configuration +@EnableCaching +@EnableConfigurationProperties(CacheTtlProperties.class) +// cached values are rebuilt from JSON by type name: reflection a native image must know about +@RegisterReflectionForBinding({SdlcSystem.class, ExternalProject.class, NullValue.class}) +public class CacheConfig { + + public static final String SDLC_SYSTEMS = "sdlc-systems"; + public static final String EXTERNAL_PROJECTS = "external-projects"; + + /** + * Starts from Boot's defaults (spring.cache.redis.*: key prefix, null values, ...) and only + * replaces the value serializer and sets each cache's time to live. External projects rely on + * null values being cached (spring.cache.redis.cache-null-values, true by default). + */ + @Bean + RedisCacheManagerBuilderCustomizer redisCacheCustomizer(CacheTtlProperties ttl) { + var typeValidator = BasicPolymorphicTypeValidator.builder() + .allowIfSubType("com.company.resourceapi.") + .allowIfSubType("java.lang.") + .allowIfSubType("java.time.") + // Spring's marker for a cached null ("the SDLC system doesn't know this project") + .allowIfSubType(NullValue.class) + .build(); + var serializer = GenericJacksonJsonRedisSerializer.builder() + .enableDefaultTyping(typeValidator) + .enableSpringCacheNullValueSupport() + .build(); + return builder -> { + var defaults = builder.cacheDefaults().serializeValuesWith(SerializationPair.fromSerializer(serializer)); + builder.cacheDefaults(defaults) + .withCacheConfiguration(SDLC_SYSTEMS, defaults.entryTtl(ttl.sdlcSystemsTtl())) + // a found project is kept longer than "unknown", so a project created there is found soon + .withCacheConfiguration(EXTERNAL_PROJECTS, defaults.entryTtl((key, value) -> + value == null || value instanceof NullValue + ? ttl.missingExternalProjectsTtl() + : ttl.externalProjectsTtl())); + }; + } + + /** + * Caching infrastructure reads this before regular beans exist; as an infrastructure bean of its + * own, without dependencies, that early creation is expected and not reported as a problem. + */ + @Configuration + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + static class CacheErrorHandling implements CachingConfigurer { + + @Override + public CacheErrorHandler errorHandler() { + return new LoggingCacheErrorHandler(); + } + } +} diff --git a/src/main/java/com/company/resourceapi/config/CacheTtlProperties.java b/src/main/java/com/company/resourceapi/config/CacheTtlProperties.java new file mode 100644 index 0000000..597d037 --- /dev/null +++ b/src/main/java/com/company/resourceapi/config/CacheTtlProperties.java @@ -0,0 +1,20 @@ +package com.company.resourceapi.config; + +import java.time.Duration; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.bind.DefaultValue; + +/** + * Time to live of the Redis caches ({@code resourceapi.cache.*}). + * + * @param sdlcSystemsTtl SDLC systems by id; they change rarely and only outside this API + * @param externalProjectsTtl projects as described by their SDLC system ({@code GET .../external}) + * @param missingExternalProjectsTtl "the SDLC system doesn't know this project" answers; short, so a + * project created there is found soon + */ +@ConfigurationProperties("resourceapi.cache") +public record CacheTtlProperties( + @DefaultValue("10m") Duration sdlcSystemsTtl, + @DefaultValue("5m") Duration externalProjectsTtl, + @DefaultValue("30s") Duration missingExternalProjectsTtl) { +} diff --git a/src/main/java/com/company/resourceapi/config/RateLimitProperties.java b/src/main/java/com/company/resourceapi/config/RateLimitProperties.java new file mode 100644 index 0000000..b580eeb --- /dev/null +++ b/src/main/java/com/company/resourceapi/config/RateLimitProperties.java @@ -0,0 +1,42 @@ +package com.company.resourceapi.config; + +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; +import java.time.Duration; +import org.springframework.aot.hint.MemberCategory; +import org.springframework.aot.hint.annotation.RegisterReflection; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.validation.annotation.Validated; + +/** + * Distributed rate limits ({@code resourceapi.rate-limit.*}), shared by all instances through Redis. + * Validated at startup: a broken limit would otherwise fail, or silently stop limiting, at runtime. + * + * @param sdlc calls to one SDLC system (protects its API quota) + * @param sync sync requests for one project + * @param client API requests from one client (IP address) + */ +@Validated +@ConfigurationProperties("resourceapi.rate-limit") +// validation reads the record fields reflectively, which a native image must know about +@RegisterReflection(classes = {RateLimitProperties.class, RateLimitProperties.Limit.class}, + memberCategories = {MemberCategory.ACCESS_DECLARED_FIELDS, MemberCategory.INVOKE_DECLARED_METHODS}) +public record RateLimitProperties(@NotNull @Valid Limit sdlc, @NotNull @Valid Limit sync, @NotNull @Valid Limit client) { + + /** + * At most {@code permits} per sliding window of {@code period}: each permit becomes available again + * one period after it was used. + * + * @param permits how many per period + * @param period the sliding window + * @param maxWait how long a caller may wait for a permit before being refused + */ + public record Limit(@Positive long permits, @NotNull Duration period, @NotNull Duration maxWait) { + + /** When a permit is surely available again: a used permit only comes back after a full period. */ + public long retryAfterSeconds() { + return Math.max(1, (long) Math.ceil(period.toMillis() / 1000.0)); + } + } +} diff --git a/src/main/java/com/company/resourceapi/config/RedissonConfig.java b/src/main/java/com/company/resourceapi/config/RedissonConfig.java new file mode 100644 index 0000000..26c96ba --- /dev/null +++ b/src/main/java/com/company/resourceapi/config/RedissonConfig.java @@ -0,0 +1,94 @@ +package com.company.resourceapi.config; + +import java.time.Duration; +import java.util.Arrays; +import java.util.stream.Stream; +import org.redisson.Redisson; +import org.redisson.api.RedissonClient; +import org.redisson.config.Config; +import org.redisson.config.NameMapper; +import org.springframework.beans.factory.config.BeanFactoryPostProcessor; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.data.redis.autoconfigure.DataRedisConnectionDetails; +import org.springframework.boot.data.redis.autoconfigure.DataRedisProperties; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +/** + * Redisson client for distributed locks and rate limiters, connected to the same Redis as the cache + * (Boot's connection details, so Docker Compose and Testcontainers connections work too). + * + *

It connects lazily, on first use: the application starts without Redis (for example the AOT + * training run while building an image), and Redis being down only affects the calls that need it. + * Only a standalone Redis is supported; with Sentinel or Cluster there is no client and its users fall + * back to running without it. TLS uses the JVM's trust store (Boot SSL bundles aren't passed on). + */ +@Configuration +@EnableConfigurationProperties(RateLimitProperties.class) +public class RedissonConfig { + + static final String REDISSON_CLIENT = "redissonClient"; + static final String TASK_EXECUTOR = "applicationTaskExecutor"; + + @Bean(name = REDISSON_CLIENT, destroyMethod = "shutdown") + @ConditionalOnExpression("'${spring.data.redis.sentinel.master:}' == '' and '${spring.data.redis.cluster.nodes:}' == ''") + RedissonClient redissonClient(DataRedisConnectionDetails connection, DataRedisProperties redis, + @Value("${resourceapi.redisson.key-prefix}") String keyPrefix) { + var standalone = connection.getStandalone(); + var config = new Config() + .setLazyInitialization(true) + .setUsername(connection.getUsername()) + .setPassword(connection.getPassword()) + .setNameMapper(new KeyPrefixNameMapper(keyPrefix)); + config.useSingleServer() + .setAddress((connection.getSslBundle() != null ? "rediss://" : "redis://") + + standalone.getHost() + ":" + standalone.getPort()) + .setDatabase(standalone.getDatabase()) + .setConnectTimeout(millis(redis.getConnectTimeout(), Duration.ofSeconds(10))) + .setTimeout(millis(redis.getTimeout(), Duration.ofSeconds(3))) + // fail fast like the cache: callers decide what to do without Redis + .setRetryAttempts(0) + // a lock or rate limit check now and then needs few connections (defaults: 24 idle, 64 max) + .setConnectionMinimumIdleSize(1) + .setConnectionPoolSize(8) + .setSubscriptionConnectionMinimumIdleSize(1) + .setSubscriptionConnectionPoolSize(4); + return Redisson.create(config); + } + + /** + * On shutdown, the task executor waits for running syncs, which release their locks when they end; + * Redisson must still be up then, so the executor depends on it and is destroyed first. + */ + @Bean + static BeanFactoryPostProcessor taskExecutorShutsDownBeforeRedisson() { + return beanFactory -> { + if (beanFactory.containsBeanDefinition(TASK_EXECUTOR) && beanFactory.containsBeanDefinition(REDISSON_CLIENT)) { + var executor = beanFactory.getBeanDefinition(TASK_EXECUTOR); + var dependsOn = executor.getDependsOn() == null ? new String[0] : executor.getDependsOn(); + executor.setDependsOn(Stream.concat(Arrays.stream(dependsOn), Stream.of(REDISSON_CLIENT)) + .distinct().toArray(String[]::new)); + } + }; + } + + private static int millis(Duration value, Duration fallback) { + return (int) (value != null ? value : fallback).toMillis(); + } + + /** Prefixes every Redisson object name (locks, rate limiters) in Redis. */ + record KeyPrefixNameMapper(String prefix) implements NameMapper { + + @Override + public String map(String name) { + return prefix + name; + } + + @Override + public String unmap(String name) { + return name.startsWith(prefix) ? name.substring(prefix.length()) : name; + } + } +} diff --git a/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java b/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java index eae8e54..ea6f9a4 100755 --- a/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java +++ b/src/main/java/com/company/resourceapi/controllers/ProjectRestController.java @@ -9,11 +9,11 @@ import static org.springframework.http.HttpStatus.SERVICE_UNAVAILABLE; import com.company.resourceapi.clients.ExternalProject; -import com.company.resourceapi.clients.SdlcProjectClient; import com.company.resourceapi.entities.Project; import com.company.resourceapi.entities.SdlcSystem; import com.company.resourceapi.exceptions.ConflictException; import com.company.resourceapi.exceptions.NotFoundException; +import com.company.resourceapi.services.ExternalProjectService; import com.company.resourceapi.services.ProjectService; import com.company.resourceapi.services.SdlcSystemService; import io.swagger.v3.oas.annotations.Operation; @@ -58,7 +58,7 @@ public class ProjectRestController { private final ProjectService projectService; private final SdlcSystemService sdlcSystemService; - private final SdlcProjectClient sdlcProjectClient; + private final ExternalProjectService externalProjectService; @Operation(summary = "Get a Project") @GetMapping(ENDPOINT_ID) @@ -74,7 +74,7 @@ public ResponseEntity getExternalProject( @Parameter(name = PATH_VARIABLE_ID, description = API_PARAM_ID, required = true) @PathVariable(PATH_VARIABLE_ID) final long projectId) throws NotFoundException { var project = projectService.getProject(projectId); - return sdlcProjectClient.fetchProject(project.getSdlcSystem(), project.getExternalId()) + return externalProjectService.findProject(project.getSdlcSystem(), project.getExternalId()) .map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.status(NOT_FOUND).build()); } diff --git a/src/main/java/com/company/resourceapi/controllers/ProjectSyncController.java b/src/main/java/com/company/resourceapi/controllers/ProjectSyncController.java index e49b860..11d12fe 100644 --- a/src/main/java/com/company/resourceapi/controllers/ProjectSyncController.java +++ b/src/main/java/com/company/resourceapi/controllers/ProjectSyncController.java @@ -5,7 +5,6 @@ import static com.company.resourceapi.controllers.ProjectRestController.PATH_VARIABLE_ID; import com.company.resourceapi.exceptions.NotFoundException; -import com.company.resourceapi.services.ProjectService; import com.company.resourceapi.services.ProjectSyncService; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; @@ -23,7 +22,6 @@ @RequiredArgsConstructor public class ProjectSyncController { - private final ProjectService projectService; private final ProjectSyncService projectSyncService; @Operation(summary = "Refresh a Project's name from its SDLC system, in the background") @@ -31,10 +29,8 @@ public class ProjectSyncController { public ResponseEntity syncProject( @Parameter(name = PATH_VARIABLE_ID, description = "ID", required = true) @PathVariable(PATH_VARIABLE_ID) final long projectId) throws NotFoundException { - // an unknown project is reported now, not only in the background (404 via ApiExceptionHandler); - // a full task executor rejects the sync (503 via ApiExceptionHandler) - projectService.getProject(projectId); - projectSyncService.syncName(projectId); + // unknown project: 404, sync already running: 409, no background slot: 503 (ApiExceptionHandler) + projectSyncService.requestSync(projectId); return ResponseEntity.accepted().build(); } } diff --git a/src/main/java/com/company/resourceapi/exceptions/RateLimitExceededException.java b/src/main/java/com/company/resourceapi/exceptions/RateLimitExceededException.java new file mode 100644 index 0000000..1163ea0 --- /dev/null +++ b/src/main/java/com/company/resourceapi/exceptions/RateLimitExceededException.java @@ -0,0 +1,20 @@ +package com.company.resourceapi.exceptions; + +import lombok.Getter; + +/** One of our own rate limits refused the request; it can be retried after {@link #getRetryAfterSeconds()}. */ +@Getter +public class RateLimitExceededException extends RuntimeException { + + /** Whose limit: the caller's own requests (429) or the application's calls to a system (503). */ + public enum Scope { CALLER, UPSTREAM } + + private final Scope scope; + private final long retryAfterSeconds; + + public RateLimitExceededException(Scope scope, String message, long retryAfterSeconds) { + super(message); + this.scope = scope; + this.retryAfterSeconds = retryAfterSeconds; + } +} diff --git a/src/main/java/com/company/resourceapi/exceptions/SyncInProgressException.java b/src/main/java/com/company/resourceapi/exceptions/SyncInProgressException.java new file mode 100644 index 0000000..d715b7c --- /dev/null +++ b/src/main/java/com/company/resourceapi/exceptions/SyncInProgressException.java @@ -0,0 +1,8 @@ +package com.company.resourceapi.exceptions; + +public class SyncInProgressException extends RuntimeException { + + public SyncInProgressException(long projectId) { + super("A sync of project %d is already running.".formatted(projectId)); + } +} diff --git a/src/main/java/com/company/resourceapi/services/ExternalProjectService.java b/src/main/java/com/company/resourceapi/services/ExternalProjectService.java new file mode 100644 index 0000000..88d39e2 --- /dev/null +++ b/src/main/java/com/company/resourceapi/services/ExternalProjectService.java @@ -0,0 +1,53 @@ +package com.company.resourceapi.services; + +import static com.company.resourceapi.config.CacheConfig.EXTERNAL_PROJECTS; + +import com.company.resourceapi.clients.ExternalProject; +import com.company.resourceapi.clients.SdlcProjectClient; +import com.company.resourceapi.config.RateLimitProperties; +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.exceptions.RateLimitExceededException; +import com.company.resourceapi.exceptions.RateLimitExceededException.Scope; +import java.util.Optional; +import lombok.RequiredArgsConstructor; +import org.springframework.cache.annotation.CachePut; +import org.springframework.cache.annotation.Cacheable; +import org.springframework.stereotype.Service; + +/** + * Projects as described by their SDLC system, cached per SDLC system and external id, and rate limited + * per SDLC system across all instances. + */ +@Service +@RequiredArgsConstructor +public class ExternalProjectService { + + private final SdlcProjectClient sdlcProjectClient; + private final RateLimiters rateLimiters; + private final RateLimitProperties rateLimits; + + /** + * Cached, including the answer "the SDLC system doesn't know this project" (cached as null), which + * is kept only briefly so a project created there is found soon (see CacheConfig). + */ + @Cacheable(cacheNames = EXTERNAL_PROJECTS, key = "#sdlcSystem.id + ':' + #externalId") + public Optional findProject(SdlcSystem sdlcSystem, String externalId) { + return fetch(sdlcSystem, externalId); + } + + /** Always asks the SDLC system and caches its answer, found or unknown, replacing the cached one. */ + @CachePut(cacheNames = EXTERNAL_PROJECTS, key = "#sdlcSystem.id + ':' + #externalId") + public Optional refreshProject(SdlcSystem sdlcSystem, String externalId) { + return fetch(sdlcSystem, externalId); + } + + /** Only real calls count against the SDLC system's rate limit, not cache hits. */ + private Optional fetch(SdlcSystem sdlcSystem, String externalId) { + var limit = rateLimits.sdlc(); + if (!rateLimiters.tryAcquire("sdlc-rate:" + sdlcSystem.getId(), limit)) { + throw new RateLimitExceededException(Scope.UPSTREAM, + "Rate limit for SdlcSystem %d reached.".formatted(sdlcSystem.getId()), limit.retryAfterSeconds()); + } + return sdlcProjectClient.fetchProject(sdlcSystem, externalId); + } +} diff --git a/src/main/java/com/company/resourceapi/services/ProjectSyncLock.java b/src/main/java/com/company/resourceapi/services/ProjectSyncLock.java new file mode 100644 index 0000000..76c58f2 --- /dev/null +++ b/src/main/java/com/company/resourceapi/services/ProjectSyncLock.java @@ -0,0 +1,75 @@ +package com.company.resourceapi.services; + +import java.time.Duration; +import java.util.Optional; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import lombok.extern.slf4j.Slf4j; +import org.redisson.api.RLock; +import org.redisson.api.RedissonClient; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Component; + +/** + * At most one sync per project at a time, across all instances (a Redisson lock per project). + * + *

The lock is taken by the request and released by the background task, on another thread, so it + * is held by a random owner id instead of a thread. It expires after the lease time, so an instance + * that dies mid-sync doesn't block the project forever. Without Redis (unreachable, or no Redisson + * client for a non-standalone Redis) the sync runs unlocked: a duplicate sync only costs an extra + * call, while refusing it would stop syncs entirely. + */ +@Component +@Slf4j +public class ProjectSyncLock { + + private final RedissonClient redisson; + private final Duration leaseTime; + + public ProjectSyncLock(ObjectProvider redisson, + @Value("${resourceapi.sync.lock-lease-time}") Duration leaseTime) { + this.redisson = redisson.getIfAvailable(); + this.leaseTime = leaseTime; + if (this.redisson == null) { + log.warn("No Redisson client (only a standalone Redis is supported): syncs run without the per-project lock"); + } + } + + /** @return the lease to release when the sync ends, or empty if a sync of the project is running */ + public Optional tryAcquire(long projectId) { + if (redisson == null) { + return Optional.of(Lease.NONE); + } + var lock = redisson.getLock("project-sync:" + projectId); + long ownerId = ThreadLocalRandom.current().nextLong(); + try { + boolean acquired = lock.tryLockAsync(0, leaseTime.toMillis(), TimeUnit.MILLISECONDS, ownerId) + .toCompletableFuture().join(); + return acquired ? Optional.of(new Lease(lock, ownerId)) : Optional.empty(); + } catch (RuntimeException e) { + log.warn("Sync lock for project {} unavailable, syncing without it: {}", projectId, e.getMessage()); + // a timed-out request may still have taken the lock in Redis: keep the lease, so the sync's + // end tries to release it instead of leaving it held until it expires + return Optional.of(new Lease(lock, ownerId)); + } + } + + /** A lock that may be held; releasing one that isn't (anymore) only logs. */ + public record Lease(RLock lock, long ownerId) { + + static final Lease NONE = new Lease(null, 0); + + public void release() { + if (lock == null) { + return; + } + try { + lock.unlockAsync(ownerId).toCompletableFuture().join(); + } catch (RuntimeException e) { + // not held: never taken, expired after its lease time, or Redis unreachable + log.debug("Sync lock {} not released: {}", lock.getName(), e.getMessage()); + } + } + } +} diff --git a/src/main/java/com/company/resourceapi/services/ProjectSyncService.java b/src/main/java/com/company/resourceapi/services/ProjectSyncService.java index d6c5d40..06ab4b0 100644 --- a/src/main/java/com/company/resourceapi/services/ProjectSyncService.java +++ b/src/main/java/com/company/resourceapi/services/ProjectSyncService.java @@ -1,70 +1,54 @@ package com.company.resourceapi.services; -import static java.util.concurrent.CompletableFuture.completedFuture; - -import com.company.resourceapi.clients.SdlcProjectClient; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicBoolean; +import com.company.resourceapi.config.RateLimitProperties; +import com.company.resourceapi.exceptions.NotFoundException; +import com.company.resourceapi.exceptions.RateLimitExceededException; +import com.company.resourceapi.exceptions.RateLimitExceededException.Scope; +import com.company.resourceapi.exceptions.SyncInProgressException; import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; -import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; -import org.springframework.web.client.RestClientResponseException; -/** Example of background work: refreshes a project's name from its SDLC system. */ +/** Starts background syncs, at most one per project at a time across all instances. */ @Service @RequiredArgsConstructor -@Slf4j public class ProjectSyncService { private final ProjectService projectService; - private final SdlcProjectClient sdlcProjectClient; + private final ProjectSyncLock projectSyncLock; + private final ProjectSyncWorker projectSyncWorker; + private final RateLimiters rateLimiters; + private final RateLimitProperties rateLimits; /** - * Runs on a virtual thread of the application task executor, in the caller's trace. Failures are - * logged here, since nobody waits for the result of a fire-and-forget call. + * Starts refreshing the project from its SDLC system in the background. * - * @return completes when the sync has finished, exceptionally if it failed + * @throws NotFoundException if the project doesn't exist + * @throws RateLimitExceededException if the project was synced too often recently + * @throws SyncInProgressException if a sync of the project is still running + * @throws org.springframework.core.task.TaskRejectedException if no background slot is free */ - @Async - public CompletableFuture syncName(long projectId) { - try { - var project = projectService.getProject(projectId); - var externalId = project.getExternalId(); - var sdlcSystemId = project.getSdlcSystem().getId(); + public void requestSync(long projectId) throws NotFoundException { + // an unknown project is reported now, not only in the background + projectService.getProject(projectId); - var externalProject = sdlcProjectClient.fetchProject(project.getSdlcSystem(), externalId); - if (externalProject.isEmpty()) { - log.warn("Project {} ({}) is unknown to its SDLC system", projectId, externalId); - return completedFuture(null); - } - var externalName = externalProject.get().name(); - if (externalName == null || externalName.equals(project.getName())) { - log.info("Project {} name is unchanged in its SDLC system", projectId); - return completedFuture(null); - } + var lease = projectSyncLock.tryAcquire(projectId) + .orElseThrow(() -> new SyncInProgressException(projectId)); - var applied = new AtomicBoolean(); - projectService.updateProject(projectId, stored -> { - // the project may have been re-keyed while its SDLC system was being asked - if (stored.getExternalId().equals(externalId) && stored.getSdlcSystem().getId() == sdlcSystemId) { - stored.setName(externalName); - applied.set(true); - } - }); - if (applied.get()) { - log.info("Synced project {} name from its SDLC system", projectId); - } else { - log.info("Project {} changed while syncing; SDLC system name not applied", projectId); - } - return completedFuture(null); - } catch (Exception e) { - // the status only: an error response's message would include the upstream body - var reason = e instanceof RestClientResponseException response - ? "HTTP " + response.getStatusCode().value() - : e.getClass().getSimpleName() + ": " + e.getMessage(); - log.warn("Syncing project {} from its SDLC system failed: {}", projectId, reason); - return CompletableFuture.failedFuture(e); + // counted only for syncs that could run: an unknown project or a running sync doesn't use a permit + var limit = rateLimits.sync(); + if (!rateLimiters.tryAcquire("sync-rate:" + projectId, limit)) { + lease.release(); + throw new RateLimitExceededException(Scope.CALLER, + "Too many sync requests for project %d.".formatted(projectId), limit.retryAfterSeconds()); + } + + try { + // the background task releases the lock when it ends, however it ends + projectSyncWorker.syncName(projectId).whenComplete((result, failure) -> lease.release()); + } catch (RuntimeException e) { + // not started, e.g. a full task executor + lease.release(); + throw e; } } } diff --git a/src/main/java/com/company/resourceapi/services/ProjectSyncWorker.java b/src/main/java/com/company/resourceapi/services/ProjectSyncWorker.java new file mode 100644 index 0000000..ddab6f5 --- /dev/null +++ b/src/main/java/com/company/resourceapi/services/ProjectSyncWorker.java @@ -0,0 +1,73 @@ +package com.company.resourceapi.services; + +import static java.util.concurrent.CompletableFuture.completedFuture; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.scheduling.annotation.Async; +import org.springframework.stereotype.Service; +import org.springframework.web.client.RestClientResponseException; + +/** + * Example of background work: refreshes a project's name from its SDLC system. Started through + * {@link ProjectSyncService#requestSync}, which makes sure only one runs per project. + */ +@Service +@RequiredArgsConstructor +@Slf4j +public class ProjectSyncWorker { + + private final ProjectService projectService; + /** Bypasses the cache (a sync must see the current state) and refreshes it. */ + private final ExternalProjectService externalProjectService; + + /** + * Runs on a virtual thread of the application task executor, in the caller's trace. Failures are + * logged here, since nobody waits for the result of a fire-and-forget call. + * + * @return completes when the sync has finished, exceptionally if it failed + */ + @Async + public CompletableFuture syncName(long projectId) { + try { + var project = projectService.getProject(projectId); + var externalId = project.getExternalId(); + var sdlcSystemId = project.getSdlcSystem().getId(); + + var externalProject = externalProjectService.refreshProject(project.getSdlcSystem(), externalId); + if (externalProject.isEmpty()) { + log.warn("Project {} ({}) is unknown to its SDLC system", projectId, externalId); + return completedFuture(null); + } + var externalName = externalProject.get().name(); + if (externalName == null || externalName.equals(project.getName())) { + log.info("Project {} name is unchanged in its SDLC system", projectId); + return completedFuture(null); + } + + var applied = new AtomicBoolean(); + projectService.updateProject(projectId, stored -> { + // the project may have been re-keyed while its SDLC system was being asked + if (stored.getExternalId().equals(externalId) && stored.getSdlcSystem().getId() == sdlcSystemId) { + stored.setName(externalName); + applied.set(true); + } + }); + if (applied.get()) { + log.info("Synced project {} name from its SDLC system", projectId); + } else { + log.info("Project {} changed while syncing; SDLC system name not applied", projectId); + } + return completedFuture(null); + } catch (Exception e) { + // the status only: an error response's message would include the upstream body + var reason = e instanceof RestClientResponseException response + ? "HTTP " + response.getStatusCode().value() + : e.getClass().getSimpleName() + ": " + e.getMessage(); + log.warn("Syncing project {} from its SDLC system failed: {}", projectId, reason); + return CompletableFuture.failedFuture(e); + } + } +} diff --git a/src/main/java/com/company/resourceapi/services/RateLimiters.java b/src/main/java/com/company/resourceapi/services/RateLimiters.java new file mode 100644 index 0000000..787c239 --- /dev/null +++ b/src/main/java/com/company/resourceapi/services/RateLimiters.java @@ -0,0 +1,124 @@ +package com.company.resourceapi.services; + +import com.company.resourceapi.config.RateLimitProperties; +import java.time.Duration; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import lombok.extern.slf4j.Slf4j; +import org.redisson.api.RRateLimiter; +import org.redisson.api.RateType; +import org.redisson.api.RedissonClient; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.stereotype.Component; + +/** + * Sliding-window rate limiters shared by all instances (Redisson {@link RRateLimiter}), one per name. + * + *

A limiter is set up with the configured rate the first time this instance uses it (updating a + * limiter created with another rate), and again if Redis dropped it after it expired from disuse. + * Without Redis (unreachable, or no Redisson client) requests are allowed: limiting is a protection, + * losing it shouldn't stop the application. After a failure Redis is left alone for a short while, + * so an outage doesn't add a Redis timeout to every request. + */ +@Component +@Slf4j +public class RateLimiters { + + /** Unused limiters are removed from Redis after this many periods, but not sooner than MIN_KEEP_ALIVE. */ + private static final int KEEP_ALIVE_PERIODS = 10; + private static final Duration MIN_KEEP_ALIVE = Duration.ofMinutes(1); + /** How long requests are allowed without asking Redis after it failed. */ + private static final Duration PAUSE_AFTER_FAILURE = Duration.ofSeconds(10); + /** Limiters this instance has set up, most recently used kept (bounded: one per project). */ + private static final int MAX_REMEMBERED = 10_000; + + private final RedissonClient redisson; + /** System.nanoTime() until which Redis isn't asked; 0 while it works. */ + private volatile long pausedUntil; + private final Map setUp = Collections.synchronizedMap(new LinkedHashMap<>(16, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > MAX_REMEMBERED; + } + }); + + public RateLimiters(ObjectProvider redisson) { + this.redisson = redisson.getIfAvailable(); + } + + /** @return whether a permit was obtained, waiting at most the limit's max wait */ + public boolean tryAcquire(String name, RateLimitProperties.Limit limit) { + if (redisson == null || isPaused()) { + return true; + } + try { + var limiter = redisson.getRateLimiter(name); + if (!setUp.containsKey(name)) { + setUp(name, limiter, limit); + } + try { + return acquire(limiter, limit); + } catch (RuntimeException e) { + if (!isNotInitialized(e)) { + throw e; + } + // expired from Redis since this instance set it up + setUp(name, limiter, limit); + return acquire(limiter, limit); + } + } catch (RuntimeException e) { + pausedUntil = System.nanoTime() + PAUSE_AFTER_FAILURE.toNanos(); + log.warn("Rate limiter {} unavailable, allowing requests without rate limits for {}: {}", + name, PAUSE_AFTER_FAILURE, e.getMessage()); + return true; + } + } + + private boolean isPaused() { + long until = pausedUntil; + if (until == 0) { + return false; + } + if (System.nanoTime() - until < 0) { + return true; + } + pausedUntil = 0; + return false; + } + + private static Duration keepAlive(RateLimitProperties.Limit limit) { + var periods = limit.period().multipliedBy(KEEP_ALIVE_PERIODS); + return periods.compareTo(MIN_KEEP_ALIVE) > 0 ? periods : MIN_KEEP_ALIVE; + } + + private static boolean acquire(RRateLimiter limiter, RateLimitProperties.Limit limit) { + return limiter.tryAcquireAsync(1, limit.maxWait()).toCompletableFuture().join(); + } + + private void setUp(String name, RRateLimiter limiter, RateLimitProperties.Limit limit) { + var keepAlive = keepAlive(limit); + boolean created = limiter.trySetRateAsync(RateType.OVERALL, limit.permits(), limit.period(), keepAlive) + .toCompletableFuture().join(); + if (!created) { + // an existing limiter keeps its rate unless replaced, e.g. after the configuration changed + var config = limiter.getConfigAsync().toCompletableFuture().join(); + if (config.getRate() != limit.permits() || config.getRateInterval() != limit.period().toMillis()) { + log.info("Updating rate limiter {} to {} per {}", name, limit.permits(), limit.period()); + limiter.setRateAsync(RateType.OVERALL, limit.permits(), limit.period(), keepAlive) + .toCompletableFuture().join(); + } + } + // only once it is set up: a failure above is retried on the next call + setUp.put(name, Boolean.TRUE); + } + + private static boolean isNotInitialized(Throwable e) { + for (Throwable cause = e; cause != null; cause = cause.getCause()) { + if (cause.getMessage() != null && cause.getMessage().contains("not initialized")) { + return true; + } + } + return false; + } +} diff --git a/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java b/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java index a9b60ec..7b783ae 100755 --- a/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java +++ b/src/main/java/com/company/resourceapi/services/impl/SdlcSystemServiceImpl.java @@ -1,10 +1,13 @@ package com.company.resourceapi.services.impl; +import static com.company.resourceapi.config.CacheConfig.SDLC_SYSTEMS; + import com.company.resourceapi.entities.SdlcSystem; import com.company.resourceapi.exceptions.NotFoundException; import com.company.resourceapi.repositories.SdlcSystemRepository; import com.company.resourceapi.services.SdlcSystemService; import lombok.RequiredArgsConstructor; +import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Service; @Service @@ -13,6 +16,11 @@ public class SdlcSystemServiceImpl implements SdlcSystemService { private final SdlcSystemRepository sdlcSystemRepository; + /** + * Cached: SDLC systems are read on every create, update and SDLC call, and rarely change. When an + * entry expires, concurrent callers wait for one database read instead of each doing their own. + */ + @Cacheable(cacheNames = SDLC_SYSTEMS, sync = true) public SdlcSystem getSdlcSystem(long id) throws NotFoundException { return sdlcSystemRepository.findById(id) .orElseThrow(() -> new NotFoundException(SdlcSystem.class, id)); diff --git a/src/main/java/com/company/resourceapi/web/ApiExceptionHandler.java b/src/main/java/com/company/resourceapi/web/ApiExceptionHandler.java index fa78cc8..cdb0b5d 100644 --- a/src/main/java/com/company/resourceapi/web/ApiExceptionHandler.java +++ b/src/main/java/com/company/resourceapi/web/ApiExceptionHandler.java @@ -1,10 +1,14 @@ package com.company.resourceapi.web; import static org.springframework.http.HttpHeaders.RETRY_AFTER; +import static org.springframework.http.HttpStatus.CONFLICT; import static org.springframework.http.HttpStatus.NOT_FOUND; import static org.springframework.http.HttpStatus.SERVICE_UNAVAILABLE; +import static org.springframework.http.HttpStatus.TOO_MANY_REQUESTS; import com.company.resourceapi.exceptions.NotFoundException; +import com.company.resourceapi.exceptions.RateLimitExceededException; +import com.company.resourceapi.exceptions.SyncInProgressException; import lombok.extern.slf4j.Slf4j; import org.springframework.core.task.TaskRejectedException; import org.springframework.http.ResponseEntity; @@ -22,6 +26,30 @@ ResponseEntity handleNotFound(NotFoundException e) { return ResponseEntity.status(NOT_FOUND).build(); } + /** Another request already started a sync of this project, which is still running. */ + @ExceptionHandler(SyncInProgressException.class) + ResponseEntity handleSyncInProgress(SyncInProgressException e) { + log.debug(e.getMessage()); + return ResponseEntity.status(CONFLICT).build(); + } + + /** + * One of our rate limits: the caller's own requests are 429; our calls to an SDLC system being + * limited is a temporary unavailability on our side, 503. + */ + @ExceptionHandler(RateLimitExceededException.class) + ResponseEntity handleRateLimitExceeded(RateLimitExceededException e) { + var caller = e.getScope() == RateLimitExceededException.Scope.CALLER; + // a refused caller is expected, possibly many times a second; holding back our own calls is worth seeing + if (caller) { + log.debug(e.getMessage()); + } else { + log.info(e.getMessage()); + } + var status = caller ? TOO_MANY_REQUESTS : SERVICE_UNAVAILABLE; + return ResponseEntity.status(status).header(RETRY_AFTER, String.valueOf(e.getRetryAfterSeconds())).build(); + } + /** The background task executor is at its concurrency limit; the client can retry shortly. */ @ExceptionHandler(TaskRejectedException.class) ResponseEntity handleTaskRejected(TaskRejectedException e) { diff --git a/src/main/java/com/company/resourceapi/web/ClientRateLimitConfig.java b/src/main/java/com/company/resourceapi/web/ClientRateLimitConfig.java new file mode 100644 index 0000000..d552051 --- /dev/null +++ b/src/main/java/com/company/resourceapi/web/ClientRateLimitConfig.java @@ -0,0 +1,27 @@ +package com.company.resourceapi.web; + +import com.company.resourceapi.config.RateLimitProperties; +import com.company.resourceapi.services.RateLimiters; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; +import org.springframework.web.servlet.HandlerExceptionResolver; + +/** Registers {@link ClientRateLimitFilter} for the API only (not Swagger UI or the OpenAPI document). */ +@Configuration +public class ClientRateLimitConfig { + + @Bean + FilterRegistrationBean clientRateLimitFilter(RateLimiters rateLimiters, + RateLimitProperties rateLimits, + @Qualifier("handlerExceptionResolver") HandlerExceptionResolver exceptionResolver) { + var registration = new FilterRegistrationBean<>( + new ClientRateLimitFilter(rateLimiters, rateLimits, exceptionResolver)); + registration.addUrlPatterns("/api/*"); + // after the server observation (+1) and trace id (+2) filters + registration.setOrder(Ordered.HIGHEST_PRECEDENCE + 3); + return registration; + } +} diff --git a/src/main/java/com/company/resourceapi/web/ClientRateLimitFilter.java b/src/main/java/com/company/resourceapi/web/ClientRateLimitFilter.java new file mode 100644 index 0000000..570a96a --- /dev/null +++ b/src/main/java/com/company/resourceapi/web/ClientRateLimitFilter.java @@ -0,0 +1,44 @@ +package com.company.resourceapi.web; + +import com.company.resourceapi.config.RateLimitProperties; +import com.company.resourceapi.exceptions.RateLimitExceededException; +import com.company.resourceapi.exceptions.RateLimitExceededException.Scope; +import com.company.resourceapi.services.RateLimiters; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import lombok.RequiredArgsConstructor; +import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.servlet.HandlerExceptionResolver; + +/** + * Limits API requests per client IP address, across all instances (429 + Retry-After). Behind a + * reverse proxy the address comes from X-Forwarded-For, which Tomcat only trusts from internal proxy + * addresses ({@code server.forward-headers-strategy=native}), so clients can't pick their own key. + * + *

Registered for {@code /api/*} by {@link ClientRateLimitConfig}, after the trace id filter, so a + * refused request still gets its X-Trace-Id. The refusal is answered like any other + * {@link RateLimitExceededException}, by the application's exception handling. + */ +@RequiredArgsConstructor +public class ClientRateLimitFilter extends OncePerRequestFilter { + + private final RateLimiters rateLimiters; + private final RateLimitProperties rateLimits; + private final HandlerExceptionResolver exceptionResolver; + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + var limit = rateLimits.client(); + if (!rateLimiters.tryAcquire("client-rate:" + request.getRemoteAddr(), limit)) { + var refusal = new RateLimitExceededException(Scope.CALLER, + "Too many requests from " + request.getRemoteAddr(), limit.retryAfterSeconds()); + exceptionResolver.resolveException(request, response, null, refusal); + return; + } + filterChain.doFilter(request, response); + } +} diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 81ccaf5..ade8268 100755 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -12,6 +12,37 @@ spring.task.execution.simple.reject-tasks-when-limit-reached=true # on shutdown, let running background tasks finish (up to 10s) spring.task.execution.shutdown.await-termination=true spring.task.execution.shutdown.await-termination-period=10s +# redis (cache); spring.data.redis.host/port default to localhost:6379. Short timeouts: the cache is an +# optimization, and with Redis unreachable each cache read/write waits this long before it is skipped +# (Lettuce's default command timeout is 60s) +spring.data.redis.connect-timeout=500ms +spring.data.redis.timeout=500ms +# Redis is used for caching only, not as a Spring Data repository store +spring.data.redis.repositories.enabled=false +# explicit: Redisson brings a JCache provider, which Boot would otherwise prefer over Redis +spring.cache.type=redis +spring.cache.redis.key-prefix=resourceapi:: +resourceapi.cache.sdlc-systems-ttl=10m +resourceapi.cache.external-projects-ttl=5m +resourceapi.cache.missing-external-projects-ttl=30s +# redisson (locks, rate limiters) uses the same Redis; its object names get this prefix +resourceapi.redisson.key-prefix=resourceapi: +# a project's sync lock expires after this even if never released (e.g. the instance died) +resourceapi.sync.lock-lease-time=60s +# distributed rate limits (Redisson): calls to one SDLC system (503 + Retry-After when exceeded after +# waiting max-wait), and sync requests per project (429 + Retry-After) +resourceapi.rate-limit.sdlc.permits=10 +resourceapi.rate-limit.sdlc.period=1s +resourceapi.rate-limit.sdlc.max-wait=1s +resourceapi.rate-limit.sync.permits=6 +resourceapi.rate-limit.sync.period=1m +resourceapi.rate-limit.sync.max-wait=0s +# API requests per client IP (429 + Retry-After); behind a reverse proxy the client IP comes from +# X-Forwarded-For, trusted only from internal proxy addresses (Tomcat's RemoteIpValve) +resourceapi.rate-limit.client.permits=100 +resourceapi.rate-limit.client.period=1s +resourceapi.rate-limit.client.max-wait=0s +server.forward-headers-strategy=native # datasource spring.datasource.url=jdbc:h2:mem:test;MODE=MySQL;DATABASE_TO_LOWER=TRUE spring.sql.init.platform=h2 diff --git a/src/test/java/com/company/resourceapi/RedisTestContainerInitializer.java b/src/test/java/com/company/resourceapi/RedisTestContainerInitializer.java new file mode 100644 index 0000000..1a42868 --- /dev/null +++ b/src/test/java/com/company/resourceapi/RedisTestContainerInitializer.java @@ -0,0 +1,44 @@ +package com.company.resourceapi; + +import java.util.Map; +import java.util.UUID; +import org.springframework.context.ApplicationContextInitializer; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.core.env.MapPropertySource; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.utility.DockerImageName; + +/** + * Gives every test application context a real Redis: one container, started on first use and + * shared by all contexts of the test run (registered in META-INF/spring.factories). + * + *

Each context gets its own key prefix: contexts have their own in-memory database, so the same + * id can mean different rows in two contexts and they must not see each other's cache entries. + * A test can still point at another Redis with its own {@code spring.data.redis.*} properties. + */ +public class RedisTestContainerInitializer implements ApplicationContextInitializer { + + private static final GenericContainer REDIS = + new GenericContainer<>(DockerImageName.parse("redis:8-alpine")).withExposedPorts(6379); + + @Override + public void initialize(ConfigurableApplicationContext context) { + startOnce(); + var propertySources = context.getEnvironment().getPropertySources(); + // defaults: a test's own spring.data.redis.* properties win + propertySources.addLast(new MapPropertySource("redisTestContainer", Map.of( + "spring.data.redis.host", REDIS.getHost(), + "spring.data.redis.port", REDIS.getMappedPort(6379)))); + // overrides application.properties + var contextId = "test-" + UUID.randomUUID(); + propertySources.addFirst(new MapPropertySource("redisTestContainerKeyPrefix", Map.of( + "spring.cache.redis.key-prefix", contextId + "::", + "resourceapi.redisson.key-prefix", contextId + ":"))); + } + + private static synchronized void startOnce() { + if (!REDIS.isRunning()) { + REDIS.start(); + } + } +} diff --git a/src/test/java/com/company/resourceapi/cache/CacheIntegrationTest.java b/src/test/java/com/company/resourceapi/cache/CacheIntegrationTest.java new file mode 100644 index 0000000..d15b903 --- /dev/null +++ b/src/test/java/com/company/resourceapi/cache/CacheIntegrationTest.java @@ -0,0 +1,242 @@ +package com.company.resourceapi.cache; + +import static com.company.resourceapi.config.CacheConfig.EXTERNAL_PROJECTS; +import static com.company.resourceapi.config.CacheConfig.SDLC_SYSTEMS; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.notFound; +import static com.github.tomakehurst.wiremock.client.WireMock.okJson; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.company.resourceapi.clients.ExternalProject; +import com.company.resourceapi.controllers.ProjectRestController; +import com.company.resourceapi.entities.Project; +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.repositories.ProjectRepository; +import com.company.resourceapi.repositories.SdlcSystemRepository; +import com.company.resourceapi.services.SdlcSystemService; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.time.Duration; +import java.time.Instant; +import java.util.function.BooleanSupplier; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.cache.CacheManager; +import org.springframework.data.redis.core.StringRedisTemplate; +import org.springframework.test.web.servlet.MockMvc; + +/** Redis cache in front of the SDLC systems, against a real Redis (RedisTestContainerInitializer). */ +@SpringBootTest(properties = "resourceapi.cache.missing-external-projects-ttl=3s") +@AutoConfigureMockMvc +class CacheIntegrationTest { + + private static final String EXTERNAL_ID = "CACHED"; + private static final String SDLC_PROJECT_URL = "/rest/api/2/project/" + EXTERNAL_ID; + + @RegisterExtension + static WireMockExtension sdlcSystemServer = WireMockExtension.newInstance() + .options(wireMockConfig().dynamicPort()) + .build(); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Autowired + private ProjectRepository projectRepository; + + @Autowired + private SdlcSystemService sdlcSystemService; + + @Autowired + private CacheManager cacheManager; + + @Autowired + private StringRedisTemplate redisTemplate; + + @Value("${spring.cache.redis.key-prefix}") + private String keyPrefix; + + private SdlcSystem sdlcSystem; + private long projectId; + + @BeforeEach + void setup() { + sdlcSystem = new SdlcSystem(); + sdlcSystem.setBaseUrl(sdlcSystemServer.baseUrl()); + sdlcSystem = sdlcSystemRepository.save(sdlcSystem); + + var project = new Project(); + project.setExternalId(EXTERNAL_ID); + project.setName("Local name"); + project.setSdlcSystem(sdlcSystem); + projectId = projectRepository.save(project).getId(); + } + + @Test + void givenRepeatedLookupsThenSdlcSystemIsAskedOnce() throws Exception { + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"CACHED\", \"name\": \"From SDLC\"}"))); + + mockMvc.perform(get(externalUrl())).andExpect(status().isOk()); + // counted after the first lookup: the HTTP client may retry a GET on a stale pooled connection + int callsForFirstLookup = sdlcSystemServer.findAll(getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))).size(); + + for (int i = 0; i < 2; i++) { + mockMvc.perform(get(externalUrl())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.name", equalTo("From SDLC"))); + } + + // the next lookups are served from the cache + sdlcSystemServer.verify(callsForFirstLookup, getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))); + // stored as JSON in Redis and read back as the record, not a map + assertThat(cacheManager.getCache(EXTERNAL_PROJECTS).get(externalKey(), ExternalProject.class)) + .isEqualTo(new ExternalProject("CACHED", "From SDLC", null)); + } + + @Test + void givenProjectUnknownToSdlcSystemThenThatIsRememberedBriefly() throws Exception { + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(notFound().withHeader("Content-Type", "application/json").withBody("{}"))); + + mockMvc.perform(get(externalUrl())).andExpect(status().isNotFound()); + int callsForFirstLookup = sdlcSystemServer.findAll(getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))).size(); + mockMvc.perform(get(externalUrl())).andExpect(status().isNotFound()); + + // the "unknown" answer is remembered (as a cached null), with the short time to live (3s here) + sdlcSystemServer.verify(callsForFirstLookup, getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))); + var cached = cacheManager.getCache(EXTERNAL_PROJECTS).get(externalKey()); + assertThat(cached).isNotNull(); + assertThat(cached.get()).isNull(); + assertThat(redisTemplate.getExpire(redisKey())).isBetween(1L, 3L); + + // the project is created in the SDLC system: found once the "unknown" answer expired + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"CACHED\", \"name\": \"Created later\"}"))); + await(() -> statusOf(externalUrl()) == 200); + // ... and kept with the long time to live (5m) + assertThat(redisTemplate.getExpire(redisKey())).isGreaterThan(60L); + } + + @Test + void givenProjectRememberedAsUnknownWhenSyncedThenItIsFoundRightAway() throws Exception { + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(notFound().withHeader("Content-Type", "application/json").withBody("{}"))); + mockMvc.perform(get(externalUrl())).andExpect(status().isNotFound()); + + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"CACHED\", \"name\": \"Created later\"}"))); + mockMvc.perform(post(ProjectRestController.ENDPOINT + "/" + projectId + "/sync")) + .andExpect(status().isAccepted()); + await(() -> projectRepository.findById(projectId).orElseThrow().getName().equals("Created later")); + int callsUntilSynced = sdlcSystemServer.findAll(getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))).size(); + + // the sync replaced the "unknown" answer with the project, well within its 3s + assertThat(cacheManager.getCache(EXTERNAL_PROJECTS).get(externalKey(), ExternalProject.class)) + .isEqualTo(new ExternalProject("CACHED", "Created later", null)); + mockMvc.perform(get(externalUrl())).andExpect(status().isOk()); + sdlcSystemServer.verify(callsUntilSynced, getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))); + } + + @Test + void givenSyncThenItBypassesAndRefreshesTheCache() throws Exception { + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"CACHED\", \"name\": \"Old name\"}"))); + mockMvc.perform(get(externalUrl())).andExpect(jsonPath("$.name", equalTo("Old name"))); + + // the SDLC system renames the project; a sync must not use the cached answer + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"CACHED\", \"name\": \"New name\"}"))); + mockMvc.perform(post(ProjectRestController.ENDPOINT + "/" + projectId + "/sync")) + .andExpect(status().isAccepted()); + await(() -> projectRepository.findById(projectId).orElseThrow().getName().equals("New name")); + + // ... and the cache now has the fresh answer, without another call + mockMvc.perform(get(externalUrl())).andExpect(jsonPath("$.name", equalTo("New name"))); + sdlcSystemServer.verify(2, getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))); + } + + @Test + void givenSdlcSystemLookupThenItIsServedFromTheCache() throws Exception { + var first = sdlcSystemService.getSdlcSystem(sdlcSystem.getId()); + + // changed behind the API's back: the cached value is used until it expires + sdlcSystem.setDescription("Changed in the database"); + sdlcSystemRepository.save(sdlcSystem); + var second = sdlcSystemService.getSdlcSystem(sdlcSystem.getId()); + + assertThat(second).isNotSameAs(first); + assertThat(second.getBaseUrl()).isEqualTo(sdlcSystemServer.baseUrl()); + assertThat(second.getDescription()).isNull(); + assertThat(cacheManager.getCache(SDLC_SYSTEMS).get(sdlcSystem.getId(), SdlcSystem.class)).isNotNull(); + } + + private String externalUrl() { + return ProjectRestController.ENDPOINT + "/" + projectId + "/external"; + } + + private String externalKey() { + return sdlcSystem.getId() + ":" + EXTERNAL_ID; + } + + private String redisKey() { + return keyPrefix + EXTERNAL_PROJECTS + "::" + externalKey(); + } + + private static void await(BooleanSupplier condition) throws InterruptedException { + var deadline = Instant.now().plus(Duration.ofSeconds(5)); + while (!condition.getAsBoolean()) { + assertThat(Instant.now()).as("condition not met within 5s").isBefore(deadline); + Thread.sleep(50); + } + } + + @Test + void givenCachedValuesThenRedisKeysCarryTheConfiguredPrefixAndTtl() throws Exception { + sdlcSystemService.getSdlcSystem(sdlcSystem.getId()); + + var keys = redisTemplate.keys(keyPrefix + SDLC_SYSTEMS + "::" + sdlcSystem.getId()); + // this context's prefix (RedisTestContainerInitializer), so contexts don't share entries + assertThat(keys).singleElement().asString().startsWith(keyPrefix + SDLC_SYSTEMS + "::"); + assertThat(redisTemplate.getExpire(keys.iterator().next())).isBetween(1L, 600L); + } + + @Test + void givenProjectGoneFromSdlcSystemWhenSyncedThenCachedEntryIsRemoved() throws Exception { + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"CACHED\", \"name\": \"Soon gone\"}"))); + mockMvc.perform(get(externalUrl())).andExpect(status().isOk()); + + // deleted in the SDLC system; the sync finds out + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(notFound().withHeader("Content-Type", "application/json").withBody("{}"))); + mockMvc.perform(post(ProjectRestController.ENDPOINT + "/" + projectId + "/sync")) + .andExpect(status().isAccepted()); + await(() -> cacheManager.getCache(EXTERNAL_PROJECTS).get(externalKey()) == null); + + mockMvc.perform(get(externalUrl())).andExpect(status().isNotFound()); + } + + private int statusOf(String url) { + try { + return mockMvc.perform(get(url)).andReturn().getResponse().getStatus(); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } +} diff --git a/src/test/java/com/company/resourceapi/cache/CacheUnavailableTest.java b/src/test/java/com/company/resourceapi/cache/CacheUnavailableTest.java new file mode 100644 index 0000000..a4aaa3e --- /dev/null +++ b/src/test/java/com/company/resourceapi/cache/CacheUnavailableTest.java @@ -0,0 +1,94 @@ +package com.company.resourceapi.cache; + +import static com.github.tomakehurst.wiremock.client.WireMock.okJson; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.company.resourceapi.controllers.ProjectRestController; +import com.company.resourceapi.entities.Project; +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.repositories.ProjectRepository; +import com.company.resourceapi.repositories.SdlcSystemRepository; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.time.Duration; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.http.MediaType; +import org.springframework.test.web.servlet.MockMvc; + +/** + * The cache is an optimization: with Redis unreachable the API keeps working, just uncached, and + * requests stay fast thanks to the short Redis timeouts in application.properties. + */ +@SpringBootTest(properties = "spring.data.redis.port=1") +@AutoConfigureMockMvc +class CacheUnavailableTest { + + @RegisterExtension + static WireMockExtension sdlcSystemServer = WireMockExtension.newInstance() + .options(wireMockConfig().dynamicPort()) + .build(); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Autowired + private ProjectRepository projectRepository; + + @Test + void givenRedisDownWhenUsingCachedEndpointsThenTheyStillWork() throws Exception { + var sdlcSystem = new SdlcSystem(); + sdlcSystem.setBaseUrl(sdlcSystemServer.baseUrl()); + sdlcSystem = sdlcSystemRepository.save(sdlcSystem); + var project = new Project(); + project.setExternalId("NOREDIS"); + project.setSdlcSystem(sdlcSystem); + long projectId = projectRepository.save(project).getId(); + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo("/rest/api/2/project/NOREDIS")) + .willReturn(okJson("{\"key\": \"NOREDIS\"}"))); + + var start = System.nanoTime(); + mockMvc.perform(get(ProjectRestController.ENDPOINT + "/" + projectId + "/external")) + .andExpect(status().isOk()); + mockMvc.perform(post(ProjectRestController.ENDPOINT) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"externalId\": \"NOREDIS-2\", \"sdlcSystem\": {\"id\": " + sdlcSystem.getId() + "}}")) + .andExpect(status().isCreated()); + + // each skipped cache read/write is bounded by the Redis timeouts, not Lettuce's 60s default + assertThat(Duration.ofNanos(System.nanoTime() - start)).isLessThan(Duration.ofSeconds(10)); + } + + @Test + void givenRedisDownWhenSyncThenItRunsWithoutTheLock() throws Exception { + var sdlcSystem = new SdlcSystem(); + sdlcSystem.setBaseUrl(sdlcSystemServer.baseUrl()); + sdlcSystem = sdlcSystemRepository.save(sdlcSystem); + var project = new Project(); + project.setExternalId("NOLOCK"); + project.setSdlcSystem(sdlcSystem); + long projectId = projectRepository.save(project).getId(); + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo("/rest/api/2/project/NOLOCK")) + .willReturn(okJson("{\"key\": \"NOLOCK\", \"name\": \"Synced without Redis\"}"))); + + mockMvc.perform(post(ProjectRestController.ENDPOINT + "/" + projectId + "/sync")) + .andExpect(status().isAccepted()); + + var deadline = System.nanoTime() + Duration.ofSeconds(10).toNanos(); + while (!"Synced without Redis".equals(projectRepository.findById(projectId).orElseThrow().getName())) { + assertThat(System.nanoTime()).as("sync not done within 10s").isLessThan(deadline); + Thread.sleep(50); + } + } +} diff --git a/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerConflictTest.java b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerConflictTest.java index 3e01367..464dd07 100644 --- a/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerConflictTest.java +++ b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerConflictTest.java @@ -7,8 +7,8 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -import com.company.resourceapi.clients.SdlcProjectClient; import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.services.ExternalProjectService; import com.company.resourceapi.services.ProjectService; import com.company.resourceapi.services.SdlcSystemService; import java.sql.SQLException; @@ -36,7 +36,7 @@ class ProjectRestControllerConflictTest { private SdlcSystemService sdlcSystemService; @MockitoBean - private SdlcProjectClient sdlcProjectClient; + private ExternalProjectService externalProjectService; @Test void givenConcurrentDuplicateWhenPostThenReturnStatusConflict() throws Exception { diff --git a/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerSdlcFailureTest.java b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerSdlcFailureTest.java index f6a7c5e..ebedcc2 100644 --- a/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerSdlcFailureTest.java +++ b/src/test/java/com/company/resourceapi/controllers/ProjectRestControllerSdlcFailureTest.java @@ -7,9 +7,9 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -import com.company.resourceapi.clients.SdlcProjectClient; import com.company.resourceapi.entities.Project; import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.services.ExternalProjectService; import com.company.resourceapi.services.ProjectService; import com.company.resourceapi.services.SdlcSystemService; import java.util.Optional; @@ -35,7 +35,7 @@ class ProjectRestControllerSdlcFailureTest { private SdlcSystemService sdlcSystemService; @MockitoBean - private SdlcProjectClient sdlcProjectClient; + private ExternalProjectService externalProjectService; @BeforeEach void setup() throws Exception { @@ -47,7 +47,7 @@ void setup() throws Exception { @Test void givenExhaustedConnectionPoolWhenGetExternalThenReturnStatusServiceUnavailable() throws Exception { - when(sdlcProjectClient.fetchProject(any(), anyString())).thenThrow(new ResourceAccessException( + when(externalProjectService.findProject(any(), anyString())).thenThrow(new ResourceAccessException( "I/O error", new ConnectionRequestTimeoutException("Timeout deadline: 2000 MILLISECONDS"))); mockMvc.perform(get(ProjectRestController.ENDPOINT + "/1/external")) @@ -58,7 +58,7 @@ void givenExhaustedConnectionPoolWhenGetExternalThenReturnStatusServiceUnavailab @Test void givenNoTracerWhenHandledThenNoTraceIdHeader() throws Exception { // this web slice loads no tracing: the filter must not send an empty trace id - when(sdlcProjectClient.fetchProject(any(), anyString())).thenReturn(Optional.empty()); + when(externalProjectService.findProject(any(), anyString())).thenReturn(Optional.empty()); mockMvc.perform(get(ProjectRestController.ENDPOINT + "/1/external")) .andExpect(header().doesNotExist("X-Trace-Id")); @@ -66,7 +66,7 @@ void givenNoTracerWhenHandledThenNoTraceIdHeader() throws Exception { @Test void givenUnreachableSdlcSystemWhenGetExternalThenReturnStatusBadGateway() throws Exception { - when(sdlcProjectClient.fetchProject(any(), anyString())) + when(externalProjectService.findProject(any(), anyString())) .thenThrow(new ResourceAccessException("I/O error", new java.net.ConnectException("refused"))); mockMvc.perform(get(ProjectRestController.ENDPOINT + "/1/external")) diff --git a/src/test/java/com/company/resourceapi/lock/NonStandaloneRedisTest.java b/src/test/java/com/company/resourceapi/lock/NonStandaloneRedisTest.java new file mode 100644 index 0000000..88fc296 --- /dev/null +++ b/src/test/java/com/company/resourceapi/lock/NonStandaloneRedisTest.java @@ -0,0 +1,74 @@ +package com.company.resourceapi.lock; + +import static com.github.tomakehurst.wiremock.client.WireMock.okJson; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.company.resourceapi.controllers.ProjectRestController; +import com.company.resourceapi.entities.Project; +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.repositories.ProjectRepository; +import com.company.resourceapi.repositories.SdlcSystemRepository; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.time.Duration; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.redisson.api.RedissonClient; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.test.web.servlet.MockMvc; + +/** A Sentinel (or Cluster) Redis doesn't stop the application: syncs run without the lock. */ +@SpringBootTest(properties = { + "spring.data.redis.sentinel.master=mymaster", + "spring.data.redis.sentinel.nodes=localhost:1"}) +@AutoConfigureMockMvc +class NonStandaloneRedisTest { + + @RegisterExtension + static WireMockExtension sdlcSystemServer = WireMockExtension.newInstance() + .options(wireMockConfig().dynamicPort()) + .build(); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Autowired + private ProjectRepository projectRepository; + + @Autowired + private ObjectProvider redisson; + + @Test + void givenSentinelRedisThenApplicationStartsAndSyncsRunUnlocked() throws Exception { + assertThat(redisson.getIfAvailable()).isNull(); + + var sdlcSystem = new SdlcSystem(); + sdlcSystem.setBaseUrl(sdlcSystemServer.baseUrl()); + sdlcSystem = sdlcSystemRepository.save(sdlcSystem); + var project = new Project(); + project.setExternalId("SENTINEL"); + project.setSdlcSystem(sdlcSystem); + long projectId = projectRepository.save(project).getId(); + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo("/rest/api/2/project/SENTINEL")) + .willReturn(okJson("{\"key\": \"SENTINEL\", \"name\": \"Synced unlocked\"}"))); + + mockMvc.perform(post(ProjectRestController.ENDPOINT + "/" + projectId + "/sync")) + .andExpect(status().isAccepted()); + + var deadline = System.nanoTime() + Duration.ofSeconds(15).toNanos(); + while (!"Synced unlocked".equals(projectRepository.findById(projectId).orElseThrow().getName())) { + assertThat(System.nanoTime()).as("sync not done within 15s").isLessThan(deadline); + Thread.sleep(50); + } + } +} diff --git a/src/test/java/com/company/resourceapi/lock/ProjectSyncLockIntegrationTest.java b/src/test/java/com/company/resourceapi/lock/ProjectSyncLockIntegrationTest.java new file mode 100644 index 0000000..60a6593 --- /dev/null +++ b/src/test/java/com/company/resourceapi/lock/ProjectSyncLockIntegrationTest.java @@ -0,0 +1,168 @@ +package com.company.resourceapi.lock; + +import static com.github.tomakehurst.wiremock.client.WireMock.anyRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.okJson; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.company.resourceapi.controllers.ProjectRestController; +import com.company.resourceapi.entities.Project; +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.repositories.ProjectRepository; +import com.company.resourceapi.repositories.SdlcSystemRepository; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.time.Duration; +import java.time.Instant; +import java.util.concurrent.TimeUnit; +import java.util.function.BooleanSupplier; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.redisson.api.RedissonClient; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.data.redis.core.StringRedisTemplate; +import org.springframework.test.web.servlet.MockMvc; + +/** + * One sync per project at a time, against a real Redis; "another instance" holds the lock directly. + * The application's lease is long, so a lock that is free again was released, not expired. + */ +@SpringBootTest(properties = "resourceapi.sync.lock-lease-time=30s") +@AutoConfigureMockMvc +class ProjectSyncLockIntegrationTest { + + private static final String EXTERNAL_ID = "LOCKED"; + private static final String SDLC_PROJECT_URL = "/rest/api/2/project/" + EXTERNAL_ID; + + @RegisterExtension + static WireMockExtension sdlcSystemServer = WireMockExtension.newInstance() + .options(wireMockConfig().dynamicPort()) + .build(); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Autowired + private ProjectRepository projectRepository; + + @Autowired + private RedissonClient redisson; + + @Autowired + private StringRedisTemplate redisTemplate; + + @Autowired + private ConfigurableListableBeanFactory beanFactory; + + @Value("${resourceapi.redisson.key-prefix}") + private String keyPrefix; + + private long projectId; + private String syncUrl; + + @BeforeEach + void setup() { + var sdlcSystem = new SdlcSystem(); + sdlcSystem.setBaseUrl(sdlcSystemServer.baseUrl()); + sdlcSystem = sdlcSystemRepository.save(sdlcSystem); + var project = new Project(); + project.setExternalId(EXTERNAL_ID); + project.setName("Old name"); + project.setSdlcSystem(sdlcSystem); + projectId = projectRepository.save(project).getId(); + syncUrl = ProjectRestController.ENDPOINT + "/" + projectId + "/sync"; + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"LOCKED\", \"name\": \"New name\"}"))); + } + + @Test + void givenSyncRunningElsewhereWhenSyncThenReturnStatusConflict() throws Exception { + // another instance holds the project's lock (same Redis, different owner) + var otherInstanceLock = redisson.getLock("project-sync:" + projectId); + assertThat(otherInstanceLock.tryLockAsync(0, 10, TimeUnit.SECONDS, 4711L).toCompletableFuture().join()).isTrue(); + try { + mockMvc.perform(post(syncUrl)).andExpect(status().isConflict()); + + Thread.sleep(200); + sdlcSystemServer.verify(0, anyRequestedFor(anyUrl())); + } finally { + otherInstanceLock.unlockAsync(4711L).toCompletableFuture().join(); + } + + // released: syncing works again + mockMvc.perform(post(syncUrl)).andExpect(status().isAccepted()); + await(() -> projectRepository.findById(projectId).orElseThrow().getName().equals("New name")); + } + + @Test + void givenSlowSyncWhenSyncedTwiceThenSecondIsRefusedUntilFirstEnds() throws Exception { + sdlcSystemServer.stubFor(WireMock.get(urlEqualTo(SDLC_PROJECT_URL)) + .willReturn(okJson("{\"key\": \"LOCKED\", \"name\": \"New name\"}").withFixedDelay(800))); + + mockMvc.perform(post(syncUrl)).andExpect(status().isAccepted()); + mockMvc.perform(post(syncUrl)).andExpect(status().isConflict()); + + // the first sync releases the lock when it ends, long before its 30s lease would expire + await(() -> projectRepository.findById(projectId).orElseThrow().getName().equals("New name")); + await(() -> !redisson.getLock("project-sync:" + projectId).isLocked(), Duration.ofMillis(500)); + // only the first sync called the SDLC system (checked before the next sync can call it) + sdlcSystemServer.verify(1, getRequestedFor(urlEqualTo(SDLC_PROJECT_URL))); + mockMvc.perform(post(syncUrl)).andExpect(status().isAccepted()); + + // let that sync finish here, not during the next test + await(() -> !redisson.getLock("project-sync:" + projectId).isLocked()); + } + + @Test + void givenLockNeverReleasedThenItExpiresAfterItsLeaseTime() throws Exception { + // an instance took the lock and died: nobody releases it + var abandonedLock = redisson.getLock("project-sync:" + projectId); + abandonedLock.tryLockAsync(0, 2, TimeUnit.SECONDS, 4712L).toCompletableFuture().join(); + mockMvc.perform(post(syncUrl)).andExpect(status().isConflict()); + + await(() -> !abandonedLock.isLocked()); + mockMvc.perform(post(syncUrl)).andExpect(status().isAccepted()); + } + + @Test + void givenLockNameThenItCarriesThisContextsPrefix() { + redisson.getLock("project-sync:" + projectId).tryLockAsync(0, 2, TimeUnit.SECONDS, 4713L) + .toCompletableFuture().join(); + + // raw Redis keys (Redisson's own key API would apply the prefix to the pattern too): + // RedisTestContainerInitializer gives each test context its own prefix + assertThat(redisTemplate.keys(keyPrefix + "project-sync:" + projectId)).hasSize(1); + } + + private static void await(BooleanSupplier condition) throws InterruptedException { + await(condition, Duration.ofSeconds(5)); + } + + private static void await(BooleanSupplier condition, Duration timeout) throws InterruptedException { + var deadline = Instant.now().plus(timeout); + while (!condition.getAsBoolean()) { + assertThat(Instant.now()).as("condition not met within " + timeout).isBefore(deadline); + Thread.sleep(50); + } + } + + @Test + void givenShutdownThenTaskExecutorStopsBeforeRedisson() { + // running syncs release their locks while the executor waits for them on shutdown + assertThat(beanFactory.getBeanDefinition("applicationTaskExecutor").getDependsOn()) + .contains("redissonClient"); + } +} diff --git a/src/test/java/com/company/resourceapi/ratelimit/ClientRateLimitIntegrationTest.java b/src/test/java/com/company/resourceapi/ratelimit/ClientRateLimitIntegrationTest.java new file mode 100644 index 0000000..5de44fd --- /dev/null +++ b/src/test/java/com/company/resourceapi/ratelimit/ClientRateLimitIntegrationTest.java @@ -0,0 +1,55 @@ +package com.company.resourceapi.ratelimit; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.company.resourceapi.controllers.ProjectRestController; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.RequestPostProcessor; + +/** Requests per client address, shared by all instances through Redis. */ +@SpringBootTest(properties = { + "resourceapi.rate-limit.client.permits=3", + "resourceapi.rate-limit.client.period=2s"}) +@AutoConfigureMockMvc +class ClientRateLimitIntegrationTest { + + private static final String PROJECT_URL = ProjectRestController.ENDPOINT + "/1"; + + @Autowired + private MockMvc mockMvc; + + @Test + void givenTooManyRequestsFromOneClientThenFurtherOnesAreRefused() throws Exception { + for (int i = 0; i < 3; i++) { + mockMvc.perform(get(PROJECT_URL).with(from("10.0.0.1"))).andExpect(status().isOk()); + } + mockMvc.perform(get(PROJECT_URL).with(from("10.0.0.1"))) + .andExpect(status().isTooManyRequests()) + .andExpect(header().string("Retry-After", "2")) + .andExpect(header().exists("X-Trace-Id")); + + // another client has its own limit + mockMvc.perform(get(PROJECT_URL).with(from("10.0.0.2"))).andExpect(status().isOk()); + } + + @Test + void givenNonApiPathsThenTheyAreNotLimited() throws Exception { + for (int i = 0; i < 5; i++) { + mockMvc.perform(get("/v3/api-docs").with(from("10.0.0.3"))).andExpect(status().isOk()); + } + mockMvc.perform(get(PROJECT_URL).with(from("10.0.0.3"))).andExpect(status().isOk()); + } + + private static RequestPostProcessor from(String address) { + return request -> { + request.setRemoteAddr(address); + return request; + }; + } +} diff --git a/src/test/java/com/company/resourceapi/ratelimit/RateLimitIntegrationTest.java b/src/test/java/com/company/resourceapi/ratelimit/RateLimitIntegrationTest.java new file mode 100644 index 0000000..28884ed --- /dev/null +++ b/src/test/java/com/company/resourceapi/ratelimit/RateLimitIntegrationTest.java @@ -0,0 +1,204 @@ +package com.company.resourceapi.ratelimit; + +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.okJson; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.company.resourceapi.controllers.ProjectRestController; +import com.company.resourceapi.entities.Project; +import com.company.resourceapi.entities.SdlcSystem; +import com.company.resourceapi.repositories.ProjectRepository; +import com.company.resourceapi.repositories.SdlcSystemRepository; +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.time.Duration; +import java.time.Instant; +import java.util.concurrent.TimeUnit; +import java.util.function.BooleanSupplier; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.redisson.api.RateType; +import org.redisson.api.RedissonClient; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.test.web.servlet.MockMvc; + +/** Distributed rate limits against a real Redis; "another instance" uses the same limiter directly. */ +@SpringBootTest(properties = { + "resourceapi.rate-limit.sdlc.permits=2", + "resourceapi.rate-limit.sdlc.period=2s", + "resourceapi.rate-limit.sdlc.max-wait=0s", + "resourceapi.rate-limit.sync.permits=2", + "resourceapi.rate-limit.sync.period=1m"}) +@AutoConfigureMockMvc +class RateLimitIntegrationTest { + + @RegisterExtension + static WireMockExtension sdlcSystemServer = WireMockExtension.newInstance() + .options(wireMockConfig().dynamicPort()) + .build(); + + @Autowired + private MockMvc mockMvc; + + @Autowired + private SdlcSystemRepository sdlcSystemRepository; + + @Autowired + private ProjectRepository projectRepository; + + @Autowired + private RedissonClient redisson; + + private SdlcSystem sdlcSystem; + + @BeforeEach + void setup() { + sdlcSystem = new SdlcSystem(); + sdlcSystem.setBaseUrl(sdlcSystemServer.baseUrl()); + sdlcSystem = sdlcSystemRepository.save(sdlcSystem); + sdlcSystemServer.stubFor(WireMock.get(urlPathMatching("/rest/api/2/project/.*")) + .willReturn(okJson("{\"key\": \"ANY\", \"name\": \"Synced\"}"))); + } + + @Test + void givenTooManyCallsToOneSdlcSystemThenFurtherCallsAreRefused() throws Exception { + mockMvc.perform(get(externalUrl(project("A")))).andExpect(status().isOk()); + mockMvc.perform(get(externalUrl(project("B")))).andExpect(status().isOk()); + mockMvc.perform(get(externalUrl(project("C")))) + .andExpect(status().isServiceUnavailable()) + .andExpect(header().string("Retry-After", "2")); + + sdlcSystemServer.verify(2, getRequestedFor(urlPathMatching("/rest/api/2/project/.*"))); + } + + @Test + void givenPermitsUsedByAnotherInstanceThenThisInstanceIsLimitedToo() throws Exception { + var shared = redisson.getRateLimiter("sdlc-rate:" + sdlcSystem.getId()); + shared.trySetRate(RateType.OVERALL, 2, Duration.ofSeconds(2), Duration.ofHours(1)); + assertThat(shared.tryAcquire(2)).isTrue(); + + mockMvc.perform(get(externalUrl(project("A")))).andExpect(status().isServiceUnavailable()); + + // the permits refill after the period + var url = externalUrl(project("B")); + await(() -> statusOf(url) == 200, Duration.ofSeconds(5)); + } + + @Test + void givenCachedAnswersThenOnlyRealCallsCount() throws Exception { + var url = externalUrl(project("A")); + for (int i = 0; i < 5; i++) { + mockMvc.perform(get(url)).andExpect(status().isOk()); + } + + sdlcSystemServer.verify(1, getRequestedFor(urlPathMatching("/rest/api/2/project/.*"))); + mockMvc.perform(get(externalUrl(project("B")))).andExpect(status().isOk()); + } + + @Test + void givenChangedConfigurationThenExistingLimiterIsUpdated() throws Exception { + var limiter = redisson.getRateLimiter("sdlc-rate:" + sdlcSystem.getId()); + limiter.trySetRate(RateType.OVERALL, 100, Duration.ofSeconds(1), Duration.ofHours(1)); + + mockMvc.perform(get(externalUrl(project("A")))).andExpect(status().isOk()); + + var config = limiter.getConfig(); + assertThat(config.getRate()).isEqualTo(2); + assertThat(config.getRateInterval()).isEqualTo(Duration.ofSeconds(2).toMillis()); + } + + @Test + void givenTooManySyncRequestsForOneProjectThenTheyAreRefused() throws Exception { + long projectId = project("SYNCED"); + var syncUrl = ProjectRestController.ENDPOINT + "/" + projectId + "/sync"; + + for (int i = 0; i < 2; i++) { + mockMvc.perform(post(syncUrl)).andExpect(status().isAccepted()); + // let it finish, so the lock doesn't refuse the next one + await(() -> !redisson.getLock("project-sync:" + projectId).isLocked(), Duration.ofSeconds(5)); + } + mockMvc.perform(post(syncUrl)) + .andExpect(status().isTooManyRequests()) + .andExpect(header().string("Retry-After", "60")); + } + + private long project(String externalId) { + var project = new Project(); + project.setExternalId(externalId); + project.setSdlcSystem(sdlcSystem); + return projectRepository.save(project).getId(); + } + + private static String externalUrl(long projectId) { + return ProjectRestController.ENDPOINT + "/" + projectId + "/external"; + } + + private int statusOf(String url) { + try { + return mockMvc.perform(get(url)).andReturn().getResponse().getStatus(); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + + private static void await(BooleanSupplier condition, Duration timeout) throws InterruptedException { + var deadline = Instant.now().plus(timeout); + while (!condition.getAsBoolean()) { + assertThat(Instant.now()).as("condition not met within " + timeout).isBefore(deadline); + Thread.sleep(100); + } + } + + @Test + void givenSyncAlreadyRunningThenRefusedRequestDoesNotUseAPermit() throws Exception { + long projectId = project("BUSY"); + var syncUrl = ProjectRestController.ENDPOINT + "/" + projectId + "/sync"; + var otherInstanceLock = redisson.getLock("project-sync:" + projectId); + otherInstanceLock.tryLockAsync(0, 10, TimeUnit.SECONDS, 4714L).toCompletableFuture().join(); + try { + for (int i = 0; i < 3; i++) { + mockMvc.perform(post(syncUrl)).andExpect(status().isConflict()); + } + } finally { + otherInstanceLock.unlockAsync(4714L).toCompletableFuture().join(); + } + + // both permits are still there + mockMvc.perform(post(syncUrl)).andExpect(status().isAccepted()); + // let that sync finish here, not during the next test + await(() -> !otherInstanceLock.isLocked(), Duration.ofSeconds(5)); + } + + @Test + void givenLimiterThenItExpiresFromRedisSoonAfterItsLastUse() throws Exception { + mockMvc.perform(get(externalUrl(project("A")))).andExpect(status().isOk()); + + // 10 periods of 2s is less than the 1-minute minimum + long ttl = redisson.getRateLimiter("sdlc-rate:" + sdlcSystem.getId()).remainTimeToLive(); + assertThat(ttl).isBetween(Duration.ofSeconds(50).toMillis(), Duration.ofMinutes(1).toMillis()); + } + + @Test + void givenLimiterRemovedFromRedisThenItIsSetUpAgain() throws Exception { + mockMvc.perform(get(externalUrl(project("A")))).andExpect(status().isOk()); + var limiter = redisson.getRateLimiter("sdlc-rate:" + sdlcSystem.getId()); + // expired from disuse (or flushed) + assertThat(limiter.delete()).isTrue(); + + mockMvc.perform(get(externalUrl(project("B")))).andExpect(status().isOk()); + + assertThat(limiter.getConfig().getRate()).isEqualTo(2); + // a fresh window: B used one permit, C uses the second, D is refused + mockMvc.perform(get(externalUrl(project("C")))).andExpect(status().isOk()); + mockMvc.perform(get(externalUrl(project("D")))).andExpect(status().isServiceUnavailable()); + } +} diff --git a/src/test/java/com/company/resourceapi/ratelimit/RateLimitPropertiesTest.java b/src/test/java/com/company/resourceapi/ratelimit/RateLimitPropertiesTest.java new file mode 100644 index 0000000..3540100 --- /dev/null +++ b/src/test/java/com/company/resourceapi/ratelimit/RateLimitPropertiesTest.java @@ -0,0 +1,31 @@ +package com.company.resourceapi.ratelimit; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.company.resourceapi.config.RateLimitProperties; +import com.company.resourceapi.config.RateLimitProperties.Limit; +import jakarta.validation.Validation; +import java.time.Duration; +import org.junit.jupiter.api.Test; + +class RateLimitPropertiesTest { + + private final jakarta.validation.Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + + @Test + void givenInvalidLimitsThenTheyAreRejected() { + var noPermits = new Limit(0, Duration.ofSeconds(1), Duration.ZERO); + var noPeriod = new Limit(5, null, Duration.ZERO); + var valid = new Limit(5, Duration.ofSeconds(1), Duration.ZERO); + + assertThat(validator.validate(new RateLimitProperties(noPermits, valid, valid))).hasSize(1); + assertThat(validator.validate(new RateLimitProperties(valid, noPeriod, valid))).hasSize(1); + assertThat(validator.validate(new RateLimitProperties(valid, valid, valid))).isEmpty(); + } + + @Test + void givenLimitThenRetryAfterIsOneFullWindow() { + assertThat(new Limit(6, Duration.ofMinutes(1), Duration.ZERO).retryAfterSeconds()).isEqualTo(60); + assertThat(new Limit(10, Duration.ofMillis(500), Duration.ZERO).retryAfterSeconds()).isEqualTo(1); + } +} diff --git a/src/test/java/com/company/resourceapi/ratelimit/RateLimitersTest.java b/src/test/java/com/company/resourceapi/ratelimit/RateLimitersTest.java new file mode 100644 index 0000000..6b6f7aa --- /dev/null +++ b/src/test/java/com/company/resourceapi/ratelimit/RateLimitersTest.java @@ -0,0 +1,49 @@ +package com.company.resourceapi.ratelimit; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.company.resourceapi.config.RateLimitProperties.Limit; +import com.company.resourceapi.services.RateLimiters; +import java.time.Duration; +import org.junit.jupiter.api.Test; +import org.redisson.api.RedissonClient; +import org.springframework.beans.factory.ObjectProvider; + +/** Behavior without a working Redis; with one, see RateLimitIntegrationTest. */ +class RateLimitersTest { + + private static final Limit LIMIT = new Limit(1, Duration.ofSeconds(1), Duration.ZERO); + + @Test + void givenRedisFailsThenRequestsAreAllowedWithoutAskingItAgainForAWhile() { + var redisson = mock(RedissonClient.class); + when(redisson.getRateLimiter(anyString())).thenThrow(new IllegalStateException("Redis is down")); + var rateLimiters = new RateLimiters(providerOf(redisson)); + + for (int i = 0; i < 5; i++) { + assertThat(rateLimiters.tryAcquire("test", LIMIT)).isTrue(); + } + + // one failed attempt, then Redis is left alone instead of each request waiting for its timeout + verify(redisson, times(1)).getRateLimiter(anyString()); + } + + @Test + void givenNoRedissonClientThenRequestsAreAllowed() { + var rateLimiters = new RateLimiters(providerOf(null)); + + assertThat(rateLimiters.tryAcquire("test", LIMIT)).isTrue(); + } + + @SuppressWarnings("unchecked") + private static ObjectProvider providerOf(RedissonClient redisson) { + ObjectProvider provider = mock(ObjectProvider.class); + when(provider.getIfAvailable()).thenReturn(redisson); + return provider; + } +} diff --git a/src/test/java/com/company/resourceapi/services/ProjectSyncBackpressureTest.java b/src/test/java/com/company/resourceapi/services/ProjectSyncBackpressureTest.java index 8a80a55..947f5fa 100644 --- a/src/test/java/com/company/resourceapi/services/ProjectSyncBackpressureTest.java +++ b/src/test/java/com/company/resourceapi/services/ProjectSyncBackpressureTest.java @@ -4,6 +4,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.okJson; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -14,6 +15,7 @@ import com.company.resourceapi.repositories.ProjectRepository; import com.company.resourceapi.repositories.SdlcSystemRepository; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.time.Duration; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.beans.factory.annotation.Autowired; @@ -22,7 +24,10 @@ import org.springframework.test.web.servlet.MockMvc; /** Background syncs are bounded: beyond the executor's limit they are rejected, not queued. */ -@SpringBootTest(properties = "spring.task.execution.simple.concurrency-limit=1") +@SpringBootTest(properties = { + "spring.task.execution.simple.concurrency-limit=1", + // this test retries refused syncs; the per-project sync rate limit isn't what it is about + "resourceapi.rate-limit.sync.permits=1000"}) @AutoConfigureMockMvc class ProjectSyncBackpressureTest { @@ -46,17 +51,31 @@ void givenBusyTaskExecutorWhenSyncThenReturnStatusServiceUnavailable() throws Ex var sdlcSystem = new SdlcSystem(); sdlcSystem.setBaseUrl(sdlcSystemServer.baseUrl()); sdlcSystem = sdlcSystemRepository.save(sdlcSystem); - var project = new Project(); - project.setExternalId("BUSY"); - project.setSdlcSystem(sdlcSystem); - var syncUrl = ProjectRestController.ENDPOINT + "/" + projectRepository.save(project).getId() + "/sync"; + var syncUrl = syncUrl(sdlcSystem, "BUSY"); + var otherSyncUrl = syncUrl(sdlcSystem, "OTHER"); sdlcSystemServer.stubFor(get(urlPathMatching("/rest/api/2/project/.*")) .willReturn(okJson("{\"key\": \"BUSY\", \"name\": \"Busy\"}").withFixedDelay(1_000))); - // when / then + // when / then: another project's sync (the same project's would be refused by its lock) mockMvc.perform(post(syncUrl)).andExpect(status().isAccepted()); - mockMvc.perform(post(syncUrl)) + mockMvc.perform(post(otherSyncUrl)) .andExpect(status().isServiceUnavailable()) .andExpect(header().string("Retry-After", "1")); + + // the rejected sync released its lock: it can be started once the first sync freed the slot + var deadline = System.nanoTime() + Duration.ofSeconds(10).toNanos(); + int status; + while ((status = mockMvc.perform(post(otherSyncUrl)).andReturn().getResponse().getStatus()) == 503) { + assertThat(System.nanoTime()).as("no free slot within 10s").isLessThan(deadline); + Thread.sleep(100); + } + assertThat(status).isEqualTo(202); + } + + private String syncUrl(SdlcSystem sdlcSystem, String externalId) { + var project = new Project(); + project.setExternalId(externalId); + project.setSdlcSystem(sdlcSystem); + return ProjectRestController.ENDPOINT + "/" + projectRepository.save(project).getId() + "/sync"; } } diff --git a/src/test/resources/META-INF/spring.factories b/src/test/resources/META-INF/spring.factories new file mode 100644 index 0000000..5256c02 --- /dev/null +++ b/src/test/resources/META-INF/spring.factories @@ -0,0 +1,2 @@ +org.springframework.context.ApplicationContextInitializer=\ +com.company.resourceapi.RedisTestContainerInitializer diff --git a/src/test/resources/config/application.properties b/src/test/resources/config/application.properties index c2a9ab1..67b2d67 100644 --- a/src/test/resources/config/application.properties +++ b/src/test/resources/config/application.properties @@ -1,2 +1,4 @@ # Each test application context gets its own in-memory database so schema.sql/data.sql run on a clean slate spring.datasource.url=jdbc:h2:mem:test-${random.uuid};MODE=MySQL;DATABASE_TO_LOWER=TRUE +# tests send many requests from one address; ClientRateLimitIntegrationTest sets its own limit +resourceapi.rate-limit.client.permits=100000