"Number of tweets" setting not working, always shows 3 tweets

Hi Laura.

So is it now working as expected?

The twitter_feed block has very aggressive cache settings (hardcoded) so it might be the reason why this happened.

Never mind, I was able to reproduce the problem. Will work on it.

Fixed, should show up in the dev release in a few minutes.

The change committed only changes the cache key, it doesn't invalidate the cache properly. This may cause 2 problems:
1) Increasing the number of records in the cache_rendered bin for no reasons.
2) Switching to a previous limit/name combination will serve old cached data.

I understand chache will be refreshed anyway in 60min, however the best way should be to invalidate the cache on changing the form configuration, either from submitting a new form or importing the configuration.

Why do we need a custom cache keys for this markup? why don't we just use Drupal default BlockViewBuilder cache?
That should do all we need, then with by implementing getCacheMaxAge() we should be able to set the max age.

1) Increasing the number of records in the cache_rendered bin for no reasons.
Not for no reason. Read below.
2) Switching to a previous limit/name combination will serve old cached data.
That is acceptable if the result is going to be the same. We only want to vary on things that may actually change the output.

@gambry, the default BlockViewBuilder cache isn't going to be enough to have enough granularity (unless I'm missing something).

We really want to vary by the cache keys that I've added otherwise different blocks with different settings might use the same data, and we don't want that, as we need to assume users might want many different blocks with differing settings.

The max-age part is definitely on the TODO list (there's a comment in the code to indicate that), and would be very welcome.

Hey @Fidelix.

My understanding is you can't create more instances of the block, can you?
It's not an entity nor derivatives are implemented in any way. The rendered block is always the same. You can change the settings but they refer to an unique instance of the block.

And because you can't Drupal would expect your key to not change. If you change your key drupal thinks you have several instances of the block to cache and it will create different cache entries any time you change either the $username or $num_tweets.
Check the 'cache_render' table to check what I'm saying.

You should not change the keys. What you need to do is either invalidate the cache or - much better - add your configuration to the cache tags. In this way the cache will be automatically invalidated as soon as you change the configuration. See the Cache tags documentation page.

@gambry, You can definitely have multiple instances of the same block, with different configuration.

In one instance you might be using a certain username, and in another instance a completely different one.

Yeah, you are right. I completely forgot about that.

What is weird is the TwitterFeedBlock instances have added their own configurations as cache tags, so when you update the configuration the cache should invalidate anyway, so this issue shouldn't have happened at all.

I'm keen to think the un-fresh cache wasn't the block one but the ones coming from 'twitter_feed_item' template, which hasn't any cache metadata on it. Can it be?
What happen if you change any configuration not in the '#cache''keys' value? Still the same issue?
Some test coverage can help with answering this questions... If that's OK with you will try to write some test. I think this is the best twitter module around the drupal space so worth having a good test coverage too.

Thanks for your time!

@gambry, absolutely. Go ahead and write some tests, that would be a great addition.

> What happen if you change any configuration not in the '#cache''keys' value? Still the same issue?

Nothing will happen, and content will still be served from the cache until it times out, as far as I know.

@gambry, can you show me what your cache entries look like?

Unlike what I said in my last comment, there should already be a mechanism in place to invalidate cache for blocks when they are saved.

Sorry for the late reply. I've been busy a lot.

Unlike what I said in my last comment, there should already be a mechanism in place to invalidate cache for blocks when they are saved.

There is! Block cache is tagged with its configuration by default so on saving it's invalidated. That's why I don't understand how the bug in this issue can happen as by saving the cache should go.
That's why I was pointing my doubts against the cache key which I presume cache things slightly differently ("Cache keys must only be set if the render array should be cached." from Cacheability of render array)