Add proxy configuration support #33
Conversation
- Add proxy configuration properties with host, port, and authentication options - Implement proxy configuration for HTTP/HTTPS requests - Add validation for proxy configuration - Add unit tests for proxy configuration - Update documentation with proxy configuration examples Closes #30
There was a problem hiding this comment.
Pull Request Overview
This PR adds proxy configuration support to the Spring AI Client Library, enabling usage in corporate environments with restricted internet access. The implementation includes comprehensive configuration options, validation, and documentation.
- Added proxy configuration properties with authentication support for OpenAI API requests
- Implemented proxy setup using Java system properties with proper validation
- Added comprehensive test coverage and documentation for proxy configuration
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/digitalsanctuary/springaiclient/adapters/openai/config/OpenAIConfigProperties.java |
Added ProxyConfig inner class with proxy settings properties |
src/main/java/com/digitalsanctuary/springaiclient/adapters/openai/config/OpenAIConfig.java |
Implemented proxy configuration logic with validation and system property setup |
src/test/java/com/digitalsanctuary/springaiclient/adapters/openai/config/OpenAIConfigProxyTest.java |
Added comprehensive unit tests for proxy configuration |
src/main/resources/config/dsspringaiconfig.properties |
Added default proxy configuration properties |
src/test/resources/application-test.yml |
Added test proxy configuration for unit tests |
README.md |
Updated documentation with proxy configuration examples and usage guidance |
gradle/wrapper/gradle-wrapper.properties |
Updated Gradle wrapper version from 8.12.1 to 8.13 |
Comments suppressed due to low confidence (1)
src/test/java/com/digitalsanctuary/springaiclient/adapters/openai/config/OpenAIConfigProxyTest.java:69
- [nitpick] The method name 'testExtraProxyConfigurationValues' is unclear. Based on the test content, 'testProxyConfigurationSettersAndGetters' or 'testProxyConfigurationDefaults' would be more descriptive.
// We'll just directly test the proxy configuration values
| return new java.net.PasswordAuthentication( | ||
| properties.getProxy().getUsername(), | ||
| properties.getProxy().getPassword().toCharArray() |
There was a problem hiding this comment.
The password could be null even when username is not null and not empty, which would cause a NullPointerException. The validation only checks if password is null or empty when username is provided, but this check happens after the validation.
| return new java.net.PasswordAuthentication( | |
| properties.getProxy().getUsername(), | |
| properties.getProxy().getPassword().toCharArray() | |
| String password = properties.getProxy().getPassword(); | |
| if (password == null) { | |
| log.error("Proxy password is null for user: {}", properties.getProxy().getUsername()); | |
| throw new IllegalStateException("Proxy password cannot be null when username is provided"); | |
| } | |
| return new java.net.PasswordAuthentication( | |
| properties.getProxy().getUsername(), | |
| password.toCharArray() |
Copilot uses AI. Check for mistakes.
| log.info("Configuring proxy for OpenAI client: {}:{}", properties.getProxy().getHost(), properties.getProxy().getPort()); | ||
|
|
||
| // Configure proxy settings using system properties | ||
| System.setProperty("http.proxyHost", properties.getProxy().getHost()); |
There was a problem hiding this comment.
Setting global system properties for proxy configuration affects the entire JVM and could interfere with other components. Consider using a more scoped approach like configuring the RestClient with a custom ClientHttpRequestFactory that handles proxy settings.
Copilot uses AI. Check for mistakes.
| host: test-proxy.example.com | ||
| port: 8080 | ||
| username: testuser | ||
| password: testpass |
There was a problem hiding this comment.
[nitpick] Hardcoded passwords in test configuration files could be a security concern if they resemble real credentials. Consider using obviously fake values like 'test-password-123' to make it clear these are test-only values.
| password: testpass | |
| password: test-password-123 |
Copilot uses AI. Check for mistakes.
Summary
Test plan
Closes #30
🤖 Generated with Claude Code