[CDX-480] Expose headers for Autocomplete and Search Responses by Mudaafi · Pull Request #187 · Constructor-io/constructorio-java

Mudaafi · 2026-06-30T22:03:51Z

PR Description

This change refactors the internal calling structure of autocomplete and search to allow headers to be exposed. Headers are now exposed via the new method getHeaders() available on the types: AutocompleteResponse & SearchResponse

getHeaders returns a Map<String, List<String>> object containing the headers.

E.g.

        SearchResponse response = constructor.search(request, userInfo);
        Map<String, List<String>> headers = response.getHeaders();

        // Get the ratelimit headers
        String rateLimit = headers.get("x-ratelimit-limit").get(0);
        String rateLimitLeft = headers.get("x-ratelimit-remaining").get(0);
        String rateLimitResetTimestamp = headers.get("x-ratelimit-reset").get(0);

Considerations

This change is backwards compatible as existing consumer patterns are unchanged
Headers are not strongly typed
Search & Autocomplete to be released first. Browse and other responses to follow shortly after

Copilot

Pull request overview

This PR refactors the autocomplete and search call paths so the resulting AutocompleteResponse and SearchResponse can expose HTTP response headers (e.g., rate-limit information) via a new getHeaders() API.

Changes:

Capture OkHttp response headers for autocomplete and search, and attach them to the parsed response objects.
Add headers storage + getHeaders()/setHeaders() to AutocompleteResponse and SearchResponse.
Add/extend tests to validate rate-limit headers are surfaced.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
constructorio-client/src/main/java/io/constructor/client/ConstructorIO.java	Refactors autocomplete/search execution to capture headers and plumb them into response creation.
constructorio-client/src/main/java/io/constructor/client/models/AutocompleteResponse.java	Adds `headers` field + accessor methods.
constructorio-client/src/main/java/io/constructor/client/models/SearchResponse.java	Adds `headers` field + accessor methods.
constructorio-client/src/test/java/io/constructor/client/ConstructorIOAutocompleteUrlEncodingTest.java	Adds header exposure assertions for autocomplete responses.
constructorio-client/src/test/java/io/constructor/client/ConstructorIOSearchUrlEncodingTest.java	Adds header exposure assertions for search responses.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

esezen

Looking good. I left some small comments. We can do them in a follow up PR if this needs to go out this week

esezen · 2026-07-02T14:32:46Z

-            String json = autocompleteAsJSON(req, userInfo);
-            return createAutocompleteResponse(json);
+            Request request = createAutocompleteRequest(req, userInfo);
+            Response response = clientWithRetry.newCall(request).execute();


Not sure where to put this comment but I think we should also expose these headers on 429. I might be missing something but I think right now we throw a ConstructorException for those without exposing the rate limit headers. Wdyt?

oooh, hmm you make a good point. I'm not sure if the Exception we throw exposes anything ngl, but I agree it should be present there so that if you do get a 429, you can adjust accordingly (impt for configuration endpoints)

Yes, I'm not too worried about the rest of the errors but the rate limiting headers are important specifically for 429

esezen · 2026-07-02T14:34:37Z

     * @return a string of JSON
     * @throws ConstructorException if the request is invalid.
     */
    public String naturalLanguageSearchAsJSON(NaturalLanguageSearchRequest req, UserInfo userInfo)


Doesn't have to be in this PR but this is also "search" and I think we should add the headers to this response as well

fair, but, we'll bundle it in a later PR anyways

esezen · 2026-07-02T14:36:13Z

Do we need to pass headers here or no?

good catch, missed the overloads

esezen · 2026-07-02T14:37:38Z

    @SerializedName("request")
    private Map<String, Object> request;

+    private transient Map<String, List<String>> headers =


Doesn't need to be in this PR but given how we are going to repeat this in all responses, we might want to extract header getHeaders setHeaders

something like an inherited class?

constructor-claude-bedrock

Code Review

This PR cleanly exposes HTTP response headers on AutocompleteResponse and SearchResponse via a new ResponseHeaders base class, with a well-structured refactor of the internal request-building logic.

Inline comments: 5 discussions added

Overall Assessment: ⚠️ Needs Work

constructor-claude-bedrock · 2026-07-02T17:26:42Z

-            return createAutocompleteResponse(json);
+            Request request = createAutocompleteRequest(req, userInfo);
+            Response response = clientWithRetry.newCall(request).execute();
+            Map<String, List<String>> headers = response.headers().toMultimap();


