-
Notifications
You must be signed in to change notification settings - Fork 328
[fit] Add comprehensive HTTP client authentication support with @RequestAuth annotation #330
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes critical architecture issue where static auth appliers and parameter appliers were mixed in the same list, causing NullPointerException and parameter count mismatch errors. **Root Cause Analysis:** 1. NullPointerException: args=null for no-parameter methods 2. Count mismatch: staticAppliers + paramAppliers != args.length 3. Architecture flaw: mixed responsibilities in single applier list **Solution - Separated Applier Architecture:** - HttpInfo: Added staticAppliers and paramAppliers fields - HttpInvocationHandler: Separated execution (static first, then param) - AnnotationParser: Separated applier construction logic - Added null-safety for args parameter **Test Results:** ✅ No-parameter methods: testBearerStatic() now works ✅ Parameter methods: testBearerDynamic(token) now works ✅ All auth types: Bearer, Basic, API Key all functional ✅ Backward compatibility: existing appliers field maintained Closes #328 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add comprehensive unit tests for HttpInvocationHandler to cover the core issues discovered and fixed in the HTTP client authentication system: - Test null args handling to prevent NullPointerException - Test parameter count mismatch detection - Test static and parameter applier separation logic - Test HTTP info missing exception handling - Test multi-parameter method invocation Additionally improve code style consistency by: - Add this. prefix to all member variable accesses - Add proper resource management for MockitoAnnotations.openMocks() - Add explanatory comments for uncommon Mockito patterns These tests fill critical coverage gaps in the HttpInvocationHandler which previously had no direct unit tests, ensuring the parameter processing bugs discovered cannot reoccur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Clean up the temporary backward-compatibility code by removing the unused appliers field, which was replaced by staticAppliers and paramAppliers. **Changes:** - HttpInfo: Remove appliers field and its getter/setter methods - AnnotationParser.parseMethod(): Directly set paramAppliers without temp variable - AnnotationParser.parseInterface(): Remove backward-compatibility merge logic - HttpInvocationHandlerTest: Remove appliers initialization from test setup **Benefits:** - Clearer separation of concerns between static and parameter appliers - Reduced code redundancy and maintenance burden - Improved code readability with more explicit variable names **Testing:** ✅ All 7 HttpInvocationHandler tests pass ✅ No functional changes, purely structural cleanup 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…er mechanism Unify the authentication architecture by making the @requestauth annotation system reuse the existing AuthorizationDestinationSetter and Authorization mechanisms, ensuring consistency with the FEL Tool system. **Architecture Improvements:** - Remove AuthDestinationSetter (duplicate implementation) - Modify StaticAuthApplier to directly create Authorization objects - Modify RequestAuthResolver to use AuthorizationDestinationSetter - Both annotation-based and JSON-based systems now use the same mechanism **Changes:** - StaticAuthApplier: Create Authorization objects directly via factory methods - RequestAuthResolver: Return AuthorizationDestinationSetter with appropriate keys - Remove AuthDestinationSetter and its test file - Update RequestAuthResolverTest to use AuthorizationDestinationSetter **Benefits:** - Eliminates code duplication in authentication logic - Ensures architectural consistency across different configuration methods - Simplifies maintenance with a single source of truth - Better follows the DestinationSetter design pattern **Testing:** ✅ All 225 tests pass ✅ RequestAuthResolverTest: 3/3 tests pass ✅ HttpInvocationHandlerTest: 7/7 tests pass ✅ No functional changes, purely architectural refactoring 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fix runtime error when using @requestauth with AuthProvider in class/method level static authentication scenarios. **Problem:** - StaticAuthApplier threw UnsupportedOperationException when encountering AuthProvider - Example 07 failed to start with "AuthProvider is not supported" error - Provider-based auth (DynamicTokenProvider, CustomSignatureProvider, ApiKeyProvider) was not working in static contexts **Solution:** - Add lazy initialization support to StaticAuthApplier - Add setBeanContainer() method to inject BeanContainer at runtime - Modify HttpInvocationHandler to call setBeanContainer() before applying static auth - Cache Authorization object after creation for performance **Implementation:** - StaticAuthApplier: Store RequestAuth annotation and delay Authorization creation - If no Provider: Create Authorization immediately in constructor - If Provider used: Create Authorization when setBeanContainer() is called - HttpInvocationHandler: Inject BeanContainer before calling staticApplier.apply() **Testing:** ✅ All 225 tests pass ✅ HttpInvocationHandlerTest: 7/7 tests pass ✅ No breaking changes to existing functionality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fix a bug in parameter-level authentication where API_KEY incorrectly mapped to annotation.name() instead of the "value" field of ApiKeyAuthorization. **Problem Analysis:** 1. Bug: RequestAuthResolver.getAuthorizationKey() for API_KEY returned annotation.name() (e.g., "X-API-Key"), but should return "value" to update ApiKeyAuthorization.value field 2. Root cause: Confusion between HTTP key name and Authorization object field name 3. Why tests didn't catch it: Tests only verified Setter type, not the internal field name **Solution: Introduce AuthFieldMapper (Approach 5)** - Create dedicated AuthFieldMapper class for centralized field mapping logic - Clear documentation explaining the mapping rationale - Maintains consistency with FEL Tool system's AuthorizationDestinationSetter mechanism **Changes:** 1. New file: AuthFieldMapper.java - Maps AuthType to Authorization object field names - Comprehensive Javadoc with table showing all mappings - BEARER → "token", BASIC → "username", API_KEY → "value" 2. Modified: RequestAuthResolver.java - Remove complex getAuthorizationKey() method - Use AuthFieldMapper.getParameterAuthField() for clear intent - Updated documentation 3. New tests: AuthFieldMapperTest.java (5 tests) - Verify correct field mappings for all auth types - Specifically test API_KEY returns "value" not "key" - Validate compatibility with Authorization implementations 4. Enhanced: RequestAuthResolverTest.java - Add comments explaining the bug that was found - Tests now serve as regression prevention **Key Insights:** - API Key has two concepts: * "key" field: HTTP Header/Query name (e.g., "X-API-Key") * "value" field: Actual API key value (parameter-level updates this) - AuthFieldMapper makes this distinction explicit and well-documented **Testing:** ✅ All 230 tests pass (5 new tests added) ✅ AuthFieldMapperTest: 5/5 tests pass ✅ RequestAuthResolverTest: 3/3 tests pass ✅ Maintains 100% consistency with FEL Tool system 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fix Javadoc compilation errors caused by HTML5 incompatible tags and attributes. **Issues Fixed:** 1. Replace <h3> headings with <p><b> for consistency (H3 requires H2, H1 hierarchy) 2. Remove 'summary' attribute from table (not supported in HTML5) 3. Keep 'caption' element for table description **Changes:** - AuthFieldMapper.java: Replace h3 tags with p+b tags, remove summary attribute - RequestAuthResolver.java: Replace h3 tags with p+b tags **Verification:** ✅ mvn javadoc:javadoc compiles successfully ✅ All documentation formatting preserved ✅ HTML5 compliant 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…eter-level authentication Enable parameter-level BASIC authentication to specify which field to update (username or password) using the name attribute, allowing dual-parameter authentication scenarios. **Core Changes:** 1. **AuthFieldMapper.java** - Modified getParameterAuthField() to accept nameAttribute parameter - BASIC auth: name="username" or name="password" selects field to update - Default behavior: returns "username" when name is not specified (backward compatible) - API_KEY: name specifies HTTP Header/Query name (different semantic) - BEARER: name attribute is ignored - Use StringUtils.isNotBlank() and StringUtils.equals() for string operations - Fixed Javadoc line lengths to comply with 120-character limit 2. **RequestAuthResolver.java** - Pass annotation.name() to AuthFieldMapper.getParameterAuthField() - Enables BASIC auth field selection based on name attribute 3. **RequestAuth annotation** - Enhanced Javadoc to document name attribute semantics for different auth types - Added table showing name attribute meaning per auth type - Clarified BASIC auth usage: name selects target field (username/password) **Testing:** 4. **AuthFieldMapperTest.java** (added 3 new tests, total 8 tests) - testBasicAuthFieldDefault(): Verify default returns "username" - testBasicAuthFieldExplicitUsername(): Test name="username" - testBasicAuthFieldPassword(): Test name="password" - testBasicAuthFieldInvalidName(): Verify invalid name throws exception - Updated existing tests to use new method signature 5. **Example 07 Enhancements** - TestAuthInterface: Added testBasicDynamicUsername() and testBasicDynamicBoth() - TestAuthClient: Implemented both methods with proper annotations - TestAuthServerController: Added corresponding endpoints - TestClientController: Added method invocation support - run_tests.sh: Added two new BASIC auth test cases **Usage Examples:** ```java // Method-level: Complete BASIC auth, parameter overrides username @requestauth(type = BASIC, username = "static-user", password = "static-password") String test(@requestauth(type = BASIC) String username); // Parameter-level: Separately override username and password @requestauth(type = BASIC, username = "base-user", password = "base-password") String test( @requestauth(type = BASIC, name = "username") String user, @requestauth(type = BASIC, name = "password") String pwd ); ``` **Key Design:** - name attribute has semantic overloading across auth types - BASIC: Specifies Authorization object field name - API_KEY: Specifies HTTP Header/Query name - Method-level BASIC auth must provide complete username+password - Parameter-level updates specific fields via authorizationInfo() **Validation:** ✅ All 233 tests pass ✅ Javadoc compiles without errors/warnings ✅ Code formatting complies with CodeFormatterFromIdea.xml ✅ Example 07 compiles and runs successfully 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…or BeanContainer Remove the setBeanContainer() method and lazy initialization pattern in StaticAuthApplier, replacing it with direct constructor injection using the BeanContainer already available in AnnotationParser. **Problem:** - StaticAuthApplier used lazy initialization with setBeanContainer() called from HttpInvocationHandler - This created unnecessary complexity and runtime dependency injection - AnnotationParser already had BeanContainer available but wasn't using it - The setBeanContainer() approach required runtime checks and state management **Solution:** - Modified StaticAuthApplier constructor to accept BeanContainer parameter - AnnotationParser now passes its BeanContainer when creating StaticAuthApplier instances - Removed setBeanContainer() method entirely - Removed lazy initialization logic and state management - Simplified HttpInvocationHandler by removing BeanContainer injection code **Changes:** 1. **StaticAuthApplier.java** - Changed constructor: `StaticAuthApplier(RequestAuth, BeanContainer)` - Removed `cachedAuthorization` field, replaced with final `authorization` field - Authorization created immediately in constructor - Removed `setBeanContainer()` method - Simplified `apply()` method - no more null checks - Reduced from 118 lines to 95 lines 2. **AnnotationParser.java** - `getClassLevelAuthAppliers()`: Pass `this.beanContainer` to StaticAuthApplier - `getMethodLevelAuthAppliers()`: Pass `this.beanContainer` to StaticAuthApplier 3. **HttpInvocationHandler.java** - Removed instanceof check and setBeanContainer() call - Simplified staticApplier loop back to basic iteration **Benefits:** - ✅ Simpler design: Constructor injection instead of setter injection - ✅ Earlier error detection: Failures happen at construction time, not runtime - ✅ Immutability: Authorization is now final, thread-safe - ✅ Less code: Removed 31 lines of complexity - ✅ Clear dependencies: BeanContainer dependency explicit in constructor - ✅ Better performance: No runtime type checks or conditional initialization **Testing:** ✅ All 233 tests pass ✅ Example 07 compiles and runs successfully ✅ No behavioral changes, purely refactoring 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…lier Add notNull validation at constructor entry point and simplify createAuthorizationFromAnnotation by removing redundant null check, following the fail-fast principle. **Changes:** - Add `notNull(beanContainer, "The bean container cannot be null.")` in constructor - Remove redundant null check in createAuthorizationFromAnnotation method - BeanContainer is guaranteed non-null by AnnotationParser, validate early at entry point **Benefits:** - ✅ Fail-fast: Errors detected immediately at construction time - ✅ Clearer contract: BeanContainer is required, not optional - ✅ Simplified logic: No need for null check in private method - ✅ Consistent validation: Follows same pattern as AnnotationParser **Testing:** ✅ AuthFieldMapperTest: 8/8 tests pass ✅ No behavioral changes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add detailed usage manual and principles guide for @requestauth annotation system, covering all authentication types, usage scenarios, design principles, and internals. **Documentation Structure:** 1. **HTTP_CLIENT_AUTH_USAGE.md** - User-facing usage manual - Quick start guide - Authentication types (Bearer, Basic, API Key, Custom Provider) - Usage scenarios (interface/method/parameter levels) - Best practices (security, configuration, Provider patterns) - Common Q&A - Complete examples with HTTP request formats 2. **HTTP_CLIENT_AUTH_PRINCIPLES.md** - Internal principles guide - Architecture overview with component diagrams - Core components (AnnotationParser, StaticAuthApplier, AuthFieldMapper, etc.) - Detailed workflow with execution traces - Key design decisions (constructor injection, name attribute overloading) - Consistency with FEL Tool system - Extension guide for custom authentication types **Content Highlights:** - 📖 80+ code examples covering all scenarios - 🎯 5 detailed execution flow diagrams - 📊 Component responsibility matrix - 🔧 Extension patterns for custom authentication - ⚖️ Design decision rationales - ✅ Best practices and anti-patterns **Coverage:** - All 4 authentication types: Bearer, Basic, API Key, Custom - 3-level configuration: Interface, Method, Parameter - Static vs dynamic (Provider) authentication - Field update mechanism for parameter-level auth - BASIC auth dual-parameter support with name attribute - Integration with BeanContainer and FEL Tool system **Target Audience:** - Usage Manual: Developers using HTTP client authentication - Principles Guide: Framework developers and contributors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔗 相关问题 / Related Issue #328
Issue 链接 / Issue Link: 内部需求,修复 HTTP 客户端认证功能的多个 Bug 并完善功能
📋 变更类型 / Type of Change
📝 变更目的 / Purpose of the Change
本 PR 为 FIT HTTP 客户端添加了完整的认证功能支持,修复了多个 Bug,并通过架构重构提升了代码质量和可维护性。
主要目标:
📋 主要变更 / Brief Changelog
🐛 Bug 修复
修复参数级别认证匹配错误 (commit: 0b3b975)
修复 API_KEY 字段映射 Bug (commit: 992326e)
添加 AuthProvider 支持 (commit: 389418f)
✨ 新功能
@RequestAuth(type=BASIC, name="username")或name="password"🔧 重构改进
移除冗余 appliers 字段 (commit: 17eddc7)
重构复用 AuthorizationDestinationSetter (commit: 7a02671)
简化 StaticAuthApplier (commit: 570595e)
添加 Fail-fast 验证 (commit: 6a2bc45)
📚 文档
修复 Javadoc 错误 (commit: 9c9aa93)
添加完整文档 (commit: d3c7137)
🧪 测试
🧪 验证变更 / Verifying this Change
测试步骤 / Test Steps
编译检查
运行所有测试
验证 Example 07
运行测试脚本
cd examples/fit-example/07-http-client-proxy ./run_tests.sh basic测试覆盖 / Test Coverage
测试结果
✅ 贡献者检查清单 / Contributor Checklist
基本要求 / Basic Requirements:
代码质量 / Code Quality:
测试要求 / Testing Requirements:
mvn -B clean package -Dmaven.test.skip=true/ Basic checks passmvn clean install/ Unit tests pass文档和兼容性 / Documentation and Compatibility:
📋 附加信息 / Additional Notes
架构改进亮点
与 FEL Tool 系统一致
简化依赖注入
字段映射清晰化
代码量减少
文档亮点
已知限制
Provider 的刷新时机
BASIC 认证的参数级别限制
后续计划
审查者注意事项 / Reviewer Notes:
重点关注:
测试验证:
文档审查:
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]