Merge pull request #2 from RedJanvier/feat/tests-and-ci

chore: add tests and ci

Author: RedJanvier

.gitattributes

@@ -0,0 +1,10 @@
+# Normalize line endings so the Maven wrapper runs on Linux CI runners.
+* text=auto
+
+# Shell scripts must be LF regardless of the OS they were committed from.
+mvnw text eol=lf
+*.sh text eol=lf
+
+# Windows scripts stay CRLF.
+*.cmd text eol=crlf
+*.bat text eol=crlf

.github/workflows/ci.yml

@@ -0,0 +1,43 @@
+name: CI
+
+on:
+  push:
+  pull_request:
+
+jobs:
+  build-and-test:
+    name: Build & Test (${{ matrix.module }})
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        module: [api-gateway, sms-service, email-service]
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Set up JDK 17
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: '17'
+          cache: maven
+
+      - name: Make Maven wrapper executable
+        working-directory: ${{ matrix.module }}
+        run: chmod +x ./mvnw
+
+      # api-gateway's rate-limit test starts a throwaway Redis via Testcontainers.
+      # ubuntu-latest ships with a running Docker daemon, so no extra setup is needed.
+      - name: Build and test
+        working-directory: ${{ matrix.module }}
+        run: ./mvnw -B verify
+
+      - name: Upload test reports
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: surefire-reports-${{ matrix.module }}
+          path: ${{ matrix.module }}/target/surefire-reports/
+          if-no-files-found: ignore

.idea/workspace.xml

@@ -140,6 +140,7 @@ The solution provided doesn't tackle all the points but it is a base for the rem
 - **Twilio** *(optional)* : Real SMS delivery provider
 - **Spring Boot** : All three microservices (Java 17)
 - **Docker / Docker Compose** : Containerization & local orchestration
+- **GitHub Actions** : CI — builds and tests every service on each push
 - **Kafka** *(future)* : Message broker for asynchronous delivery
 
 ## Security
@@ -203,6 +204,20 @@ docker compose up --build
 
 The default configuration needs **no external credentials**: SMS uses a mock provider (messages appear in the `sms-service` logs) and Email is delivered to Mailpit, viewable at **http://localhost:8025**.
 
