Allow source counts to be cached: implement ::doCount() instead of ::count()

The typical pattern is that there's extra work happening in ::initializeIterator(), and hence ::count() is overridden too.

Because of crucial bugs in SourcePluginBase::count() and especially SqlBase::count(), people have been forced to override ::count() instead of ::doCount().

Therefore, once #3190815: Source count caching broken: impossible to enable source count caching for SqlBase-based source plugins (plus, unneeded cache I/O) is fixed, we'll be able to land this too, as will many contrib modules. For example: #3190804: Allow source counts to be cached: implement ::doCount() instead of ::count() and #3190800: Allow source counts to be cached: implement ::doCount() instead of ::count().

+++ b/core/modules/content_translation/src/Plugin/migrate/source/d7/EntityTranslationSettings.php
@@ -188,7 +188,7 @@ public function getIds() {
-  public function count($refresh = FALSE) {
+  public function doCount() {

Nit: should be protected.

#3190815: Source count caching broken: impossible to enable source count caching for SqlBase-based source plugins (plus, unneeded cache I/O) landed! 🥳

Addressed #3.

Looks like #3 which I fixed in #5 wasn't the only one! 🙈

Well that is an interesting test failure:

1) Drupal\Tests\migrate\Kernel\Plugin\source\MigrationSourceCacheTest::testCacheCountsNotContaminated
PHPUnit\Framework\Exception: mmap() failed: [12] Cannot allocate memory

mmap() failed: [12] Cannot allocate memory
PHP Fatal error:  Out of memory (allocated 66328723456) (tried to allocate 262144 bytes) in /var/www/html/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php on line 500
Fatal error: Out of memory (allocated 66328723456) (tried to allocate 262144 bytes) in /var/www/html/core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.php on line 500

Re-testing…

I have not yet been able to figure out what exactly in \MigrationSourceCacheTest::testCacheCountsNotContaminated() is causing the memory leak, but it sure smells like an infinite recursion problem 🤔

Wow, great catch! Infinite recursion 🙈

+++ b/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php
@@ -117,4 +117,11 @@ class EmbeddedDataSource extends SourcePluginBase {
+  /**
+   * {@inheritdoc}
+   */
+  protected function doCount() {
+    return count($this->dataRows);
+  }

Instead of this, let's do

  /**
   * {@inheritdoc}
   */
  public function count($refresh = FALSE) {
    // We do not want this source plugin to have a cacheable count.
    // @see \Drupal\migrate_cache_counts_test\Plugin\migrate\source\CacheableEmbeddedDataSource
    return count($this->dataRows);
  }

i.e. let's not add ::doCount() because it's dead code then.

Let's instead update the existing ::count() implementation and explain in a comment why it's implementing ::count() and not ::doCount(): to bypass caching. Its subclass is the one that is cacheable.

+++ b/core/modules/migrate/src/Plugin/migrate/source/EmbeddedDataSource.php
@@ -114,6 +114,8 @@ class EmbeddedDataSource extends SourcePluginBase {
   public function count($refresh = FALSE) {
+    // We do not want this source plugin to have a cacheable count.
+    // @see \Drupal\migrate_cache_counts_test\Plugin\migrate\source\CacheableEmbeddedDataSource
     return count($this->dataRows);
   }

… and that's why this is the only example where we do not change ::count() to ::doCount()!

#24: Yes, exactly. They would all be inheriting \Drupal\migrate\Plugin\migrate\source\SourcePluginBase::count().

Awesome, thanks!