Skip to content

Commit efe06c2

Browse files
committed
feedback fixes
1 parent 217c58a commit efe06c2

2 files changed

Lines changed: 20 additions & 38 deletions

File tree

csharp/src/Drivers/Databricks/Auth/OAuthClientCredentialsService.cs renamed to csharp/src/Drivers/Databricks/Auth/OAuthClientCredentialsProvider.cs

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ namespace Apache.Arrow.Adbc.Drivers.Databricks.Auth
2828
/// <summary>
2929
/// Service for obtaining OAuth access tokens using the client credentials grant type.
3030
/// </summary>
31-
internal class OAuthClientCredentialsService : IDisposable
31+
internal class OAuthClientCredentialsProvider : IDisposable
3232
{
33-
private readonly Lazy<HttpClient> _httpClient;
33+
private readonly HttpClient _httpClient;
3434
private readonly string _clientId;
3535
private readonly string _clientSecret;
36-
private readonly Uri _baseUri;
36+
private readonly string _host;
3737
private readonly string _tokenEndpoint;
3838
private readonly int _timeoutMinutes;
3939
private readonly SemaphoreSlim _tokenLock = new SemaphoreSlim(1, 1);
@@ -56,36 +56,27 @@ private class TokenInfo
5656
/// <param name="clientId">The OAuth client ID.</param>
5757
/// <param name="clientSecret">The OAuth client secret.</param>
5858
/// <param name="baseUri">The base URI of the Databricks workspace.</param>
59-
public OAuthClientCredentialsService(
59+
public OAuthClientCredentialsProvider(
6060
string clientId,
6161
string clientSecret,
62-
Uri baseUri,
63-
int timeoutMinutes = 1,
64-
HttpClient? httpClient = null)
62+
string host,
63+
int timeoutMinutes = 1)
6564
{
6665
_clientId = clientId ?? throw new ArgumentNullException(nameof(clientId));
6766
_clientSecret = clientSecret ?? throw new ArgumentNullException(nameof(clientSecret));
68-
_baseUri = baseUri ?? throw new ArgumentNullException(nameof(baseUri));
67+
_host = host ?? throw new ArgumentNullException(nameof(host));
6968
_timeoutMinutes = timeoutMinutes;
7069
_tokenEndpoint = DetermineTokenEndpoint();
7170

72-
_httpClient = httpClient != null
73-
? new Lazy<HttpClient>(() => httpClient)
74-
: new Lazy<HttpClient>(() =>
75-
{
76-
var client = new HttpClient();
77-
client.Timeout = TimeSpan.FromMinutes(_timeoutMinutes);
78-
return client;
79-
});
71+
_httpClient = new HttpClient();
72+
_httpClient.Timeout = TimeSpan.FromMinutes(_timeoutMinutes);
8073
}
8174

82-
private HttpClient HttpClient => _httpClient.Value;
83-
8475
private string DetermineTokenEndpoint()
8576
{
8677
// For workspace URLs, the token endpoint is always /oidc/v1/token
8778
// TODO: Might be different for Azure AAD SPs
88-
return $"{_baseUri.Scheme}://{_baseUri.Host}/oidc/v1/token";
79+
return $"https://{_host}/oidc/v1/token";
8980
}
9081

9182
private string? GetValidCachedToken()
@@ -103,7 +94,7 @@ private async Task<string> RefreshTokenInternalAsync(CancellationToken cancellat
10394
HttpResponseMessage response;
10495
try
10596
{
106-
response = await HttpClient.SendAsync(request, cancellationToken);
97+
response = await _httpClient.SendAsync(request, cancellationToken);
10798
response.EnsureSuccessStatusCode();
10899
}
109100
catch (HttpRequestException ex)
@@ -129,7 +120,7 @@ private HttpRequestMessage CreateTokenRequest()
129120
var requestContent = new FormUrlEncodedContent(new[]
130121
{
131122
new KeyValuePair<string, string>("grant_type", "client_credentials"),
132-
new KeyValuePair<string, string>("scope", "sql")
123+
new KeyValuePair<string, string>("scope", "all-apis")
133124
});
134125

135126
var request = new HttpRequestMessage(HttpMethod.Post, _tokenEndpoint)
@@ -194,12 +185,7 @@ public async Task<string> GetAccessTokenAsync(CancellationToken cancellationToke
194185
return cachedToken;
195186
}
196187

197-
// If token needs refresh, acquire lock with timeout
198-
var lockTimeout = TimeSpan.FromSeconds(30); // Reasonable timeout for lock acquisition
199-
if (!await _tokenLock.WaitAsync(lockTimeout, cancellationToken).ConfigureAwait(false))
200-
{
201-
throw new TimeoutException("Timeout waiting for token refresh lock");
202-
}
188+
await _tokenLock.WaitAsync(cancellationToken);
203189

204190
try
205191
{
@@ -209,22 +195,18 @@ public async Task<string> GetAccessTokenAsync(CancellationToken cancellationToke
209195
return refreshedToken;
210196
}
211197

212-
return await RefreshTokenInternalAsync(cancellationToken).ConfigureAwait(false);
198+
return await RefreshTokenInternalAsync(cancellationToken);
213199
}
214200
finally
215201
{
216202
_tokenLock.Release();
217203
}
218204
}
219205

220-
221206
public void Dispose()
222207
{
223208
_tokenLock.Dispose();
224-
if (_httpClient.IsValueCreated)
225-
{
226-
HttpClient.Dispose();
227-
}
209+
_httpClient.Dispose();
228210
}
229211

230212
}

csharp/test/Drivers/Databricks/Auth/OAuthClientCredentialsServiceTests.cs renamed to csharp/test/Drivers/Databricks/Auth/OAuthClientCredentialsProviderTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,17 @@
2424

2525
namespace Apache.Arrow.Adbc.Tests.Drivers.Databricks.Auth
2626
{
27-
public class OAuthClientCredentialsServiceTests : TestBase<DatabricksTestConfiguration, DatabricksTestEnvironment>, IDisposable
27+
public class OAuthClientCredentialsProviderTests : TestBase<DatabricksTestConfiguration, DatabricksTestEnvironment>, IDisposable
2828
{
29-
private readonly OAuthClientCredentialsService _service;
29+
private readonly OAuthClientCredentialsProvider _service;
3030

31-
public OAuthClientCredentialsServiceTests(ITestOutputHelper? outputHelper)
31+
public OAuthClientCredentialsProviderTests(ITestOutputHelper? outputHelper)
3232
: base(outputHelper, new DatabricksTestEnvironment.Factory())
3333
{
34-
_service = new OAuthClientCredentialsService(
34+
_service = new OAuthClientCredentialsProvider(
3535
TestConfiguration.OAuthClientId,
3636
TestConfiguration.OAuthClientSecret,
37-
new Uri(TestConfiguration.Uri),
37+
TestConfiguration.HostName,
3838
timeoutMinutes: 1);
3939
}
4040

0 commit comments

Comments
 (0)