Important Issue: Headers are read from the Response object before getResponseBody(response) is called. getResponseBody calls response.close() in its finally block. While OkHttp headers are buffered and safe to read before closing, this ordering is subtly fragile — if getResponseBody were ever refactored to consume the response object earlier, headers could be lost. A defensive improvement would be to call getResponseBody first and hold onto both the json string and headers, or document the ordering requirement with a comment. This same pattern appears in both the synchronous autocomplete(), synchronous search(), and async search() callback.

Alternatively, just swap the lines so getResponseBody (which throws on error) runs first, and the headers assignment only happens on success:

String json = getResponseBody(response); // closes response, throws on error Map<String, List<String>> headers = response.headers().toMultimap(); // still safe post-close return createSearchResponse(json, headers);

OkHttp Headers are already materialised before the body stream, so reading them after response.close() is safe — making this the safer order.

constructor-claude-bedrock · 2026-07-02T17:26:42Z

+import java.util.Map;
+
+/** Base class that provides access to HTTP response headers. */
+public abstract class ResponseHeaders {


Suggestion: The transient keyword on the headers field prevents it from being Java-serialised, which is good. However, because these response objects are deserialised from JSON by Gson, the transient keyword also prevents Gson from ever setting this field via reflection. This is currently harmless because headers is set explicitly via setHeaders() after Gson deserialisation, but it is an implicit assumption that could silently break if the deserialization flow changes. A comment explaining why transient is used would improve maintainability:

// transient: excluded from Gson deserialization (headers are set manually after HTTP response) private transient Map<String, List<String>> headers = Collections.emptyMap();

Also, the Collections.<String, List<String>>emptyMap() type witness is unnecessary in Java 7+ — Collections.emptyMap() is fine.

constructor-claude-bedrock · 2026-07-02T17:26:43Z

+     * @return an autocomplete OkHttp request
+     * @throws ConstructorException
+     */
+    protected Request createAutocompleteRequest(AutocompleteRequest req, UserInfo userInfo)


Suggestion: The Javadoc @throws ConstructorException tag is missing a description, whereas the existing createSearchRequest at line 1046 follows the same pattern. Consider being consistent with the rest of the file — e.g., @throws ConstructorException if the request is invalid — even if only to match the style used by all other create*Request methods in this class.

constructor-claude-bedrock · 2026-07-02T17:26:43Z

+
+        Map<String, List<String>> headers = response.getHeaders();
+        assertNotNull("headers should not be null", headers);
+        assertEquals("rate limit header", "100", headers.get("x-ratelimit-limit").get(0));


Important Issue: The test calls headers.get("x-ratelimit-limit").get(0) without a null-guard. If the header key is absent (e.g., due to a case-sensitivity change or an OkHttp version difference in how toMultimap() normalises keys), this will throw a NullPointerException rather than a clear assertion failure. The assertNotNull above only verifies the map itself is non-null. Consider asserting the individual header value is present before accessing index 0:

assertNotNull("x-ratelimit-limit header present", headers.get("x-ratelimit-limit")); assertEquals("rate limit header", "100", headers.get("x-ratelimit-limit").get(0));

The same applies to the x-ratelimit-remaining and x-ratelimit-reset assertions, and identically in ConstructorIOSearchUrlEncodingTest.

constructor-claude-bedrock · 2026-07-02T17:26:44Z

+        AutocompleteRequest request = new AutocompleteRequest("peanut");
+        AutocompleteResponse response = constructor.autocomplete(request, null);
+
+        mockServer.takeRequest();


Suggestion: mockServer.takeRequest() is called after response.getHeaders() is already asserted. The takeRequest() call here is superfluous for verification purposes (the response has already been consumed synchronously). This mirrors the pattern in the search test. While harmless, it may be confusing to future readers — either remove it or move it before the assertions to verify the request was made, which is the typical usage pattern in the other tests in this file.

expose headers

8ec49c9

Mudaafi requested a review from mocca102 June 30, 2026 22:03

Mudaafi requested a review from a team as a code owner June 30, 2026 22:03

Copilot AI review requested due to automatic review settings June 30, 2026 22:03

Copilot started reviewing on behalf of Mudaafi June 30, 2026 22:04 View session

This comment was marked as outdated.

Sign in to view

Copilot AI reviewed Jun 30, 2026

View reviewed changes

address comments

81b2e93

This comment was marked as outdated.

Sign in to view

declare types

b8ff557

This comment was marked as outdated.

Sign in to view

esezen reviewed Jul 2, 2026

View reviewed changes

address comments

4e44993

constructor-claude-bedrock Bot reviewed Jul 2, 2026

View reviewed changes

Uh oh!

Conversation

Mudaafi commented Jun 30, 2026

PR Description

Considerations

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

esezen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

constructor-claude-bedrock Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants