feat: cache peer locations for 1 week

[hacdias]

Taking in account what I said on #730, I tried to do a little experiment with Peer Locations. It doesn't add much more code and it is clean and improves performance over time. I set the peer location cached for a week, but maybe we could set it to expire in more time or even don't expire at all.

It uses IndexedDB, which in case of not being available in that browser, just fails silently. This cache is supposed to be something to improve the app and not really required to be used.

License: MIT
Signed-off-by: Henrique Dias hacdias@gmail.com

[hacdias]

@olizilla @alanshaw @fsdiogo what's your thoughts on this PR?

[olizilla]

@hacdias did you see https://reduxbundler.com/api/included-bundles.html#createcachebundlecachingfunction
and bundle.persistActions ?

With those configured we'd have a mechanism to cache entire bundle states after specified actions occur. That is a different idea to having certain data look ups cached, so it might be that we need both techniques, but I wanted to check that you'd seen the built in mechanism, as it may be that we can refactor things to just use that.

[fsdiogo]

LGTM.

But isn't one week a bit too much? 🤔

[hacdias]

@olizilla I did try that. And then some objects who had functions (mainly objects from Big.js) would cause lots of errors because with serialization and deserialization they would lost add and other fns. But I may have done sth wrong though

[hacdias]

@fsdiogo I don't think it's too much time for locations. I doubt that every peer is moving from city to city every day 😄

[olizilla]

@hacdias that's interesting! We shouldn't be storing non-serializable things in the redux tree, as a lot of the extra magic of redux assumes that the tree is safe to serialize. If we fixed the other bundles to only strore serializable stuff, would you be able to use the existing caching features of redux-bundler to achieve this?

[hacdias]

@olizilla I guess so

[hacdias]

Some more thoughts: I have been trying with createCacheBundler and I've hit only one issue: on Peers Location bundle, for example, we have the following state:

{
	locations: [...],
	queuingPeers: [...],
    resolvingPeers: [...]
}

And the cache bundle saves it all even though we just want to cache the locations themselves and not anything else. We would need to make some adaptations to the bundle.

Second point: if we're not using createAsyncBundle the expire date will be the same for all data. Otherwise we would need to create a mechanism to check that.

Third point: since it saves everything on one entry on IndexedDB, if we set the expire date for one week, all of the data might happen to expire at the same time or never expire if we're always updating that bundle data (hence updating the last modified date).

I hope I've been clear enough because I know I wasn't crystal clear with this message 😞

To finish: I'd say the idea on this PR is much more flexible but at the same time it doesn't have the initial data all set up. I'm not sure if we want to have all data cached. So I have two suggestions:

1. If a bundle wants to have a cache, it will get it through a selector or even something more generic like this:

export bundleCreator(opts) {
	opts.cache = opts.cache || {}
	const cache = getCache(opts.cache)

	return {
		...
		// use the cache in the bundle
		...
	}
}

2. Adapt createCacheBundle in order to support overriding/removing unwanted fields.

I'd like to hear your opinions.

Which types of data do we want to cache right now? Peers locations? Bandwidth? Explore data (since it's static)?

[olizilla]

So we can provide a version number here in the future if we want to invalidate existing caches, say if we totally change the structure of the data. Very cool.

https://github.com/HenrikJoreteg/redux-bundler-example/blob/0f23da51a32853a2d83aaf26e39e78f34997debc/src/utils/cache.js#L5-L14

That example also talks about adding in some unique identify where cached data is specific to a given user... we might want to cache info specific to a given ipfs node identity at some point so it's nice to know that there is a way to manage that. ✨

[olizilla]

@hacdias I think this is a cool solution. Your write up of how this compares with the existing caching solution is super helpful.

I think we can use both caching styles. The proposal in the PR makes sense for storing specific lookups, and doing so explicitly and procedurally, so the dev has full control of when it happens.

For simpler situations where the bundle is created with caching in mind, then we can just use the built in persistActions support and have it store the whole bundle.

As for "how long should we cache peer locations for" I think a week is fine. As a future PR if we cached the look up of IP address to location rather than peer address to location, we could cache them for much longer as the mapping will only change when the geoip db gets updated.

[olizilla]

This is rad ✨ Update the conflict and lets get it merged.