Fix cache.get() with default on PyLibMCCache if None is cached

[Jakub Szafrański]

If None is saved to the cache, the default keyword argument in cache.get() shadows it if using any Memcached backend. This is not consistent with LocMemCache.

The issue can be very easily fixed within PyLibMCCache, as it supports retrieving a default value if key is not present.

For MemcachedCache, unfortunatelly, this is not possible because of python-memcached limitations, although I've opened up an issue and PR to the upstream package maintainer that would add functionality that would allow this in MemcachedCache as well: ​https://github.com/linsomniac/python-memcached/issues/159

I will fix this issue for PyLibMCCache for now, and once (if) the upstream PR for python-memcached gets approved, merged and released, I'll also submit a PR that would fix this issue for MemcachedCache (although that would require a minimum version of python-memcached, so that one would probably be a little more complex).

[Jakub Szafrański]

PR that fixes this ticket: ​PR

[Tim Graham]

A test is required.

[Jakub Szafrański]

Test has been added

[Tim Graham]

There's documentation in topics/cache.txt about this that would need to be updated: "We advise against storing the literal value None in the cache, because you won't be able to distinguish between your stored None value and a cache miss signified by a return value of None."

I'm not sure if fixing this on only one backend is a good idea. Django 3.0 won't be released until December so we can't wait a bit and hopefully ​python-memcached will be fixed before then.

It looks like #29867 could also be addressed.

[Jakub Szafrański]

I fear that the python-memcached package may be abandoned, as it looks like there are many issues that have been reported but are not addressed. Anyway, right now I see a bunch of options we could go for:

1) Wait for python-memcached to accept my PR (or wait if somebody will take over maintenance of this package) and make an unified change to BaseMemcachedCache class (although this will introduce a change that's breaking to all users who have older python-memcached package installed)
2) Introduce only the change to PyLibMCCache and change the documentation to only mention the None inconsistency at MemcachedCache (because that will be the only core backend that's going to have this issue, more on that later)
3) Remove support for python-memcached alltogether and remove the None inconsistency mention from the docs entirely.

As for backends, I tested the current 2.1.7 Django release against the None and default keyword param issue and it looks like LocMemCache, FileBasedCache and DatabaseCache all support None + default properly. Only the Memcached-based backends are the ones that are not.

Personally I think that even introducing this change to one backend for now is good, as you'll have 1/5 backends not working correctly instead of 2/5, which is an improvement IMO, but this is up to you.

[Tim Graham <timograham@…>]

In 0104b5a:

Fixed #30181 -- Made cache.get() with default work correctly on PyLibMCCache if None is cached.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d23dad57:

Refs #30181 -- Corrected note about storing None in the cache.

[Mariusz Felisiak <felisiak.mariusz@…>]

In b044b41:

[3.1.x] Refs #30181 -- Corrected note about storing None in the cache.

Backport of d23dad5778b3610a5f870b4757ba628780924dd1 from master