addding interface for AuthenticationHandlerProvider #217
Conversation
WalkthroughThe pull request introduces enhancements to the Varadhi project's authentication system. It includes the addition of a new exported package Changes
Sequence DiagramsequenceDiagram
participant App
participant AuthProvider as AuthenticationProvider
participant UserContext
participant RequestContext
App->>AuthProvider: init(AuthenticationOptions)
AuthProvider-->>App: Future<Boolean>
App->>AuthProvider: authenticate(Org, RequestContext)
AuthProvider-->>App: Future<UserContext>
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
spi/build.gradle(1 hunks)spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationHandlerProvider.java(1 hunks)spi/src/main/java/module-info.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
🔇 Additional comments (2)
spi/src/main/java/module-info.java (1)
7-7: LGTM!The module changes correctly:
- Add the required vertx-web dependency
- Export the new authentication package
Also applies to: 11-11
spi/build.gradle (1)
10-10: LGTM!The vertx-web dependency is correctly added using the same version variable as vertx-core.
| public interface AuthenticationHandlerProvider { | ||
| AuthenticationHandler provideHandler(Vertx vertx, String fileName); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation and error handling specifications.
The interface should include:
- JavaDoc explaining its purpose and usage
- Method documentation specifying:
- Parameter requirements (e.g., file format, path conventions)
- Error conditions and thrown exceptions
- Thread safety guarantees
Example improvement:
+/**
+ * Provider interface for creating authentication handlers.
+ * Implementations should be thread-safe and handle configuration parsing.
+ */
public interface AuthenticationHandlerProvider {
+ /**
+ * Creates an authentication handler using the specified configuration file.
+ *
+ * @param vertx The Vertx instance to use for handler creation
+ * @param fileName Path to the authentication configuration file
+ * @return Configured authentication handler
+ * @throws IllegalArgumentException if the file is invalid or missing
+ * @throws ConfigurationException if the configuration is invalid
+ */
AuthenticationHandler provideHandler(Vertx vertx, String fileName);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public interface AuthenticationHandlerProvider { | |
| AuthenticationHandler provideHandler(Vertx vertx, String fileName); | |
| } | |
| /** | |
| * Provider interface for creating authentication handlers. | |
| * Implementations should be thread-safe and handle configuration parsing. | |
| */ | |
| public interface AuthenticationHandlerProvider { | |
| /** | |
| * Creates an authentication handler using the specified configuration file. | |
| * | |
| * @param vertx The Vertx instance to use for handler creation | |
| * @param fileName Path to the authentication configuration file | |
| * @return Configured authentication handler | |
| * @throws IllegalArgumentException if the file is invalid or missing | |
| * @throws ConfigurationException if the configuration is invalid | |
| */ | |
| AuthenticationHandler provideHandler(Vertx vertx, String fileName); | |
| } |
| import io.vertx.ext.web.handler.AuthenticationHandler; | ||
|
|
||
| public interface AuthenticationHandlerProvider { | ||
| AuthenticationHandler provideHandler(Vertx vertx, String fileName); |
There was a problem hiding this comment.
why this method signature specifically?
which fileName is this?
Why is vertx instance required?
There was a problem hiding this comment.
Also, we now have exposed vertx as the dependency in our spi. Is that correct direction?
There was a problem hiding this comment.
yeah.. better to avoid any specific framework here.
I think earlier also AuthorizationProvider is also using Vertx for its Future. We can relook at it as well.
There was a problem hiding this comment.
- fileName is config file name for configuring specific authentication handler/provider. will update the variable name.
- vertx instance is needed to create AuthenticationProviders, which are embedded in the AuthenticationHandlers for JWT/Oauth2 providers as mentioned here - https://vertx.io/docs/vertx-auth-oauth2/java/
There was a problem hiding this comment.
can't the provider create their own vertx instance?
AuthenticationHandler is just a Handler<RoutingContext>, which is very vertx specific. Any vertx specific AuthenticationHandler implementation should come OOB and should be configurable.
Anything else, we need to strip it down. like our own
VRDAuthenticationHandler::handle(URI, params, headers, Map<String, Object> context) -> Future
this stripped down interface can be part of spi.
Just a thought.
There was a problem hiding this comment.
we will strip down the interface in the spi with basic params.
we will move the instantiation part to the server itself, where instantiation would assume that a handlerProvider class would have a static constructor with the definition same as the interface currently mentioned.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
entities/src/testFixtures/java/com/flipkart/varadhi/entities/TestUser.java (1)
Line range hint
12-32: Enhance test helper to support configurable organization and attributes.The current implementation with fixed empty returns limits testing scenarios. Consider implementing a builder pattern to allow tests to configure these values.
Consider this implementation:
public final class TestUser { + private static final Map<String, String> EMPTY_ATTRIBUTES = Map.of(); + private TestUser() { } - public static UserContext testUser(String name, boolean expired) { + public static TestUserBuilder builder(String name) { + return new TestUserBuilder(name); + } + + public static UserContext testUser(String name, boolean expired) { + return builder(name).expired(expired).build(); + } + + public static class TestUserBuilder { + private final String name; + private boolean expired; + private String org = ""; + private Map<String, String> attributes = EMPTY_ATTRIBUTES; + + private TestUserBuilder(String name) { + this.name = name; + } + + public TestUserBuilder expired(boolean expired) { + this.expired = expired; + return this; + } + + public TestUserBuilder org(String org) { + this.org = org; + return this; + } + + public TestUserBuilder attributes(Map<String, String> attributes) { + this.attributes = new HashMap<>(attributes); + return this; + } + + public UserContext build() { return new UserContext() { @Override public String getSubject() { return name; } @Override public boolean isExpired() { return expired; } @Override public String getOrg() { - return ""; + return org; } @Override public Map<String, String> getAttributes() { - return Map.of(); + return Collections.unmodifiableMap(attributes); } }; + } + } }This allows tests to create users with specific organizations and attributes:
UserContext user = TestUser.builder("test") .expired(false) .org("org1") .attributes(Map.of("role", "admin")) .build();
🧹 Nitpick comments (1)
server/src/main/java/com/flipkart/varadhi/entities/VertxUserContext.java (1)
33-36: Optimize empty map return and consider extracting attributes from Vert.x User.
- Cache the empty map as a constant to avoid creating new instances.
- The Vert.x User might contain attributes that we're not utilizing.
Consider this implementation:
+ private static final Map<String, String> EMPTY_ATTRIBUTES = Map.of(); @Override public Map<String, String> getAttributes() { - return Map.of(); + Map<String, String> userAttributes = extractAttributesFromUser(user); + return userAttributes.isEmpty() ? EMPTY_ATTRIBUTES : Collections.unmodifiableMap(userAttributes); } + + private Map<String, String> extractAttributesFromUser(User user) { + // Extract attributes from Vert.x User if available + return new HashMap<>(); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
entities/src/main/java/com/flipkart/varadhi/entities/auth/UserContext.java(1 hunks)entities/src/testFixtures/java/com/flipkart/varadhi/entities/TestUser.java(2 hunks)server/src/main/java/com/flipkart/varadhi/entities/VertxUserContext.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
🔇 Additional comments (1)
server/src/main/java/com/flipkart/varadhi/entities/VertxUserContext.java (1)
29-31: Consider extracting organization from Vert.x User.The current implementation returns an empty string, but the organization information might be available in the Vert.x User object.
Let's verify if Vert.x User contains organization information:
| String getOrg(); | ||
|
|
||
| Map<String, String> getAttributes(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation for new methods and consider null safety.
The new methods lack documentation explaining their purpose and return value semantics. Additionally, consider null safety:
- Should
getOrg()returnOptional<String>if organization is optional? - Should
getAttributes()return an unmodifiable map to prevent modifications?
Add Javadoc and consider this implementation:
+ /**
+ * Returns the organization identifier associated with this user context.
+ * @return Optional<String> containing the organization identifier, or empty if none exists
+ */
- String getOrg();
+ Optional<String> getOrg();
+ /**
+ * Returns an unmodifiable map of user attributes.
+ * @return Map<String, String> containing user attributes, never null
+ */
- Map<String, String> getAttributes();
+ Map<String, String> getAttributes();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String getOrg(); | |
| Map<String, String> getAttributes(); | |
| /** | |
| * Returns the organization identifier associated with this user context. | |
| * @return the organization identifier, or null if none exists | |
| */ | |
| String getOrg(); | |
| /** | |
| * Returns a map of user attributes. | |
| * @return Map<String, String> containing user attributes, never null | |
| */ | |
| Map<String, String> getAttributes(); | |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #217 +/- ##
============================================
+ Coverage 63.11% 65.38% +2.27%
- Complexity 543 1194 +651
============================================
Files 133 221 +88
Lines 2790 5900 +3110
Branches 168 400 +232
============================================
+ Hits 1761 3858 +2097
- Misses 964 1869 +905
- Partials 65 173 +108 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationOptions.java (1)
5-10: Consider making the class immutable using the builder pattern.Since this class represents configuration options, it would benefit from immutability to prevent accidental modifications after initialization.
Here's a suggested implementation using the builder pattern:
package com.flipkart.varadhi.spi.authn; import com.flipkart.varadhi.spi.ConfigFile; public class AuthenticationOptions { - private String providerClassName; + private final String providerClassName; @ConfigFile - private String configFile; + private final String configFile; + + private AuthenticationOptions(Builder builder) { + this.providerClassName = builder.providerClassName; + this.configFile = builder.configFile; + } + + public String getProviderClassName() { + return providerClassName; + } + + public String getConfigFile() { + return configFile; + } + + public static class Builder { + private String providerClassName; + private String configFile; + + public Builder providerClassName(String providerClassName) { + this.providerClassName = providerClassName; + return this; + } + + public Builder configFile(String configFile) { + this.configFile = configFile; + return this; + } + + public AuthenticationOptions build() { + if (providerClassName == null || providerClassName.isEmpty()) { + throw new IllegalStateException("providerClassName is required"); + } + return new AuthenticationOptions(this); + } + } }spi/src/main/java/com/flipkart/varadhi/spi/authn/VaradhiAuthenticationProvider.java (1)
11-11: Type-safe the context parameter.The
contextparameter usingMap<String, Object>is too generic and could lead to runtime errors. Consider:
- Creating a dedicated context class
- Using an enum for known context keys
Example:
public class AuthenticationContext { private final Map<ContextKey, Object> attributes; public enum ContextKey { REQUEST_ID, TIMESTAMP, CLIENT_IP // ... other known keys } public <T> Optional<T> getAttribute(ContextKey key, Class<T> type) { return Optional.ofNullable(attributes.get(key)) .filter(type::isInstance) .map(type::cast); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationOptions.java(1 hunks)spi/src/main/java/com/flipkart/varadhi/spi/authn/VaradhiAuthenticationProvider.java(1 hunks)
🔇 Additional comments (2)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationOptions.java (1)
8-9: Document the purpose of @configfile annotation.The
@ConfigFileannotation's purpose and implications are not clear from the code.Let's verify the annotation's documentation:
✅ Verification successful
@configfile marks a field containing the path to a YAML configuration file
This annotation marks a String field that specifies the path to an external YAML configuration file. The configuration file is loaded and validated by YamlLoader at runtime.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for documentation of @ConfigFile annotation # and its usage patterns in the codebase echo "Searching for @ConfigFile annotation definition..." rg -A 5 "public.*@interface.*ConfigFile" echo "Searching for @ConfigFile usage patterns..." rg -A 2 "@ConfigFile"Length of output: 1522
Script:
#!/bin/bash # Get the complete ConfigFile annotation definition with comments echo "ConfigFile.java content:" cat spi/src/main/java/com/flipkart/varadhi/spi/ConfigFile.java echo -e "\nSearching for code that processes @ConfigFile annotation..." rg -l "ConfigFile" | xargs rg -l "getAnnotation.*ConfigFile|AnnotatedElement.*ConfigFile"Length of output: 512
Script:
#!/bin/bash # Search for configuration loading patterns and Options usage echo "Searching for Options classes usage and config loading..." rg -A 5 "class \w+Options" echo -e "\nSearching for config loading patterns..." rg -l "load.*[Cc]onfig|read.*[Cc]onfig" | xargs rg -A 5 "load.*[Cc]onfig|read.*[Cc]onfig"Length of output: 78510
spi/src/main/java/com/flipkart/varadhi/spi/authn/VaradhiAuthenticationProvider.java (1)
1-8: LGTM! Clean and well-organized imports.The package structure and imports are appropriate for an SPI interface, following good practices with minimal necessary dependencies.
| public class AuthenticationOptions { | ||
| private String providerClassName; | ||
|
|
||
| @ConfigFile | ||
| private String configFile; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add documentation and proper encapsulation.
The class lacks essential documentation and proper encapsulation. As this is part of the SPI (Service Provider Interface), clear documentation is crucial for implementers.
Consider applying these improvements:
package com.flipkart.varadhi.spi.authn;
import com.flipkart.varadhi.spi.ConfigFile;
+/**
+ * Configuration options for authentication providers.
+ * This class holds the necessary configuration parameters required to initialize
+ * and configure an authentication provider implementation.
+ */
public class AuthenticationOptions {
+ /**
+ * The fully qualified class name of the authentication provider implementation.
+ */
private String providerClassName;
+ /**
+ * Path to the configuration file containing provider-specific settings.
+ */
@ConfigFile
private String configFile;
+ public String getProviderClassName() {
+ return providerClassName;
+ }
+
+ public void setProviderClassName(String providerClassName) {
+ this.providerClassName = providerClassName;
+ }
+
+ public String getConfigFile() {
+ return configFile;
+ }
+
+ public void setConfigFile(String configFile) {
+ this.configFile = configFile;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class AuthenticationOptions { | |
| private String providerClassName; | |
| @ConfigFile | |
| private String configFile; | |
| } | |
| package com.flipkart.varadhi.spi.authn; | |
| import com.flipkart.varadhi.spi.ConfigFile; | |
| /** | |
| * Configuration options for authentication providers. | |
| * This class holds the necessary configuration parameters required to initialize | |
| * and configure an authentication provider implementation. | |
| */ | |
| public class AuthenticationOptions { | |
| /** | |
| * The fully qualified class name of the authentication provider implementation. | |
| */ | |
| private String providerClassName; | |
| /** | |
| * Path to the configuration file containing provider-specific settings. | |
| */ | |
| @ConfigFile | |
| private String configFile; | |
| public String getProviderClassName() { | |
| return providerClassName; | |
| } | |
| public void setProviderClassName(String providerClassName) { | |
| this.providerClassName = providerClassName; | |
| } | |
| public String getConfigFile() { | |
| return configFile; | |
| } | |
| public void setConfigFile(String configFile) { | |
| this.configFile = configFile; | |
| } | |
| } |
| public interface VaradhiAuthenticationProvider { | ||
| Future<Boolean> init(AuthenticationOptions authenticationOptions); | ||
| Future<UserContext> handle(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive JavaDoc documentation.
The interface methods lack documentation explaining:
- Purpose and contract of each method
- Parameter requirements and validation rules
- Error scenarios and their handling
- Expected behavior for edge cases
Example documentation:
/**
* Service Provider Interface for authentication providers in Varadhi.
*/
public interface VaradhiAuthenticationProvider {
/**
* Initializes the authentication provider with the given options.
*
* @param authenticationOptions Configuration options for the provider
* @return Future<Boolean> completing with true if initialization succeeds, false otherwise
* @throws IllegalArgumentException if authenticationOptions is null or invalid
*/
Future<Boolean> init(AuthenticationOptions authenticationOptions);
/**
* Handles an authentication request.
*
* @param uri The request URI
* @param params Query parameters from the request
* @param headers HTTP headers from the request
* @param context Additional context for authentication
* @return Future<UserContext> completing with authenticated user context
* @throws AuthenticationException if authentication fails
* @throws IllegalArgumentException if required parameters are null or invalid
*/
Future<UserContext> handle(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context);
}|
|
||
| public interface VaradhiAuthenticationProvider { | ||
| Future<Boolean> init(AuthenticationOptions authenticationOptions); | ||
| Future<UserContext> handle(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context); |
There was a problem hiding this comment.
💡 Codebase verification
Add documentation for initialization behavior and include cleanup method
The init method needs documentation to clarify:
- Success/failure conditions
- Retry behavior
- Idempotency guarantees
Also add a cleanup method:
/**
* Performs cleanup and releases resources.
*
* @return Future<Void> completing when cleanup is done
*/
Future<Void> close();🔗 Analysis chain
Consider adding validation details for initialization.
The init method should clarify:
- What constitutes a successful initialization
- How to handle failed initialization
- Whether retries are supported
- Whether it's idempotent
Also, consider adding a cleanup method:
+ /**
+ * Performs cleanup and releases resources.
+ *
+ * @return Future<Void> completing when cleanup is done
+ */
+ Future<Void> close();🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find the complete interface definition
ast-grep --pattern 'interface VaradhiAuthenticationProvider {
$$$
}'
# Search for implementations
rg "implements.*VaradhiAuthenticationProvider" -l
Length of output: 124
Script:
#!/bin/bash
# View the complete interface file
cat spi/src/main/java/com/flipkart/varadhi/spi/authn/VaradhiAuthenticationProvider.java
# Search for implementations with a simpler pattern
rg "VaradhiAuthenticationProvider" -l
Length of output: 628
💡 Codebase verification
Interface needs documented error contracts for implementations
The interface currently lacks defined error scenarios and handling requirements. Since it's an authentication provider that returns Future, implementations need clear guidance on:
- What exceptions to throw for specific authentication failures (invalid credentials, expired tokens, etc.)
- How to handle network/infrastructure errors
- Whether to complete the Future exceptionally or return a failed authentication result
Add @throws documentation and define custom exceptions for common authentication scenarios.
🔗 Analysis chain
Verify error handling across implementations.
The interface should define common error scenarios and how they should be handled by implementations.
Run this script to analyze existing implementations:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for implementations to analyze error handling patterns
echo "Searching for implementations..."
rg -l "implements.*VaradhiAuthenticationProvider"
echo "Analyzing error handling patterns..."
ast-grep --pattern 'class $_ implements VaradhiAuthenticationProvider {
$$$
handle($$$) {
try {
$$$
} catch ($$$) {
$$$
}
}
}'
Length of output: 368
Script:
#!/bin/bash
# Broader search for the interface and its usage
echo "Searching for interface mentions..."
rg "VaradhiAuthenticationProvider" -B 2 -A 2
echo -e "\nSearching for implementations with looser pattern..."
ast-grep --pattern 'class $_ implements VaradhiAuthenticationProvider {
$$$
}'
echo -e "\nSearching for imports..."
rg "import.*VaradhiAuthenticationProvider"
Length of output: 1127
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
entities/src/main/java/com/flipkart/varadhi/entities/auth/UserContext.java (1)
20-20: Convert TODO into a tracked issue.The TODO comment indicates planned enhancements to include more authentication context details.
Would you like me to help create a GitHub issue to track this enhancement? This would help ensure the TODO is properly tracked and implemented in future updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
entities/src/main/java/com/flipkart/varadhi/entities/auth/UserContext.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
🔇 Additional comments (3)
entities/src/main/java/com/flipkart/varadhi/entities/auth/UserContext.java (3)
3-4: LGTM!The required imports are correctly added for the new methods.
12-14: Add Javadoc for the getOrg method.The method implementation correctly uses Optional as suggested in the past review, but documentation is still missing.
Add Javadoc to explain the purpose and semantics:
+ /** + * Returns the organization identifier associated with this user context. + * @return Optional<String> containing the organization identifier, or empty if none exists + */ default Optional<String> getOrg() { return Optional.empty(); }
16-18: Add Javadoc for the getAttributes method.The method implementation correctly returns an immutable map via Map.of(), but documentation is still missing.
Add Javadoc to explain the purpose and semantics:
+ /** + * Returns an unmodifiable map of user attributes. + * @return Map<String, String> containing user attributes, never null + */ default Map<String, String> getAttributes() { return Map.of(); }
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
entities/src/main/java/com/flipkart/varadhi/entities/auth/UserContext.java(1 hunks)spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationOptions.java(1 hunks)spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- entities/src/main/java/com/flipkart/varadhi/entities/auth/UserContext.java
- spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationOptions.java
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
| import java.util.Map; | ||
|
|
||
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(AuthenticationOptions authenticationOptions); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance init method contract.
The boolean return type doesn't provide enough context for initialization failures. Consider:
- Adding method documentation
- Using a more descriptive return type
+ /**
+ * Initializes the authentication provider with the given options.
+ * This method is called once during provider initialization.
+ *
+ * @param authenticationOptions configuration options for the provider
+ * @return Future that completes with initialization result
+ * @throws IllegalArgumentException if options are invalid
+ */
- Future<Boolean> init(AuthenticationOptions authenticationOptions);
+ Future<Void> init(AuthenticationOptions authenticationOptions);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Future<Boolean> init(AuthenticationOptions authenticationOptions); | |
| /** | |
| * Initializes the authentication provider with the given options. | |
| * This method is called once during provider initialization. | |
| * | |
| * @param authenticationOptions configuration options for the provider | |
| * @return Future that completes with initialization result | |
| * @throws IllegalArgumentException if options are invalid | |
| */ | |
| Future<Void> init(AuthenticationOptions authenticationOptions); |
|
|
||
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(AuthenticationOptions authenticationOptions); | ||
| Future<UserContext> handle(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve handle method contract and type safety.
The current implementation has several areas for improvement:
- Missing method documentation
- No validation requirements specified
- Using raw Object type in context map
+ /**
+ * Authenticates a request and returns the user context.
+ *
+ * @param uri the request URI
+ * @param params query parameters from the request
+ * @param headers HTTP headers from the request
+ * @param context request-specific context
+ * @return Future that completes with UserContext on successful authentication
+ * @throws AuthenticationException if authentication fails
+ * @throws IllegalArgumentException if required parameters are missing
+ */
- Future<UserContext> handle(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context);
+ Future<UserContext> handle(
+ URI uri,
+ Map<String, String> params,
+ Map<String, String> headers,
+ Map<String, AuthenticationContext> context);Consider creating a dedicated AuthenticationContext class to replace the generic Object type for better type safety and documentation.
Committable suggestion skipped: line range outside the PR's diff.
| import java.net.URI; | ||
| import java.util.Map; | ||
|
|
||
| public interface AuthenticationProvider { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add interface documentation.
Please add Javadoc to describe:
- The purpose and responsibility of this authentication provider
- The lifecycle management expectations
- Thread-safety guarantees
- Error handling contract
+/**
+ * Service provider interface for implementing authentication mechanisms.
+ * Implementations must be thread-safe and handle concurrent requests.
+ * <p>
+ * The lifecycle of an AuthenticationProvider:
+ * 1. Instance created
+ * 2. init() called once
+ * 3. handle() called multiple times concurrently
+ * </p>
+ */
public interface AuthenticationProvider {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public interface AuthenticationProvider { | |
| /** | |
| * Service provider interface for implementing authentication mechanisms. | |
| * Implementations must be thread-safe and handle concurrent requests. | |
| * <p> | |
| * The lifecycle of an AuthenticationProvider: | |
| * 1. Instance created | |
| * 2. init() called once | |
| * 3. handle() called multiple times concurrently | |
| * </p> | |
| */ | |
| public interface AuthenticationProvider { |
| package com.flipkart.varadhi.spi.authn; | ||
|
|
||
| import com.flipkart.varadhi.entities.auth.UserContext; | ||
| import io.vertx.core.Future; | ||
|
|
||
| import java.net.URI; | ||
| import java.util.Map; | ||
|
|
||
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(AuthenticationOptions authenticationOptions); | ||
| Future<UserContext> handle(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Test coverage required for security-critical authentication interface
The verification confirms there are no tests for this newly introduced authentication interface. Given that this is security-critical code handling user authentication, comprehensive test coverage is essential. Please add tests covering all scenarios as originally requested:
- Successful initialization
- Failed initialization
- Successful authentication
- Failed authentication
- Invalid input handling
🔗 Analysis chain
Add test coverage.
The PR objectives indicate 0% test coverage for the changes. Please add comprehensive tests covering:
- Successful initialization
- Failed initialization
- Successful authentication
- Failed authentication
- Invalid input handling
Here's a script to verify current test coverage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check test coverage for AuthenticationProvider implementations
# Find test files for AuthenticationProvider
echo "Looking for test files..."
fd -e java -e kt test.*AuthenticationProvider
# Find implementations to ensure they're tested
echo -e "\nLooking for implementations..."
ast-grep --pattern 'class $_ implements AuthenticationProvider'
Length of output: 253
Script:
#!/bin/bash
# Broader search for test files and implementations
echo "Looking for test files containing AuthenticationProvider..."
rg -l "AuthenticationProvider" -g "**/*Test.java" -g "**/*Tests.java"
echo -e "\nLooking for implementations (checking imports)..."
rg -l "import.*AuthenticationProvider" --type java
echo -e "\nChecking test directories specifically..."
fd . -t f -e java --full-path ".*test.*" | xargs rg -l "AuthenticationProvider"
Length of output: 718
Script:
#!/bin/bash
# Final verification for authentication-related code
echo "Looking for authentication-related test files..."
rg -l "class.*Test.*authentication" -g "*.java" --ignore-case
echo -e "\nChecking for potential implementing classes..."
rg -l "class.*Authentication.*Provider" -g "*.java"
echo -e "\nLooking for test methods related to authentication..."
ast-grep --pattern 'class $_ {
$$$
@Test
$$$
void $_($$$) {
$$$authentication$$$
}
$$$
}'
Length of output: 564
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
🔇 Additional comments (3)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java (3)
1-8: LGTM! Well-structured package and imports.The package placement in the SPI module and the choice of imports are appropriate for an authentication provider interface.
9-9: Add interface documentation.Please add Javadoc to describe:
- The purpose and responsibility of this authentication provider
- The lifecycle management expectations
- Thread-safety guarantees
- Error handling contract
10-10: Enhance init method contract.The boolean return type doesn't provide enough context for initialization failures.
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(AuthenticationOptions authenticationOptions); | ||
| Future<UserContext> authenticate(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Security-critical authentication interface requires test coverage
The verification confirms that there are no tests for the newly added AuthenticationProvider interface. Given that this interface handles user authentication, which is security-critical, comprehensive test coverage is essential to ensure correct behavior and security.
Required test coverage:
- Initialization scenarios (success/failure)
- Authentication scenarios (success/failure)
- Input validation
- Async operation handling
🔗 Analysis chain
Add test coverage for security-critical authentication interface.
The PR objectives indicate 0% test coverage for these changes. Given that this is security-critical code handling user authentication, comprehensive test coverage is essential.
Please add tests covering:
- Successful initialization
- Failed initialization
- Successful authentication
- Failed authentication
- Invalid input handling
Run this script to verify test coverage:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check test coverage for AuthenticationProvider
echo "Looking for test files..."
fd -e java -e kt test.*AuthenticationProvider
echo -e "\nLooking for implementations to ensure they're tested..."
ast-grep --pattern 'class $_ implements AuthenticationProvider'
echo -e "\nChecking test methods..."
ast-grep --pattern 'class $_ {
$$$
@Test
$$$
void $_($$$) {
$$$authentication$$$
}
$$$
}'
Length of output: 479
Script:
#!/bin/bash
# Broader search for authentication-related tests and implementations
echo "Looking for auth-related test files..."
fd -e java -e kt -t f "test" | grep -i "auth"
echo -e "\nLooking for implementation files..."
fd -e java -e kt -t f "src" | grep -i "auth"
echo -e "\nChecking for authentication-related test methods..."
ast-grep --pattern '@Test
void $_($$$) {
$$$
}'
echo -e "\nLooking for auth-related classes..."
ast-grep --pattern 'class $_ {
$$$
authenticate($$$) {
$$$
}
$$$
}'
Length of output: 66394
Script:
#!/bin/bash
# Search specifically for authentication-related files and tests
echo "Looking for authentication files in spi module..."
fd -e java -t f . spi/src | grep -i "auth"
echo -e "\nLooking for authentication test files..."
fd -e java -t f "test" | grep -i "authentication"
echo -e "\nLooking for AuthenticationProvider implementations..."
ast-grep --pattern 'class $_ implements AuthenticationProvider'
echo -e "\nChecking for authentication-related test methods..."
ast-grep --pattern '@Test
void $_($$$) {
$$$authentication$$$
}'
Length of output: 977
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
🔇 Additional comments (3)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java (3)
1-9: LGTM!Package structure and imports are well-organized and appropriate for an SPI module.
10-10: Add interface documentation.Please add Javadoc to describe:
- The purpose and responsibility of this authentication provider
- The lifecycle management expectations
- Thread-safety guarantees
- Error handling contract
10-13: Security-critical authentication interface requires test coverage.The verification confirms there are no tests for this newly introduced authentication interface. Given that this is security-critical code handling user authentication, comprehensive test coverage is essential.
Required test coverage:
- Initialization scenarios (success/failure)
- Authentication scenarios (success/failure)
- Input validation
- Async operation handling
|
|
||
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(ConfigFileResolver resolver, AuthenticationOptions authenticationOptions); | ||
| Future<UserContext> authenticate(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve authentication method contract and type safety.
The method needs several improvements:
- Method documentation
- Type-safe context map
- Validation requirements
- Error handling specifications
+ /**
+ * Authenticates a request and returns the user context.
+ *
+ * @param uri the request URI to authenticate
+ * @param params query parameters from the request
+ * @param headers HTTP headers from the request
+ * @param context request-specific context
+ * @return Future that completes with UserContext on successful authentication
+ * @throws AuthenticationException if authentication fails
+ * @throws IllegalArgumentException if required parameters are missing
+ */
- Future<UserContext> authenticate(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, Object> context);
+ Future<UserContext> authenticate(URI uri, Map<String,String> params, Map<String,String> headers, Map<String, AuthenticationContext> context);Consider creating a dedicated context type:
/**
* Represents the context for an authentication request.
*/
public interface AuthenticationContext {
String getClientId();
String getRequestId();
// Add other relevant context fields
}| import java.util.Map; | ||
|
|
||
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(ConfigFileResolver resolver, AuthenticationOptions authenticationOptions); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve initialization method contract.
The method needs several improvements:
- Method documentation describing parameters, return values, and exceptions
- Consider using a more descriptive return type than boolean
- Add validation requirements for the input parameters
+ /**
+ * Initializes the authentication provider with the given configuration.
+ * This method is called once during provider initialization.
+ *
+ * @param resolver resolver for loading configuration files
+ * @param authenticationOptions configuration options for the provider
+ * @return Future that completes successfully if initialization succeeds,
+ * or fails with an appropriate exception if initialization fails
+ * @throws IllegalArgumentException if options are invalid
+ * @throws ConfigurationException if configuration loading fails
+ */
Future<Boolean> init(ConfigFileResolver resolver, AuthenticationOptions authenticationOptions);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Future<Boolean> init(ConfigFileResolver resolver, AuthenticationOptions authenticationOptions); | |
| /** | |
| * Initializes the authentication provider with the given configuration. | |
| * This method is called once during provider initialization. | |
| * | |
| * @param resolver resolver for loading configuration files | |
| * @param authenticationOptions configuration options for the provider | |
| * @return Future that completes successfully if initialization succeeds, | |
| * or fails with an appropriate exception if initialization fails | |
| * @throws IllegalArgumentException if options are invalid | |
| * @throws ConfigurationException if configuration loading fails | |
| */ | |
| Future<Boolean> init(ConfigFileResolver resolver, AuthenticationOptions authenticationOptions); |
| public interface UserContext { | ||
|
|
||
| String getSubject(); | ||
|
|
||
| boolean isExpired(); | ||
|
|
||
| // TODO: enhance it to include more details like context around the authentication | ||
| default Optional<String> getOrg() { |
There was a problem hiding this comment.
what does it mean for the user to have Org?
There was a problem hiding this comment.
org here is associated with the tenants of AuthN. org/tenants have been used interchangeably by authN.
for varadhi also, cross-org resource access should not be allowed.
| import java.util.Map; | ||
|
|
||
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(ConfigFileResolver resolver, AuthenticationOptions authenticationOptions); |
There was a problem hiding this comment.
for now I think we can omit the resolver parameter. Why is it needed?
There was a problem hiding this comment.
yes. this can be removed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java (2)
8-8: 🛠️ Refactor suggestionAdd interface documentation.
Please add Javadoc to describe:
- The purpose and responsibility of this authentication provider
- The lifecycle management expectations
- Thread-safety guarantees
- Error handling contract
9-9: 🛠️ Refactor suggestionEnhance init method contract.
The boolean return type doesn't provide enough context for initialization failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
entities/src/main/java/com/flipkart/varadhi/entities/utils/RequestContext.java(1 hunks)spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
🔇 Additional comments (1)
spi/src/main/java/com/flipkart/varadhi/spi/authn/AuthenticationProvider.java (1)
8-11: Security-critical authentication interface requires test coverage.The verification confirms that there are no tests for this newly added interface. Given that this interface handles user authentication, which is security-critical, comprehensive test coverage is essential.
| public interface RequestContext { | ||
|
|
||
| URI getURI(); | ||
|
|
||
| Map<String, String> getParams(); | ||
|
|
||
| Map<String, String> getHeaders(); | ||
|
|
||
| Map<String, Object> getContext(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add interface documentation and improve type safety.
- Add Javadoc to describe:
- Purpose and responsibility of this interface
- Contract for each method
- Thread-safety guarantees
- Consider using a type-safe context map
Apply this diff:
+/**
+ * Represents the context of an HTTP request, providing access to URI, parameters,
+ * headers, and additional context information.
+ * <p>
+ * Implementations must be thread-safe.
+ */
public interface RequestContext {
+ /**
+ * @return the URI of the request
+ */
URI getURI();
+ /**
+ * @return unmodifiable map of query parameters
+ */
Map<String, String> getParams();
+ /**
+ * @return unmodifiable map of HTTP headers
+ */
Map<String, String> getHeaders();
- Map<String, Object> getContext();
+ /**
+ * @return unmodifiable map of typed context attributes
+ */
+ Map<String, RequestAttribute> getContext();
}
+
+/**
+ * Type-safe container for request context attributes.
+ */
+public interface RequestAttribute {
+ String getType();
+ <T> T getValue(Class<T> type);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public interface RequestContext { | |
| URI getURI(); | |
| Map<String, String> getParams(); | |
| Map<String, String> getHeaders(); | |
| Map<String, Object> getContext(); | |
| } | |
| /** | |
| * Represents the context of an HTTP request, providing access to URI, parameters, | |
| * headers, and additional context information. | |
| * <p> | |
| * Implementations must be thread-safe. | |
| */ | |
| public interface RequestContext { | |
| /** | |
| * @return the URI of the request | |
| */ | |
| URI getURI(); | |
| /** | |
| * @return unmodifiable map of query parameters | |
| */ | |
| Map<String, String> getParams(); | |
| /** | |
| * @return unmodifiable map of HTTP headers | |
| */ | |
| Map<String, String> getHeaders(); | |
| /** | |
| * @return unmodifiable map of typed context attributes | |
| */ | |
| Map<String, RequestAttribute> getContext(); | |
| } | |
| /** | |
| * Type-safe container for request context attributes. | |
| */ | |
| public interface RequestAttribute { | |
| String getType(); | |
| <T> T getValue(Class<T> type); | |
| } |
|
|
||
| public interface AuthenticationProvider { | ||
| Future<Boolean> init(AuthenticationOptions authenticationOptions); | ||
| Future<UserContext> authenticate(Org org, RequestContext requestContext); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve authenticate method contract.
Add method documentation specifying:
- Parameter validation requirements
- Error handling expectations
- Thread-safety guarantees
Apply this diff:
+ /**
+ * Authenticates a request in the context of an organization.
+ *
+ * @param org organization context for the authentication
+ * @param requestContext context containing request details
+ * @return Future that completes with UserContext on successful authentication
+ * @throws IllegalArgumentException if org or requestContext is null
+ * @throws AuthenticationException if authentication fails
+ */
Future<UserContext> authenticate(Org org, RequestContext requestContext);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Future<UserContext> authenticate(Org org, RequestContext requestContext); | |
| /** | |
| * Authenticates a request in the context of an organization. | |
| * | |
| * @param org organization context for the authentication | |
| * @param requestContext context containing request details | |
| * @return Future that completes with UserContext on successful authentication | |
| * @throws IllegalArgumentException if org or requestContext is null | |
| * @throws AuthenticationException if authentication fails | |
| */ | |
| Future<UserContext> authenticate(Org org, RequestContext requestContext); |
entities/src/main/java/com/flipkart/varadhi/entities/utils/RequestContext.java
Outdated
Show resolved
Hide resolved
entities/src/main/java/com/flipkart/varadhi/entities/utils/RequestContext.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
entities/src/main/java/com/flipkart/varadhi/entities/utils/RequestContext.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: call-workflow-docker-build / build
- GitHub Check: build
🔇 Additional comments (1)
entities/src/main/java/com/flipkart/varadhi/entities/utils/RequestContext.java (1)
1-7: LGTM!Package location and imports are appropriate for this utility class.
| @Data | ||
| public class RequestContext { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making the class immutable and adding documentation.
The class holds request context that should be immutable after creation to prevent unintended modifications during request processing.
Apply this diff:
+/**
+ * Represents the context of an HTTP request, providing access to URI, parameters,
+ * headers, and additional context information.
+ * <p>
+ * This class is immutable and thread-safe.
+ */
-@Data
+@Value
public class RequestContext {Note: @Value is Lombok's immutable variant of @Data that makes all fields private and final.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Data | |
| public class RequestContext { | |
| /** | |
| * Represents the context of an HTTP request, providing access to URI, parameters, | |
| * headers, and additional context information. | |
| * <p> | |
| * This class is immutable and thread-safe. | |
| */ | |
| @Value | |
| public class RequestContext { |
| private URI uri; | ||
| private Map<String, String> params; | ||
| private Map<String, String> headers; | ||
| private Map<String, Object> context; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve field declarations for better type safety and immutability.
The current implementation has several potential issues:
- Mutable maps could be modified after creation
- Raw Object type in context map is not type-safe
- HTTP headers should be case-insensitive
Apply this diff:
- private URI uri;
- private Map<String, String> params;
- private Map<String, String> headers;
- private Map<String, Object> context;
+ private final URI uri;
+ private final Map<String, String> params;
+ private final TreeMap<String, String> headers; // Use TreeMap with String.CASE_INSENSITIVE_ORDER
+ private final Map<String, RequestAttribute<?>> context;
+
+ public RequestContext(URI uri,
+ Map<String, String> params,
+ Map<String, String> headers,
+ Map<String, RequestAttribute<?>> context) {
+ this.uri = uri;
+ this.params = Map.copyOf(params); // Create immutable copy
+ this.headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
+ this.headers.putAll(headers);
+ this.context = Map.copyOf(context);
+ }
+
+ /**
+ * Type-safe container for request context attributes.
+ * @param <T> The type of the attribute value
+ */
+ public interface RequestAttribute<T> {
+ String getKey();
+ Class<T> getType();
+ T getValue();
+ }This improves the implementation by:
- Making fields final and creating immutable copies of maps
- Using case-insensitive TreeMap for headers
- Adding type-safe RequestAttribute interface
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private URI uri; | |
| private Map<String, String> params; | |
| private Map<String, String> headers; | |
| private Map<String, Object> context; | |
| private final URI uri; | |
| private final Map<String, String> params; | |
| private final TreeMap<String, String> headers; // Use TreeMap with String.CASE_INSENSITIVE_ORDER | |
| private final Map<String, RequestAttribute<?>> context; | |
| public RequestContext(URI uri, | |
| Map<String, String> params, | |
| Map<String, String> headers, | |
| Map<String, RequestAttribute<?>> context) { | |
| this.uri = uri; | |
| this.params = Map.copyOf(params); // Create immutable copy | |
| this.headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); | |
| this.headers.putAll(headers); | |
| this.context = Map.copyOf(context); | |
| } | |
| /** | |
| * Type-safe container for request context attributes. | |
| * @param <T> The type of the attribute value | |
| */ | |
| public interface RequestAttribute<T> { | |
| String getKey(); | |
| Class<T> getType(); | |
| T getValue(); | |
| } |
Summary by CodeRabbit
New Features
AuthenticationOptionsto manage authentication configurations.RequestContextfor structured access to request components.Improvements