feat: support short-lived token endpoint #369
Conversation
WalkthroughThis pull request introduces a new authentication feature into the Deepgram SDK. It adds an Changes
Sequence Diagram(s)sequenceDiagram
participant P as Program
participant F as ClientFactory
participant AC as AuthClient / IAuthClient
participant API as Deepgram API
P->>F: Call CreateAuthClient(apiKey, options, httpId)
F->>AC: Instantiate AuthClient
P->>AC: Invoke GrantToken(...)
AC->>API: POST request to '/grant' endpoint
API-->>AC: Return GrantTokenResponse
AC-->>P: Return token details
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
Deepgram/ClientFactory.cs (1)
59-62: Consider adding versioned auth client creation.The ClientFactory includes versioned methods for other clients (e.g., CreateListenWebSocketClient with version parameter), but not for the new AuthClient. Consider adding a versioned method for consistency with the pattern used for other clients.
+ /// <summary> + /// This method allows you to create an AuthClient with a specific version of the client. + /// TODO: this should be revisited at a later time. Opportunity to use reflection to get the type of the client + /// </summary> + public static object CreateAuthClient(int version, string apiKey = "", DeepgramHttpClientOptions? options = null, string? httpId = null) + { + // Currently only a single version of the AuthClient exists + return new AuthClient(apiKey, options, httpId); + }examples/auth/grant-token/Auth.csproj (4)
3-8: Inconsistent indentation in project file.The project file uses tabs for indentation, which is inconsistent with the standard .NET project format that typically uses spaces. Consider standardizing the indentation across all project files.
- <PropertyGroup> - <OutputType>Exe</OutputType> - <TargetFramework>net8.0</TargetFramework> - <ImplicitUsings>enable</ImplicitUsings> - <Nullable>enable</Nullable> - </PropertyGroup> + <PropertyGroup> + <OutputType>Exe</OutputType> + <TargetFramework>net8.0</TargetFramework> + <ImplicitUsings>enable</ImplicitUsings> + <Nullable>enable</Nullable> + </PropertyGroup>
10-12: Inconsistent indentation in ItemGroup.The ItemGroup element also uses tabs for indentation, which should be standardized to match other project files in the repository.
- <ItemGroup> - <PackageReference Include="Microsoft.Extensions.Http" Version="8.0.1" /> - </ItemGroup> + <ItemGroup> + <PackageReference Include="Microsoft.Extensions.Http" Version="8.0.1" /> + </ItemGroup>
14-16: Inconsistent indentation in ProjectReference ItemGroup.This ItemGroup also uses tabs for indentation and has an inconsistent spacing in the element.
- <ItemGroup> - <ProjectReference Include="..\..\..\Deepgram\Deepgram.csproj" /> - </ItemGroup> + <ItemGroup> + <ProjectReference Include="..\..\..\Deepgram\Deepgram.csproj" /> + </ItemGroup>
18-20: Inconsistent spacing in Using ItemGroup.The Using ItemGroup uses spaces but has a different level of indentation compared to the other ItemGroup with spaces.
- <ItemGroup> - <Using Include="Deepgram" /> - </ItemGroup> + <ItemGroup> + <Using Include="Deepgram" /> + </ItemGroup>examples/auth/grant-token/Program.cs (2)
22-26: Consider enhancing error handling.The current implementation only checks if the token response is null. Consider adding more comprehensive error handling such as try/catch blocks to handle potential API errors or network issues.
// generate token var tokenResp = await deepgramClient.GrantToken(); if (tokenResp == null) { Console.WriteLine("GrantToken failed."); Environment.Exit(1); } +catch (Exception ex) +{ + Console.WriteLine($"Error obtaining token: {ex.Message}"); + Environment.Exit(1); +}
28-31: Consider handling null token values.The code directly accesses
AccessTokenandExpiresInproperties without checking if they are null, which could lead to potential null reference exceptions.-string token = tokenResp.AccessToken; -string ttl = tokenResp.ExpiresIn.ToString(); +string token = tokenResp.AccessToken ?? "Token not provided"; +string ttl = tokenResp.ExpiresIn?.ToString() ?? "TTL not provided"; Console.WriteLine($"Token: {token}"); Console.WriteLine($"TTL: {ttl}");Deepgram/Models/Auth/v1/GrantTokenResponse.cs (2)
12-14: JSON annotation inconsistency.The indentation of the
[JsonPropertyName]attribute is inconsistent with the[JsonIgnore]attribute.[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] - [JsonPropertyName("access_token")] +[JsonPropertyName("access_token")] public string? AccessToken { get; set; }
19-21: JSON annotation inconsistency.The indentation of the
[JsonPropertyName]attribute is inconsistent with the[JsonIgnore]attribute.[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] - [JsonPropertyName("expires_in")] +[JsonPropertyName("expires_in")] public decimal? ExpiresIn { get; set; }Deepgram/Clients/Auth/v1/Client.cs (3)
5-8: Potential unused namespace import.The
Deepgram.Models.Authenticate.v1namespace is imported but doesn't appear to be used in the class.-using Deepgram.Models.Authenticate.v1; using Deepgram.Models.Auth.v1; using Deepgram.Clients.Interfaces.v1; using Deepgram.Abstractions.v1;
13-13: Incorrect XML documentation.The XML comment references "Models Client" but this is actually an "Auth Client".
/// <summary> -/// Implements version 1 of the Models Client. +/// Implements version 1 of the Auth Client. /// </summary>
24-37: Implementation looks good but could benefit from exception handling.The
GrantTokenmethod implementation is correct, but consider adding explicit exception handling to log errors and provide more context in case of failures.public async Task<GrantTokenResponse> GrantToken(CancellationTokenSource? cancellationToken = default, Dictionary<string, string>? addons = null, Dictionary<string, string>? headers = null) { Log.Verbose("AuthClient.GrantToken", "ENTER"); + try + { var uri = GetUri(_options, $"auth/{UriSegments.GRANTTOKEN}"); var result = await PostAsync<object, GrantTokenResponse>(uri, null, cancellationToken, addons, headers); Log.Information("GrantToken", $"{uri} Succeeded"); Log.Debug("GrantToken", $"result: {result}"); Log.Verbose("AuthClient.GrantToken", "LEAVE"); return result; + } + catch (Exception ex) + { + Log.Error("AuthClient.GrantToken", $"Failed to grant token: {ex.Message}"); + Log.Verbose("AuthClient.GrantToken", "LEAVE"); + throw; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Deepgram/AuthClient.cs(1 hunks)Deepgram/ClientFactory.cs(1 hunks)Deepgram/Clients/Auth/v1/Client.cs(1 hunks)Deepgram/Clients/Auth/v1/UriSegments.cs(1 hunks)Deepgram/Clients/Interfaces/v1/IAuthClient.cs(1 hunks)Deepgram/Models/Auth/v1/GrantTokenResponse.cs(1 hunks)examples/auth/grant-token/Auth.csproj(1 hunks)examples/auth/grant-token/Program.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
Deepgram/Clients/Interfaces/v1/IAuthClient.cs (3)
Deepgram/Clients/Auth/v1/Client.cs (1)
Task(24-37)examples/auth/grant-token/Program.cs (1)
Task(11-38)Deepgram.Tests/UnitTests/HttpExtensionsTests/HttpClientExtensionTests.cs (1)
Dictionary(143-153)
Deepgram/Models/Auth/v1/GrantTokenResponse.cs (1)
Deepgram/Utilities/JsonSerializeOptions.cs (1)
JsonSerializeOptions(7-13)
Deepgram/Clients/Auth/v1/Client.cs (4)
Deepgram/Clients/Interfaces/v1/IAuthClient.cs (1)
Task(20-21)examples/auth/grant-token/Program.cs (1)
Task(11-38)Deepgram/Logger/Log.cs (3)
Verbose(75-78)Information(91-94)Debug(83-86)Deepgram/Clients/Auth/v1/UriSegments.cs (1)
UriSegments(7-12)
🔇 Additional comments (10)
Deepgram/AuthClient.cs (1)
10-19: Good implementation of the auth client class.The AuthClient class correctly extends the base Client class and is well-documented with appropriate XML comments. The implementation follows the same pattern as other client classes in the SDK, providing a consistent API for users.
Deepgram/Clients/Auth/v1/UriSegments.cs (1)
7-12: Good practice using constants instead of magic strings.Using a dedicated constants class for URI segments improves maintainability and helps prevent errors from typos. The naming convention (uppercase) follows standard practices for constants.
Deepgram/ClientFactory.cs (1)
52-62: Well-integrated factory method for AuthClient.The CreateAuthClient method follows the same pattern as other client creation methods in the factory, maintaining consistency in the API. The implementation and documentation are appropriate.
examples/auth/grant-token/Auth.csproj (1)
1-22: Well-structured example project.The example project is properly configured with appropriate dependencies and references. It targets .NET 8.0 and includes the necessary reference to the Deepgram project.
Deepgram/Clients/Interfaces/v1/IAuthClient.cs (1)
20-21: Interface method implementation looks good.The
GrantTokenmethod signature is well-defined with appropriate optional parameters and return type.examples/auth/grant-token/Program.cs (1)
11-38: Example shows clear implementation of token acquisition.The example provides a clear demonstration of how to use the new auth client to obtain a token, including proper initialization and cleanup.
Deepgram/Models/Auth/v1/GrantTokenResponse.cs (2)
7-30: Well-structured response model.The response model is well-structured using C# records, with appropriate JSON attributes and null handling, which aligns with the rest of the SDK design patterns.
28-28:✅ Verification successful
Verify if Regex.Unescape is necessary.
The use of
Regex.Unescapemight be unnecessary depending on the expected content. Typically,JsonSerializer.Serializealready produces well-formed JSON.
🏁 Script executed:
#!/bin/bash # Check usage of Regex.Unescape in other model classes # Find other ToString methods that use JsonSerializer.Serialize rg -A 1 -B 1 "override\s+string\s+ToString" --glob="*.cs" | grep -A 1 -B 1 "JsonSerializer.Serialize"Length of output: 101
Regex.Unescape Usage Confirmed as Intentional
Based on our review and prior learnings from the codebase (see Deepgram/Models/Speak/v1/WebSocket/MetadataResponse.cs), the use of
Regex.Unescapein theToStringmethod for JSON serialization is intentional. Although the shell script did not return additional instances, the project convention supports using it to produce the desired JSON output.Deepgram/Clients/Auth/v1/Client.cs (2)
17-38: Implementation provides clean and concise solution for token acquisition.The client implementation follows the SDK's patterns with proper inheritance, logging, and HTTP request construction. The code is well-organized and easy to understand.
18-18:❓ Verification inconclusive
Constructor uses recent C# feature.
The class uses C# 12's primary constructor syntax, which is good for conciseness but ensure the SDK's minimum required .NET version supports this feature.
🏁 Script executed:
#!/bin/bash # Check the target framework version in the project file fd -e csproj | xargs cat | grep -A 5 -B 5 "TargetFramework"Length of output: 13068
Action: Verify .NET Target Compatibility for C#12 Primary Constructors
The updated constructor uses the concise, C# 12 primary constructor syntax—which is fully supported when building for .NET 8.0 as indicated by the
<TargetFramework>settings and<LangVersion>latest</LangVersion>. However, since the SDK also targets netstandard2.0, please verify that:
- The net8.0 build (which supports C#12 features) is isolated from any issues that might arise from multi-targeting.
- The netstandard2.0 target compiles and functions correctly with this syntax—or that it’s conditionally excluded if needed.
If any compatibility concerns appear on the netstandard2.0 side, consider either conditionally compiling this feature or updating the minimal supported framework accordingly.
Co-authored-by: John Vajda <[email protected]>
Proposed changes
Types of changes
What types of changes does your code introduce to the community .NET SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
Summary by CodeRabbit