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.

Command icon 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

claudiu.cristea created an issue. See original summary.

joelpittet’s picture

Issue tags: +Needs tests

This could use a kernel test

joelpittet’s picture

Status: Active » Needs work
joelpittet’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

I 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.

joelpittet’s picture

Oh 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.

joelpittet’s picture

Why the tests failed: the static cache was never cleared. The tests save an entity, then enable scan on 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 resetScannableFieldDefinitions and we pepper it around linkchecker_entity_[insert|update|delete] #bringBack_drupal_static_reset()... likely overkill

claudiu.cristea’s picture

I think using memory cache bin with proper tagging might solve the static cache.

claudiu.cristea’s picture

Assigned: Unassigned » claudiu.cristea
Status: Needs review » Needs work

I 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...

claudiu.cristea’s picture

Assigned: claudiu.cristea » Unassigned
Issue summary: View changes
Status: Needs work » Needs review

With the scope limited to the queued entities keys, this is ready for review.

joelpittet’s picture

This is way easier to review, thanks for splitting up the hard part!

joelpittet’s picture

Title: Strengthen queued entities key » Queued entities of different types with the same ID overwrite each other

retitling, while looking am pondering if cleanUpForEntity needs the same fix for collisions. scratch that you fixed it, I just added a test for that too.

joelpittet’s picture

Status: Needs review » Fixed

Much easier to merge, thanks you we can move to bigger and harder problems now 🙌 thanks @claudiu.cristea

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.