Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In the constructor of SourcePluginBase
the cache_key configuration value for the count cache is used in a surprising way:
$this->cacheKey = !empty($configuration['cache_key']) ? !empty($configuration['cache_key']) : NULL;
As a result when I set a cache key in my migration configurations they all share the same count cache. I assume that it is a typo and instead it should be:
$this->cacheKey = !empty($configuration['cache_key']) ? $configuration['cache_key'] : NULL;
I can provide a patch if that is the correct behaviour.
I have also noticed that the default cache key provided is the same for all migrations sharing a source plugin.
if (!isset($this->cacheKey)) {
$this->cacheKey = hash('sha256', $this->getPluginId());
}
I am a not sure how to obtain a safe default.
Comment | File | Size | Author |
---|---|---|---|
#11 | count-cache-2723115-11.patch | 2.13 KB | mayurjadhav |
#7 | interdiff.txt | 1.03 KB | Stefan Freudenberg |
#7 | count-cache-2723115-7.patch | 2.51 KB | Stefan Freudenberg |
#5 | count-cache-2723115-4.patch | 986 bytes | Stefan Freudenberg |
Comments
Comment #2
Stefan Freudenberg CreditAttribution: Stefan Freudenberg commentedComment #3
Stefan Freudenberg CreditAttribution: Stefan Freudenberg commentedComment #4
Stefan Freudenberg CreditAttribution: Stefan Freudenberg at Agaric commentedComment #5
Stefan Freudenberg CreditAttribution: Stefan Freudenberg at Agaric commentedPease review attached patch addressing the cache key issue.
Comment #6
mikeryanLooks good, but let's have a test demonstrating the failure.
Comment #7
Stefan Freudenberg CreditAttribution: Stefan Freudenberg at Agaric commentedI have added a test to the patch.
Comment #9
Stefan Freudenberg CreditAttribution: Stefan Freudenberg at Agaric commentedEr. Not sure what to do. The test failure looks completely unrelated.
Comment #11
mayurjadhav CreditAttribution: mayurjadhav commentedAdded updated patch.
Comment #12
vasi CreditAttribution: vasi at Evolving Web commentedThis is perfect. We just need a follow-up for the issue with the default cache key.
Comment #13
mikeryanFollow-up issue #2751829: Default source plugin cache key is insufficiently unique created - looks good!
Comment #14
alexpottCommitted b0bc0c5 and pushed to 8.1.x and 8.2.x. Thanks!