Skip to content

Commit 134f1fe

Browse files
authored
Handle nested query parameters in ApiClient.withQuery (#125)
## Changes The Query History list API filter_by query parameter is modeled by a dictionary, rather than a primitive type, defining the allowed filters including query_start_time_range, statuses, user_ids, and warehouse_ids. To be compatible with gRPC transcoding, query parameters modeled by message types as opposed to primitives need to be separated into one query parameter per nested field, where the key is the path to that query parameter. For example: ``` new ListQueryHistoryRequest() .setFilterBy(new QueryFilter().setUserIds(Arrays.asList(123L, 456L))) ``` becomes ``` filter_by.user_ids=123&filter_by.user_ids=456 ``` For this to be compatible with the requests library we use today, we need to recursively compute the path of each field in the request object that is annotated with the `QueryParam` annotation, then serialize the value according to Jackson. As part of this, I've also generalized the `Request` class to support repeated query parameter values for lists (see the added integration test). This resolves one of the problems from databricks/databricks-sdk-py#99. The issue with the conflict between filter_by and next_page_token will be resolved by the backend service. ## Tests Added an integration test for SQL Query History API.
1 parent b71d391 commit 134f1fe

File tree

8 files changed

+216
-31
lines changed

8 files changed

+216
-31
lines changed

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

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,13 @@
66
import com.databricks.sdk.core.http.Response;
77
import com.databricks.sdk.core.utils.RealTimer;
88
import com.databricks.sdk.core.utils.Timer;
9-
import com.databricks.sdk.support.QueryParam;
109
import com.fasterxml.jackson.annotation.JsonInclude;
1110
import com.fasterxml.jackson.core.JsonProcessingException;
1211
import com.fasterxml.jackson.databind.DeserializationFeature;
1312
import com.fasterxml.jackson.databind.JavaType;
1413
import com.fasterxml.jackson.databind.ObjectMapper;
1514
import com.fasterxml.jackson.databind.SerializationFeature;
1615
import java.io.IOException;
17-
import java.lang.reflect.Field;
1816
import java.util.Collection;
1917
import java.util.Map;
2018
import java.util.Random;
@@ -91,25 +89,11 @@ private <I> Request withQuery(Request in, I entity) {
9189
if (entity == null) {
9290
return in;
9391
}
94-
try {
95-
// deterministic query string: in the order of class fields
96-
for (Field field : entity.getClass().getDeclaredFields()) {
97-
QueryParam param = field.getAnnotation(QueryParam.class);
98-
if (param == null) {
99-
continue;
100-
}
101-
field.setAccessible(true);
102-
Object value = field.get(entity);
103-
field.setAccessible(false);
104-
if (value == null) {
105-
continue;
106-
}
107-
in.withQueryParam(param.value(), value.toString());
108-
}
109-
return in;
110-
} catch (IllegalAccessException e) {
111-
throw new DatabricksException("Cannot create query string: " + e.getMessage(), e);
92+
for (GrpcTranscodingQueryParamsSerializer.HeaderEntry e :
93+
GrpcTranscodingQueryParamsSerializer.serialize(entity)) {
94+
in.withQueryParam(e.getKey(), e.getValue());
11295
}
96+
return in;
11397
}
11498

11599
public <I, O> Collection<O> getCollection(String path, I in, Class<O> element) {
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
package com.databricks.sdk.core;
2+
3+
import com.databricks.sdk.support.QueryParam;
4+
import com.fasterxml.jackson.annotation.JsonProperty;
5+
import java.lang.reflect.Field;
6+
import java.util.*;
7+
8+
/**
9+
* Serializes an object into a list of query parameter entries compatible with gRPC-transcoding.
10+
*
11+
* <p>The Databricks REST API uses gRPC transcoding to expose gRPC services as REST APIs. This
12+
* serializer is used to serialize objects into a map of query parameter entries that can be used to
13+
* invoke a gRPC service via REST.
14+
*
15+
* <p>See <a
16+
* href="https://cloud.google.com/endpoints/docs/grpc-service-config/reference/rpc/google.api#google.api.HttpRule">the
17+
* documentation for gRPC transcoding</a> for more details.
18+
*/
19+
public class GrpcTranscodingQueryParamsSerializer {
20+
public static class HeaderEntry {
21+
private final String key;
22+
private final String value;
23+
24+
public HeaderEntry(String key, String value) {
25+
this.key = key;
26+
this.value = value;
27+
}
28+
29+
public String getKey() {
30+
return key;
31+
}
32+
33+
public String getValue() {
34+
return value;
35+
}
36+
}
37+
/**
38+
* Serializes an object into a map of query parameter values compatible with gRPC-transcoding.
39+
*
40+
* <p>This method respects the QueryParam and JsonProperty annotations on the object's fields when
41+
* serializing the field name. If both annotations are present, the value of the QueryParam
42+
* annotation is used.
43+
*
44+
* <p>The returned object does not contain any top-level fields that are not annotated with
45+
* QueryParam. All nested fields are included, even if they are not annotated with QueryParam.
46+
*
47+
* @param o The object to serialize.
48+
* @return A list of query parameter entries compatible with gRPC-transcoding.
49+
*/
50+
public static List<HeaderEntry> serialize(Object o) {
51+
Map<String, Object> flattened = flattenObject(o);
52+
for (Field f : o.getClass().getDeclaredFields()) {
53+
QueryParam queryParam = f.getAnnotation(QueryParam.class);
54+
if (queryParam == null) {
55+
flattened.remove(getFieldName(f));
56+
}
57+
}
58+
59+
List<HeaderEntry> result = new ArrayList<>();
60+
for (Map.Entry<String, Object> entry : flattened.entrySet()) {
61+
String key = entry.getKey();
62+
Object value = entry.getValue();
63+
if (value instanceof Collection) {
64+
for (Object v : (Collection<Object>) value) {
65+
result.add(new HeaderEntry(key, v.toString()));
66+
}
67+
} else {
68+
result.add(new HeaderEntry(key, value.toString()));
69+
}
70+
}
71+
return result;
72+
}
73+
74+
private static final List<Class<?>> primitiveTypes =
75+
Arrays.asList(
76+
boolean.class,
77+
Boolean.class,
78+
byte.class,
79+
Byte.class,
80+
char.class,
81+
Character.class,
82+
short.class,
83+
Short.class,
84+
int.class,
85+
Integer.class,
86+
long.class,
87+
Long.class,
88+
float.class,
89+
Float.class,
90+
double.class,
91+
Double.class,
92+
String.class);
93+
94+
private static String getFieldName(Field f) {
95+
QueryParam queryParam = f.getAnnotation(QueryParam.class);
96+
JsonProperty jsonProperty = f.getAnnotation(JsonProperty.class);
97+
if (queryParam != null) {
98+
return queryParam.value();
99+
} else if (jsonProperty != null) {
100+
return jsonProperty.value();
101+
} else {
102+
return f.getName();
103+
}
104+
}
105+
106+
private static Map<String, Object> flattenObject(Object o) {
107+
// LinkedHashMap ensures consistent ordering of fields.
108+
Map<String, Object> result = new LinkedHashMap<>();
109+
Field[] fields = o.getClass().getDeclaredFields();
110+
for (Field f : fields) {
111+
f.setAccessible(true);
112+
try {
113+
String name = getFieldName(f);
114+
Object value = f.get(o);
115+
if (value == null) {
116+
continue;
117+
}
118+
// check if object is a primitive type or a collection of some kind
119+
if (primitiveTypes.contains(f.getType()) || Iterable.class.isAssignableFrom(f.getType())) {
120+
result.put(name, value);
121+
} else {
122+
// recursively flatten the object
123+
Map<String, Object> flattened = flattenObject(value);
124+
for (Map.Entry<String, Object> entry : flattened.entrySet()) {
125+
result.put(name + "." + entry.getKey(), entry.getValue());
126+
}
127+
}
128+
} catch (IllegalAccessException e) {
129+
throw new RuntimeException(e);
130+
} finally {
131+
f.setAccessible(false);
132+
}
133+
}
134+
return result;
135+
}
136+
}
Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,22 @@
11
package com.databricks.sdk.core.http;
22

3-
import java.util.Map;
3+
import java.util.*;
44

55
public class FormRequest extends Request {
66
public FormRequest(String url, Map<String, String> form) {
77
this(POST, url, form);
88
}
99

1010
public FormRequest(String method, String url, Map<String, String> form) {
11-
super(method, url, mapToQuery(form));
11+
super(method, url, mapToQuery(wrapValuesInList(form)));
1212
withHeader("Content-Type", "application/x-www-form-urlencoded");
1313
}
14+
15+
static Map<String, List<String>> wrapValuesInList(Map<String, String> map) {
16+
Map<String, List<String>> m = new LinkedHashMap<>();
17+
for (Map.Entry<String, String> entry : map.entrySet()) {
18+
m.put(entry.getKey(), Collections.singletonList(entry.getValue()));
19+
}
20+
return m;
21+
}
1422
}

databricks-sdk-java/src/main/java/com/databricks/sdk/core/http/Request.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public class Request {
1616
private final String method;
1717
private String url;
1818
private final Map<String, String> headers = new HashMap<>();
19-
private final Map<String, String> query = new TreeMap<>();
19+
private final Map<String, List<String>> query = new TreeMap<>();
2020
private final String body;
2121

2222
public Request(String method, String url) {
@@ -40,7 +40,8 @@ public Request withHeader(String key, String value) {
4040
}
4141

4242
public Request withQueryParam(String key, String value) {
43-
query.put(key, value);
43+
List<String> values = query.computeIfAbsent(key, k -> new ArrayList<>());
44+
values.add(value);
4445
return this;
4546
}
4647

@@ -49,13 +50,15 @@ public Request withUrl(String url) {
4950
return this;
5051
}
5152

52-
protected static String mapToQuery(Map<String, String> in) {
53+
protected static String mapToQuery(Map<String, List<String>> in) {
5354
StringJoiner joiner = new StringJoiner("&");
54-
for (Map.Entry<String, String> entry : in.entrySet()) {
55+
for (Map.Entry<String, List<String>> entry : in.entrySet()) {
5556
try {
5657
String encodedKey = URLEncoder.encode(entry.getKey(), StandardCharsets.UTF_8.name());
57-
String encodedValue = URLEncoder.encode(entry.getValue(), StandardCharsets.UTF_8.name());
58-
joiner.add(encodedKey + "=" + encodedValue);
58+
for (String value : entry.getValue()) {
59+
String encodedValue = URLEncoder.encode(value, StandardCharsets.UTF_8.name());
60+
joiner.add(encodedKey + "=" + encodedValue);
61+
}
5962
} catch (UnsupportedEncodingException e) {
6063
throw new DatabricksException("Unable to encode query parameter: " + e.getMessage(), e);
6164
}
@@ -116,7 +119,7 @@ public Map<String, String> getHeaders() {
116119
return headers;
117120
}
118121

119-
public Map<String, String> getQuery() {
122+
public Map<String, List<String>> getQuery() {
120123
return query;
121124
}
122125

databricks-sdk-java/src/main/java/com/databricks/sdk/support/Paginator.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ private Iterator<T> outerIterator() {
7171
return new Iterator<T>() {
7272
@Override
7373
public boolean hasNext() {
74+
if (currentPage == null) {
75+
return false;
76+
}
7477
if (currentPage.hasNext()) {
7578
return true;
7679
}

databricks-sdk-java/src/test/java/com/databricks/sdk/core/http/FormRequestTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22

33
import static org.junit.jupiter.api.Assertions.*;
44

5+
import java.util.LinkedHashMap;
56
import java.util.Map;
6-
import java.util.TreeMap;
77
import org.junit.jupiter.api.Test;
88

99
class FormRequestTest {
1010
@Test
1111
public void itWorks() {
12-
Map<String, String> data = new TreeMap<>();
12+
Map<String, String> data = new LinkedHashMap<>();
1313
data.put("foo", "bar");
1414
data.put("new", "foo");
1515
FormRequest request = new FormRequest("/foo", data);

databricks-sdk-java/src/test/java/com/databricks/sdk/core/http/RequestTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,15 @@ void requestUriWithQueryWithForwardSlash() {
3131
URI uri = new Request("GET", "/foo").withQueryParam("foo", "bar/baz").getUri();
3232
assertEquals("/foo?foo=bar%2Fbaz", uri.toString());
3333
}
34+
35+
// Test that multiple values for the same query parameter are appropriately encoded.
36+
@Test
37+
void requestUriWithQueryWithMultipleValues() {
38+
URI uri =
39+
new Request("GET", "/foo")
40+
.withQueryParam("foo", "bar")
41+
.withQueryParam("foo", "baz")
42+
.getUri();
43+
assertEquals("/foo?foo=bar&foo=baz", uri.toString());
44+
}
3445
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package com.databricks.sdk.integration;
2+
3+
import com.databricks.sdk.WorkspaceClient;
4+
import com.databricks.sdk.integration.framework.EnvContext;
5+
import com.databricks.sdk.integration.framework.EnvTest;
6+
import com.databricks.sdk.service.sql.ListQueryHistoryRequest;
7+
import com.databricks.sdk.service.sql.QueryFilter;
8+
import com.databricks.sdk.service.sql.QueryInfo;
9+
import com.databricks.sdk.service.sql.TimeRange;
10+
import java.util.Arrays;
11+
import org.junit.jupiter.api.Test;
12+
import org.junit.jupiter.api.extension.ExtendWith;
13+
14+
@EnvContext("workspace")
15+
@ExtendWith(EnvTest.class)
16+
public class SqlIT {
17+
@Test
18+
void listQueryHistoryTimeRange(WorkspaceClient w) {
19+
TimeRange timeRange =
20+
new TimeRange().setStartTimeMs(1690243200000L).setEndTimeMs(1690329600000L);
21+
ListQueryHistoryRequest request =
22+
new ListQueryHistoryRequest()
23+
.setFilterBy(new QueryFilter().setQueryStartTimeRange(timeRange));
24+
Iterable<QueryInfo> queries = w.queryHistory().list(request);
25+
for (QueryInfo query : queries) {
26+
System.out.println(query);
27+
}
28+
}
29+
30+
@Test
31+
void listQueryHistoryUserIds(WorkspaceClient w) {
32+
ListQueryHistoryRequest request =
33+
new ListQueryHistoryRequest()
34+
.setFilterBy(new QueryFilter().setUserIds(Arrays.asList(123L, 456L)));
35+
Iterable<QueryInfo> queries = w.queryHistory().list(request);
36+
for (QueryInfo query : queries) {
37+
System.out.println(query);
38+
}
39+
}
40+
}

0 commit comments

Comments
 (0)