Cache Control #55

pradumnk-mahanta · 2021-10-04T15:21:24Z

PR Checklist

The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.
There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
All existing tests are passing
Tests for the changes are included

What is the current behavior?

No Cache Control.

What is the new behavior?

Ability to control Cache for HTTP Requests.

Fixes/Implements/Closes #54.

Hi @jerbob92 ,

I tried implementing cache control in the plugin and it works partially. For iOS I was able to directly set Cache Policy as urlRequest.cachePolicy = NSURLRequestCachePolicy.UseProtocolCachePolicy; or whatever.

But for Android I noticed that you are making the request with the AAR created by you and not directly with okhttp3.

I thought of rewriting the request function for android of the plugin with OkHttp. However before I start that, I thought of getting in touch with you for a possibility to add cache control parameter to com.klippa.NativeScriptHTTP.Async.Http.MakeRequest(requestOptions, callbackComplete, requestId)

Currently there are 3 parameters in MakeRequest

RequestOptions
CallbackComplete
Request Id Counter

We can add in a fourth Parameter CacheControlPolicy which will be added to the okhttp request.

You must be building a request of okHttp inside the MakeRequest function similar to var request = new okhttp3.Request.Builder(); we can just add the cache control builder to that request to make it work request.cacheControl(cacheControlPolicy);

I have committed my changes to the forked repository here.

Please have a look at the request function in http.ios.ts and http.android.ts and let me know.

jerbob92 · 2021-10-12T07:21:06Z

src/http.android.ts

@@ -365,6 +366,32 @@ export function request(options: HttpRequestOptions): Promise<HttpResponse> {
                NetworkAgent.requestWillBeSent(requestIdCounter, options);
            }

+            let cacheControlBuilder = new okhttp3.CacheControl.Builder();


We probably want more control right? Like where to store and the max size?

I think we do. I will add the option to add the cache location and size.

jerbob92 · 2021-10-12T07:22:42Z

src/http.android.ts

@@ -377,6 +404,27 @@ export function request(options: HttpRequestOptions): Promise<HttpResponse> {
            // make the actual async call
            com.klippa.NativeScriptHTTP.Async.Http.MakeRequest(javaOptions, completeCallback, new java.lang.Integer(requestIdCounter));

+
+            //Temporary - Replace with cacheControlBuilder
+            if (options.cachePolicy) {


Why is this necessary?

This is not necessary, I will remove this. This was just a temporary add on to handle the cache post the request.

jerbob92 · 2021-10-12T07:23:24Z

src/http.android.ts

@@ -377,6 +404,27 @@ export function request(options: HttpRequestOptions): Promise<HttpResponse> {
            // make the actual async call
            com.klippa.NativeScriptHTTP.Async.Http.MakeRequest(javaOptions, completeCallback, new java.lang.Integer(requestIdCounter));


Can't we add the CacheControl to the javaOptions?

Yes, we can. I was not sure if you'll accept changes to the Async.java, so I just suggested the additional parameter way.

Yeah, the java code is part of this repository and is an integral part of how the HTTP client works, it's also only used by this plugin, so you are free to make changes to it.

jerbob92 · 2021-10-12T07:24:21Z

src/http.common.ts

+    NOCACHE
+}
+
+export interface HttpsRequestOptions extends HttpRequestOptions {


I'm not sure if you can just rename this everywhere, won't this break support with the Core HTTP methods?

I think we need to take a call on this one. If people are using this plugin then, probably they won't be using the native ones and there won't be a problem, and if they use the native ones and pass those options with this plugin then they will have everything except the cache control.

Well the thing is that this plugin allows you to automatically replace all http calls with this module. To make that work, the functions and interfaces have to match the original, I suppose, we would have to check that.

jerbob92 · 2021-10-12T07:25:29Z

src/http.ios.ts

@@ -130,6 +132,29 @@ export function request(options: HttpRequestOptions): Promise<HttpResponse> {

            urlRequest.HTTPMethod = Utils.isDefined(options.method) ? options.method : GET;

+            if (options.cachePolicy) {


Do we also have something like the cache builder for iOS to have more control over it?

jerbob92 · 2021-10-12T07:27:16Z

The code for the AAR is here: https://github.com/klippa-app/nativescript-http/blob/master/src/platforms/android/java/com/klippa/NativeScriptHTTP/Async.java

You can directly edit that code and NativeScript should re-generate the AAR.

pradumnk-mahanta added 3 commits October 4, 2021 12:21

Add Option to Clear Cache

ef9f3a8

Added Default Opotion to Android Cache Policy

cc280b5

Mapped Cached Control Builder to Cache Control

ca76d70

jerbob92 requested changes Oct 12, 2021

View reviewed changes

jerbob92 closed this Jul 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache Control #55

Cache Control #55

pradumnk-mahanta commented Oct 4, 2021

jerbob92 Oct 12, 2021

pradumnk-mahanta Oct 12, 2021

jerbob92 Oct 12, 2021

pradumnk-mahanta Oct 12, 2021

jerbob92 Oct 12, 2021

pradumnk-mahanta Oct 12, 2021

jerbob92 Oct 12, 2021

jerbob92 Oct 12, 2021

pradumnk-mahanta Oct 12, 2021

jerbob92 Oct 12, 2021

jerbob92 Oct 12, 2021

jerbob92 commented Oct 12, 2021

		@@ -377,6 +404,27 @@ export function request(options: HttpRequestOptions): Promise<HttpResponse> {
		// make the actual async call
		com.klippa.NativeScriptHTTP.Async.Http.MakeRequest(javaOptions, completeCallback, new java.lang.Integer(requestIdCounter));

		@@ -130,6 +132,29 @@ export function request(options: HttpRequestOptions): Promise<HttpResponse> {

		urlRequest.HTTPMethod = Utils.isDefined(options.method) ? options.method : GET;

		if (options.cachePolicy) {

Cache Control #55

Cache Control #55

Conversation

pradumnk-mahanta commented Oct 4, 2021

PR Checklist

What is the current behavior?

What is the new behavior?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerbob92 commented Oct 12, 2021