Problem/Motivation
Quoting from #3581562-14: Add option to disable Linkchecker (esp. entity hooks):
(...) the queues also collect entities that have no scannable fields at all (content_moderation_state here), and they are keyed by entity id alone, so an entity of another type with the same id as a queued node replaces it. array_shift() in destruct() renumbers the remaining integer keys too, so a linkcheckerlink saved during destruct() can land on a still-queued entity.
Proposed resolution
This issue is only addressing the queue key overlapping. The issue of scannable entities has been postponed to #3627371: Provide service/methods to detect scannable entity types, bundles, fields, which is an improvement and not blocker bug
Remaining tasks
None.
User interface changes
None.
API changes
None.
Data model changes
None.
Issue fork linkchecker-3626994
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
joelpittetThis could use a kernel test
Comment #4
joelpittetComment #5
joelpittetI added some tests while I was in there. I hope I didn't step on your toes on this one (let me know and I will revert). I left a comment about the
assert()code path messing with the signature. Hope that is clear.Comment #6
joelpittetOh I see, I should have realized that, normally static cache is solving that kinda thing... it is why the tests were failing but I was really reluctant to solve by fixing the tests (your earlier commit passed)... I'll revert though and see what that looks like.
Comment #7
joelpittetWhy the tests failed: the static cache was never cleared. The tests save an entity, then enable
scanon the field, then save again, and the second save still saw the old "no scannable fields" result. This mostly affects tests and long-running scripts that change field settings and save content in the same process; normal requests start with an empty cache.So maybe the most pragmatic fix is just in the tests? The alternative is a public reset method like
resetScannableFieldDefinitionsand we pepper it aroundlinkchecker_entity_[insert|update|delete]#bringBack_drupal_static_reset()... likely overkillComment #8
claudiu.cristeaI think using memory cache bin with proper tagging might solve the static cache.
Comment #9
claudiu.cristeaI split the scannable entities/fields to #3627371: Provide service/methods to detect scannable entity types, bundles, fields because that's just a performance improvement and API consolidation. Definitely not a bug or a blocker for release. Reworking the MR...
Comment #10
claudiu.cristeaWith the scope limited to the queued entities keys, this is ready for review.
Comment #11
joelpittetThis is way easier to review, thanks for splitting up the hard part!
Comment #12
joelpittetretitling,
while looking am pondering ifscratch that you fixed it, I just added a test for that too.cleanUpForEntityneeds the same fix for collisions.Comment #13
joelpittetMuch easier to merge, thanks you we can move to bigger and harder problems now 🙌 thanks @claudiu.cristea