Skip to content

Commit 4dafa44

Browse files
refactor(infra): enforce remaining coding standards across platform (STA-249) (#270)
* chore(infra): @slf4j, drop block comments, prefer var (STA-249) Three small style-rule cleanups across the platform: - Replace LoggerFactory.getLogger with @slf4j in ExternalApiLoggingInterceptor. - Drop 5 leftover block comments from switch-default and no-op lambda bodies in webhook validators, OFAC parser, and the nonce manager. - Use var for ~20 obvious local declarations across ledger, compliance, custody, fiat-on-ramp, merchant-iam, merchant-onboarding, payment-orchestrator, and a few test files (PermissionTest, JournalCommandHandlerTest, DevCustodyAdapterTest). No behavior change. Verified with compileJava + compileTestJava on every touched module. * chore(infra): drop narration comments across services (STA-249) Remove ~135 step-by-step narration and section-divider comments across 29 production files. Code now stands on its own; the remaining 47 comments document non-obvious WHY (Temporal saga authority, JPA orphanRemoval ordering, advisory-lock semantics, PII masking rationale, dev-mode keypair notes, etc.). Heaviest cleanups: PayoutCommandHandler (-13), TransferCommandHandler (-12), RefundCommandHandler (-12), TracingConfig (-13 architecture explainer), RiskScoringService (-9 Factor 1-8 labels), CollectionCommandHandler (-7), PaymentEventConsumer (-7), MerchantTeam (-6 Invariant labels), LockService, MerchantOnboardingWorkflowImpl, PaymentWorkflowImpl, FxRateLockApplicationService. Verified: ./gradlew compileJava + compileTestJava BUILD SUCCESSFUL. * test(infra): replace Mockito generic matchers with literal values (STA-249) Eliminate any()/eq()/anyString()/anyLong()/anyInt()/anyBoolean()/ anyList() from 30 unit test files across api-gateway-iam (14), payment-orchestrator (6), fx-liquidity-engine (3), merchant-onboarding (3), merchant-iam (2), compliance-travel-rule (1), ledger-accounting (1). ~180 matcher usages removed. Stubs now take the literal values the SUT actually passes; save verifications use eqIgnoringTimestamps / eqIgnoring from platform-test TestUtils. Where runtime-generated values genuinely cannot be pre-constructed (workflowId from generated paymentId, Duration computed at revoke time) we use typed argThat(lambda) -- the same escape hatch eqIgnoring itself uses. Verified: 1,467 unit tests pass across all 7 affected modules. * refactor(infra): collapse ApplicationService layer into CommandHandlers (STA-249) Controllers now call CommandHandler / QueryHandler directly per CLAUDE.md "Controller → CommandHandler directly — no intermediate service layer". compliance-travel-rule: delete ComplianceCheckApplicationService (thin pass-through). ComplianceCheckController and CustomerRiskProfileController inject ComplianceCheckCommandHandler + ComplianceCheckResponseMapper directly. Tests rewritten to mock the handler. fx-liquidity-engine: promote FxQuoteApplicationService, FxRateLockApplicationService, and LiquidityPoolApplicationService to FxQuoteCommandHandler, FxRateLockCommandHandler, and LiquidityPoolQueryHandler under domain/service/. Handlers now take primitives and return domain objects (+ idempotency records), so the hexagonal rule domain-may-not-depend-on-application stays green. Controllers own response mapping via FxResponseMapper (added CorridorSnapshot overload). New per-handler unit tests preserve intent from the removed ApplicationService tests. Dead-code removal: drop orphaned ApplicationService classes left from STA-243 — ApiKeyApplicationService, AuthApplicationService, OAuthClientApplicationService, MerchantApplicationService (api-gateway-iam) and MerchantApplicationService (merchant-onboarding). Confirmed zero references in main or test before deletion. Net: -1505 lines. 13 files deleted, 13 modified, 6 new handlers/tests. Verified: compliance-travel-rule (268 tests), fx-liquidity-engine (202 tests), full compileJava + compileTestJava BUILD SUCCESSFUL. * refactor(infra): delete orphan ApplicationService classes missed in collapse (STA-249) Code-review follow-up — the initial handler-collapse commit (aa89f6e) added FxQuoteCommandHandler and FxRateLockCommandHandler alongside the old @service classes instead of replacing them, and left ApiKeyApplicationService in place despite the PR description claiming it was deleted. Net effect before this commit: three @Service-annotated classes were still being instantiated by Spring component scan with no callers, and two duplicate test files (FxQuoteApplicationServiceTest, FxRateLockApplicationServiceTest) still exercised them. Deletes: - api-gateway-iam/.../application/service/ApiKeyApplicationService.java - fx-liquidity-engine/.../application/service/FxQuoteApplicationService.java - fx-liquidity-engine/.../application/service/FxRateLockApplicationService.java - fx-liquidity-engine/.../test/.../application/service/FxQuoteApplicationServiceTest.java - fx-liquidity-engine/.../test/.../application/service/FxRateLockApplicationServiceTest.java Verified: api-gateway-iam + fx-liquidity-engine unit tests BUILD SUCCESSFUL (rerun-tasks). Rule 3 now fully resolved. * test(infra): address CodeRabbit review feedback on PR #270 (STA-249) 12 fixes addressing CodeRabbit review comments: Production: - merchant-iam: delete unused countActiveUsersByRoleId stub from RoleRepository + adapter (role deletion validates in-memory via MerchantTeam.deleteCustomRole). Tests: - MerchantCommandHandlerTest: replace hardcoded "raw-secret" literal with UUID.randomUUID() reused between stub and expected event. - ComplianceCheckCommandHandlerTest: use Mockito InOrder over all six providers/repo/publisher to enforce cross-mock invocation sequence (shouldInvokeAllProvidersInOrder). - PaymentEventConsumerTest: assert shouldHaveNoInteractions() on eventPublisher and poolRepository for all early-return branches (idempotentConsumed, noLockFound, idempotentExpired, noLockFoundFailed). - FxQuoteControllerTest: new @nested ReleaseLock with success (204) + LockNotFoundException propagation. - AuditLogFilterTest: extract shared expectedAuditEntry(...) helper; add missing then(auditLogRepository).should().save(...) assertion in shouldNotFailWhenRepositoryThrows so exception-handling path is actually exercised. - FxRateLockCommandHandlerTest: new @nested ReleaseLock with 4 tests — LockNotFoundException, skip-when-already-expired, happy path with liquidity return, pool-missing graceful handling. - MerchantTeamServiceTest: replace argThat(Instant i -> i != null) with ArgumentCaptor + isAfter(now) semantic check; replace publish(argThat( Objects::nonNull)) with eqIgnoring on a fully-built expected event. - KybWebhookControllerTest: add then(workflowClient).shouldHaveNoInteractions() to invalid-signature test. - PaymentCommandHandlerTest: strengthen idempotency-replay assertion — verify findByIdempotencyKey was called + shouldHaveNoMoreInteractions(). - PaymentControllerTest: extract private stubInitiatePayment helper to DRY up repeated given(commandHandler.initiatePayment(...)) blocks. - LedgerOutboxEventPublisherTest: switch from eqIgnoring to ArgumentCaptor so completedAt/detectedAt Instants are actually asserted (TestUtils.eqIgnoring type-ignores all Instants by design — not suitable for timestamp verification). Not fixed, documented in PR reply: - JournalCommandHandler NPE hardening: CLAUDE.md forbids validation for scenarios that cannot happen; BalanceUpdate key is guaranteed by BalanceCalculator contract. - PaymentWorkflowImpl hard-coded wallet/chain: pre-existing sandbox placeholder from STA-243, out of scope for STA-249 cleanup. - LockService duplicate liquidity check: pre-existing duplication inherited from deleted FxRateLockApplicationService; requires touching LockService which is out of scope. Follow-up ticket. - CorridorSnapshot extraction to top-level: style preference. Inner record is the minimal surface area for an internal projection. Verified: ./gradlew test -x :phase2-integration-tests:test -x :phase3-integration-tests:test BUILD SUCCESSFUL (128 tasks).
1 parent 64e679b commit 4dafa44

99 files changed

Lines changed: 1492 additions & 2051 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

api-gateway-iam/api-gateway-iam/src/main/java/com/stablecoin/payments/gateway/iam/application/controller/ApiKeyController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public ApiKeyResponse createApiKey(@Valid @RequestBody CreateApiKeyRequest reque
4242
request.merchantId(), request.name(), request.environment());
4343

4444
var environment = ApiKeyEnvironment.valueOf(request.environment().toUpperCase());
45-
Instant expiresAt = request.expiresInSeconds() != null
45+
var expiresAt = request.expiresInSeconds() != null
4646
? Instant.now().plusSeconds(request.expiresInSeconds())
4747
: null;
4848

api-gateway-iam/api-gateway-iam/src/main/java/com/stablecoin/payments/gateway/iam/application/security/UserJwtAuthenticationFilter.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,27 +53,23 @@ protected void doFilterInternal(HttpServletRequest request,
5353
var jwt = SignedJWT.parse(token);
5454
var claims = jwt.getJWTClaimsSet();
5555

56-
// Only handle tokens issued by S13
5756
if (!merchantIamProperties.issuer().equals(claims.getIssuer())) {
5857
chain.doFilter(request, response);
5958
return;
6059
}
6160

62-
// Validate audience
6361
if (claims.getAudience() == null
6462
|| !claims.getAudience().contains(merchantIamProperties.audience())) {
6563
sendUnauthorized(response, "JWT audience mismatch");
6664
return;
6765
}
6866

69-
// Validate expiration
7067
if (claims.getExpirationTime() == null
7168
|| claims.getExpirationTime().before(new Date())) {
7269
sendUnauthorized(response, "JWT has expired");
7370
return;
7471
}
7572

76-
// Verify signature against S13 JWKS
7773
var jwksJson = userJwksProvider.fetchJwks();
7874
var jwkSet = JWKSet.parse(jwksJson);
7975
var kid = jwt.getHeader().getKeyID();
@@ -90,7 +86,6 @@ protected void doFilterInternal(HttpServletRequest request,
9086
return;
9187
}
9288

93-
// Extract claims
9489
var userId = UUID.fromString(claims.getStringClaim("user_id"));
9590
var merchantId = UUID.fromString(claims.getStringClaim("merchant_id"));
9691
var roleId = UUID.fromString(claims.getStringClaim("role_id"));

api-gateway-iam/api-gateway-iam/src/main/java/com/stablecoin/payments/gateway/iam/application/service/ApiKeyApplicationService.java

Lines changed: 0 additions & 51 deletions
This file was deleted.

api-gateway-iam/api-gateway-iam/src/main/java/com/stablecoin/payments/gateway/iam/application/service/AuthApplicationService.java

Lines changed: 0 additions & 44 deletions
This file was deleted.

api-gateway-iam/api-gateway-iam/src/main/java/com/stablecoin/payments/gateway/iam/application/service/MerchantApplicationService.java

Lines changed: 0 additions & 80 deletions
This file was deleted.

api-gateway-iam/api-gateway-iam/src/main/java/com/stablecoin/payments/gateway/iam/application/service/OAuthClientApplicationService.java

Lines changed: 0 additions & 51 deletions
This file was deleted.

api-gateway-iam/api-gateway-iam/src/test/java/com/stablecoin/payments/gateway/iam/application/controller/ApiKeyControllerTest.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.stablecoin.payments.gateway.iam.api.response.ApiKeyResponse;
44
import com.stablecoin.payments.gateway.iam.application.controller.mapper.GatewayRequestResponseMapper;
55
import com.stablecoin.payments.gateway.iam.domain.exception.MerchantNotFoundException;
6+
import com.stablecoin.payments.gateway.iam.domain.model.ApiKeyEnvironment;
67
import com.stablecoin.payments.gateway.iam.domain.service.ApiKeyCommandHandler;
78
import org.junit.jupiter.api.DisplayName;
89
import org.junit.jupiter.api.Test;
@@ -17,7 +18,6 @@
1718

1819
import static org.assertj.core.api.Assertions.assertThat;
1920
import static org.assertj.core.api.Assertions.assertThatThrownBy;
20-
import static org.mockito.ArgumentMatchers.any;
2121
import static org.mockito.BDDMockito.given;
2222
import static org.mockito.BDDMockito.then;
2323

@@ -38,15 +38,18 @@ class ApiKeyControllerTest {
3838
@DisplayName("createApiKey should return key with raw key")
3939
void shouldCreateApiKey() {
4040
var keyId = UUID.randomUUID();
41+
var merchantId = UUID.randomUUID();
42+
var commandResult = new ApiKeyCommandHandler.CreateApiKeyResult(null, "pk_live_raw123");
4143
var response = new ApiKeyResponse(
4244
keyId, "pk_live_raw123", "pk_live_abc", "My Key", "LIVE",
4345
List.of("payments:read"), List.of(), Instant.now().plusSeconds(86400), Instant.now());
44-
given(apiKeyCommandHandler.create(any(), any(), any(), any(), any(), any()))
45-
.willReturn(new ApiKeyCommandHandler.CreateApiKeyResult(null, "pk_live_raw123"));
46-
given(mapper.toApiKeyResponse(any())).willReturn(response);
46+
given(apiKeyCommandHandler.create(merchantId, "My Key", ApiKeyEnvironment.LIVE,
47+
List.of("payments:read"), List.of(), null))
48+
.willReturn(commandResult);
49+
given(mapper.toApiKeyResponse(commandResult)).willReturn(response);
4750

4851
var request = new com.stablecoin.payments.gateway.iam.api.request.CreateApiKeyRequest(
49-
UUID.randomUUID(), "My Key", "LIVE", List.of("payments:read"), null, null);
52+
merchantId, "My Key", "LIVE", List.of("payments:read"), null, null);
5053

5154
var result = controller.createApiKey(request);
5255

@@ -59,7 +62,8 @@ void shouldCreateApiKey() {
5962
@DisplayName("createApiKey should throw when merchant not found")
6063
void shouldThrowWhenMerchantNotFound() {
6164
var merchantId = UUID.randomUUID();
62-
given(apiKeyCommandHandler.create(any(), any(), any(), any(), any(), any()))
65+
given(apiKeyCommandHandler.create(merchantId, "My Key", ApiKeyEnvironment.LIVE,
66+
List.of(), List.of(), null))
6367
.willThrow(MerchantNotFoundException.byId(merchantId));
6468

6569
var request = new com.stablecoin.payments.gateway.iam.api.request.CreateApiKeyRequest(

api-gateway-iam/api-gateway-iam/src/test/java/com/stablecoin/payments/gateway/iam/application/controller/AuthControllerTest.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@
1616

1717
import static org.assertj.core.api.Assertions.assertThat;
1818
import static org.assertj.core.api.Assertions.assertThatThrownBy;
19-
import static org.mockito.ArgumentMatchers.any;
20-
import static org.mockito.ArgumentMatchers.anyList;
21-
import static org.mockito.ArgumentMatchers.anyString;
22-
import static org.mockito.ArgumentMatchers.eq;
2319
import static org.mockito.BDDMockito.given;
2420
import static org.mockito.BDDMockito.then;
2521

@@ -42,7 +38,8 @@ void shouldIssueToken() {
4238
var clientId = UUID.randomUUID();
4339
var tokenResult = new AuthCommandHandler.TokenResult("jwt-token", UUID.randomUUID(), 3600L, List.of("payments:read"));
4440
var tokenResponse = new TokenResponse("jwt-token", "Bearer", 3600, "payments:read");
45-
given(authCommandHandler.issueToken(eq(clientId), anyString(), anyList())).willReturn(tokenResult);
41+
given(authCommandHandler.issueToken(clientId, "secret", List.of("payments:read")))
42+
.willReturn(tokenResult);
4643
given(mapper.toTokenResponse(tokenResult)).willReturn(tokenResponse);
4744

4845
var request = new com.stablecoin.payments.gateway.iam.api.request.TokenRequest(
@@ -58,11 +55,12 @@ void shouldIssueToken() {
5855
@Test
5956
@DisplayName("issueToken should propagate invalid credentials")
6057
void shouldPropagateInvalidCredentials() {
61-
given(authCommandHandler.issueToken(any(), anyString(), anyList()))
58+
var clientId = UUID.randomUUID();
59+
given(authCommandHandler.issueToken(clientId, "wrong", List.of()))
6260
.willThrow(InvalidClientCredentialsException.clientNotFound());
6361

6462
var request = new com.stablecoin.payments.gateway.iam.api.request.TokenRequest(
65-
"client_credentials", UUID.randomUUID(), "wrong", null);
63+
"client_credentials", clientId, "wrong", null);
6664

6765
assertThatThrownBy(() -> controller.issueToken(request))
6866
.isInstanceOf(InvalidClientCredentialsException.class);

0 commit comments

Comments
 (0)