- 
                Notifications
    You must be signed in to change notification settings 
- Fork 21
fix: ensure headers can be set and overriden #226
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
…lientBatchCheckOptions
| WalkthroughDocumentation adds examples for default and per-request headers. ClientConfiguration gains a fluent  Changes
 Sequence Diagram(s)sequenceDiagram
  autonumber
  actor Caller
  participant Client as OpenFgaClient
  participant Config as ClientConfiguration
  Caller->>Client: readAuthorizationModel(options)
  alt options has non-blank authorizationModelId
    Client->>Client: use options.getAuthorizationModelIdChecked()
  else fallback
    Client->>Config: configuration.getAuthorizationModelIdChecked()
    Config-->>Client: authorizationModelId
  end
  Client->>Client: prepare request (defensive headers copy, merge)
  Client-->>Caller: AuthorizationModel
sequenceDiagram
  autonumber
  actor Caller
  participant Client as OpenFgaClient
  participant Config as ClientConfiguration
  participant HTTP as HttpClient
  Caller->>Client: any API call (opts may include additionalHeaders)
  Client->>Config: config.getDefaultHeaders()
  Client->>Client: build headers map
  Note right of Client #DDEBF7: start from defaultHeaders\nif additionalHeaders != null use new HashMap<>(additionalHeaders)\nputIfAbsent/put to merge without mutating caller map
  Client->>HTTP: send HTTP request with merged headers
  HTTP-->>Client: response
  Client-->>Caller: mapped result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
 Suggested reviewers
 Pre-merge checks and finishing touches❌ Failed checks (1 warning)
 ✅ Passed checks (2 passed)
 ✨ Finishing touches
 🧪 Generate unit tests (beta)
 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
 🚧 Files skipped from review as they are similar to previous changes (1)
 🧰 Additional context used🪛 Gitleaks (8.28.0)src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java[high] 38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) 🔇 Additional comments (3)
 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. Comment  | 
        
          
                src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      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: 2
🧹 Nitpick comments (1)
README.md (1)
239-267: Docs: add a call using options; note mutable map for some ops
- Consider adding a line showing how options are passed, e.g.,
fgaClient.read(request, options);for completeness.- For operations that add SDK headers (e.g., batchCheck, listRelations, non-transactional write), prefer a mutable map (e.g., new HashMap<>(Map.of(...))) for
additionalHeaders.Also applies to: 279-299
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
- README.md(1 hunks)
- src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java(1 hunks)
- src/main/java/dev/openfga/sdk/api/configuration/ClientConfiguration.java(1 hunks)
- src/main/java/dev/openfga/sdk/api/configuration/ClientListRelationsOptions.java(1 hunks)
- src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test and Build OpenFGA (21)
- GitHub Check: Test and Build OpenFGA (17)
- GitHub Check: Analyze (java)
🔇 Additional comments (2)
src/main/java/dev/openfga/sdk/api/configuration/ClientConfiguration.java (1)
144-148: Fluent override for defaultHeaders looks goodEnables proper chaining, delegates correctly.
src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java (1)
329-358: Great coverage; add two edge-case tests for readAuthorizationModel
- options provided with no model ID (null/blank) should fall back to configuration.
- null options passed to the overload should behave like the no-arg variant (and not NPE).
        
          
                src/main/java/dev/openfga/sdk/api/configuration/ClientListRelationsOptions.java
          
            Show resolved
            Hide resolved
        
      | @coderabbitai review | 
| ✅ Actions performedReview triggered. 
 | 
| Codecov Report❌ Patch coverage is  Additional details and impacted files@@             Coverage Diff              @@
##               main     #226      +/-   ##
============================================
+ Coverage     35.19%   35.97%   +0.78%     
- Complexity     1071     1116      +45     
============================================
  Files           187      187              
  Lines          7087     7096       +9     
  Branches        803      806       +3     
============================================
+ Hits           2494     2553      +59     
+ Misses         4483     4438      -45     
+ Partials        110      105       -5     ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
bf73cd5    to
    90294e3      
    Compare
  
    | @coderabbitai review | 
| ✅ Actions performedReview triggered. 
 | 
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
- src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java(3 hunks)
- src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/dev/openfga/sdk/api/client/OpenFgaClient.java
🔇 Additional comments (2)
src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java (2)
1-73: LGTM! Well-structured test setup.The test class initialization is clean and comprehensive:
- Clear constant definitions for test data
- Proper mock setup for HttpClient and ApiClient
- Default headers configured in ClientConfiguration for testing merge/override scenarios
76-942: Excellent comprehensive test coverage.The test suite thoroughly validates header behavior across all client operations:
- Default header merging and per-call overrides
- Null and empty header map handling
- Edge cases: no default headers, multiple overrides, special characters
- Consistent Given-When-Then structure with proper assertions
- Good use of mocks to verify HTTP method, URL, headers, and request bodies
        
          
                src/test/java/dev/openfga/sdk/api/client/OpenFgaClientHeadersTest.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …le map for headers
| @coderabbitai review | 
| ✅ Actions performedReview triggered. 
 | 
Description
What problem is being solved?
The project does not document or test custom headers support
How is it being solved?
Documentation and tests added, fixes made where relevant
What changes are made to solve it?
Adds tests and documentation for setting default headers and overriding them per-request, this also extended to a few fixes due to issue uncovered, they are:
defaultHeadersonClientConfigurationso typing works correctlyreadAuthorizationModelwith options but no model ID setClientBatchCheckoptionsThere is one discussion point I have that I will comment in the PR regarding
References
openfga/sdk-generator#569
Review Checklist
mainSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests