Skip to content

Commit e51c2c6

Browse files
Convert All Usage of LocalDateTime to Instant (#456)
## What changes are proposed in this pull request? This pull request refactors the Databricks Java SDK to replace all usages of LocalDateTime with Instant for representing date-time values. The primary motivation is to ensure that all date-time handling is unambiguous, always in UTC, and consistent across the SDK. ## How does time currently operate in the SDK? Currently, tokens in the SDK use LocalDateTime for expiry timestamps, which by itself is timezone-agnostic and could be ambiguous. Thus, it is not clear how time is handled from the type alone and we need to look at the implementation details of the ClockSupplier used. The SDK currently uses system time through the `SystemClockSupplier` class as defined [here](https://github.com/databricks/databricks-sdk-java/blob/main/databricks-sdk-java/src/main/java/com/databricks/sdk/core/utils/SystemClockSupplier.java). This implementation of the ClockSupplier interface returns `Clock.systemDefaultZone()`, meaning it uses the actual system time from the operating system. The `SystemClockSupplier` is the used throughout the SDK to conduct OAuth token expiration checks as seen [here](https://github.com/databricks/databricks-sdk-java/blob/main/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/Token.java) in the `isExpired()` method of the Token class. While `LocalDateTime` doesn't explicitly store timezone information, the system's timezone is being implicitly used for these comparisons through `Clock.systemDefaultZone()`. In other words, token expiry times are implicitly measured in the system's timezone. ## Converting to UTC To convert these expiry times to UTC while maintaining the SDK's current behavior, we need to handle two distinct scenarios: ### 1. Tokens with `expires_in` - The response includes the time until expiry. - **Conversion:** Add the `expires_in` value to the current UTC time. - **Result:** This approach maintains the original expiration behavior while using UTC internally. --- ### 2. Tokens with `expires_on` (e.g., Azure CLI, Databricks CLI) #### With Time Zone Indicator - Tokens (such as those from the Databricks CLI) provide expiry times in RFC 3339 format, including a time zone indicator. - **Conversion:** These can be easily converted to UTC using standard library methods. #### Without Time Zone Indicator - Example: The `expiresOn` field from Azure CLI [Reference](https://github.com/Azure/azure-cli/blob/dev/src/azure-cli-core/azure/cli/core/_profile.py#L384) - Tokens provide expiry timestamps without explicit time zone information. - The timestamp is implicitly in the system’s local time zone. - **Conversion steps:** 1. Interpret the timestamp as local time. 2. Add the system timezone. 3. Convert this to UTC. We do this with **`LocalDateTime.atZone(systemDefault).toInstant()`** ## How is this tested? - Existing unit tests to ensure no regressions. - Added new tests in #462 which verifies that this change does not change overall behavior of CilTokenSource. ## Future Work - The SDK currently parses expiry strings from external sources like Azure CLI - Azure CLI provides two timestamp fields: - `expiresOn`: Local datetime without timezone (maintained for backward compatibility) - `expires_on`: POSIX timestamp (seconds since epoch) - preferred format ### Current Implementation - The Java SDK currently uses the deprecated `expiresOn` field - `LocalDateTime` creates timezone interpretation challenges ### To Do 1. **Phase out `expiresOn`** This legacy field should be deprecated in favor of `expires_on` 2. **Update parsing logic** Future PRs should prioritize using `expires_on` (POSIX timestamp) which: - Provides unambiguous UTC-based expiration time - Avoids timezone conversion complexities NO_CHANGELOG=true --------- Co-authored-by: Parth Bansal <parth.bansal@databricks.com>
1 parent 7567b41 commit e51c2c6

17 files changed

+142
-98
lines changed

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

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88
import com.fasterxml.jackson.databind.ObjectMapper;
99
import java.io.IOException;
1010
import java.io.InputStream;
11+
import java.time.Instant;
1112
import java.time.LocalDateTime;
13+
import java.time.OffsetDateTime;
14+
import java.time.ZoneId;
1215
import java.time.format.DateTimeFormatter;
1316
import java.time.format.DateTimeParseException;
1417
import java.util.Arrays;
@@ -36,26 +39,45 @@ public CliTokenSource(
3639
this.env = env;
3740
}
3841

39-
static LocalDateTime parseExpiry(String expiry) {
42+
/**
43+
* Parses an expiry time string and returns the corresponding {@link Instant}. The method attempts
44+
* to parse the input in the following order: 1. RFC 3339/ISO 8601 format with offset (e.g.
45+
* "2024-03-20T10:30:00Z") - If the timestamp is compliant with ISO 8601 and RFC 3339, the
46+
* timezone indicator (Z or +/-HH:mm) is used as part of the conversion process to UTC 2. Local
47+
* date-time format "yyyy-MM-dd HH:mm:ss" (e.g. "2024-03-20 10:30:00") 3. Local date-time format
48+
* with optional fractional seconds of varying precision (e.g. "2024-03-20 10:30:00.123")
49+
*
50+
* <p>For timestamps without a timezone indicator (formats 2 and 3), the system's default time
51+
* zone is assumed and used for the conversion to UTC.
52+
*
53+
* @param expiry expiry time string in one of the supported formats
54+
* @return the parsed {@link Instant}
55+
* @throws DateTimeParseException if the input string cannot be parsed in any of the supported
56+
* formats
57+
*/
58+
static Instant parseExpiry(String expiry) {
59+
DateTimeParseException parseException;
60+
try {
61+
return OffsetDateTime.parse(expiry).toInstant();
62+
} catch (DateTimeParseException e) {
63+
parseException = e;
64+
}
65+
4066
String multiplePrecisionPattern =
4167
"[SSSSSSSSS][SSSSSSSS][SSSSSSS][SSSSSS][SSSSS][SSSS][SSS][SS][S]";
4268
List<String> datePatterns =
43-
Arrays.asList(
44-
"yyyy-MM-dd HH:mm:ss",
45-
"yyyy-MM-dd HH:mm:ss." + multiplePrecisionPattern,
46-
"yyyy-MM-dd'T'HH:mm:ss." + multiplePrecisionPattern + "XXX",
47-
"yyyy-MM-dd'T'HH:mm:ss." + multiplePrecisionPattern + "'Z'");
48-
DateTimeParseException lastException = null;
69+
Arrays.asList("yyyy-MM-dd HH:mm:ss", "yyyy-MM-dd HH:mm:ss." + multiplePrecisionPattern);
4970
for (String pattern : datePatterns) {
5071
try {
5172
DateTimeFormatter formatter = DateTimeFormatter.ofPattern(pattern);
5273
LocalDateTime dateTime = LocalDateTime.parse(expiry, formatter);
53-
return dateTime;
74+
return dateTime.atZone(ZoneId.systemDefault()).toInstant();
5475
} catch (DateTimeParseException e) {
55-
lastException = e;
76+
parseException.addSuppressed(e);
5677
}
5778
}
58-
throw lastException;
79+
80+
throw parseException;
5981
}
6082

6183
private String getProcessStream(InputStream stream) throws IOException {
@@ -83,7 +105,7 @@ protected Token refresh() {
83105
String tokenType = jsonNode.get(tokenTypeField).asText();
84106
String accessToken = jsonNode.get(accessTokenField).asText();
85107
String expiry = jsonNode.get(expiryField).asText();
86-
LocalDateTime expiresOn = parseExpiry(expiry);
108+
Instant expiresOn = parseExpiry(expiry);
87109
return new Token(accessToken, tokenType, expiresOn);
88110
} catch (DatabricksException e) {
89111
throw e;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import com.databricks.sdk.core.DatabricksException;
44
import com.databricks.sdk.core.http.HttpClient;
55
import com.google.common.base.Strings;
6-
import java.time.LocalDateTime;
6+
import java.time.Instant;
77
import java.util.HashMap;
88
import java.util.Map;
99
import java.util.Objects;
@@ -166,7 +166,7 @@ public Token refresh() {
166166
throw e;
167167
}
168168

169-
LocalDateTime expiry = LocalDateTime.now().plusSeconds(response.getExpiresIn());
169+
Instant expiry = Instant.now().plusSeconds(response.getExpiresIn());
170170
return new Token(
171171
response.getAccessToken(), response.getTokenType(), response.getRefreshToken(), expiry);
172172
}

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

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

33
import com.databricks.sdk.core.DatabricksException;
44
import com.databricks.sdk.core.http.HttpClient;
5-
import java.time.LocalDateTime;
5+
import java.time.Instant;
66
import java.util.HashMap;
77
import java.util.Map;
88
import java.util.Objects;
@@ -87,7 +87,7 @@ protected Token refresh() {
8787
throw e;
8888
}
8989

90-
LocalDateTime expiry = LocalDateTime.now().plusSeconds(oauthResponse.getExpiresIn());
90+
Instant expiry = Instant.now().plusSeconds(oauthResponse.getExpiresIn());
9191
return new Token(
9292
oauthResponse.getAccessToken(),
9393
oauthResponse.getTokenType(),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import com.google.common.base.Strings;
99
import com.google.common.collect.ImmutableMap;
1010
import java.io.IOException;
11-
import java.time.LocalDateTime;
11+
import java.time.Instant;
1212

1313
/**
1414
* {@code OidcTokenSource} is responsible for obtaining OAuth tokens using the OpenID Connect (OIDC)
@@ -77,7 +77,7 @@ protected Token refresh() {
7777
if (resp.getErrorCode() != null) {
7878
throw new IllegalArgumentException(resp.getErrorCode() + ": " + resp.getErrorSummary());
7979
}
80-
LocalDateTime expiry = LocalDateTime.now().plusSeconds(resp.getExpiresIn());
80+
Instant expiry = Instant.now().plusSeconds(resp.getExpiresIn());
8181
return new Token(resp.getAccessToken(), resp.getTokenType(), resp.getRefreshToken(), expiry);
8282
}
8383
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55
import com.databricks.sdk.core.http.FormRequest;
66
import com.databricks.sdk.core.http.HttpClient;
77
import com.databricks.sdk.core.http.Request;
8-
import java.time.LocalDateTime;
9-
import java.time.temporal.ChronoUnit;
8+
import java.time.Instant;
109
import java.util.Base64;
1110
import java.util.Map;
1211
import org.apache.http.HttpHeaders;
@@ -70,7 +69,7 @@ protected static Token retrieveToken(
7069
if (resp.getErrorCode() != null) {
7170
throw new IllegalArgumentException(resp.getErrorCode() + ": " + resp.getErrorSummary());
7271
}
73-
LocalDateTime expiry = LocalDateTime.now().plus(resp.getExpiresIn(), ChronoUnit.SECONDS);
72+
Instant expiry = Instant.now().plusSeconds(resp.getExpiresIn());
7473
return new Token(resp.getAccessToken(), resp.getTokenType(), resp.getRefreshToken(), expiry);
7574
} catch (Exception e) {
7675
throw new DatabricksException("Failed to refresh credentials: " + e.getMessage(), e);

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
import com.databricks.sdk.core.utils.SystemClockSupplier;
55
import com.fasterxml.jackson.annotation.JsonCreator;
66
import com.fasterxml.jackson.annotation.JsonProperty;
7-
import java.time.LocalDateTime;
8-
import java.time.temporal.ChronoUnit;
7+
import java.time.Instant;
98
import java.util.Objects;
109

1110
public class Token {
@@ -19,21 +18,20 @@ public class Token {
1918
* The expiry time of the token.
2019
*
2120
* <p>OAuth token responses include the duration of the lifetime of the access token. When the
22-
* token is retrieved, this is converted to a LocalDateTime tracking the expiry time of the token
23-
* with respect to the current clock.
21+
* token is retrieved, this is converted to an Instant tracking the expiry time of the token with
22+
* respect to the current clock.
2423
*/
25-
@JsonProperty private LocalDateTime expiry;
24+
@JsonProperty private Instant expiry;
2625

2726
private final ClockSupplier clockSupplier;
2827

2928
/** Constructor for non-refreshable tokens (e.g. M2M). */
30-
public Token(String accessToken, String tokenType, LocalDateTime expiry) {
29+
public Token(String accessToken, String tokenType, Instant expiry) {
3130
this(accessToken, tokenType, null, expiry, new SystemClockSupplier());
3231
}
3332

3433
/** Constructor for non-refreshable tokens (e.g. M2M) with ClockSupplier */
35-
public Token(
36-
String accessToken, String tokenType, LocalDateTime expiry, ClockSupplier clockSupplier) {
34+
public Token(String accessToken, String tokenType, Instant expiry, ClockSupplier clockSupplier) {
3735
this(accessToken, tokenType, null, expiry, clockSupplier);
3836
}
3937

@@ -43,7 +41,7 @@ public Token(
4341
@JsonProperty("accessToken") String accessToken,
4442
@JsonProperty("tokenType") String tokenType,
4543
@JsonProperty("refreshToken") String refreshToken,
46-
@JsonProperty("expiry") LocalDateTime expiry) {
44+
@JsonProperty("expiry") Instant expiry) {
4745
this(accessToken, tokenType, refreshToken, expiry, new SystemClockSupplier());
4846
}
4947

@@ -52,7 +50,7 @@ public Token(
5250
String accessToken,
5351
String tokenType,
5452
String refreshToken,
55-
LocalDateTime expiry,
53+
Instant expiry,
5654
ClockSupplier clockSupplier) {
5755
Objects.requireNonNull(accessToken, "accessToken must be defined");
5856
Objects.requireNonNull(tokenType, "tokenType must be defined");
@@ -71,8 +69,8 @@ public boolean isExpired() {
7169
}
7270
// Azure Databricks rejects tokens that expire in 30 seconds or less,
7371
// so we refresh the token 40 seconds before it expires.
74-
LocalDateTime potentiallyExpired = expiry.minus(40, ChronoUnit.SECONDS);
75-
LocalDateTime now = LocalDateTime.now(clockSupplier.getClock());
72+
Instant potentiallyExpired = expiry.minusSeconds(40);
73+
Instant now = Instant.now(clockSupplier.getClock());
7674
return potentiallyExpired.isBefore(now);
7775
}
7876

databricks-sdk-java/src/main/java/com/databricks/sdk/core/utils/SystemClockSupplier.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
public class SystemClockSupplier implements ClockSupplier {
66
@Override
77
public Clock getClock() {
8-
return Clock.systemDefaultZone();
8+
return Clock.systemUTC();
99
}
1010
}

databricks-sdk-java/src/test/java/com/databricks/sdk/core/AzureCliCredentialsProviderTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import com.databricks.sdk.core.oauth.Token;
99
import com.databricks.sdk.core.oauth.TokenSource;
10-
import java.time.LocalDateTime;
1110
import java.util.Arrays;
1211
import java.util.List;
1312
import org.junit.jupiter.api.Test;
@@ -25,7 +24,7 @@ class AzureCliCredentialsProviderTest {
2524
private static CliTokenSource mockTokenSource() {
2625
CliTokenSource tokenSource = Mockito.mock(CliTokenSource.class);
2726
Mockito.when(tokenSource.getToken())
28-
.thenReturn(new Token(TOKEN, TOKEN_TYPE, LocalDateTime.now()));
27+
.thenReturn(new Token(TOKEN, TOKEN_TYPE, java.time.Instant.now()));
2928
return tokenSource;
3029
}
3130

databricks-sdk-java/src/test/java/com/databricks/sdk/core/CliTokenSourceTest.java

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.databricks.sdk.core;
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertThrows;
45
import static org.mockito.ArgumentMatchers.any;
56
import static org.mockito.Mockito.mock;
67
import static org.mockito.Mockito.mockConstruction;
@@ -14,7 +15,9 @@
1415
import java.io.ByteArrayInputStream;
1516
import java.io.IOException;
1617
import java.time.Duration;
18+
import java.time.Instant;
1719
import java.time.LocalDateTime;
20+
import java.time.ZoneId;
1821
import java.time.ZonedDateTime;
1922
import java.time.format.DateTimeFormatter;
2023
import java.time.format.DateTimeParseException;
@@ -27,7 +30,6 @@
2730
import java.util.stream.Collectors;
2831
import java.util.stream.IntStream;
2932
import java.util.stream.Stream;
30-
import org.junit.jupiter.api.Test;
3133
import org.junit.jupiter.params.ParameterizedTest;
3234
import org.junit.jupiter.params.provider.Arguments;
3335
import org.junit.jupiter.params.provider.MethodSource;
@@ -157,30 +159,58 @@ public void testRefreshWithExpiry(
157159
}
158160
}
159161

160-
@Test
161-
public void testParseExpiryWithoutTruncate() {
162-
LocalDateTime parsedDateTime = CliTokenSource.parseExpiry("2023-07-17T09:02:22.330612218Z");
163-
assertEquals(LocalDateTime.of(2023, 7, 17, 9, 2, 22, 330612218), parsedDateTime);
162+
private static Stream<Arguments> expiryProvider() {
163+
return Stream.of(
164+
Arguments.of(
165+
"2023-07-17T09:02:22.330612218Z",
166+
Instant.parse("2023-07-17T09:02:22.330612218Z"),
167+
"9-digit nanos"),
168+
Arguments.of(
169+
"2023-07-17T09:02:22.33061221Z",
170+
Instant.parse("2023-07-17T09:02:22.330612210Z"),
171+
"8-digit nanos"),
172+
Arguments.of(
173+
"2023-07-17T09:02:22.330612Z",
174+
Instant.parse("2023-07-17T09:02:22.330612000Z"),
175+
"6-digit nanos"),
176+
Arguments.of(
177+
"2023-07-17T10:02:22.330612218+01:00",
178+
Instant.parse("2023-07-17T09:02:22.330612218Z"),
179+
"+01:00 offset, 9-digit nanos"),
180+
Arguments.of(
181+
"2023-07-17T04:02:22.330612218-05:00",
182+
Instant.parse("2023-07-17T09:02:22.330612218Z"),
183+
"-05:00 offset, 9-digit nanos"),
184+
Arguments.of(
185+
"2023-07-17T10:02:22.330612+01:00",
186+
Instant.parse("2023-07-17T09:02:22.330612000Z"),
187+
"+01:00 offset, 6-digit nanos"),
188+
Arguments.of("2023-07-17T09:02:22.33061221987Z", null, "Invalid: >9 nanos"),
189+
Arguments.of("17-07-2023 09:02:22", null, "Invalid date format"),
190+
Arguments.of(
191+
"2023-07-17 09:02:22.330612218",
192+
LocalDateTime.parse("2023-07-17T09:02:22.330612218")
193+
.atZone(ZoneId.systemDefault())
194+
.toInstant(),
195+
"Space separator, 9-digit nanos"),
196+
Arguments.of(
197+
"2023-07-17 09:02:22.330612",
198+
LocalDateTime.parse("2023-07-17T09:02:22.330612")
199+
.atZone(ZoneId.systemDefault())
200+
.toInstant(),
201+
"Space separator, 6-digit nanos"),
202+
Arguments.of(
203+
"2023-07-17 09:02:22.33061221987", null, "Space separator, Invalid: >9 nanos"));
164204
}
165205

166-
@Test
167-
public void testParseExpiryWithTruncate() {
168-
LocalDateTime parsedDateTime = CliTokenSource.parseExpiry("2023-07-17T09:02:22.33061221Z");
169-
assertEquals(LocalDateTime.of(2023, 7, 17, 9, 2, 22, 330612210), parsedDateTime);
170-
}
171-
172-
@Test
173-
public void testParseExpiryWithTruncateAndLessNanoSecondDigits() {
174-
LocalDateTime parsedDateTime = CliTokenSource.parseExpiry("2023-07-17T09:02:22.330612Z");
175-
assertEquals(LocalDateTime.of(2023, 7, 17, 9, 2, 22, 330612000), parsedDateTime);
176-
}
177-
178-
@Test
179-
public void testParseExpiryWithMoreThanNineNanoSecondDigits() {
180-
try {
181-
CliTokenSource.parseExpiry("2023-07-17T09:02:22.33061221987Z");
182-
} catch (DateTimeParseException e) {
183-
assert (e.getMessage().contains("could not be parsed"));
206+
@ParameterizedTest(name = "{2}")
207+
@MethodSource("expiryProvider")
208+
public void testParseExpiry(String input, Instant expectedInstant, String description) {
209+
if (expectedInstant == null) {
210+
assertThrows(DateTimeParseException.class, () -> CliTokenSource.parseExpiry(input));
211+
} else {
212+
Instant parsedInstant = CliTokenSource.parseExpiry(input);
213+
assertEquals(expectedInstant, parsedInstant);
184214
}
185215
}
186216
}

databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import com.databricks.sdk.core.oauth.TokenSource;
1313
import com.databricks.sdk.core.utils.Environment;
1414
import java.io.IOException;
15-
import java.time.LocalDateTime;
1615
import java.util.ArrayList;
1716
import java.util.HashMap;
1817
import java.util.List;
@@ -232,7 +231,7 @@ public void testGetTokenSourceWithOAuth() {
232231
HttpClient httpClient = mock(HttpClient.class);
233232
TokenSource mockTokenSource = mock(TokenSource.class);
234233
when(mockTokenSource.getToken())
235-
.thenReturn(new Token("test-token", "Bearer", LocalDateTime.now().plusHours(1)));
234+
.thenReturn(new Token("test-token", "Bearer", java.time.Instant.now().plusSeconds(3600)));
236235
OAuthHeaderFactory mockHeaderFactory = OAuthHeaderFactory.fromTokenSource(mockTokenSource);
237236
CredentialsProvider mockProvider = mock(CredentialsProvider.class);
238237
when(mockProvider.authType()).thenReturn("test");

0 commit comments

Comments
 (0)