Purge specific URLs

[marcus-steinbach]

Fixes issue #2

I have taken the inspiration and expanded it.

Now you can clean up the cache in e.g. wagatil_hooks.py .

EXAMPLE:

@hooks.register('after_create_page')
@hooks.register('after_edit_page')
def clear_wagtailcache(request, page):
    purge_cache(
        [
            page.full_url, # page
            page.get_parent().full_url, # category page
            page.get_url_parts()[1], # root page
        ]
    )

[marcus-steinbach]

I have then cleaned up.

Now you can clean up the cache in e.g. wagtail_hooks.py .

EXAMPLE:

@hooks.register('after_create_page')
@hooks.register('after_edit_page')
def clear_wagtailcache(request, page):
    clear_cache(
        [
            page.full_url, # page
            page.get_parent().full_url, # category page
            page.get_url_parts()[1], # root page
        ]
    )

Then I have already integrated this part https://github.com/coderedcorp/wagtail-cache/issues/23#issuecomment-738169467.

[vsalvino]

This change looks good.

I think the WAGTAIL_CACHE_KEYRING_WITH_QUERYS setting is not needed through... the default behavior (keyring stores full URI as-is) should be the only behavior, for now.

I would rather leave it up to the developer to call clear_cache() as they see fit. They could do regex matching on URIs, etc. and build the list of URIs to clear as necessary.

Reason being... we will have to address querystrings as its own separate feature. Once we add this setting we cannot take it away, and it might complicate things in the future when we address querystrings as a whole.

Other work needed to get this released:

• Documentation on new behavior, function calls.
• Unit tests of new behavior.
• (unrelated - unit tests are all broken in main branch due to some pip package change... need to hunt that down).

[marcus-steinbach]

I think the WAGTAIL_CACHE_KEYRING_WITH_QUERYS setting is not needed through... the default behavior (keyring stores full URI as-is) should be the only behavior, for now.

Reason being... we will have to address querystrings as its own separate feature. Once we add this setting we cannot take it away, and it might complicate things in the future when we address querystrings as a whole.

Well I understand. I have changed the _clean_uri(), but not removed it.

def _clean_uri(uri: str):
    """
    remove Trailingslash , remove protocol and subdomain and decode URI.
    Reasons:
            - clean keyring
            - problems with reverse Proxy
            - problems with special characters e.g. Ü,Ä,ö,ß
    """
    uri = re.sub(r'https://www\.|http://www\.|https://www|http://|https://|www\.', "", uri).strip('/')

    return unquote(uri)

I would rather leave it up to the developer to call clear_cache() as they see fit. They could do regex matching on URIs, etc. and build the list of URIs to clear as necessary.

Here it makes sense to pass another parameter(list regex) to the function clear_cache()?

Other work needed to get this released:

Documentation on new behavior, function calls.
Unit tests of new behavior.

Once we have reached agreement. ;)

(unrelated - unit tests are all broken in main branch due to some pip package change... need to hunt that down).

Is there already a fix here? At the moment all are broken. 24 x AttributeError: 'NoneType' object has no attribute 'objects'

[marcus-steinbach]

(unrelated - unit tests are all broken in main branch due to some pip package change... need to hunt that down).

1. setting.py remove wagtail.core.middleware.SiteMiddleware

   https://docs.wagtail.io/en/v2.8.1/getting_started/integrating_into_django.html Wagtail is currently incompatible with projects using django.contrib.sites.middleware.CurrentSiteMiddleware, as both this and wagtail.core.middleware.SiteMiddleware set the attribute request.site.

2. tests.py(61) change to cls.page_wagtailpage = WagtailPage.objects.get(slug="home").get_parent().specific

Now I receive the following issue.

FAILED testproject/home/tests.py::WagtailCacheTest::test_page_hit - AssertionError: 'hit' != 'miss'
FAILED testproject/home/tests.py::WagtailCacheTest::test_page_hit_without_auth - AssertionError: 'hit' != 'miss'
FAILED testproject/home/tests.py::WagtailCacheTest::test_page_miss - AssertionError: 'hit' != 'miss'
FAILED testproject/home/tests.py::WagtailCacheTest::test_page_miss_without_auth - AssertionError: 'hit' != 'miss'
FAILED testproject/home/tests.py::WagtailCacheTest::test_page_restricted - TypeError: Cannot encode None for key 'return_url' as POST data. Did you mean to pass an empty string or omit the value?
FAILED testproject/home/tests.py::WagtailCacheTest::test_page_restricted_without_auth - TypeError: Cannot encode None for key 'return_url' as POST data. Did you mean to pass an empty string or omit the value?
FAILED testproject/home/tests.py::WagtailCacheTest::test_page_skip - AssertionError: 'miss' != 'skip'
FAILED testproject/home/tests.py::WagtailCacheTest::test_page_skip_without_auth - AssertionError: 'miss' != 'skip'
FAILED testproject/home/tests.py::WagtailCacheTest::test_request_hook_any - AssertionError: 'hit' != 'miss'
FAILED testproject/home/tests.py::WagtailCacheTest::test_response_hook_any - AssertionError: 'hit' != 'miss'

[timonweb]

That's a very nice feature! Any idea if it's going to be merged? Do you need any help?

[marcus-steinbach]

I'm waiting for vsalvino to give me his answer.

[andruten]

I'm also really interested in this feature!

[marcus-steinbach]

@vsalvino I still have to write unit tests and documentation. Then the code coverage also works. It would be nice if you could take a look beforehand.

[vsalvino]

@klausfuchs thanks for putting in all your hard work on this feature... you will be making many people happy :)

I left a few comments in the code review. Overall the diff looks nice and clean.

Unit tests will of course be the not-so-fun part. I think a good test test would be: hits several URLs, verifies they are cached, then looks up those URLs in the keyring to verify they are there. And the reverse for clearing the cache (verify those URLs are no longer in the keyring).

[marcus-steinbach]

@vsalvino

I will add an optional regex parameter to the clear_cache function. Otherwise, not all keys in the keyring will be matched, or only the passed keys will be matched.

At that point I set another filter. I think robots.txt and favicon.icodo not need to be in the keyring. :smirk:

if not re.findall("robots.txt|favicon.ico", uri):
    keyring[uri] = keyring.get(uri, []) + [cache_key]
    self._wagcache.set("keyring", keyring)

[marcus-steinbach]

you will be making many people happy :)

i hope... ;)

https://github.com/coderedcorp/wagtail-cache/issues/23 , https://github.com/coderedcorp/wagtail-cache/issues/2 I think the issues can then be closed.

[timonweb]

Hi @klausfuchs @vsalvino, are there any blockers left? What's required to get it merged?

[marcus-steinbach]

Hi @timonweb , I'm ready. Waiting for the review of @vsalvino .

[timonweb]

@vsalvino could you please take a look at this PR?

[timonweb]

Hi @vsalvino, this PR has been in limbo for about a year, is there any chance to get this merged? Or should we just fork wagtail-cache? Is this project abandoned or not? Thank you in advance for your answer.

[vsalvino]

Sorry for the delay on this @timonweb , PR looks good; I will merge it in and probably make a couple minor tweaks. Or course the project is not abandoned, it is simply stable and does not need frequent changes.

@klausfuchs if you are finished committing, can you mark the PR as ready for review? It is still in draft status and cannot be merged.

[marcus-steinbach]

Hi @vsalvino, I make ready the PR for review. After that, I'll think about this.

Not necessary, but an idea for a cool feature: show the list of URLs in the keyring on the wagtail cache admin page. Could possibly include a little button to clear each individual page from the cache.

Adjustments will also be made here. https://github.com/coderedcorp/wagtail-cache/blob/main/wagtailcache/management/commands/clear_wagtail_cache.py

[timonweb]

@vsalvino @klausfuchs great news, thank you guys for not dropping this one!

[marcus-steinbach]

Found another bug, still need to fix it. :astonished:

[marcus-steinbach]

Hi @vsalvino can you help me? I do not understand this error. :scream:

https://github.com/coderedcorp/wagtail-cache/pull/22/checks?check_run_id=4606816888

[vsalvino]

@klausfuchs that error was from trying to compare code coverage of your branch against the main branch. But the main branch test has not been run in some time and expired, so I re-ran both branches. Looks like the CI is working now.

[marcus-steinbach]

@vsalvino thank you, now is ready for preview.

[mheppner]

Any updates on this PR?

[MasonLyons]

Any updates on this?