Skip to content

Commit 89c523b

Browse files
committed
refactor: Carry the gzip collaborators instead of plumbing their sizes
HandlerConfig now holds the RequestBodyReader and the ResponseRenderer themselves rather than the two longs they are built from, and Builder constructs both. Since HandlerConfig already reached every wiring method, the extra parameters threaded through wireBindings, wireBinding and wireExtras all go away, and with them both java:S107 suppressions — the parameter counts were the signal that the numbers were at the wrong altitude. Also reuses ContentTypeHeader.parameter for the Accept-Encoding q weight instead of hand-rolling a second parameter parser, replaces the capped inflate loop with a single bounded readNBytes, and drops the gzipStream alias for the JDK constructor it wrapped. The five gzip files go 530 to 470 lines and OpenApiServer sheds 36.
1 parent 8bf4e13 commit 89c523b

6 files changed

Lines changed: 58 additions & 141 deletions

File tree

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

Lines changed: 20 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ record HandlerConfig(
6666
Map<String, RequestHandler> extras,
6767
boolean externalAuth,
6868
List<AfterResponseHook> afterHooks,
69-
long maxDecompressedRequestBytes,
70-
long minimumGzipResponseBytes) {}
69+
RequestBodyReader bodyReader,
70+
ResponseRenderer renderer) {}
7171

7272
OpenApiServer(
7373
List<SpecBinding> bindings,
@@ -86,7 +86,6 @@ record HandlerConfig(
8686
requireNonNull(bodyMappers, "bodyMappers must not be null");
8787

8888
long t0 = System.currentTimeMillis();
89-
ExceptionHandler exceptionHandler = handlerConfig.exceptionHandler();
9089

9190
InetSocketAddress socketAddress =
9291
(bindAddress == null)
@@ -95,26 +94,8 @@ record HandlerConfig(
9594
this.httpServer = createHttpServer(socketAddress, sslContext);
9695
httpServer.setExecutor(newThreadPerTaskExecutor(ofVirtual().name("http-", 0).factory()));
9796

98-
ResponseRenderer renderer =
99-
new ResponseRenderer(bodyMappers, handlerConfig.minimumGzipResponseBytes());
100-
RequestBodyReader bodyReader =
101-
new RequestBodyReader(handlerConfig.maxDecompressedRequestBytes());
102-
boolean anyBindingAtRoot =
103-
wireBindings(
104-
httpServer,
105-
bindings,
106-
bodyMappers,
107-
handlerConfig,
108-
exceptionHandler,
109-
renderer,
110-
bodyReader);
111-
wireExtras(
112-
httpServer,
113-
anyBindingAtRoot,
114-
handlerConfig.extras(),
115-
exceptionHandler,
116-
renderer,
117-
bodyReader);
97+
boolean anyBindingAtRoot = wireBindings(httpServer, bindings, bodyMappers, handlerConfig);
98+
wireExtras(httpServer, anyBindingAtRoot, handlerConfig);
11899

119100
httpServer.start();
120101
this.shutdownTimeoutSeconds = shutdownTimeoutSeconds;
@@ -131,42 +112,26 @@ private static HttpServer createHttpServer(InetSocketAddress addr, SSLContext ss
131112
return HttpServer.create(addr, 0);
132113
}
133114

134-
@SuppressWarnings("java:S107")
135115
private static boolean wireBindings(
136116
HttpServer httpServer,
137117
List<SpecBinding> bindings,
138118
Map<String, TypeMapper> bodyMappers,
139-
HandlerConfig handlerConfig,
140-
ExceptionHandler exceptionHandler,
141-
ResponseRenderer renderer,
142-
RequestBodyReader bodyReader) {
119+
HandlerConfig handlerConfig) {
143120
boolean anyBindingAtRoot = false;
144121
for (SpecBinding binding : bindings) {
145122
String basePath = Optional.ofNullable(binding.spec().basePath()).orElse("/");
146123
anyBindingAtRoot |= "/".equals(basePath);
147-
wireBinding(
148-
httpServer,
149-
basePath,
150-
binding,
151-
bodyMappers,
152-
handlerConfig,
153-
exceptionHandler,
154-
renderer,
155-
bodyReader);
124+
wireBinding(httpServer, basePath, binding, bodyMappers, handlerConfig);
156125
}
157126
return anyBindingAtRoot;
158127
}
159128

160-
@SuppressWarnings("java:S107")
161129
private static void wireBinding(
162130
HttpServer httpServer,
163131
String basePath,
164132
SpecBinding binding,
165133
Map<String, TypeMapper> bodyMappers,
166-
HandlerConfig handlerConfig,
167-
ExceptionHandler exceptionHandler,
168-
ResponseRenderer renderer,
169-
RequestBodyReader bodyReader) {
134+
HandlerConfig handlerConfig) {
170135
Map<String, Operation> operationsById =
171136
binding.spec().operations().stream()
172137
.collect(Collectors.toUnmodifiableMap(Operation::operationId, op -> op));
@@ -178,10 +143,10 @@ private static void wireBinding(
178143
binding.router(),
179144
binding.validator(),
180145
bodyMappers,
181-
exceptionHandler,
182-
renderer,
146+
handlerConfig.exceptionHandler(),
147+
handlerConfig.renderer(),
183148
handlerConfig.afterHooks(),
184-
bodyReader));
149+
handlerConfig.bodyReader()));
185150
ctx.getFilters()
186151
.add(
187152
new SecurityFilter(
@@ -195,26 +160,25 @@ private static void wireBinding(
195160
binding.handlers(),
196161
handlerConfig.interceptors(),
197162
handlerConfig.decorators(),
198-
renderer));
163+
handlerConfig.renderer()));
199164
}
200165

