Invalidate /aggregator page cache when feeds update

The aggregator module has been removed from Core in 10.0.x-dev and now lives on as a contrib module. Issues in the Core queue about the aggregator module, like this one, have been moved to the contrib module queue.

I verified that this is still an issue. The steps to reproduce it worked for me.

This test demonstrates the bug by requesting /aggregator, updating feed items, then requesting /aggregator again. The test fails correctly on my local.

Unfortunately, the test is no good because I think it will invalidate the assertions that follow it. Those assertions were added by #3273873: Aggregator page contents could be empty due to missing cache context and they depend on the page not being cached. They were a problem for getting a failing test though. Requesting /aggregator?page=2 causes /aggregator to start working properly. I don't know enough to understand why that was happening, but I assume that requesting a different page's context somehow cleared the first page's context too. As a result, it took me a while just to get a failing test. I haven't figured out if the tests for both issues can go into the same method yet. Maybe if I delete all the items and then re-update the feed...

Nevermind. That was easy to solve. All it needs is a cache clear before the next assertion. I verified that the cache context assertion is still working properly by undoing the fix from #3273873: Aggregator page contents could be empty due to missing cache context and testing it locally. It failed just like it's supposed to. Here's a good failing test.

This passes and works for me locally.

I'm hoping to review this tomorrow

+++ b/tests/src/Functional/AggregatorRenderingTest.php
@@ -96,9 +96,20 @@ class AggregatorRenderingTest extends AggregatorTestBase {
+    // Clear the cache so that the paginated cache context can be tested.
+    drupal_flush_all_caches();

Instead of this, we *should* be able to just call $feed->save(), because the Aggregator item includes 'aggregator_feed_list' in its cache tags, saving a feed should invalidate the render cache.

Using drupal_flush_all_caches feels wrong, because effectively we're saying 'if we clear all caches does the correct thing show' which we know is the case. I think we want to instead test that saving a feed invalidates the listing.

Yes, that change worked.

Looks good to me - thanks!

I'll cut a new release

This can apply to the 1.x branch too. I just haven't fixed its tests yet. We could apply the patch without fixing the tests first. I ran them locally without and with the fix and got the same results that I did for the 2.x branch. But I was trying to do things properly.

Yeah fine to backport we know tests on 1.x are in a bad place

It passed for the 1.x branch too.

This has been committed to contrib Aggregator. The patch in #14 can be rerolled for core, if anyone cares to do it.

Please review.

9.5 has perhaps seen its last bug fix release already, but for posterity let's get this in anyway.

Committed and pushed 3892aea923 to 9.5.x. Thanks!