Add support for aggregator API keys #17
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for sending API keys to the subscription proxy of aggregators, enabling authentication for protected endpoints. It also introduces new exception types for handling authorization and rate limiting errors.
Key changes:
- Adds API key support to AggregatorClient with optional authentication
- Introduces UnauthorizedException and RateLimitExceededException for error handling
- Updates JsonRpcHttpTransport to handle API key headers and HTTP error responses
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| TestApiKeyIntegration.java | Comprehensive test suite for API key authentication scenarios |
| MockAggregatorServer.java | Test mock server with API key validation and rate limiting simulation |
| UnauthorizedException.java | New exception for HTTP 401 unauthorized errors |
| RateLimitExceededException.java | New exception for HTTP 429 rate limit errors with retry information |
| JsonRpcHttpTransport.java | Enhanced transport layer with API key support and error handling |
| AggregatorClient.java | Updated client with optional API key constructor and authentication |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,20 @@ | |||
| package org.unicitylabs.sdk.jsonrpc; | |||
|
|
|||
There was a problem hiding this comment.
The RateLimitExceededException class is missing a Javadoc comment. Add documentation explaining when this exception is thrown and the purpose of the retryAfterSeconds field.
| /** | |
| * Exception thrown when a rate limit is exceeded in the API. | |
| * <p> | |
| * The {@code retryAfterSeconds} field indicates the number of seconds | |
| * the client should wait before retrying the request. | |
| */ |
Copilot uses AI. Check for mistakes.
| private final Set<String> protectedMethods; | ||
| private volatile boolean simulateRateLimit = false; | ||
| private volatile int rateLimitRetryAfter = 60; | ||
| private volatile String expectedApiKey = null; |
There was a problem hiding this comment.
The simulateRateLimit flag is reset after one use but rateLimitRetryAfter persists. Consider resetting rateLimitRetryAfter to its default value when simulateRateLimit is reset to avoid unexpected behavior in subsequent tests.
| private volatile String expectedApiKey = null; | |
| private volatile String expectedApiKey = null; | |
| /** | |
| * Resets rate limit simulation flags to their default values. | |
| */ | |
| public void resetRateLimitSimulation() { | |
| this.simulateRateLimit = false; | |
| this.rateLimitRetryAfter = 60; | |
| } |
Copilot uses AI. Check for mistakes.
| } else if (response.code() == HTTP_TOO_MANY_REQUESTS) { | ||
| int retryAfterSeconds = extractRetryAfterSeconds(response); | ||
| future.completeExceptionally(new RateLimitExceededException( | ||
| "Rate limit exceeded. Please retry after " + retryAfterSeconds + " seconds", |
There was a problem hiding this comment.
this message could be put together inside RateLimitExceededException constructor
This adds support for sending API keys to the subscription proxy of aggregators. It also adds a couple of exceptions for error cases.