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

Enable Manual Response Caching #3488

Open
1 task done
Alexandredenis35 opened this issue Aug 18, 2021 · 7 comments
Open
1 task done

Enable Manual Response Caching #3488

Alexandredenis35 opened this issue Aug 18, 2021 · 7 comments
Assignees

Comments

@Alexandredenis35
Copy link

Alexandredenis35 commented Aug 18, 2021

ℹ Please fill out this template when filing a feature request.
All lines beginning with an ℹ symbol instruct you with what info we expect. You can delete those lines once you've filled in the info.

Feature requests should code examples whenever possible.

Per our *CONTRIBUTING guidelines, we use GitHub for
bugs and feature requests, not general support. Other issues should be opened on Stack Overflow with the tag alamofire.

Please remove this line and everything above it before submitting.

Problem

I would like to retrieve the response returned by the webservice and modify its header before it is interpreted by the Alamofire library such as a Response Interceptor. This functionality seems to be present using a ResponseCacher to modify a response before caching it.
However, if you want to modify the headers to allow the response to be cached, there is currently no solution.

Feature Request

Have the ability to intercept the response and modify any header. This feature is available with the OkHttp library on Android https://square.github.io/okhttp/interceptors/#rewriting-responses.

Value to Alamofire

As mentioned above, this would allow the behavior of the response to be changed on the client side and, for example, to be able to cache the response.

@jshier
Copy link
Contributor

jshier commented Aug 18, 2021

This is actually a feature we recently discussed adding. Can I ask what your use case would be? On first glance this doesn't seem to be a very useful capability. The use case you mention, caching, is already possible with the CachedResponseHandler protocol: simply modify the response to have the relevant caching headers and tell the system to cache it. The added headers should then be used by the system to determine whether future requests should use the cache.

We discussed adding a general ResponseInterceptor and decided against it, as there's no way to integrate such modified responses with the rest of URLSession's response pipeline. We'd have to maintain our own, separate caching pipeline, since we'd lose access to the cache maintained by URLSession. That level of complexity just didn't seem worth it without extremely motivating use cases, which we couldn't find.

@jshier jshier self-assigned this Aug 18, 2021
@cnoon
Copy link
Member

cnoon commented Aug 18, 2021

I thought the same thing @Alexandredenis35. It's just a very complex feature to support. I'm with @jshier, do you have any actual use cases? I myself didn't which made it difficult to try to justify.

@Alexandredenis35
Copy link
Author

I would like to cache a response that the server does not allow, by removing the no-store header from the Cache-Control header.
The ResponseCacher is only usable if the response can be cached. Except in my case, the header prevents me from caching the response.

@jshier
Copy link
Contributor

jshier commented Aug 20, 2021

@Alexandredenis35 As I said, you should already be able to do that with the CacheResponseHandler protocol or ResponseCacher type. Have you tried implementing what you need?

@Alexandredenis35
Copy link
Author

Alexandredenis35 commented Aug 20, 2021

Yes, indeed I have already implemented a responseCacher to remove some headers from cached requests.
However, in this case, the responseCacher with the modify behavior is never called because the response cannot be cached (Cache-Control: no-store). As the documentation mentioned,

The conditions under which a response will be considered for caching are extensive, so it’s best to review the documentation of the URLSessionDataDelegate method urlSession(_:dataTask:willCacheResponse:completionHandler:). Once a response is considered for caching, there are variety of valuable manipulations that can be made

The response has to be considered for caching before manipulate it.
If you have a solution, I'm interested.

@jshier
Copy link
Contributor

jshier commented Aug 20, 2021

Ah yes, sorry. Unfortunately there's no other way to hook into URLSession's caching process, at least automatically. You could try manually caching the response in the URLCache associated with your request. It's likely the only place you could really do it is in or after your response handler. Something like this may work:

// Assuming Session is in scope somewhere.
session.request(...).responseDecodable(of: Type.self) { response in
  // Unwrap urlRequest, httpResponse, data, and check for success.
  // Modify the httpResponse to remove the caching attributes.
  let cachedResponse = CachedURLResponse(response: httpResponse, data: data)
  session.configuration.urlCache.storeCachedResponse(cachedResponse, for: urlRequest)
}

That should allow future requests using the same URLRequest to come from the cache automatically, depending on the cache settings you add to the response. One issue here may be the URLRequest used for caching here, as it's unclear how precise you need to be. The URLRequest returned by Alamofire's response values are the URLRequests that Alamofire sees, not necessarily the URLRequests performed by the underlying URLSession. If it doesn't work with this URLRequest, you may need to access your underlying URLSessionTask's currentRequest to get the proper one. You can do that by capturing the Alamofire Request you created and accessing the currentTask to find the latest URLRequest that was performed.

Given this limitation of URLSession's caching delegation, it may be appropriate to build a feature that performs the above steps for you more easily. Something like a ManuallyCaching protocol and associated types. We'd have to work out the details of where and when this method would fire and how you might connect it to the result of attached response handlers, but it's much more possible than a general ResponseInterceptor protocol.

@jshier jshier changed the title Response Interceptor Enable Manual Response Caching Oct 21, 2021
@jshier
Copy link
Contributor

jshier commented Oct 21, 2021

Thinking about this more, proper integration of manual caching into Alamofire is deeply impacted by our response serializer APIs and the fact that Request may have multiple attached response handlers. We'd want to perform manual caching only after the success of the response serializer, but we'd also want to avoid multiply caching responses from multiple handlers.

My current thinking is that we'd create a wrapper protocol for our serializers which would allow manual caching after running the serializer. It would look to the rest of the system that's it's just a response serializer with additional parameters injected. Perhaps something like:

ManuallyCaching(DecodableResponseSerializer())

with a convenience method:

request.manuallyCachingResponse(.decodable(of: Type.self)) { response in }

But this requires further investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants