Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.stablecoin.payments.fx.infrastructure.provider.frankfurter;

import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "app.fx.frankfurter")
public record FrankfurterProperties(
String baseUrl,
Integer readTimeoutMs
) {
public FrankfurterProperties {
if (baseUrl == null || baseUrl.isBlank()) {
baseUrl = "https://api.frankfurter.app";
}
if (readTimeoutMs == null || readTimeoutMs <= 0) {
readTimeoutMs = 5000;
}
}
Comment on lines +6 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Properties record is well-structured with sensible defaults.

Compact constructor with null/blank checks and defaults is a clean pattern. Consider adding a separate connectTimeoutMs property—the adapter currently reuses readTimeoutMs for both connect and read timeouts, which conflates two distinct concerns.

♻️ Add separate connect timeout property
 `@ConfigurationProperties`(prefix = "app.fx.frankfurter")
 public record FrankfurterProperties(
         String baseUrl,
-        Integer readTimeoutMs
+        Integer readTimeoutMs,
+        Integer connectTimeoutMs
 ) {
     public FrankfurterProperties {
         if (baseUrl == null || baseUrl.isBlank()) {
             baseUrl = "https://api.frankfurter.app";
         }
         if (readTimeoutMs == null || readTimeoutMs <= 0) {
             readTimeoutMs = 5000;
         }
+        if (connectTimeoutMs == null || connectTimeoutMs <= 0) {
+            connectTimeoutMs = 3000;
+        }
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public record FrankfurterProperties(
String baseUrl,
Integer readTimeoutMs
) {
public FrankfurterProperties {
if (baseUrl == null || baseUrl.isBlank()) {
baseUrl = "https://api.frankfurter.app";
}
if (readTimeoutMs == null || readTimeoutMs <= 0) {
readTimeoutMs = 5000;
}
}
public record FrankfurterProperties(
String baseUrl,
Integer readTimeoutMs,
Integer connectTimeoutMs
) {
public FrankfurterProperties {
if (baseUrl == null || baseUrl.isBlank()) {
baseUrl = "https://api.frankfurter.app";
}
if (readTimeoutMs == null || readTimeoutMs <= 0) {
readTimeoutMs = 5000;
}
if (connectTimeoutMs == null || connectTimeoutMs <= 0) {
connectTimeoutMs = 3000;
}
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/frankfurter/FrankfurterProperties.java`
around lines 6 - 17, Add a separate connectTimeoutMs field to the
FrankfurterProperties record and initialize it in the compact constructor with
its own default (e.g., 5000) instead of reusing readTimeoutMs; update the record
signature to include Integer connectTimeoutMs, update the compact constructor to
check/connectTimeoutMs for null or <=0 and set the default there, and then
update any adapter code that currently uses readTimeoutMs for both timeouts to
reference FrankfurterProperties.connectTimeoutMs for the connect timeout and
FrankfurterProperties.readTimeoutMs for the read timeout.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package com.stablecoin.payments.fx.infrastructure.provider.frankfurter;

import com.stablecoin.payments.fx.domain.model.CorridorRate;
import com.stablecoin.payments.fx.domain.port.RateProvider;
import io.github.resilience4j.circuitbreaker.annotation.CircuitBreaker;
import io.github.resilience4j.retry.annotation.Retry;
import lombok.extern.slf4j.Slf4j;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.http.client.JdkClientHttpRequestFactory;
import org.springframework.stereotype.Component;
import org.springframework.web.client.RestClient;

import java.net.http.HttpClient;
import java.net.http.HttpClient.Version;
import java.time.Duration;
import java.util.Optional;

@Slf4j
@Component
@ConditionalOnProperty(name = "app.fx.rate-provider", havingValue = "frankfurter")
@EnableConfigurationProperties(FrankfurterProperties.class)
public class FrankfurterRateAdapter implements RateProvider {

private static final int DEFAULT_SPREAD_BPS = 30;
private static final int DEFAULT_FEE_BPS = 30;

private final RestClient restClient;

public FrankfurterRateAdapter(FrankfurterProperties properties) {
var httpClient = HttpClient.newBuilder()
.version(Version.HTTP_1_1)
.connectTimeout(Duration.ofMillis(properties.readTimeoutMs()))
.build();

var requestFactory = new JdkClientHttpRequestFactory(httpClient);
requestFactory.setReadTimeout(Duration.ofMillis(properties.readTimeoutMs()));

this.restClient = RestClient.builder()
.baseUrl(properties.baseUrl())
.requestFactory(requestFactory)
.build();
}
Comment on lines +30 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

connectTimeout incorrectly uses readTimeoutMs.

Line 33 sets connectTimeout using properties.readTimeoutMs(). These are distinct concerns: connect timeout guards against slow network handshakes (typically 2-5s), while read timeout guards against slow responses (often longer). Using the same value means either connect is too long or read is too short.

🔧 Separate connect and read timeouts
     public FrankfurterRateAdapter(FrankfurterProperties properties) {
         var httpClient = HttpClient.newBuilder()
                 .version(Version.HTTP_1_1)
-                .connectTimeout(Duration.ofMillis(properties.readTimeoutMs()))
+                .connectTimeout(Duration.ofMillis(properties.connectTimeoutMs()))
                 .build();

This requires adding connectTimeoutMs to FrankfurterProperties as suggested in that file's review.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/frankfurter/FrankfurterRateAdapter.java`
around lines 30 - 43, Constructor FrankfurterRateAdapter incorrectly uses
properties.readTimeoutMs() for the HTTP connect timeout; add a distinct
connectTimeoutMs to FrankfurterProperties and use properties.connectTimeoutMs()
when building the HttpClient (keep
requestFactory.setReadTimeout(Duration.ofMillis(properties.readTimeoutMs())) for
read timeout) so connect and read timeouts are separated; update
FrankfurterProperties interface/class to expose connectTimeoutMs() and adjust
any configuration bindings.


@Override
@Retry(name = "frankfurter", fallbackMethod = "frankfurterFallback")
@CircuitBreaker(name = "frankfurter", fallbackMethod = "frankfurterFallback")
public Optional<CorridorRate> getRate(String fromCurrency, String toCurrency) {
log.info("[FRANKFURTER] Fetching rate for {}:{}", fromCurrency, toCurrency);

var response = restClient.get()
.uri("/latest?from={from}&to={to}", fromCurrency, toCurrency)
.retrieve()
.body(FrankfurterRateResponse.class);

if (response == null || response.rates() == null || response.rates().isEmpty()) {
log.warn("[FRANKFURTER] No rate returned for {}:{}", fromCurrency, toCurrency);
return Optional.empty();
}
Comment on lines +48 to +59

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider DEBUG level for routine request logging.

Line 49 logs at INFO for every rate fetch. In production with frequent polling, this creates significant log volume. Consider DEBUG for the request and reserve INFO for warnings/errors.

♻️ Reduce log verbosity
-        log.info("[FRANKFURTER] Fetching rate for {}:{}", fromCurrency, toCurrency);
+        log.debug("[FRANKFURTER] Fetching rate for {}:{}", fromCurrency, toCurrency);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public Optional<CorridorRate> getRate(String fromCurrency, String toCurrency) {
log.info("[FRANKFURTER] Fetching rate for {}:{}", fromCurrency, toCurrency);
var response = restClient.get()
.uri("/latest?from={from}&to={to}", fromCurrency, toCurrency)
.retrieve()
.body(FrankfurterRateResponse.class);
if (response == null || response.rates() == null || response.rates().isEmpty()) {
log.warn("[FRANKFURTER] No rate returned for {}:{}", fromCurrency, toCurrency);
return Optional.empty();
}
public Optional<CorridorRate> getRate(String fromCurrency, String toCurrency) {
log.debug("[FRANKFURTER] Fetching rate for {}:{}", fromCurrency, toCurrency);
var response = restClient.get()
.uri("/latest?from={from}&to={to}", fromCurrency, toCurrency)
.retrieve()
.body(FrankfurterRateResponse.class);
if (response == null || response.rates() == null || response.rates().isEmpty()) {
log.warn("[FRANKFURTER] No rate returned for {}:{}", fromCurrency, toCurrency);
return Optional.empty();
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/frankfurter/FrankfurterRateAdapter.java`
around lines 48 - 59, Change the routine request log in
FrankfurterRateAdapter.getRate from INFO to DEBUG to avoid high-volume logging
during frequent polling; update the log statement that currently reads
log.info("[FRANKFURTER] Fetching rate for {}:{}", fromCurrency, toCurrency) to
use log.debug instead, keeping the existing warning log (log.warn) for missing
responses unchanged so only notable issues remain at higher levels.


var rate = response.rates().get(toCurrency);
if (rate == null) {
log.warn("[FRANKFURTER] Rate for {} not found in response for {}:{}", toCurrency, fromCurrency, toCurrency);
return Optional.empty();
}

var corridorRate = CorridorRate.builder()
.fromCurrency(fromCurrency)
.toCurrency(toCurrency)
.rate(rate)
.spreadBps(DEFAULT_SPREAD_BPS)
.feeBps(DEFAULT_FEE_BPS)
.provider("frankfurter")
.ageMs(0)
.build();

log.info("[FRANKFURTER] Rate fetched {}:{}={}", fromCurrency, toCurrency, rate);
return Optional.of(corridorRate);
}
Comment on lines +67 to +79

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

ageMs=0 hardcoded—document the limitation.

The Frankfurter API returns only a date, not a timestamp, so actual rate staleness cannot be determined. Hardcoding ageMs=0 is pragmatic for a sandbox provider, but consider adding a brief code comment explaining this limitation for future maintainers.

📝 Add explanatory comment
         var corridorRate = CorridorRate.builder()
                 .fromCurrency(fromCurrency)
                 .toCurrency(toCurrency)
                 .rate(rate)
                 .spreadBps(DEFAULT_SPREAD_BPS)
                 .feeBps(DEFAULT_FEE_BPS)
                 .provider("frankfurter")
+                // Frankfurter API provides date only (no timestamp), so ageMs cannot be accurately determined
                 .ageMs(0)
                 .build();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var corridorRate = CorridorRate.builder()
.fromCurrency(fromCurrency)
.toCurrency(toCurrency)
.rate(rate)
.spreadBps(DEFAULT_SPREAD_BPS)
.feeBps(DEFAULT_FEE_BPS)
.provider("frankfurter")
.ageMs(0)
.build();
log.info("[FRANKFURTER] Rate fetched {}:{}={}", fromCurrency, toCurrency, rate);
return Optional.of(corridorRate);
}
var corridorRate = CorridorRate.builder()
.fromCurrency(fromCurrency)
.toCurrency(toCurrency)
.rate(rate)
.spreadBps(DEFAULT_SPREAD_BPS)
.feeBps(DEFAULT_FEE_BPS)
.provider("frankfurter")
// Frankfurter API provides date only (no timestamp), so ageMs cannot be accurately determined
.ageMs(0)
.build();
log.info("[FRANKFURTER] Rate fetched {}:{}={}", fromCurrency, toCurrency, rate);
return Optional.of(corridorRate);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/frankfurter/FrankfurterRateAdapter.java`
around lines 67 - 79, Add a brief explanatory code comment in
FrankfurterRateAdapter next to the CorridorRate.builder() usage (or specifically
the ageMs(0) line) that states the Frankfurter API provides only a date (no
timestamp), so true staleness cannot be computed and we set ageMs to 0 as a
pragmatic/sandbox choice; reference the Frankfurter API behavior and note this
limitation for future maintainers and that it should be revisited if a
timestamped provider is used.


@Override
public String providerName() {
return "frankfurter";
}

@SuppressWarnings("unused")
private Optional<CorridorRate> frankfurterFallback(String fromCurrency, String toCurrency, Exception ex) {
log.error("[FRANKFURTER] Resilience fallback for {}:{} due to {}",
fromCurrency, toCurrency, ex.getClass().getSimpleName(), ex);
return Optional.empty();
}
Comment on lines +86 to +91

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find usages of RateProvider.getRate and check how empty is handled
ast-grep --pattern 'getRate($_, $_)' | head -50
rg -n -A5 'getRate\(' --type=java -g '!*Test*' -g '!FrankfurterRateAdapter.java'

Repository: Puneethkumarck/stablebridge-platform

Length of output: 6316


🏁 Script executed:

#!/bin/bash
# Check FxQuoteApplicationService full method context around getRate call
rg -n -B10 -A15 'rateProvider.getRate' fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/application/service/FxQuoteApplicationService.java | head -60

Repository: Puneethkumarck/stablebridge-platform

Length of output: 1561


🏁 Script executed:

#!/bin/bash
# Check RateRefreshJob full method context
rg -n -B5 -A15 'rateProvider.getRate' fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/scheduling/RateRefreshJob.java

Repository: Puneethkumarck/stablebridge-platform

Length of output: 1084


🏁 Script executed:

#!/bin/bash
# Check how FrankfurterRateAdapter.getRate is decorated (circuit breaker, resilience4j)
rg -n -B20 'public Optional<CorridorRate> getRate' fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/frankfurter/FrankfurterRateAdapter.java | head -40

Repository: Puneethkumarck/stablebridge-platform

Length of output: 1041


🏁 Script executed:

#!/bin/bash
# Verify frankfurterFallback is used as fallback method in circuit breaker config
rg -n 'frankfurterFallback\|@CircuitBreaker\|@Fallback' fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/frankfurter/FrankfurterRateAdapter.java

Repository: Puneethkumarck/stablebridge-platform

Length of output: 62


Circuit breaker fallback returns Optional.empty() without distinguishing provider-down from legitimate empty rates.

Both FxQuoteApplicationService and RateRefreshJob correctly handle the empty Optional—the former throws RateUnavailableException (critical path), the latter logs and continues (background refresh). However, since the fallback returns the same Optional.empty() regardless of whether the provider is down or legitimately has no rate, callers cannot distinguish these cases. Consider having frankfurterFallback() throw a distinct exception (e.g., ProviderUnavailableException) instead of returning empty, enabling callers to implement provider-specific fallback strategies.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@fx-liquidity-engine/fx-liquidity-engine/src/main/java/com/stablecoin/payments/fx/infrastructure/provider/frankfurter/FrankfurterRateAdapter.java`
around lines 86 - 91, frankfurterFallback currently always returns
Optional.empty(), hiding provider-down failures; change
frankfurterFallback(String fromCurrency, String toCurrency, Exception ex) so
that when ex indicates the provider is unavailable it throws a distinct
ProviderUnavailableException (include fromCurrency, toCurrency and ex as cause)
instead of returning Optional.empty(), and only return Optional.empty() for
legitimate “no rate” conditions; update the method signature to allow throwing
ProviderUnavailableException (or use an unchecked
ProviderUnavailableRuntimeException) and keep references to CorridorRate,
Optional.empty(), FxQuoteApplicationService and RateRefreshJob in logs/exception
message so callers can detect provider-down vs legitimate empty rates.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.stablecoin.payments.fx.infrastructure.provider.frankfurter;

import java.math.BigDecimal;
import java.util.Map;

record FrankfurterRateResponse(
BigDecimal amount,
String base,
String date,
Map<String, BigDecimal> rates
) {}
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ app:
resilience4j:
circuitbreaker:
circuit-breaker-aspect-order: 1
instances:
frankfurter:
slidingWindowSize: 10
failureRateThreshold: 50
waitDurationInOpenState: 30s
retry:
retry-aspect-order: 3
instances:
Expand All @@ -144,6 +149,18 @@ resilience4j:
ignore-exceptions:
- org.springframework.web.client.HttpClientErrorException
- io.github.resilience4j.circuitbreaker.CallNotPermittedException
frankfurter:
maxAttempts: 3
waitDuration: 1s
retry-exceptions:
- java.io.IOException
- java.net.http.HttpTimeoutException
- java.net.http.HttpConnectTimeoutException
- org.springframework.web.client.ResourceAccessException
- org.springframework.web.client.HttpServerErrorException
ignore-exceptions:
- org.springframework.web.client.HttpClientErrorException
- io.github.resilience4j.circuitbreaker.CallNotPermittedException
Comment on lines +152 to +163

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Inconsistent property naming style in retry configuration.

The frankfurter retry config uses camelCase (maxAttempts, waitDuration) while the existing fxRate config uses kebab-case (max-attempts, wait-duration). Spring Boot accepts both, but mixing styles in the same file reduces maintainability.

Also, fxRate enables exponential backoff; consider whether frankfurter should too for better retry distribution under load.

♻️ Align naming style with existing config
       frankfurter:
-        maxAttempts: 3
-        waitDuration: 1s
+        max-attempts: 3
+        wait-duration: 1s
+        enable-exponential-backoff: true
+        exponential-backoff-multiplier: 2
         retry-exceptions:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fx-liquidity-engine/fx-liquidity-engine/src/main/resources/application.yml`
around lines 152 - 163, The frankfurter retry block uses camelCase keys
(maxAttempts, waitDuration) which is inconsistent with the existing fxRate
block's kebab-case (max-attempts, wait-duration); change frankfurter property
names to kebab-case (max-attempts, wait-duration) to match style and
maintainability, and optionally add the same exponential backoff settings used
by fxRate (e.g., enable exponential-backoff with multiplier/delay settings)
under the frankfurter configuration so retry behavior is consistent with fxRate.


logging:
level:
Expand Down
Loading
Loading