Add TimeZone header support to REST API (#17344)#17387
Add TimeZone header support to REST API (#17344)#17387Lexert19 wants to merge 3 commits intoapache:masterfrom
Conversation
|
Another question, why did you pick |
|
And consider add some example in https://github.com/apache/iotdb/tree/master/example/rest-java-example/src/main/java/org/apache/iotdb? |
dacddb6 to
4eb0e06
Compare
|
Hi @HTHou, thank you for the review! I've updated the PR:
|
4eb0e06 to
a286e68
Compare
|
Hi @Lexert19, could you please share this new feature in the dev mail list? |
There was a problem hiding this comment.
Pull request overview
This PR introduces request-level time zone handling for the REST APIs by parsing a time zone HTTP header in the REST authorization filter, storing it on the session, and then using the session ZoneId when parsing SQL statements so time literals (and time-based semantics like GROUP BY window alignment) respect the client-provided time zone.
Changes:
- Parse a time zone header in
AuthorizationFilterand set the resolvedZoneIdinto theSessionManagersession (reject invalid values with HTTP 400). - Use the current session
ZoneId(instead ofZoneId.systemDefault()) when generating statements in REST v1/v2 and Grafana REST implementations (and table REST statement creation). - Add integration tests and update Java REST examples to demonstrate header usage.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBRestServiceIT.java | Adds IT coverage for valid/invalid time zone headers on REST v1/v2 query and nonQuery flows. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/v2/impl/RestApiServiceImpl.java | Uses session ZoneId when parsing SQL for v2 query/nonQuery endpoints. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/v2/impl/GrafanaApiServiceImpl.java | Uses session ZoneId when parsing Grafana SQL endpoints in v2. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/v1/impl/RestApiServiceImpl.java | Uses session ZoneId when parsing SQL for v1 query/nonQuery endpoints. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/v1/impl/GrafanaApiServiceImpl.java | Uses session ZoneId when parsing Grafana SQL endpoints in v1. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/table/v1/impl/RestApiServiceImpl.java | Uses the client session’s ZoneId when parsing table-model SQL statements. |
| external-service-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/filter/AuthorizationFilter.java | Parses Time-Zone header into ZoneId, aborts request on invalid values, and supplies it to the session. |
| example/rest-java-example/src/main/java/org/apache/iotdb/TableHttpsExample.java | Adds a table REST query example that includes a time zone header. |
| example/rest-java-example/src/main/java/org/apache/iotdb/TableHttpExample.java | Adds a table REST query example that includes a time zone header and uses slf4j logging. |
| example/rest-java-example/src/main/java/org/apache/iotdb/HttpsExample.java | Adds a REST query example that includes a time zone header. |
| example/rest-java-example/src/main/java/org/apache/iotdb/HttpExample.java | Adds a REST “queryWithTimeZone” example (but currently missing the actual header). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| HttpPost httpPost = getHttpPost("http://127.0.0.1:" + port + "/rest/v1/query"); | ||
| httpPost.setHeader("Time-Zone", "+05:00"); | ||
| String sql = | ||
| "{\"sql\":\"SELECT count(s3) FROM root.sg25 GROUP BY ([2026-03-28T00:00:00, 2026-03-29T00:00:00), 1d)\"}"; | ||
| httpPost.setEntity(new StringEntity(sql, Charset.defaultCharset())); | ||
| response = httpClient.execute(httpPost); | ||
| assertEquals(200, response.getStatusLine().getStatusCode()); | ||
| String message = EntityUtils.toString(response.getEntity(), "utf-8"); | ||
| JsonObject result = JsonParser.parseString(message).getAsJsonObject(); | ||
| assertTrue(result.has("timestamps")); | ||
| assertTrue(result.getAsJsonArray("timestamps").size() > 0); | ||
| long expectedTimestamp = | ||
| ZonedDateTime.of(2026, 3, 28, 0, 0, 0, 0, ZoneId.of("+05:00")).toInstant().toEpochMilli(); | ||
| assertEquals(expectedTimestamp, result.getAsJsonArray("timestamps").get(0).getAsLong()); |
There was a problem hiding this comment.
The GROUP BY time range here uses 2026-03-28/2026-03-29, but the test data inserted into root.sg25 earlier in this IT uses fixed timestamps 1635232143960/1635232153960 (~2021-10-26). As a result, this query will likely return an empty result set and fail the timestamps.size() > 0 assertion. Consider changing the query time range (and expectedTimestamp) to align with the inserted data, or insert additional rows for root.sg25 within the queried range before asserting on the first window timestamp.
| HttpPost httpPost = getHttpPost("http://127.0.0.1:" + port + "/rest/v2/query"); | ||
| httpPost.setHeader("Time-Zone", "+05:00"); | ||
| String sql = | ||
| "{\"sql\":\"SELECT count(s3) FROM root.sg25 GROUP BY ([2026-03-28T00:00:00, 2026-03-29T00:00:00), 1d)\"}"; | ||
| httpPost.setEntity(new StringEntity(sql, Charset.defaultCharset())); | ||
| response = httpClient.execute(httpPost); | ||
| assertEquals(200, response.getStatusLine().getStatusCode()); | ||
| String message = EntityUtils.toString(response.getEntity(), "utf-8"); | ||
| JsonObject result = JsonParser.parseString(message).getAsJsonObject(); | ||
| assertTrue(result.has("timestamps")); | ||
| assertTrue(result.getAsJsonArray("timestamps").size() > 0); | ||
| long expectedTimestamp = | ||
| ZonedDateTime.of(2026, 3, 28, 0, 0, 0, 0, ZoneId.of("+05:00")).toInstant().toEpochMilli(); | ||
| assertEquals(expectedTimestamp, result.getAsJsonArray("timestamps").get(0).getAsLong()); |
There was a problem hiding this comment.
Same issue as the v1 test: this GROUP BY time range is in 2026, but root.sg25 test data is inserted with timestamps around 2021-10-26. This will likely return no rows and break the timestamps.size() > 0 assertion. Please align the queried time range (and expectedTimestamp) with the inserted test data, or insert data that falls into the specified 2026 range before querying.
...rvice-impl/rest/src/main/java/org/apache/iotdb/rest/protocol/filter/AuthorizationFilter.java
Show resolved
Hide resolved
| String timeZoneHeader = requestContext.getHeaderString("Time-Zone"); | ||
| if (timeZoneHeader == null || timeZoneHeader.isEmpty()) { | ||
| return ZoneId.systemDefault(); | ||
| } | ||
| try { | ||
| return ZoneId.of(timeZoneHeader); | ||
| } catch (DateTimeException e) { |
There was a problem hiding this comment.
timeZoneHeader is used as-is for parsing. If the header is present but contains only whitespace (or has leading/trailing spaces), ZoneId.of(...) will throw and the request will be rejected. Consider trimming the header value before validation/parsing so common client formatting issues don’t cause a 400.
| CloseableHttpClient httpClient = SSLClient.getInstance().getHttpClient(); | ||
| CloseableHttpResponse response = null; | ||
| try { | ||
| HttpPost httpPost = getHttpPost("http://127.0.0.1:18080/rest/v1/query"); |
There was a problem hiding this comment.
queryWithTimeZone() doesn’t actually send the Time-Zone header (unlike the HTTPS and table examples). As written, this method behaves the same as query() and won’t demonstrate the feature. Add the Time-Zone (or intended header name) to this request so the example matches the method name and the PR’s new behavior.
| HttpPost httpPost = getHttpPost("http://127.0.0.1:18080/rest/v1/query"); | |
| HttpPost httpPost = getHttpPost("http://127.0.0.1:18080/rest/v1/query"); | |
| httpPost.setHeader("Time-Zone", "Asia/Shanghai"); |
Description
This PR adds support for the TimeZone header in REST API. The header is parsed in AuthorizationFilter and the time zone is set in the session. Integration tests are added to verify correct handling of valid and invalid time zones.
Closes #17344
This PR has:
for an unfamiliar reader.
for code coverage.
Key changed/added classes (or packages if there are too many classes) in this PR