Skip to content

Commit 337471c

Browse files
committed
feat!: Request.queryParam/header return Optional<String>, treat blank as absent
Every list handler was writing: int limit = Optional.ofNullable(req.queryParam("limit")) .filter(v -> !v.isBlank()) .map(Integer::parseInt) .orElse(DEFAULT_LIMIT); The blank-vs-absent question is the kind of HTTP-API trap that's better solved once in the library than copied across every consumer. Returning Optional<String> with blank already filtered shrinks the call site to: int limit = req.queryParam("limit").map(Integer::parseInt).orElse(DEFAULT_LIMIT); Same treatment for header(name). pathParam(name) is unchanged (still nullable) — path parameters come from the route template, not from the client, so blank is genuinely a different case.
1 parent 8464ff2 commit 337471c

3 files changed

Lines changed: 56 additions & 20 deletions

File tree

README.md

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ public class PostDataHandler implements RequestHandler {
4848
// TypedTypeMapper).
4949
MyDto dto = request.asPojo(MyDto.class);
5050
// Path parameters, query parameters, and headers are also available.
51-
String id = request.pathParam("id");
52-
String filter = request.queryParam("filter");
53-
String corr = request.header("correlation-id");
51+
String id = request.pathParam("id"); // null if absent
52+
Optional<String> filter = request.queryParam("filter"); // empty if absent or blank
53+
Optional<String> corr = request.header("correlation-id");
5454

5555
return Response.ok(dto);
5656
}
@@ -182,7 +182,7 @@ OpenApiServer.builder()
182182
.handlers(handlers)
183183
.interceptor((request, next) -> {
184184
// Resolve once per request; bind to a ScopedValue for the rest of the chain.
185-
String tenant = request.header("X-Tenant-Id");
185+
String tenant = request.header("X-Tenant-Id").orElse("public");
186186
return ScopedValue.where(TENANT, tenant).call(next::proceed);
187187
})
188188
.interceptor((request, next) -> {
@@ -213,8 +213,7 @@ OpenApiServer.builder()
213213
// 1. Resolve once per request and bind to ScopedValues.
214214
.interceptor((request, next) -> {
215215
String correlationId =
216-
Optional.ofNullable(request.header("X-Correlation-Id"))
217-
.orElseGet(() -> UUID.randomUUID().toString());
216+
request.header("X-Correlation-Id").orElseGet(() -> UUID.randomUUID().toString());
218217
String tenantId = resolveTenant(request);
219218
return ScopedValue.where(CORRELATION_ID, correlationId)
220219
.where(TENANT_ID, tenantId)
@@ -282,10 +281,9 @@ public final class App {
282281
.handlers(Map.of("get-promotion", getPromotion))
283282
// Bind tenant + correlation id once per request.
284283
.interceptor((req, next) -> {
285-
String tenant = req.header("X-Tenant-Id");
284+
String tenant = req.header("X-Tenant-Id").orElse("public");
286285
String correlationId =
287-
Optional.ofNullable(req.header("X-Correlation-Id"))
288-
.orElseGet(() -> UUID.randomUUID().toString());
286+
req.header("X-Correlation-Id").orElseGet(() -> UUID.randomUUID().toString());
289287
return ScopedValue.where(TENANT, tenant)
290288
.where(CORRELATION_ID, correlationId)
291289
.call(next::proceed);

src/main/java/com/retailsvc/http/Request.java

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java.util.LinkedHashMap;
77
import java.util.Map;
88
import java.util.Objects;
9+
import java.util.Optional;
910

1011
/**
1112
* Read-only per-request handle passed to {@link RequestHandler}. Carries the parsed body, path
@@ -70,12 +71,13 @@ public <T> T asPojo(Class<T> type) {
7071
if (parsed != null && type.isInstance(parsed)) {
7172
return type.cast(parsed);
7273
}
74+
String contentType = exchange.getRequestHeaders().getFirst(CONTENT_TYPE);
7375
if (bodyMapper instanceof TypedTypeMapper typed) {
74-
return typed.readAs(body, header(CONTENT_TYPE), type);
76+
return typed.readAs(body, contentType, type);
7577
}
7678
throw new IllegalStateException(
7779
"body mapper for "
78-
+ header(CONTENT_TYPE)
80+
+ contentType
7981
+ " does not support typed conversion; the mapper must implement TypedTypeMapper");
8082
}
8183

@@ -92,8 +94,14 @@ public String pathParam(String name) {
9294
return pathParameters.get(name);
9395
}
9496

95-
public String header(String name) {
96-
return exchange.getRequestHeaders().getFirst(name);
97+
/**
98+
* First value of the request header {@code name}, or {@link Optional#empty()} if absent or blank.
99+
* Blank values are treated as missing so callers can write {@code req.header("X").map(...)}
100+
* without the extra {@code filter(v -> !v.isBlank())} step.
101+
*/
102+
public Optional<String> header(String name) {
103+
String raw = exchange.getRequestHeaders().getFirst(name);
104+
return raw == null || raw.isBlank() ? Optional.empty() : Optional.of(raw);
97105
}
98106

99107
/**
@@ -115,9 +123,15 @@ public Map<String, String> queryParams() {
115123
return queryParamCache;
116124
}
117125

118-
/** First decoded value for {@code name}, or {@code null} if absent. */
119-
public String queryParam(String name) {
120-
return queryParams().get(name);
126+
/**
127+
* First decoded value for query parameter {@code name}, or {@link Optional#empty()} if absent or
128+
* blank. Blank values are treated as missing so callers can write {@code
129+
* req.queryParam("limit").map(Integer::parseInt).orElse(DEFAULT)} without the extra {@code
130+
* filter(v -> !v.isBlank())} step.
131+
*/
132+
public Optional<String> queryParam(String name) {
133+
String raw = queryParams().get(name);
134+
return raw == null || raw.isBlank() ? Optional.empty() : Optional.of(raw);
121135
}
122136

123137
private static Map<String, String> parseQuery(String query) {

src/test/java/com/retailsvc/http/RequestTest.java

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,9 @@ void exposesQueryParams() {
130130
Request req = new Request(exchange, new byte[0], null, null, "op", Map.of());
131131

132132
assertThat(req.rawQuery()).isEqualTo("name=Alice%20Smith&active=true&active=false");
133-
assertThat(req.queryParam("name")).isEqualTo("Alice Smith");
134-
assertThat(req.queryParam("active")).isEqualTo("true");
135-
assertThat(req.queryParam("missing")).isNull();
133+
assertThat(req.queryParam("name")).contains("Alice Smith");
134+
assertThat(req.queryParam("active")).contains("true");
135+
assertThat(req.queryParam("missing")).isEmpty();
136136
assertThat(req.queryParams())
137137
.containsEntry("name", "Alice Smith")
138138
.containsEntry("active", "true");
@@ -146,6 +146,30 @@ void queryParamsEmptyWhenNoQuery() {
146146

147147
assertThat(req.rawQuery()).isNull();
148148
assertThat(req.queryParams()).isEmpty();
149-
assertThat(req.queryParam("anything")).isNull();
149+
assertThat(req.queryParam("anything")).isEmpty();
150+
}
151+
152+
@Test
153+
void queryParamBlankIsTreatedAsAbsent() {
154+
HttpExchange exchange = mock(HttpExchange.class);
155+
when(exchange.getRequestURI()).thenReturn(URI.create("http://h/x?limit=&offset=%20"));
156+
Request req = new Request(exchange, new byte[0], null, null, "op", Map.of());
157+
158+
assertThat(req.queryParam("limit")).isEmpty();
159+
assertThat(req.queryParam("offset")).isEmpty();
160+
}
161+
162+
@Test
163+
void headerReturnsOptionalAndBlankIsAbsent() {
164+
HttpExchange exchange = mock(HttpExchange.class);
165+
com.sun.net.httpserver.Headers h = new com.sun.net.httpserver.Headers();
166+
h.add("X-Trace", "abc");
167+
h.add("X-Empty", " ");
168+
when(exchange.getRequestHeaders()).thenReturn(h);
169+
Request req = new Request(exchange, new byte[0], null, null, "op", Map.of());
170+
171+
assertThat(req.header("X-Trace")).contains("abc");
172+
assertThat(req.header("X-Empty")).isEmpty();
173+
assertThat(req.header("Missing")).isEmpty();
150174
}
151175
}

0 commit comments

Comments
 (0)