Add dynamic request header provider support#18
Conversation
…rrect Content-Type
…ccurately reflect that transport-level retries may reuse request headers. Added guidance for implementers to configure retry policies appropriately for security-sensitive headers.
| if (httpHeaders != null) { | ||
| mergedHeaders.putAll(httpHeaders); | ||
| } | ||
|
|
There was a problem hiding this comment.
It looks like we are already setting these static http headers in the request in the above createRequestOutputStream() method. This probably doesn't hurt to set them again, but seems like it might not be necessary?
| request.getFirstHeader(HttpHeaders.CONTENT_TYPE) != null | ||
| ? request.getFirstHeader(HttpHeaders.CONTENT_TYPE).getValue() | ||
| : null |
There was a problem hiding this comment.
Is this any different than just calling getContentType() directly to get the Content Type?
There was a problem hiding this comment.
I’ve refactored this to use a helper method that prioritizes the request’s current Content-Type over the subclass default. This ensures providers see the actual execution-level values and correctly respects any external header overrides.
| null); | ||
| var mergedHeaders = new LinkedHashMap<String, String>(); | ||
| if (httpHeaders != null) { | ||
| mergedHeaders.putAll(httpHeaders); |
There was a problem hiding this comment.
Same as previous comment, aren't we already setting these headers in createRequest()?
…viders to avoid double-processing.
…adoc for internal and protected methods
There was a problem hiding this comment.
This is good, but we're missing something. It's too generic. The SDK really needs to include an SPP-specific implementation of the RequestHeaderProvider, the allows clients to connect to the SPP systems as described at It should be easy to configure and a first-class part of the builder. As we discussed in that design email chain, nobody cares about "dynamic headers". That's not a thing.
What is a thing is connecting to SPP's TROLIE. The RequestHeader provider is a BUILDING BLOCK of that, not the whole solution. There should be concrete code, in the TROLIE SDK, that implements the requirements at https://spp.org/documents/75215/two-factor%20authentication%20technical%20specifications%20v1.4.pdf. Then, improve the readme docs with a page describing how to use the SDK to connect to an SPP system.
I can see the intent of the design is to be generic. I appreciate that. However, the mission of the TROLIE project is to aid FERC 881 participants with interop. With that, interoperating with SPP is something a LOT of people have to do. There are 10s of companies that are SPP TOs, and various neighboring RCs. There are a small number of ISOs in the US, and I think it makes sense that we certify the Java SDK with all ISO systems. That means SPP's as well, so features specific to integrating with SPP's systems very much belong here.
…ample on how to use it. Modified readme.md file
getorymckeag
left a comment
getorymckeag
left a comment
There was a problem hiding this comment.
Hi Sammy, this looks great! Looks like the build is currently failing though. Please fix and we can merge.
3 participants
This PR introduces RequestHeaderProvider support to the Trolie SDK. This allows users to dynamically inject headers (such as authentication tokens) into requests.
Added RequestHeaderProvider interface.
Updated TrolieClientBuilder to accept header providers.
Closes #17