201166
private static void wireExtras(
202-
HttpServer httpServer,
203-
boolean anyBindingAtRoot,
204-
Map<String, RequestHandler> extras,
205-
ExceptionHandler exceptionHandler,
206-
ResponseRenderer renderer,
207-
RequestBodyReader bodyReader) {
167+
HttpServer httpServer, boolean anyBindingAtRoot, HandlerConfig handlerConfig) {
168+
Map<String, RequestHandler> extras = handlerConfig.extras();
208169
if (anyBindingAtRoot) {
209170
if (!extras.isEmpty()) {
210171
throw new IllegalStateException(
211172
"extras cannot be registered when a binding owns basePath '/'");
212173
}
213174
return;
214175
}
215-
ExtrasRouter extrasRouter = new ExtrasRouter(extras, renderer, bodyReader);
176+
ExtrasRouter extrasRouter =
177+
new ExtrasRouter(extras, handlerConfig.renderer(), handlerConfig.bodyReader());
216178
HttpContext extrasCtx = httpServer.createContext("/", extrasRouter);
217-
extrasCtx.getFilters().add(new ExceptionFilter(exceptionHandler, renderer));
179+
extrasCtx
180+
.getFilters()
181+
.add(new ExceptionFilter(handlerConfig.exceptionHandler(), handlerConfig.renderer()));
218182
}
219183

220184
private void logStartup(long t0) {
@@ -510,8 +474,8 @@ public OpenApiServer build() throws IOException {
510474
extras,
511475
externalAuth,
512476
List.copyOf(afterHooks),
513-
maxDecompressedRequestBytes,
514-
minimumGzipResponseBytes);
477+
new RequestBodyReader(maxDecompressedRequestBytes),
478+
new ResponseRenderer(resolved, minimumGzipResponseBytes));
515479
int resolvedPort = resolvePort();
516480
SSLContext sslContext =
517481
httpsCertChain != null ? PemSslContext.load(httpsCertChain, httpsPrivateKey) : null;

