Fix and improve comment cache tag usage

Actually I like to use placeholders for the whole comment area, to avoid especially that problem that the node is invalidated when comments are newly created.

However removing the list cache tag is fine I think.

1. +++ b/core/modules/comment/src/Tests/CommentDefaultFormatterCacheTagsTest.php
   --- a/core/modules/comment/src/Tests/Views/CommentUserNameTest.php
   +++ b/core/modules/comment/src/Tests/Views/CommentUserNameTest.php
   

   Unnecessary change.

2. +++ b/core/modules/node/src/NodeViewBuilder.php
   @@ -34,15 +34,7 @@ public function buildComponents(array &$build, array $entities, array $displays,
   -        $build[$id]['links'] = array(
   -          '#lazy_builder' => [get_called_class() . '::renderLinks', [
   -            $entity->id(),
   -            $view_mode,
   -            $langcode,
   -            !empty($entity->in_preview),
   -          ]],
   -          '#create_placeholder' => TRUE,
   -        );
   +        $build[$id]['links'] = $this->renderLinks($entity, $view_mode, $langcode, !empty($entity->in_preview));
   

   <3

   We can do this thanks to #2429257: Bubble cache contexts, which predates the use of placeholdering for node/comment links!

   This should yield a small performance improvement too.

3. +++ b/core/modules/node/src/NodeViewBuilder.php
   @@ -75,7 +67,7 @@ protected function getBuildDefaults(EntityInterface $entity, $view_mode, $langco
       *   The node entity ID.
   

   Now outdated.

4. +++ b/core/modules/node/src/NodeViewBuilder.php
   @@ -87,23 +79,25 @@ protected function getBuildDefaults(EntityInterface $entity, $view_mode, $langco
   +      '#cache' => [
   +        'contexts' => ['user.permissions'],
   +      ]
   

   How do we know that we want user.permissions here? Is it just a "sane default"?

Statistics should probably use a placeholder itself? Or mixed placeholder/js like history module.

1 hour TTL will affect cache hit rate for everything else and there's an issue to bubble max age to the page cache.

#17: The trick is to do both:

Compared to #post_render_cache, #placeholders can be lazy created _and_ cached at the same time! :)

Example:

$build['lazy'] = [
  '#cache' => [
    'keys' => ['node_statistics', $this->getId()],
     'max-age' => 3600,
     'tags' => ['node_read:' . $this->getId()],
  ],
  '#lazy_builder' => ['SomeClass::someMethod', ['foo']],
];

That way the nodes will be cached indefinitely, while the statistics are refreshed e.g. every hour.

Once we have multipleGet (on the roadmap, but no API change needed, so just major priority), all placeholders can be lazily gotten at the page / smart cache / render strategy level.

And with render_strategies someone would be flexible to convert that even into a JS callback instead or remove it or ...

  .-''''-. _    
 ('    '  '0)-/)
 '..____..:    \._
   \u  u (        '-..------._
   |     /      :   '.        '--.
  .nn_nn/ (      :   '            '\
 ( '' '' /      ;     .             \
  ''----' "\          :            : '.
         .'/                           '.
        / /                             '.
       /_|       )                     .\|
         |      /\                     . '
         '--.__|  '--._  ,            /
                      /'-,          .'
                     /   |        _.' 
                snd (____\       /    
                          \      \    
                           '-'-'-'    

1. +++ b/core/modules/comment/src/Tests/CommentCacheTagsTest.php
   @@ -29,6 +31,16 @@ class CommentCacheTagsTest extends EntityWithUriCacheTagsTestBase {
   +  protected $entityTestHippopotamidae;
   

   This is actually plural, even though we only ever herd a single hippopotamus. In which case the singular form would be hippopotamida.

2. +++ b/core/modules/comment/src/Tests/CommentCacheTagsTest.php
   @@ -81,6 +93,45 @@ protected function createEntity() {
   +  public function testCommentEntity() {
   +
   +    $this->verifyPageCache($this->entityTestCamelid->urlInfo(), 'MISS');
   

   Extraneous newline.

3. +++ b/core/modules/comment/src/Tests/CommentCacheTagsTest.php
   @@ -81,6 +93,45 @@ protected function createEntity() {
   +    $this->entity->save();
   +    $this->verifyPageCache($this->entityTestCamelid->urlInfo(), 'MISS');
   +    $this->verifyPageCache($this->entityTestHippopotamidae->urlInfo(), 'HIT');
   

   Hrm, aren't the camelid and hippopotamida completely separate entities? Ah, this is because before this patch, we set the comment list cache tags. Can you add a comment to the test to make that clear?

1. YAY!!! :P The Wikipedia page says: the family Hippopotamidae, which means it's plural. And, the typehint prevents us from having multiple hippos assigned to that one variable, so :)

3. A comment like "Ensure only the commented entity is affected" should be sufficient, and not suffer from the "X in the past" problem?

Hippopotamidae is the family for hippopotamus in the sense that canidae is the family of canis and vulpes. So it's not really plural, it's 'the thing above a genus'. By the way it is fun discussing taxonomy on a d.o issue :P

So it's hippopotamus for singular and any of hippopotamuses/hippopotami/hippos for plural according to wikipedia, although we could probably use family/genus if we wanted to.

Or... there is the collective noun which can be any of bloat/pod/dale/herd.

NERDS!! :P ;)

Rerolled and fixed #22.2 and #22.3.

Thanks for the throughput graph, flat throughput graphs are nicer.

Committed/pushed to 8.0.x, thanks!