Skip to content
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

FilterableService<T> doesn't work if you have multiple request/response filters? #458

Closed
avranju opened this issue Jan 13, 2015 · 3 comments

Comments

@avranju
Copy link
Contributor

avranju commented Jan 13, 2015

The FilterableService<T> interface works by accepting objects that implement ServiceRequestFilter. This is a thin wrapper around the support for request/response interceptors in the Apache HTTP Client library. The code adapting objects implementing ServiceRequestFilter as objects that implement Apache's HttpRequestInterceptor interface is in the class ServiceClient (from which all key management client classes inherit). Specifically, ServiceClient implements FilterableService and in its implementation of the methods from FilterableService it adapts the ServiceRequestFilter objects into HttpRequestInterceptor objects via HttpRequestInterceptorAdapter.

This means that if a given client needs 3 different types of filters added to the request/response pipeline then they'd have to write 3 different classes all implementingServiceRequestFilter - which is perfectly fine. As described above this should cause 3 instances of HttpRequestInterceptorAdapter to be created and added to the HttpClientBuilder's first or last interceptor. See here for a sample implementation.

The problem with this setup is that, for some reason, the Apache HTTP Client library allows only one instance to be added to the interception chain of each class that implements HttpRequestInterceptor for a particular type of interceptor (request/response). The relevant code from the library appears to be in the ensureUnique method of the ChainBuilder class. So in the example above, though there are 3 different classes implementing ServiceRequestFilter, all of them end up getting adapted via instances of HttpRequestInterceptorAdapter. As far as Apache HTTP Client is concerned all 3 objects are of the same type - HttpRequestInterceptorAdapter - so only one of them ends up getting added to the interception chain (as far as I can tell, the last interceptor that happens to get added).

Though I have only 1 object added to the request pipeline it turns out that the Azure SDK adds a filter of it's own to the pipeline (ClientRequestTrackingHandler) which ends up replacing my filter instance.

avranju added a commit to avranju/azure-sdk-for-java that referenced this issue Jan 14, 2015
avranju added a commit to avranju/azure-sdk-for-java that referenced this issue Jan 14, 2015
@xuezhai
Copy link
Contributor

xuezhai commented Feb 2, 2015

@avranju

Thank for pointing this out. this is a known issue and we will fix it in a future release.
In the meantime, you can apply a workaround for this by reflecting on httpClientBuilder property in ServiceClient class and call addInterceptorFirst() method on it and pass in your own adapter. This is not recommended but it should unblock your work.

@avranju
Copy link
Contributor Author

avranju commented Feb 2, 2015

Thanks @xuezhai. For our purposes we ended up forking the repo and creating a fix for this. The commit in question which has the fixes is here: avranju@0fa83ae. I can create a pull request from this if you think the approach is acceptable. + @BeatriceOltean, @martinsawicki

@markcowl
Copy link
Member

This should be fixed. Please reopen if you see a recurrence

@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants