-
Notifications
You must be signed in to change notification settings - Fork 44
feat: adds ttl_seconds support #306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change introduces the ability to request temporary access tokens with a custom time-to-live (TTL) by extending the Changes
Sequence Diagram(s)sequenceDiagram
participant UserApp
participant AuthClient
participant API
UserApp->>AuthClient: GrantToken(ctx, &GrantTokenRequest{TTLSeconds: 60})
AuthClient->>API: POST /grant-token (body: {"ttl_seconds":60})
API-->>AuthClient: Token response (with TTL)
AuthClient-->>UserApp: GrantToken result (token, TTL)
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/unit_test/auth/grant_token_test.go (1)
101-101: Consider extracting repeated string constant.The empty JSON object
"{}"appears multiple times. Consider extracting it as a constant to improve maintainability.+const emptyJSON = "{}" + func TestGrantTokenRequestJSON(t *testing.T) { // ... other code ... - expectedJSON := `{}` + expectedJSON := emptyJSON if string(jsonData) != expectedJSON { t.Errorf("Expected JSON: %s, got: %s", expectedJSON, string(jsonData)) } }) t.Run("Test_GrantTokenRequest_JSON_unmarshaling_empty", func(t *testing.T) { - jsonData := `{}` + jsonData := emptyJSONAlso applies to: 126-126
pkg/api/auth/v1/grant-token.go (1)
28-37: Optimize request body handling for better performance.The current implementation unnecessarily converts the buffer to a string and then back to a reader. Since
bytes.Bufferimplementsio.Reader, you can use it directly.Apply this diff for better performance:
var body io.Reader if req != nil { var buf bytes.Buffer if err := json.NewEncoder(&buf).Encode(req); err != nil { klog.V(1).Infof("GrantToken json.NewEncoder().Encode() failed. Err: %v\n", err) klog.V(6).Infof("auth.GrantToken() LEAVE\n") return nil, err } - body = strings.NewReader(buf.String()) + body = &buf }This eliminates the unnecessary string conversion and memory allocation.
tests/response_data/642c86c60eedbc4af873632b86d68164149599cf97131d81a63a2711f0563d37-response.json (1)
1-1: Snapshots churn due to dynamic metadata—consider normalizingrequest_id&createdRefreshing UUIDs and timestamps adds noise to PRs and can break brittle assertions. Prefer stripping or hard-coding these dynamic fields in test fixtures to keep snapshots deterministic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/auth/grant-token/main.go(3 hunks)pkg/api/auth/v1/grant-token.go(2 hunks)pkg/api/auth/v1/interfaces/types.go(1 hunks)tests/response_data/642c86c60eedbc4af873632b86d68164149599cf97131d81a63a2711f0563d37-response.json(1 hunks)tests/response_data/bfae00d50d521f470ff9d1943f32225fcfeffe51eff47984886930b71fae0929-response.json(1 hunks)tests/unit_test/auth/grant_token_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
examples/auth/grant-token/main.go (7)
Learnt from: dvonthenen
PR: deepgram/deepgram-go-sdk#252
File: examples/manage/usage/main.go:17-17
Timestamp: 2024-06-25T18:05:26.824Z
Learning: The `pkg/client/rest/v1` directory in the `deepgram-go-sdk` is not deprecated, and its usage is still valid except for the direct use of the helper file `pkg/client/rest/client.go`.
Learnt from: dvonthenen
PR: deepgram/deepgram-go-sdk#252
File: examples/manage/scopes/main.go:14-14
Timestamp: 2024-06-25T18:05:47.921Z
Learning: The `pkg/client/rest/v1` directory in the deepgram-go-sdk is not deprecated, only the direct use of `pkg/client/rest/client.go` is affected. This distinction is important for accurate codebase analysis and comments.
Learnt from: dvonthenen
PR: deepgram/deepgram-go-sdk#252
File: examples/manage/usage/main.go:17-17
Timestamp: 2024-10-09T02:19:48.728Z
Learning: The `pkg/client/rest/v1` directory in the `deepgram-go-sdk` is not deprecated, and its usage is still valid except for the direct use of the helper file `pkg/client/rest/client.go`.
Learnt from: dvonthenen
PR: deepgram/deepgram-go-sdk#258
File: pkg/api/speak/v1/websocket/interfaces/interfaces.go:31-43
Timestamp: 2024-10-09T02:19:46.086Z
Learning: The `SpeakMessageCallback` and `SpeakMessageChan` interfaces in the Deepgram Go SDK are distinct because they accommodate different transport mechanisms.
Learnt from: dvonthenen
PR: deepgram/deepgram-go-sdk#258
File: pkg/api/speak/v1/websocket/interfaces/interfaces.go:31-43
Timestamp: 2024-08-23T04:42:57.605Z
Learning: The `SpeakMessageCallback` and `SpeakMessageChan` interfaces in the Deepgram Go SDK are distinct because they accommodate different transport mechanisms.
Learnt from: dvonthenen
PR: deepgram/deepgram-go-sdk#252
File: examples/manage/usage/main.go:17-17
Timestamp: 2024-06-25T18:06:34.961Z
Learning: User dvonthenen clarified that the introduction of the `manage` client in the `deepgram-go-sdk` is to prevent direct creation of a `rest` client and to add a level of indirection. This design choice encapsulates functionalities and controls the interface more effectively.
Learnt from: dvonthenen
PR: deepgram/deepgram-go-sdk#252
File: examples/manage/usage/main.go:17-17
Timestamp: 2024-10-09T02:19:46.086Z
Learning: User dvonthenen clarified that the introduction of the `manage` client in the `deepgram-go-sdk` is to prevent direct creation of a `rest` client and to add a level of indirection. This design choice encapsulates functionalities and controls the interface more effectively.
🧬 Code Graph Analysis (3)
examples/auth/grant-token/main.go (1)
pkg/api/auth/v1/interfaces/types.go (2)
GrantToken(20-23)GrantTokenRequest(15-17)
tests/unit_test/auth/grant_token_test.go (1)
pkg/api/auth/v1/interfaces/types.go (1)
GrantTokenRequest(15-17)
pkg/api/auth/v1/grant-token.go (4)
pkg/api/auth/v1/auth.go (1)
Client(21-23)pkg/client/auth/v1/types.go (1)
Client(13-15)pkg/api/auth/v1/interfaces/types.go (2)
GrantToken(20-23)GrantTokenRequest(15-17)pkg/api/version/auth-version.go (1)
GrantTokenURI(16-16)
🪛 GitHub Check: Lint
tests/unit_test/auth/grant_token_test.go
[failure] 101-101:
string {} has 2 occurrences, make it a constant (goconst)
🔇 Additional comments (11)
tests/response_data/bfae00d50d521f470ff9d1943f32225fcfeffe51eff47984886930b71fae0929-response.json (1)
1-1: Test data refresh with updated metadata.Similar to the other response file, only metadata fields are updated. This is routine test data maintenance.
pkg/api/auth/v1/interfaces/types.go (1)
14-17: Well-designed struct for optional TTL specification.The implementation follows Go best practices:
- Pointer type allows for proper optional field handling
- JSON tag uses appropriate snake_case convention
omitemptyensures clean JSON when field is not specified- Field name follows Go naming conventions
examples/auth/grant-token/main.go (3)
13-13: Import addition supports new functionality.The import of
authInterfacesis correctly added to support the newGrantTokenRequeststruct usage.
39-39: Explicit nil parameter maintains clarity.Good practice to explicitly pass
nilto demonstrate the optional nature of the request parameter and maintain backward compatibility.
51-71: Excellent demonstration of custom TTL functionality.The new Phase 1.5 effectively demonstrates:
- Custom TTL value assignment
- Proper struct initialization
- Error handling
- Secure token display (truncated)
- Clear output formatting
tests/unit_test/auth/grant_token_test.go (3)
14-70: Comprehensive struct testing with excellent coverage.The test functions thoroughly cover all scenarios:
- Struct creation with various TTL values
- Nil value handling
- Boundary conditions (0, 3600 seconds)
- Proper pointer dereferencing
Test organization and naming conventions are excellent.
72-138: Thorough JSON serialization testing.The JSON marshaling/unmarshaling tests effectively verify:
- Correct JSON field naming (
ttl_seconds)- Proper
omitemptybehavior when TTL is nil- Bidirectional serialization consistency
- Empty JSON handling
140-177: Comprehensive validation testing covers expected use cases.The validation tests cover the documented TTL range (1-3600 seconds) and include typical values that users would likely use.
pkg/api/auth/v1/grant-token.go (3)
12-12: LGTM! Import additions are appropriate.The new imports are correctly added to support JSON encoding and request body handling functionality.
Also applies to: 14-16
25-25: LGTM! Method signature change maintains backward compatibility.The addition of the optional
reqparameter using a pointer type allows for backward compatibility while enabling the new TTL functionality.
40-40: LGTM! APIRequest call correctly updated.The APIRequest call properly passes the body parameter while maintaining existing error handling and return logic.
PR Summary: Add TTL Support to Grant Token Endpoint
TL;DR
ttl_secondsparameter support to the grant token endpoint in the Deepgram Go SDK.🚀 Changes Made
Core Implementation
Added
GrantTokenRequeststruct (pkg/api/auth/v1/interfaces/types.go)ttl_secondsparameter (1-3600 seconds)*intwithomitemptyfor proper JSON marshalingUpdated
GrantTokenmethod (pkg/api/auth/v1/grant-token.go)*GrantTokenRequestparameternilparameterExamples & Documentation
examples/auth/grant-token/main.go)Testing
tests/unit_test/auth/grant_token_test.go)🔄 Backward Compatibility
client.GrantToken(ctx, nil)works as before📋 Usage Examples
🧪 Testing Results
🎯 Files Changed
pkg/api/auth/v1/interfaces/types.go- Added GrantTokenRequest structpkg/api/auth/v1/grant-token.go- Updated method signature and implementationexamples/auth/grant-token/main.go- Enhanced example with TTL demonstrationtests/unit_test/auth/grant_token_test.go- Added comprehensive unit testsTypes of changes
What types of changes does your code introduce to the community Go 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.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores