Caching cleaned URLs not working after url changes

[roperto]

If you move a course module to another section, it hold the cache for the old URL.

We may have other similar problems.

Should we watch for that event and remove that URL from the cache (what other scenarios should we think about too?)

I think it would also be nice to have a way to disable Clean URL caching (dev or debugging mode) or even a quick-link to clean it.

Brendan's spec ideas:

• [ ] different components of the url could change, eg the course shortcode, the section title, or the username, activity name etc. So we want a generic set of helpers that any uncleaner object can use to store and query for stubs. Possibly it would be some base object that they all extend, or maybe this is just built directly into the uncleaner base class.
• [ ] there would be a generic db table, eg _local_cleanurls_stub_history: id, type, refid, stub, created, lastused eg 123, course, 12345, 'COMP100', timestamp, timestamp
• [ ] the 'type refers 1-1 with the uncleaner type, eg that record would be managed by the course_uncleaner object.
• [ ] refid depends on the uncleaner but is the primary id of whatever objects we are dealing with. There might be a history of 10 stubs for the whole history of the same object
• [ ] stub is the old stub, so in this case would be the old course shortcode
• [ ] created is the time this stub was archived. Only archived stubs are in this table. This table is only consulted if no other primary match is found.
• [ ] generally old urls won't get used much, but bookmarks or whatever will persist. robots will crawl and should be given the correct canonical url and never come back to the old url. So it's useful to see how many old versions exist, and also when they were last used. We can use this to automatically purge old stubs after some time, eg keep old urls working until they haven't been accessed for say 6 months
• [ ] each cleaner / uncleaner is responsible for watching the various events for mutating objects and then inserting the new history object. eg watch for the course_updated event and pull out the shortcode, and id and add to the history.
• [ ] this common code should also be the one central place that does all the lookup, caching and cache invalidation of stubs, as this shouldl be the same across all the cleaners / uncleaners.

ie if a course shortcode changes then

• [ ] the event fires
• [ ] the event listener says: I have course with id 7 with old stub of COMP100 and new stub of COMP200
• [ ] if passes this to the helper
• [ ] help stores 'course', 7, 'COMP100' as a new record with timestamp of now
• [ ] it overwrites COMP200 as the new outgoing cache record to cleaned urls
• [ ] for incoming urls it add the COMP200 to the cache too, the old COMP100 cache item will continue to exist

when a pageview for COMP100 hits the router and gets to the course uncleaner if will

• [ ] do the cache lookup for COMP100, if hits then it stops
• [ ] do the normal lookup in mdl_course for a shortcode = COMP100, if there is a hit then it stops
• [ ] do the history lookup in mdl_local_cleanurls_history for a 'course', 'stub' = COMP100, if there is a hit then it stops

[brendanheywood]

There are a few moving parts to this:

• [ ] when a url changes for whatever reason, then the old incoming url should still work and be cached for some time as a transition. Both old and new urls should work. The big questions is whether we persist this or just leave it in the cache and let it expire at some random point. Some thoughts are here https://github.com/brendanheywood/moodle-local_cleanurls/issues/9

• [ ] when a course category name changes, or the course is moved within the category, then the event should be captured and the outgoing cached url should be reset (but not the incoming one).

• [ ] if either of these change, then we really need to invalidate every single cached url for activities and sections withing the course. This feels quite wrong to me. What I think we should do is have each component of the url cached independently, so that just the course category stub, and then the format components and activity components are cached independently. We probably need 4 caches (eg 1) category, 2) course, 3) format 4) activity ) ie if we change a courses format from topics to social, then the only cache item to be invalidated is the cache format item. Performance wise it will still all get cached it just means we will do 4 cache gets instead of 1.

[roperto]

• "old incoming url should still work and be cached": Just using caches, or storing it in the DB so it would retain the data after purging caches? If keeping on the DB, any time limit or keep it until an address conflicts and is overwritten?

• category name change: if using the "cache" above we don't need to worry about incoming URLs when resetting the cleaning cache anyway. I think ideally we would make a redirect instead of using the deprecated incoming URLs.

• If the course section (not category) changes we have similar problems. If you move a course module inside another section the URL also changes. Same thing here.

• The biggest issue I see so far in the "cleaning" stale URLs is that they replace the address bar of the browser, it should not happen.

I didnt quite understand the 4 caches idea, we can discuss that later.

Cheers,

Daniel

[roperto]

After changing a custom activity path as in PR #105 the outgoing cache old entry is removed so the new one can be generated, but ont the reverse uncleaning so the previous URL can still be used.

I think using the cache to implement instead of the DB is very flawed, IMHO cache should only be used to improve performance, clearing caches should not affect the behaviour of the uncleaner like it does now (after cleaning caches the old urls become invalid).

This is not major, but something to think about.

[brendanheywood]

Yup totally agree this should be persisted in db, and then cached on top. For public urls that have been crawled by google as an example we will need to persist these for say a month until google can re-scrape in order to re-canonicalize these urls. And for long history permalinks we need to keep them essentially forever.

[roperto]

@brendanheywood Please note I made a change in the way course URLs are uncleaned, see commit message: https://github.com/brendanheywood/moodle-local_cleanurls/commit/ef44e16210cd7c3b762280b0127092e8cf2f7259

[brendanheywood]

+1