+### 3. Run the tests
+
+Each service is an independent Maven module with its own test suite. Run one with its wrapper:
+
+```bash
+cd sms-service && ./mvnw test
+```
+
+No Docker is required to run the tests: persistence tests use an in-memory H2 database, and the API-gateway rate-limit test starts an in-process Redis (embedded-redis). All suites also run automatically in CI on every push (see [Continuous Integration](#continuous-integration)).
+
+## Continuous Integration
+
+Every push and pull request triggers the GitHub Actions workflow at [`.github/workflows/ci.yml`](.github/workflows/ci.yml). It runs a matrix job — one per service (`api-gateway`, `sms-service`, `email-service`) — that sets up JDK 17, restores the Maven cache, and runs `./mvnw verify`. The build fails if any test fails, so regressions are caught before merge.
+
 ## Documentation
 > Base path for all endpoints (through the gateway) is `http://localhost:8081/api/v1`
 

README.md

@@ -44,6 +44,20 @@
 			<artifactId>reactor-test</artifactId>
 			<scope>test</scope>
 		</dependency>
+		<!-- Real (embedded, in-process) Redis for rate-limiter integration tests.
+		     No Docker required, so the test runs locally and in CI alike. -->
+		<dependency>
+			<groupId>com.github.codemonstur</groupId>
+			<artifactId>embedded-redis</artifactId>
+			<version>1.4.3</version>
+			<scope>test</scope>
+		</dependency>
+		<!-- Stub downstream service for routing/rate-limit tests -->
+		<dependency>
+			<groupId>com.squareup.okhttp3</groupId>
+			<artifactId>mockwebserver</artifactId>
+			<scope>test</scope>
+		</dependency>
 	</dependencies>
 	<dependencyManagement>
 		<dependencies>

api-gateway/pom.xml

@@ -7,11 +7,32 @@
 
 import reactor.core.publisher.Mono;
 
+/**
+ * Key resolvers for the gateway's {@code RequestRateLimiter} filter.
+ *
+ * <p>The rate limiter buckets requests by the key returned here. Two strategies
+ * are provided so both README requirements are covered:
+ *
+ * <ul>
+ *   <li>{@link #systemKeyResolver()} — one shared bucket for the whole system.
+ *       This enforces the "limit amount of requests per time window across the
+ *       whole system" requirement (the {@code 10 req / 3s} system-wide limit).</li>
+ *   <li>{@link #clientKeyResolver()} — one bucket per client, keyed by the
+ *       {@code X-Client-Id} header (falling back to the caller's IP). This
+ *       enforces the "limit too many requests from a client" requirement.</li>
+ * </ul>
+ *
+ * <p>The active strategy is chosen in {@code application.yml} via
+ * {@code key-resolver: "#{@systemKeyResolver}"} (or {@code #{@clientKeyResolver}}).
+ * {@code systemKeyResolver} is {@link Primary} so it is used if a route omits an
+ * explicit key resolver.
+ */
 @Configuration
 public class RateLimiterConfig {
 
-	private static final String SYSTEM_BUCKET = "system";
-	private static final String CLIENT_HEADER = "X-Client-Id";
+	static final String SYSTEM_BUCKET = "system";
+	static final String CLIENT_HEADER = "X-Client-Id";
+	static final String UNKNOWN_CLIENT = "unknown";
 
 	@Bean
 	@Primary
@@ -26,7 +47,7 @@ public KeyResolver clientKeyResolver() {
 			if (clientId == null || clientId.isBlank()) {
 				clientId = exchange.getRequest().getRemoteAddress() != null
 						? exchange.getRequest().getRemoteAddress().getAddress().getHostAddress()
-						: "unknown";
+						: UNKNOWN_CLIENT;
 			}
 			return Mono.just(clientId);
 		};

api-gateway/src/main/java/com/redjanvier/apigateway/config/RateLimiterConfig.java

@@ -0,0 +1,113 @@
+package com.redjanvier.apigateway;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.ServerSocket;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
+import org.springframework.test.context.DynamicPropertyRegistry;
+import org.springframework.test.context.DynamicPropertySource;
+import org.springframework.test.web.reactive.server.WebTestClient;
+
+import okhttp3.mockwebserver.Dispatcher;
+import okhttp3.mockwebserver.MockResponse;
+import okhttp3.mockwebserver.MockWebServer;
+import okhttp3.mockwebserver.RecordedRequest;
+import redis.embedded.RedisServer;
+
+/**
+ * End-to-end test of the gateway's two headline responsibilities:
+ * <ol>
+ *   <li>routing a request to the correct downstream service, and</li>
+ *   <li>enforcing the Redis-backed rate limit (HTTP 429 once the bucket drains).</li>
+ * </ol>
+ *
+ * <p>Uses a real, in-process Redis (embedded-redis, no Docker) and a stubbed
+ * downstream (MockWebServer), so it exercises the actual {@code RequestRateLimiter}
+ * Lua script and runs anywhere — locally and in CI.
+ */
+@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
+class RateLimitingIntegrationTest {
+
+	static RedisServer redisServer;
+	static MockWebServer downstream;
+
+	@DynamicPropertySource
+	static void properties(DynamicPropertyRegistry registry) throws IOException {
+		int redisPort = findFreePort();
+		redisServer = new RedisServer(redisPort);
+		redisServer.start();
+
+		downstream = new MockWebServer();
+		downstream.setDispatcher(new Dispatcher() {
+			@Override
+			public MockResponse dispatch(RecordedRequest request) {
+				return new MockResponse().setResponseCode(200).setBody("ok");
+			}
+		});
+		downstream.start();
+
+		String base = "http://" + downstream.getHostName() + ":" + downstream.getPort();
+		registry.add("spring.data.redis.host", () -> "localhost");
+		registry.add("spring.data.redis.port", () -> redisPort);
+		registry.add("SMS_SERVICE_URI", () -> base);
+		registry.add("EMAIL_SERVICE_URI", () -> base);
+	}
+
+	@AfterAll
+	static void tearDown() throws IOException {
+		if (downstream != null) {
+			downstream.shutdown();
+		}
+		if (redisServer != null) {
+			redisServer.stop();
+		}
+	}
+
+	private static int findFreePort() throws IOException {
+		try (ServerSocket socket = new ServerSocket(0)) {
+			return socket.getLocalPort();
+		}
+	}
+
+	@Autowired
+	private WebTestClient client;
+
+	@Test
+	void routesToDownstreamThenRateLimitsTheBurst() {
+		// 1) Routing: the first request is proxied to the stub downstream.
+		client.post().uri("/api/v1/notifications/sms")
+				.exchange()
+				.expectStatus().isOk()
+				.expectBody(String.class).isEqualTo("ok");
+
+		// 2) Rate limiting: a rapid burst must be partly rejected with 429.
+		int ok = 1;
+		int limited = 0;
+		for (int i = 0; i < 40; i++) {
+			int status = client.post().uri("/api/v1/notifications/sms")
+					.exchange()
+					.returnResult(String.class)
+					.getStatus()
+					.value();
+			if (status == 429) {
+				limited++;
+			} else if (status == 200) {
+				ok++;
+			}
+		}
+
+		assertThat(limited)
+				.as("the rate limiter must reject part of a 41-request burst")
+				.isGreaterThan(0);
+		assertThat(ok)
+				.as("some requests should still pass through")
+				.isGreaterThanOrEqualTo(1)
+				.isLessThan(41);
+	}
+}