FetchFromCacheMiddleware throws an exception if AuthenticationMiddleware is not enabled.

[m3brown]

FetchFromCacheMiddleware throws an exception if AuthenticationMiddleware is not enabled.

File "/usr/lib/python3.6/site-packages/wagtailcache/cache.py", line 71, in process_request
AttributeError: 'WSGIRequest' object has no attribute 'user'

Why this is a problem

The configuration I'm deploying has a read-only Wagtail service. As part of the hardening process, we're disabling all administrative/login functionality, including AuthenticationMiddleware.

Removing AuthenticationMiddleware removes the request.user objectm, which is currently required by FetchFromCacheMiddleware.

Proposed solution

We should be able to adjust the conditional logic in FetchFromCacheMiddleware.process_request to work both with and without a defined request.user

I believe the following tweak would be appropriate and logically identical as far as the cache middleware is concerned.

  is_cacheable = \
              request.method in ('GET', 'HEAD') and \
              not getattr(request, 'is_preview', False) and \
-             not request.user.is_authenticated
+             not (hasattr(request, 'user') and request.user.is_authenticated)

Another approach would be to decouple is_cacheable into its own function so others can extend the mixin and override the functionality.

Potential concerns

Consider this configuration:

MIDDLEWARE = [
   ...
   'FetchFromCacheMiddleware',
   'AuthenticationMiddleware',
   ...
]

In this situation, I believe my proposed solution would continue processing under the assumption that the user is not logged in, when in fact the request.user objects just has not been instantiated yet.

We could certainly add more complexity to the logic recommended above to confirm MIDDLEWARE ordering is what we expect. Since the documentation clearly states that FetchFromCacheMiddleware should be at the end of the MIDDLEWARE list, I would argue any codified checking is unnecessary.

[vsalvino]

Great find, and thank you for the very detailed bug report. I agree that it should work if there is no request.user, which should behave the same as if request.user were None.

Double thank you for provide a pull request as well!