Skip to content

Commit c750aae

Browse files
authored
[Fix] Correctly include query parameters for APIs whose request objects contain the body as a field (#401)
## What changes are proposed in this pull request? The Databricks API spec has two different styles of APIs, those where the request body is a field in the `Request` structure, and those where the request body is said structure (not counting the parameters passed via the URI path and query string). The former style of APIs was incorrectly implemented, preventing such APIs from using any query parameters. Generally, the approach taken here is to move the construction of the request object into the autogenerated `Impl` classes and providing a fully materialized `Request` object to the `ApiClient`. To do so, we expose the `<T> T execute(Request in, Target Class<T>)` method as public. Separately, I've removed the per-HTTP-method methods from `ApiClient`, opting instead to use the singular `execute`, as they purely create more maintenance burden for us and decrease the surface area we need to maintain. ## How is this tested? Added `AppsImplTest` to verify that the no_compute flag is included in the request passed to the `ApiClient`.
1 parent 6b76d2a commit c750aae

File tree

139 files changed

+6730
-2956
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

139 files changed

+6730
-2956
lines changed

databricks-sdk-java/src/main/java/com/databricks/sdk/core/ApiClient.java

Lines changed: 8 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ private ApiClient(Builder builder) {
151151
bodyLogger = new BodyLogger(mapper, 1024, debugTruncateBytes);
152152
}
153153

154-
private static <I> void setQuery(Request in, I entity) {
154+
public static <I> void setQuery(Request in, I entity) {
155155
if (entity == null) {
156156
return;
157157
}
@@ -161,140 +161,33 @@ private static <I> void setQuery(Request in, I entity) {
161161
}
162162
}
163163

164-
private static <I> void setHeaders(Request in, Map<String, String> headers) {
165-
if (headers == null) {
166-
return;
167-
}
168-
for (Map.Entry<String, String> e : headers.entrySet()) {
169-
in.withHeader(e.getKey(), e.getValue());
170-
}
171-
}
172-
173-
public <I, O> Collection<O> getCollection(
174-
String path, I in, Class<O> element, Map<String, String> headers) {
164+
public <O> Collection<O> getCollection(Request req, Class<O> element) {
175165
return withJavaType(
176-
path,
177-
in,
178-
mapper.getTypeFactory().constructCollectionType(Collection.class, element),
179-
headers);
166+
req, mapper.getTypeFactory().constructCollectionType(Collection.class, element));
180167
}
181168

182-
public <I> Map<String, String> getStringMap(String path, I in, Map<String, String> headers) {
169+
public Map<String, String> getStringMap(Request req) {
183170
return withJavaType(
184-
path,
185-
in,
186-
mapper.getTypeFactory().constructMapType(Map.class, String.class, String.class),
187-
headers);
171+
req, mapper.getTypeFactory().constructMapType(Map.class, String.class, String.class));
188172
}
189173

190-
protected <I, O> O withJavaType(
191-
String path, I in, JavaType javaType, Map<String, String> headers) {
174+
protected <I, O> O withJavaType(Request request, JavaType javaType) {
192175
try {
193-
Request request = prepareRequest("GET", path, in, headers);
194176
Response response = getResponse(request);
195177
return deserialize(response.getBody(), javaType);
196178
} catch (IOException e) {
197179
throw new DatabricksException("IO error: " + e.getMessage(), e);
198180
}
199181
}
200182

201-
public <O> O HEAD(String path, Class<O> target, Map<String, String> headers) {
202-
return HEAD(path, null, target, headers);
203-
}
204-
205-
public <I, O> O HEAD(String path, I in, Class<O> target, Map<String, String> headers) {
206-
try {
207-
return execute(prepareRequest("HEAD", path, in, headers), target);
208-
} catch (IOException e) {
209-
throw new DatabricksException("IO error: " + e.getMessage(), e);
210-
}
211-
}
212-
213-
public <O> O GET(String path, Class<O> target, Map<String, String> headers) {
214-
return GET(path, null, target, headers);
215-
}
216-
217-
public <I, O> O GET(String path, I in, Class<O> target, Map<String, String> headers) {
218-
try {
219-
return execute(prepareRequest("GET", path, in, headers), target);
220-
} catch (IOException e) {
221-
throw new DatabricksException("IO error: " + e.getMessage(), e);
222-
}
223-
}
224-
225-
public <O> O POST(String path, Class<O> target, Map<String, String> headers) {
226-
try {
227-
return execute(prepareRequest("POST", path, null, headers), target);
228-
} catch (IOException e) {
229-
throw new DatabricksException("IO error: " + e.getMessage(), e);
230-
}
231-
}
232-
233-
public <I, O> O POST(String path, I in, Class<O> target, Map<String, String> headers) {
234-
try {
235-
return execute(prepareRequest("POST", path, in, headers), target);
236-
} catch (IOException e) {
237-
throw new DatabricksException("IO error: " + e.getMessage(), e);
238-
}
239-
}
240-
241-
public <I, O> O PUT(String path, I in, Class<O> target, Map<String, String> headers) {
242-
try {
243-
return execute(prepareRequest("PUT", path, in, headers), target);
244-
} catch (IOException e) {
245-
throw new DatabricksException("IO error: " + e.getMessage(), e);
246-
}
247-
}
248-
249-
public <I, O> O PATCH(String path, I in, Class<O> target, Map<String, String> headers) {
250-
try {
251-
return execute(prepareRequest("PATCH", path, in, headers), target);
252-
} catch (IOException e) {
253-
throw new DatabricksException("IO error: " + e.getMessage(), e);
254-
}
255-
}
256-
257-
public <I, O> O DELETE(String path, I in, Class<O> target, Map<String, String> headers) {
258-
try {
259-
return execute(prepareRequest("DELETE", path, in, headers), target);
260-
} catch (IOException e) {
261-
throw new DatabricksException("IO error: " + e.getMessage(), e);
262-
}
263-
}
264-
265-
private boolean hasBody(String method) {
266-
return !method.equals("GET") && !method.equals("DELETE") && !method.equals("HEAD");
267-
}
268-
269-
private <I> Request prepareBaseRequest(String method, String path, I in)
270-
throws JsonProcessingException {
271-
if (in == null || !hasBody(method)) {
272-
return new Request(method, path);
273-
} else if (InputStream.class.isAssignableFrom(in.getClass())) {
274-
InputStream body = (InputStream) in;
275-
return new Request(method, path, body);
276-
} else {
277-
String body = (in instanceof String) ? (String) in : serialize(in);
278-
return new Request(method, path, body);
279-
}
280-
}
281-
282-
private <I> Request prepareRequest(String method, String path, I in, Map<String, String> headers)
283-
throws JsonProcessingException {
284-
Request req = prepareBaseRequest(method, path, in);
285-
setQuery(req, in);
286-
setHeaders(req, headers);
287-
return req;
288-
}
289-
290183
/**
291184
* Executes HTTP request with retries and converts it to proper POJO
292185
*
293186
* @param in Commons HTTP request
294187
* @param target Expected pojo type
295188
* @return POJO of requested type
296189
*/
297-
private <T> T execute(Request in, Class<T> target) throws IOException {
190+
public <T> T execute(Request in, Class<T> target) throws IOException {
298191
Response out = getResponse(in);
299192
if (target == Void.class) {
300193
return null;
@@ -533,7 +426,7 @@ public <T> void deserialize(Response response, T object) throws IOException {
533426
}
534427
}
535428

536-
private String serialize(Object body) throws JsonProcessingException {
429+
public String serialize(Object body) throws JsonProcessingException {
537430
if (body == null) {
538431
return null;
539432
}

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -586,10 +586,13 @@ private OpenIDConnectEndpoints fetchDefaultOidcEndpoints() throws IOException {
586586
.withHttpClient(getHttpClient())
587587
.withGetHostFunc(v -> getHost())
588588
.build();
589-
return apiClient.GET(
590-
"/oidc/.well-known/oauth-authorization-server",
591-
OpenIDConnectEndpoints.class,
592-
new HashMap<>());
589+
try {
590+
return apiClient.execute(
591+
new Request("GET", "/oidc/.well-known/oauth-authorization-server"),
592+
OpenIDConnectEndpoints.class);
593+
} catch (IOException e) {
594+
throw new DatabricksException("IO error: " + e.getMessage(), e);
595+
}
593596
}
594597

595598
@Override

databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/RefreshableTokenSource.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.databricks.sdk.core.DatabricksException;
55
import com.databricks.sdk.core.http.FormRequest;
66
import com.databricks.sdk.core.http.HttpClient;
7+
import com.databricks.sdk.core.http.Request;
78
import java.time.LocalDateTime;
89
import java.time.temporal.ChronoUnit;
910
import java.util.Base64;
@@ -61,12 +62,11 @@ protected static Token retrieveToken(
6162
break;
6263
}
6364
headers.put("Content-Type", "application/x-www-form-urlencoded");
65+
Request req = new Request("POST", tokenUrl, FormRequest.wrapValuesInList(params));
66+
req.withHeaders(headers);
6467
try {
6568
ApiClient apiClient = new ApiClient.Builder().withHttpClient(hc).build();
66-
67-
OAuthResponse resp =
68-
apiClient.POST(
69-
tokenUrl, FormRequest.wrapValuesInList(params), OAuthResponse.class, headers);
69+
OAuthResponse resp = apiClient.execute(req, OAuthResponse.class);
7070
if (resp.getErrorCode() != null) {
7171
throw new IllegalArgumentException(resp.getErrorCode() + ": " + resp.getErrorSummary());
7272
}

0 commit comments

Comments
 (0)