Skip to content

[CDX-480] Expose headers for Autocomplete and Search Responses #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Mudaafi wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[CDX-480] Expose headers for Autocomplete and Search Responses #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ec49c9
expose headers
Mudaafi Jun 30, 2026
81b2e93
address comments
Mudaafi Jun 30, 2026
b8ff557
declare types
Mudaafi Jun 30, 2026
4e44993
address comments
Mudaafi Jul 2, 2026
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
58 changes: 48 additions & 10 deletions constructorio-client/src/main/java/io/constructor/client/ConstructorIO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -926,8 +926,11 @@ public String retrieveVariationsAsJson(VariationsRequest req) throws Constructor
public AutocompleteResponse autocomplete(AutocompleteRequest req, UserInfo userInfo)
throws ConstructorException {
try {
String json = autocompleteAsJSON(req, userInfo);
return createAutocompleteResponse(json);
Request request = createAutocompleteRequest(req, userInfo);
Response response = clientWithRetry.newCall(request).execute();

@esezen esezen Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

@Mudaafi Mudaafi Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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)

@esezen esezen Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Map<String, List<String>> headers = response.headers().toMultimap();
Comment thread
Mudaafi marked this conversation as resolved.

@constructor-claude-bedrock constructor-claude-bedrock Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

String json = getResponseBody(response);
return createAutocompleteResponse(json, headers);
} catch (Exception exception) {
throw new ConstructorException(exception);
}
Expand All @@ -947,6 +950,25 @@ public AutocompleteResponse autocomplete(AutocompleteRequest req, UserInfo userI
*/
public String autocompleteAsJSON(AutocompleteRequest req, UserInfo userInfo)
throws ConstructorException {
try {
Request request = createAutocompleteRequest(req, userInfo);
Response response = clientWithRetry.newCall(request).execute();
return getResponseBody(response);
} catch (Exception exception) {
throw new ConstructorException(exception);
}
}

/**
* Creates an autocomplete OkHttp request
*
* @param req the autocomplete request
* @param userInfo optional information about the user
* @return an autocomplete OkHttp request
* @throws ConstructorException
*/
protected Request createAutocompleteRequest(AutocompleteRequest req, UserInfo userInfo)
Comment thread
Mudaafi marked this conversation as resolved.

@constructor-claude-bedrock constructor-claude-bedrock Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

throws ConstructorException {
try {
List<String> paths = Arrays.asList("autocomplete", req.getQuery());
HttpUrl url = (userInfo == null) ? this.makeUrl(paths) : this.makeUrl(paths, userInfo);
Expand Down Expand Up @@ -1009,10 +1031,7 @@ public String autocompleteAsJSON(AutocompleteRequest req, UserInfo userInfo)
.build();
}

Request request = this.makeUserRequestBuilder(userInfo).url(url).get().build();

Response response = clientWithRetry.newCall(request).execute();
return getResponseBody(response);
return this.makeUserRequestBuilder(userInfo).url(url).get().build();
} catch (Exception exception) {
throw new ConstructorException(exception);
}
Expand Down Expand Up @@ -1171,8 +1190,9 @@ public SearchResponse search(SearchRequest req, UserInfo userInfo) throws Constr
try {
Request request = createSearchRequest(req, userInfo);
Response response = clientWithRetry.newCall(request).execute();
Map<String, List<String>> headers = response.headers().toMultimap();
String json = getResponseBody(response);
return createSearchResponse(json);
return createSearchResponse(json, headers);
} catch (Exception exception) {
throw new ConstructorException(exception);
}
Expand Down Expand Up @@ -1207,8 +1227,10 @@ public void onFailure(Call call, IOException e) {
public void onResponse(Call call, final Response response)
throws IOException {
try {
Map<String, List<String>> headers =
response.headers().toMultimap();
String json = getResponseBody(response);
SearchResponse res = createSearchResponse(json);
SearchResponse res = createSearchResponse(json, headers);
c.onResponse(res);
} catch (Exception e) {
c.onFailure(new ConstructorException(e));
Expand Down Expand Up @@ -2200,6 +2222,11 @@ protected String getVersion() {
* to do it in a Gson Type Adapter.
*/
protected static AutocompleteResponse createAutocompleteResponse(String string) {
return createAutocompleteResponse(string, null);
}

protected static AutocompleteResponse createAutocompleteResponse(
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
String string, Map<String, List<String>> headers) {
JSONObject json = new JSONObject(string);
JSONObject sections = json.getJSONObject("sections");
for (Object sectionKey : sections.keySet()) {
Expand All @@ -2208,7 +2235,11 @@ protected static AutocompleteResponse createAutocompleteResponse(String string)
moveMetadataOutOfResultData(results);
}
String transformed = json.toString();
return new Gson().fromJson(transformed, AutocompleteResponse.class);
AutocompleteResponse autocompleteResponse =
new Gson().fromJson(transformed, AutocompleteResponse.class);
autocompleteResponse.setHeaders(headers);

return autocompleteResponse;
}

/**
Expand All @@ -2217,6 +2248,11 @@ protected static AutocompleteResponse createAutocompleteResponse(String string)
* in a Gson Type Adapter.
*/
protected static SearchResponse createSearchResponse(String string) {
return createSearchResponse(string, null);
}

protected static SearchResponse createSearchResponse(
String string, Map<String, List<String>> headers) {
JSONObject json = new JSONObject(string);
JSONObject response = json.getJSONObject("response");
JSONArray results;
Expand All @@ -2227,7 +2263,9 @@ protected static SearchResponse createSearchResponse(String string) {
}

String transformed = json.toString();
return new Gson().fromJson(transformed, SearchResponse.class);
SearchResponse searchResponse = new Gson().fromJson(transformed, SearchResponse.class);
searchResponse.setHeaders(headers);
return searchResponse;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion constructorio-client/src/main/java/io/constructor/client/models/AutocompleteResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import java.util.Map;

/** Constructor.io Autocomplete Response ... uses Gson/Reflection to load data in */
public class AutocompleteResponse {
public class AutocompleteResponse extends ResponseHeaders {

@SerializedName("sections")
private Map<String, List<Result>> sections;
Expand Down
26 changes: 26 additions & 0 deletions constructorio-client/src/main/java/io/constructor/client/models/ResponseHeaders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package io.constructor.client.models;

import java.util.Collections;
import java.util.List;
import java.util.Map;

/** Base class that provides access to HTTP response headers. */
public abstract class ResponseHeaders {

@constructor-claude-bedrock constructor-claude-bedrock Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.


private transient Map<String, List<String>> headers =
Collections.<String, List<String>>emptyMap();

/**
* @return the HTTP response headers
*/
public Map<String, List<String>> getHeaders() {
return headers;
}

/**
* @param headers the HTTP response headers to set. If null, defaults to an empty map.
*/
public void setHeaders(Map<String, List<String>> headers) {
this.headers = (headers != null) ? headers : Collections.<String, List<String>>emptyMap();
}
}
2 changes: 1 addition & 1 deletion constructorio-client/src/main/java/io/constructor/client/models/SearchResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import java.util.Map;

/** Constructor.io Search Response ... uses Gson/Reflection to load data in */
public class SearchResponse {
public class SearchResponse extends ResponseHeaders {

@SerializedName("result_id")
private String resultId;
Expand Down
41 changes: 35 additions & 6 deletions ...-client/src/test/java/io/constructor/client/ConstructorIOAutocompleteUrlEncodingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package io.constructor.client;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;

import io.constructor.client.models.AutocompleteResponse;
import java.util.List;
import java.util.Map;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
Expand Down Expand Up @@ -41,8 +45,7 @@ public void AutocompleteWithPlusShouldBeEncodedInUrl() throws Exception {
constructor.autocomplete(request, null);

RecordedRequest recordedRequest = mockServer.takeRequest();
String expectedPath =
String.format("/autocomplete/r%%2Bco?key=%s&c=ciojava-7.5.0", apiKey);
String expectedPath = String.format("/autocomplete/r%%2Bco?key=%s&c=ciojava-7.5.0", apiKey);
String actualPath = recordedRequest.getPath();
assertEquals("recorded request is encoded correctly", actualPath, expectedPath);
}
Expand All @@ -59,8 +62,7 @@ public void AutocompleteWithSpaceShouldBeEncodedInUrl() throws Exception {
constructor.autocomplete(request, null);

RecordedRequest recordedRequest = mockServer.takeRequest();
String expectedPath =
String.format("/autocomplete/r%%20co?key=%s&c=ciojava-7.5.0", apiKey);
String expectedPath = String.format("/autocomplete/r%%20co?key=%s&c=ciojava-7.5.0", apiKey);
String actualPath = recordedRequest.getPath();
assertEquals("recorded request is encoded correctly", actualPath, expectedPath);
}
Expand All @@ -77,8 +79,7 @@ public void AutocompleteWithSlashShouldBeEncodedInUrl() throws Exception {
constructor.autocomplete(request, null);

RecordedRequest recordedRequest = mockServer.takeRequest();
String expectedPath =
String.format("/autocomplete/r%%2Fco?key=%s&c=ciojava-7.5.0", apiKey);
String expectedPath = String.format("/autocomplete/r%%2Fco?key=%s&c=ciojava-7.5.0", apiKey);
String actualPath = recordedRequest.getPath();
assertEquals("recorded request is encoded correctly", actualPath, expectedPath);
}
Expand All @@ -99,4 +100,32 @@ public void AutocompleteWithSingleQuoteShouldBeAllowedInUrl() throws Exception {
String actualPath = recordedRequest.getPath();
assertEquals("recorded request is encoded correctly", actualPath, expectedPath);
}

@Test
public void AutocompleteShouldReturnRateLimitHeaders() throws Exception {
String string = Utils.getTestResource("response.autocomplete.peanut.json");
MockResponse mockResponse =
new MockResponse()
.setResponseCode(200)
.setBody(string)
.addHeader("X-RateLimit-Limit", "100")
.addHeader("X-RateLimit-Remaining", "99")
.addHeader("X-RateLimit-Reset", "1620000000");
mockServer.enqueue(mockResponse);

ConstructorIO constructor =
new ConstructorIO("", apiKey, false, "127.0.0.1", mockServer.getPort());
AutocompleteRequest request = new AutocompleteRequest("peanut");
AutocompleteResponse response = constructor.autocomplete(request, null);

mockServer.takeRequest();
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
Comment thread
Mudaafi marked this conversation as resolved.

@constructor-claude-bedrock constructor-claude-bedrock Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.


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));
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.

@constructor-claude-bedrock constructor-claude-bedrock Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

assertEquals(
"rate limit remaining header", "99", headers.get("x-ratelimit-remaining").get(0));
assertEquals(
"rate limit reset header", "1620000000", headers.get("x-ratelimit-reset").get(0));
Comment thread
Mudaafi marked this conversation as resolved.
}
}
32 changes: 32 additions & 0 deletions ...ctorio-client/src/test/java/io/constructor/client/ConstructorIOSearchUrlEncodingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package io.constructor.client;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;

import io.constructor.client.models.SearchResponse;
import java.util.List;
import java.util.Map;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
Expand Down Expand Up @@ -108,4 +112,32 @@ public void SearchWithSingleQuoteShouldBeAllowedInUrl() throws Exception {
String actualPath = recordedRequest.getPath();
assertEquals("recorded request is encoded correctly", actualPath, expectedPath);
}

@Test
public void SearchShouldReturnRateLimitHeaders() throws Exception {
Comment thread
Mudaafi marked this conversation as resolved.
String string = Utils.getTestResource("response.search.peanut.json");
MockResponse mockResponse =
new MockResponse()
.setResponseCode(200)
.setBody(string)
.addHeader("X-RateLimit-Limit", "100")
.addHeader("X-RateLimit-Remaining", "99")
.addHeader("X-RateLimit-Reset", "1620000000");
mockServer.enqueue(mockResponse);

ConstructorIO constructor =
new ConstructorIO("", apiKey, false, "127.0.0.1", mockServer.getPort());
SearchRequest request = new SearchRequest("peanut");
SearchResponse response = constructor.search(request, null);

mockServer.takeRequest();
Comment thread
Mudaafi marked this conversation as resolved.

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));
assertEquals(
"rate limit remaining header", "99", headers.get("x-ratelimit-remaining").get(0));
assertEquals(
"rate limit reset header", "1620000000", headers.get("x-ratelimit-reset").get(0));
Comment thread
Mudaafi marked this conversation as resolved.
}
}
Loading