src/main/java/com/retailsvc/http/internal/AcceptEncodingHeader.java

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
/** Parses {@code Accept-Encoding} request header values (RFC 9110 §12.5.3). */
66
public final class AcceptEncodingHeader {
77

8-
private static final double DEFAULT_QUALITY = 1.0;
9-
108
private AcceptEncodingHeader() {}
119

1210
/**
@@ -22,14 +20,9 @@ public static boolean acceptsGzip(String header) {
2220
boolean gzipAccepted = false;
2321
boolean wildcardAccepted = false;
2422
for (String token : header.split(",")) {
25-
String trimmed = token.trim();
26-
if (trimmed.isEmpty()) {
27-
continue;
28-
}
29-
int semi = trimmed.indexOf(';');
30-
String coding =
31-
(semi < 0 ? trimmed : trimmed.substring(0, semi)).trim().toLowerCase(Locale.ROOT);
32-
boolean accepted = quality(semi < 0 ? null : trimmed.substring(semi + 1)) > 0;
23+
int semi = token.indexOf(';');
24+
String coding = (semi < 0 ? token : token.substring(0, semi)).trim().toLowerCase(Locale.ROOT);
25+
boolean accepted = positiveWeight(token);
3326
if ("gzip".equals(coding) || "x-gzip".equals(coding)) {
3427
gzipSeen = true;
3528
gzipAccepted |= accepted;
@@ -41,28 +34,18 @@ public static boolean acceptsGzip(String header) {
4134
}
4235

4336
/**
44-
* Reads the {@code q} weight from a token's parameter list. An absent or unparsable weight is
45-
* read as the default 1.0 — a malformed header should not silently disable compression.
37+
* Whether the token's {@code q} weight admits the coding. An absent or unparsable weight reads as
38+
* the default 1.0 — a malformed header should not silently disable compression.
4639
*/
47-
private static double quality(String parameters) {
48-
if (parameters == null) {
49-
return DEFAULT_QUALITY;
40+
private static boolean positiveWeight(String token) {
41+
String weight = ContentTypeHeader.parameter(token, "q").orElse(null);
42+
if (weight == null) {
43+
return true;
5044
}
51-
for (String parameter : parameters.split(";")) {
52-
String trimmed = parameter.trim();
53-
int equals = trimmed.indexOf('=');
54-
if (equals <= 0) {
55-
continue;
56-
}
57-
String name = trimmed.substring(0, equals).trim().toLowerCase(Locale.ROOT);
58-
if ("q".equals(name)) {
59-
try {
60-
return Double.parseDouble(trimmed.substring(equals + 1).trim());
61-
} catch (NumberFormatException e) {
62-
return DEFAULT_QUALITY;
63-
}
64-
}
45+
try {
46+
return Double.parseDouble(weight) > 0;
47+
} catch (NumberFormatException _) {
48+
return true;
6549
}
66-
return DEFAULT_QUALITY;
6750
}
6851
}

src/main/java/com/retailsvc/http/internal/RequestBodyReader.java

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import com.sun.net.httpserver.Headers;
99
import com.sun.net.httpserver.HttpExchange;
1010
import java.io.ByteArrayInputStream;
11-
import java.io.ByteArrayOutputStream;
1211
import java.io.EOFException;
1312
import java.io.IOException;
1413
import java.util.function.UnaryOperator;
@@ -29,13 +28,15 @@ public final class RequestBodyReader {
2928
private static final int BUFFER_SIZE = 8192;
3029

3130
private final long maxDecompressedBytes;
31+
private final int readLimit;
3232

3333
public RequestBodyReader(long maxDecompressedBytes) {
3434
if (maxDecompressedBytes <= 0) {
3535
throw new IllegalArgumentException(
3636
"maxDecompressedBytes must be positive, got " + maxDecompressedBytes);
3737
}
3838
this.maxDecompressedBytes = maxDecompressedBytes;
39+
this.readLimit = (int) Math.min(maxDecompressedBytes, Integer.MAX_VALUE - 1) + 1;
3940
}
4041

4142
/**
@@ -67,20 +68,13 @@ private byte[] inflate(byte[] raw) throws IOException {
6768
return raw;
6869
}
6970
try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(raw), BUFFER_SIZE)) {
70-
ByteArrayOutputStream out = new ByteArrayOutputStream();
71-
byte[] buffer = new byte[BUFFER_SIZE];
72-
long total = 0;
73-
int read;
74-
while ((read = in.read(buffer)) != -1) {
75-
total += read;
76-
if (total > maxDecompressedBytes) {
77-
throw new BadRequestException(
78-
HTTP_ENTITY_TOO_LARGE,
79-
"decompressed request body exceeds " + maxDecompressedBytes + " bytes");
80-
}
81-
out.write(buffer, 0, read);
71+
byte[] inflated = in.readNBytes(readLimit);
72+
if (inflated.length > maxDecompressedBytes) {
73+
throw new BadRequestException(
74+
HTTP_ENTITY_TOO_LARGE,
75+
"decompressed request body exceeds " + maxDecompressedBytes + " bytes");
8276
}
83-
return out.toByteArray();
77+
return inflated;
8478
} catch (ZipException | EOFException e) {
8579
throw new BadRequestException(HTTP_BAD_REQUEST, "malformed gzip request body", e);
8680
}

src/main/java/com/retailsvc/http/internal/ResponseCompression.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import java.io.ByteArrayOutputStream;
44
import java.io.IOException;
5-
import java.io.OutputStream;
65
import java.util.Set;
76
import java.util.zip.GZIPOutputStream;
87

@@ -53,12 +52,4 @@ public static byte[] gzip(byte[] body) throws IOException {
5352
}
5453
return out.toByteArray();
5554
}
56-
57-
/**
58-
* Wraps {@code out} so a streamed body is deflated as it is written. Closing the returned stream
59-
* writes the gzip trailer and releases the deflater's native memory, so the caller must close it.
60-
*/
61-
public static OutputStream gzipStream(OutputStream out) throws IOException {
62-
return new GZIPOutputStream(out);
63-
}
6455
}

src/main/java/com/retailsvc/http/internal/ResponseRenderer.java

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.io.OutputStream;
1515
import java.util.Locale;
1616
import java.util.Map;
17+
import java.util.zip.GZIPOutputStream;
1718

1819
/** Writes a {@link Response} to an {@link HttpExchange}. */
1920
public final class ResponseRenderer {
@@ -65,9 +66,7 @@ public void render(HttpExchange exchange, Response response) throws IOException
6566
*/
6667
private void renderEmpty(HttpExchange exchange, Headers headers, int status, String contentType)
6768
throws IOException {
68-
if (contentType != null && !headers.containsKey(CONTENT_TYPE)) {
69-
headers.add(CONTENT_TYPE, contentType);
70-
}
69+
defaultContentType(headers, contentType);
7170
long declared = declaredLength(headers);
7271
if (shouldCompress(exchange, headers, status, contentType, declared) && declared >= 0) {
7372
headers.remove(CONTENT_LENGTH);
@@ -78,24 +77,26 @@ private void renderEmpty(HttpExchange exchange, Headers headers, int status, Str
7877
private void renderStream(
7978
HttpExchange exchange, Headers headers, int status, String contentType, BodyWriter writer)
8079
throws IOException {
81-
if (contentType != null && !headers.containsKey(CONTENT_TYPE)) {
82-
headers.add(CONTENT_TYPE, contentType);
83-
}
80+
defaultContentType(headers, contentType);
8481
long declared = writer instanceof BodyWriter.Sized sized ? sized.length() : UNKNOWN_LENGTH;
85-
if (shouldCompress(exchange, headers, status, contentType, declared)) {
82+
boolean gzip = shouldCompress(exchange, headers, status, contentType, declared);
83+
if (gzip) {
8684
headers.set(CONTENT_ENCODING, GZIP);
87-
exchange.sendResponseHeaders(status, CHUNKED);
88-
try (OutputStream out = ResponseCompression.gzipStream(exchange.getResponseBody())) {
89-
writer.writeTo(out);
90-
}
91-
return;
9285
}
93-
exchange.sendResponseHeaders(status, Math.max(declared, CHUNKED));
94-
try (OutputStream out = exchange.getResponseBody()) {
86+
exchange.sendResponseHeaders(status, gzip ? CHUNKED : Math.max(declared, CHUNKED));
87+
try (OutputStream out =
88+
gzip ? new GZIPOutputStream(exchange.getResponseBody()) : exchange.getResponseBody()) {
9589
writer.writeTo(out);
9690
}
9791
}
9892

93+
/** Adds the response's own content type unless the handler already set one. */
94+
private static void defaultContentType(Headers headers, String contentType) {
95+
if (contentType != null && !headers.containsKey(CONTENT_TYPE)) {
96+
headers.add(CONTENT_TYPE, contentType);
97+
}
98+
}
99+
99100
/**
100101
* Whether a body of {@code length} bytes should be gzipped, marking the response as varying by
101102
* {@code Accept-Encoding} whenever it could have been. A negative length means unknown, which
@@ -120,7 +121,7 @@ private static long declaredLength(Headers headers) {
120121
}
121122
try {
122123
return Long.parseLong(declared.trim());
123-
} catch (NumberFormatException e) {
124+
} catch (NumberFormatException _) {
124125
return UNKNOWN_LENGTH;
125126
}
126127
}
@@ -137,9 +138,7 @@ private void renderBytes(
137138
effectiveContentType = contentType != null ? contentType : DEFAULT_JSON;
138139
bytes = serialize(body, effectiveContentType);
139140
}
140-
if (!headers.containsKey(CONTENT_TYPE)) {
141-
headers.add(CONTENT_TYPE, effectiveContentType);
142-
}
141+
defaultContentType(headers, effectiveContentType);
143142
byte[] payload = maybeCompress(exchange, headers, status, effectiveContentType, bytes);
144143
exchange.sendResponseHeaders(status, payload.length == 0 ? -1 : payload.length);
145144
if (payload.length > 0) {

src/test/java/com/retailsvc/http/internal/ResponseCompressionTest.java

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44
import static org.assertj.core.api.Assertions.assertThat;
55

66
import java.io.ByteArrayInputStream;
7-
import java.io.ByteArrayOutputStream;
87
import java.io.IOException;
9-
import java.io.OutputStream;
108
import java.util.zip.GZIPInputStream;
119
import org.junit.jupiter.api.Test;
1210

@@ -74,18 +72,6 @@ void gzipRoundTripsBytes() throws IOException {
7472
assertThat(gunzip(compressed)).isEqualTo(plain);
7573
}
7674

77-
@Test
78-
void gzipStreamRoundTripsBytes() throws IOException {
79-
byte[] plain = "stream me".repeat(20).getBytes(UTF_8);
80-
ByteArrayOutputStream sink = new ByteArrayOutputStream();
81-
82-
try (OutputStream out = ResponseCompression.gzipStream(sink)) {
83-
out.write(plain);
84-
}
85-
86-
assertThat(gunzip(sink.toByteArray())).isEqualTo(plain);
87-
}
88-
8975
private static byte[] gunzip(byte[] data) throws IOException {
9076
try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(data))) {
9177
return in.readAllBytes();

0 commit comments

Comments
 (0)