SimonHalvdansson / Harmonic-HN

Modern Android client for Hacker News
https://play.google.com/store/apps/details?id=com.simon.harmonichackernews
Apache License 2.0
611 stars 40 forks source link

Allow Forced Refresh on Comment View #170

Open Tukajo opened 4 months ago

Tukajo commented 4 months ago

This is to allow for comment view's "refresh" button to bypass the cache. Otherwise, the network disk cache will always be used. Which is not the intent of a refresh.

Tukajo commented 4 months ago

@SimonHalvdansson TODO: It would probably be better to just use the volley cache all over instead of relying on your internal caching to resolve stories. That way we won't have two caches that are conflated with each other easily.

I am referring to the cache that @AppearamidGuy made in NetworkComponent.

AppearamidGuy commented 3 months ago

Does it actually use cache for any requests? I think no, because responses don't contain any Cache-Control or Expires headers, so it always runs networks request.

Still I do agree that using Volley's cache is a good idea. Looking through CacheDispatcher it is possible to make it both return cached request and make a network request by overriding StringRequest#parseNetworkResponse like this (barely tested code, but I do see success callback twice in logs):

StringRequest stringRequest = new StringRequest(Request.Method.GET, url, response ->{...}, error ->{...}) {
            @Override
            protected Response<String> parseNetworkResponse(NetworkResponse response) {
                Response<String> res = super.parseNetworkResponse(response);
                if (res != null && res.cacheEntry != null) {
                    res.cacheEntry.ttl = Long.MAX_VALUE;
                }
                return res;
            }
        };
Tukajo commented 3 months ago

@AppearamidGuy I honestly don't remember now because it's been a few months since I looked at this and my computer has since been bricked (waiting for a repair).

What I do remember is, using a debugger to step through scenarios on my S23 Ultra and my changes had performed some extra network requests that were otherwise not happening before (meaning they were cached).

I am definitely open to somehow testing this better but my point still stands that this should just be using your cache as it is much better IMO to cache in the way they you had set it up.

Too bad there's no unit tests and I'm far too lazy to set those up right now 😅.