Skip to content

Commit 394082e

Browse files
author
Andy Ngo
committed
#122 Add all missing required params to exception
Refactor error report to show all missing query parameters in one Exception
1 parent dee877e commit 394082e

File tree

4 files changed

+63
-12
lines changed

4 files changed

+63
-12
lines changed

src/main/java/dev/sorn/fmp4j/services/FmpService.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,14 @@ protected Map<String, String> headers() {
7676

7777
public final R download() {
7878
final var required = requiredParams();
79-
for (final var req : required.keySet()) {
80-
if (!params.containsKey(req)) {
81-
throw new FmpServiceException("'%s' is a required query param for endpoint [%s]", req, url());
82-
}
79+
final var missing = required.keySet().stream()
80+
.filter(req -> !params.containsKey(req))
81+
.toList();
82+
83+
if (!missing.isEmpty()) {
84+
throw new FmpServiceException("%s are required query params for endpoint [%s]", missing, url());
8385
}
86+
8487
return filter(http.get(typeRef, url(), headers(), params));
8588
}
8689
}

src/test/java/dev/sorn/fmp4j/services/FmpFullQuoteServiceTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ void missing_symbol_throws() {
7676

7777
// then
7878
var e = assertThrows(FmpServiceException.class, () -> f.accept(service));
79-
assertEquals(format("'symbol' is a required query param for endpoint [%s]", service.url()), e.getMessage());
79+
assertEquals(format("[symbol] are required query params for endpoint [%s]", service.url()), e.getMessage());
8080
}
8181

8282
@Test

src/test/java/dev/sorn/fmp4j/services/FmpPartialQuoteServiceTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ void missing_symbol_throws() {
7676

7777
// then
7878
var e = assertThrows(FmpServiceException.class, () -> f.accept(service));
79-
assertEquals(format("'symbol' is a required query param for endpoint [%s]", service.url()), e.getMessage());
79+
assertEquals(format("[symbol] are required query params for endpoint [%s]", service.url()), e.getMessage());
8080
}
8181

8282
@Test

src/test/java/dev/sorn/fmp4j/services/FmpServiceTest.java

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import dev.sorn.fmp4j.cfg.FmpConfig;
1313
import dev.sorn.fmp4j.http.FmpHttpClient;
1414
import dev.sorn.fmp4j.types.FmpApiKey;
15+
import dev.sorn.fmp4j.types.FmpLimit;
16+
import dev.sorn.fmp4j.types.FmpPage;
1517
import dev.sorn.fmp4j.types.FmpSymbol;
1618
import java.time.LocalDate;
1719
import java.util.Arrays;
@@ -34,15 +36,19 @@ class FmpServiceTest {
3436

3537
private ConcreteFmpService service;
3638

39+
private MultiRequiredFmpService multiRequiredService;
40+
3741
@BeforeEach
3842
void setup() {
3943
openMocks(this);
4044
when(mockConfig.fmpApiKey()).thenReturn(new FmpApiKey("abcdefghij1234567890abcdefghij12"));
4145
service = new ConcreteFmpService(mockConfig, mockHttpClient);
46+
47+
multiRequiredService = new MultiRequiredFmpService(mockConfig, mockHttpClient);
4248
}
4349

4450
@Test
45-
void accepts_list_with_correct_type() {
51+
void shouldAcceptListWithCorrectType() {
4652
// given
4753
List<FmpSymbol> symbols = List.of(symbol("AAPL"), symbol("GOOGL"), symbol("MSFT"));
4854

@@ -84,12 +90,11 @@ void handles_empty_collection() {
8490
}
8591

8692
@Test
87-
@DisplayName("Should accept Optional with correct type")
8893
void accepts_optional_with_correct_type() {
8994
// given
9095
Optional<FmpSymbol> optionalSymbol = Optional.of(symbol("AAPL"));
9196

92-
// when // then
97+
// when //then
9398
assertDoesNotThrow(() -> service.param("symbol", optionalSymbol));
9499
assertEquals(optionalSymbol, service.params.get("symbol"));
95100
}
@@ -114,7 +119,7 @@ void handles_empty_optional() {
114119
// given
115120
Optional<String> emptyOptional = Optional.empty();
116121

117-
// when then
122+
// when //then
118123
assertDoesNotThrow(() -> service.param("symbol", emptyOptional));
119124
assertEquals(emptyOptional, service.params.get("symbol"));
120125
}
@@ -130,15 +135,37 @@ void handles_null_key() {
130135
}
131136

132137
@Test
133-
@DisplayName("Should reject nested Collection")
134138
void validates_nested_collections() {
135-
// given: a nested list
139+
// given
136140
List<List<String>> nestedList = Arrays.asList(Arrays.asList("AAPL", "GOOGL"));
137141

138142
// when // then
139143
assertThrows(FmpServiceException.class, () -> service.param("symbol", nestedList));
140144
}
141145

146+
@Test
147+
void should_show_all_missing_required_params() {
148+
// given //when
149+
FmpServiceException thrownEx = assertThrows(FmpServiceException.class, multiRequiredService::download);
150+
var msg = thrownEx.getMessage();
151+
// then
152+
assertTrue(
153+
msg.contains("limit") || msg.contains("page"),
154+
"Expected exception message to mention either 'limit' and 'page' as the required param, but got: "
155+
+ msg);
156+
}
157+
158+
@Test
159+
void should_not_throw_missing_required_params() {
160+
// given
161+
multiRequiredService.param("page", FmpPage.page(10));
162+
multiRequiredService.param("limit", FmpLimit.limit(10));
163+
164+
// when //then
165+
assertDoesNotThrow(multiRequiredService::download);
166+
}
167+
168+
// Concrete implementation for testing
142169
private static class ConcreteFmpService extends FmpService<String> {
143170
public ConcreteFmpService(FmpConfig cfg, FmpHttpClient http) {
144171
super(cfg, http, new TypeReference<String>() {});
@@ -159,4 +186,25 @@ protected String relativeUrl() {
159186
return "/test/endpoint";
160187
}
161188
}
189+
190+
private static class MultiRequiredFmpService extends FmpService<String> {
191+
public MultiRequiredFmpService(FmpConfig cfg, FmpHttpClient http) {
192+
super(cfg, http, new TypeReference<String>() {});
193+
}
194+
195+
@Override
196+
protected Map<String, Class<?>> requiredParams() {
197+
return Map.of("limit", FmpLimit.class, "page", FmpPage.class);
198+
}
199+
200+
@Override
201+
protected Map<String, Class<?>> optionalParams() {
202+
return Map.of();
203+
}
204+
205+
@Override
206+
protected String relativeUrl() {
207+
return "/test/endpoint";
208+
}
209+
}
162210
}

0 commit comments

Comments
 (0)