Skip to content

Commit dfce414

Browse files
committed
Fix for comments
1 parent 3e65109 commit dfce414

File tree

6 files changed

+22
-102
lines changed

6 files changed

+22
-102
lines changed

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

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,20 @@ private enum TokenState {
3636
// Default duration before expiry to consider a token as 'stale'.
3737
private static final Duration DEFAULT_STALE_DURATION = Duration.ofMinutes(3);
3838

39-
/** The current OAuth token. May be null if not yet fetched. */
39+
// The current OAuth token. May be null if not yet fetched.
4040
protected Token token;
41-
/** Whether asynchronous refresh is enabled. */
41+
// Whether asynchronous refresh is enabled.
4242
private boolean asyncEnabled = false;
43-
/** Duration before expiry to consider a token as 'stale'. */
43+
// Duration before expiry to consider a token as 'stale'.
4444
private Duration staleDuration = DEFAULT_STALE_DURATION;
45-
/** Additional buffer before expiry to consider a token as expired. */
46-
private Duration expiryBuffer = Duration.ZERO;
47-
/** Whether a refresh is currently in progress (for async refresh). */
45+
// Additional buffer before expiry to consider a token as expired.
46+
private Duration expiryBuffer = Duration.ofSeconds(40);
47+
// Whether a refresh is currently in progress (for async refresh).
4848
private boolean refreshInProgress = false;
49-
/** Whether the last refresh attempt succeeded. */
49+
// Whether the last refresh attempt succeeded.
5050
private boolean lastRefreshSucceeded = true;
5151

52-
/** Default constructor. */
52+
/** Constructs a new {@code RefreshableTokenSource} with no initial token. */
5353
public RefreshableTokenSource() {}
5454

5555
/**
@@ -103,7 +103,7 @@ public RefreshableTokenSource withExpiryBuffer(Duration buffer) {
103103
*
104104
* @return The current valid token
105105
*/
106-
public synchronized Token getToken() {
106+
public Token getToken() {
107107
if (!asyncEnabled) {
108108
return getTokenBlocking();
109109
}
@@ -143,15 +143,10 @@ protected synchronized Token getTokenBlocking() {
143143
if (state != TokenState.EXPIRED) {
144144
return token;
145145
}
146-
try {
147-
Token newToken = refresh();
148-
token = newToken;
149-
lastRefreshSucceeded = true;
150-
return newToken;
151-
} catch (Exception e) {
152-
lastRefreshSucceeded = false;
153-
throw e;
154-
}
146+
lastRefreshSucceeded = false;
147+
token = refresh(); // May throw an exception
148+
lastRefreshSucceeded = true;
149+
return token;
155150
}
156151

157152
/**
@@ -170,16 +165,15 @@ protected Token getTokenAsync() {
170165
state = getTokenState();
171166
currentToken = token;
172167
}
173-
if (state == TokenState.FRESH) {
174-
return currentToken;
175-
}
176-
if (state == TokenState.STALE) {
177-
// Trigger background refresh, return current token
178-
triggerAsyncRefresh();
179-
return token;
180-
} else {
181-
// Token is expired, block to refresh
182-
return getTokenBlocking();
168+
switch (state) {
169+
case FRESH:
170+
return currentToken;
171+
case STALE:
172+
triggerAsyncRefresh();
173+
return currentToken;
174+
case EXPIRED:
175+
default:
176+
return getTokenBlocking();
183177
}
184178
}
185179

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import com.fasterxml.jackson.annotation.JsonProperty;
77
import java.time.Duration;
88
import java.time.LocalDateTime;
9-
import java.time.temporal.ChronoUnit;
109
import java.util.Objects;
1110

1211
public class Token {
@@ -66,33 +65,6 @@ public Token(
6665
this.clockSupplier = clockSupplier;
6766
}
6867

69-
/**
70-
* Checks if the token is expired. Tokens are considered expired 40 seconds before their actual
71-
* expiry time to account for Azure Databricks rejecting tokens that expire in 30 seconds or less.
72-
*
73-
* @return true if the token is expired or about to expire, false otherwise
74-
*/
75-
public boolean isExpired() {
76-
if (expiry == null) {
77-
return false;
78-
}
79-
// Azure Databricks rejects tokens that expire in 30 seconds or less,
80-
// so we refresh the token 40 seconds before it expires.
81-
LocalDateTime potentiallyExpired = expiry.minus(40, ChronoUnit.SECONDS);
82-
LocalDateTime now = LocalDateTime.now(clockSupplier.getClock());
83-
return potentiallyExpired.isBefore(now);
84-
}
85-
86-
/**
87-
* Checks if the token is valid. A token is valid if it has a non-null access token and is not
88-
* expired.
89-
*
90-
* @return true if the token is valid, false otherwise
91-
*/
92-
public boolean isValid() {
93-
return accessToken != null && !isExpired();
94-
}
95-
9668
/**
9769
* Returns the type of the token (e.g., "Bearer").
9870
*

databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/DataPlaneTokenSourceTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ void testDataPlaneTokenSource(
196196
assertEquals(expectedToken.getAccessToken(), token.getAccessToken());
197197
assertEquals(expectedToken.getTokenType(), token.getTokenType());
198198
assertEquals(expectedToken.getRefreshToken(), token.getRefreshToken());
199-
assertTrue(token.isValid());
200199
}
201200
}
202201

databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/DatabricksOAuthTokenSourceTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,6 @@ void testTokenSource(TestCase testCase) {
330330
assertEquals(TOKEN, token.getAccessToken());
331331
assertEquals(TOKEN_TYPE, token.getTokenType());
332332
assertEquals(REFRESH_TOKEN, token.getRefreshToken());
333-
assertFalse(token.isExpired());
334333

335334
// Verify correct audience was used
336335
verify(testCase.idTokenSource, atLeastOnce()).getIDToken(testCase.expectedAudience);

databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/FileTokenCacheTest.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,24 +54,6 @@ void testSaveAndLoadToken() {
5454
assertEquals("access-token", loadedToken.getAccessToken());
5555
assertEquals("Bearer", loadedToken.getTokenType());
5656
assertEquals("refresh-token", loadedToken.getRefreshToken());
57-
assertFalse(loadedToken.isExpired(), "Token should not be expired");
58-
}
59-
60-
@Test
61-
void testTokenExpiry() {
62-
// Create an expired token
63-
LocalDateTime pastTime = LocalDateTime.now().minusHours(1);
64-
Token expiredToken = new Token("access-token", "Bearer", "refresh-token", pastTime);
65-
66-
// Verify it's marked as expired
67-
assertTrue(expiredToken.isExpired(), "Token should be expired");
68-
69-
// Create a valid token
70-
LocalDateTime futureTime = LocalDateTime.now().plusMinutes(30);
71-
Token validToken = new Token("access-token", "Bearer", "refresh-token", futureTime);
72-
73-
// Verify it's not marked as expired
74-
assertFalse(validToken.isExpired(), "Token should not be expired");
7557
}
7658

7759
@Test

databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/TokenTest.java

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ void createNonRefreshableToken() {
3030
assertEquals(accessToken, token.getAccessToken());
3131
assertEquals(tokenType, token.getTokenType());
3232
assertNull(token.getRefreshToken());
33-
assertTrue(token.isValid());
3433
}
3534

3635
@Test
@@ -45,31 +44,6 @@ void createRefreshableToken() {
4544
assertEquals(accessToken, token.getAccessToken());
4645
assertEquals(tokenType, token.getTokenType());
4746
assertEquals(refreshToken, token.getRefreshToken());
48-
assertTrue(token.isValid());
49-
}
50-
51-
@Test
52-
void tokenExpiryMoreThan40Seconds() {
53-
Token token =
54-
new Token(accessToken, tokenType, currentLocalDateTime.plusSeconds(50), fakeClockSupplier);
55-
assertFalse(token.isExpired());
56-
assertTrue(token.isValid());
57-
}
58-
59-
@Test
60-
void tokenExpiryLessThan40Seconds() {
61-
Token token =
62-
new Token(accessToken, tokenType, currentLocalDateTime.plusSeconds(30), fakeClockSupplier);
63-
assertTrue(token.isExpired());
64-
assertFalse(token.isValid());
65-
}
66-
67-
@Test
68-
void expiredToken() {
69-
Token token =
70-
new Token(accessToken, tokenType, currentLocalDateTime.minusSeconds(10), fakeClockSupplier);
71-
assertTrue(token.isExpired());
72-
assertFalse(token.isValid());
7347
}
7448

7549
@Test

0 commit comments

Comments
 (0)