Comment migrations produce fatal errors when RDF module is enabled

After a few solid hours of debugging, I have figured out the incredibly sneaky and insidious cause of this fatal. Give me a freaking medal! :)

While importing a comment:

1. The migration process plugin tries to look up the comment's pid. If it cannot be found, it creates and saves a stub comment. That stub has a specific ID (the pid which could not be found), but little else. Crucially, it does not have an entity that "owns" it (i.e., the entity that was commented upon) -- its entity_id field is NULL.
2. At some further point in the migration, the comment which was stubbed is being imported for real. The Entity destination's getEntity() method is called, which results in the comment storage's load() method being called (because Entity is trying to ascertain if the comment already exists or needs to be created).
3. The comment storage handler queries the database for the comment, and finds it -- because it was stubbed back in step 1.
4. The comment storage handler invokes hook_comment_storage_load() on the comments loaded from the DB. RDF, which implements this hook, innocently calls $comment->getCommentedEntity() on the loaded stub -- which, you'll remember, does not refer to an entity. Because it was a stub. So getCommentedEntity() returns NULL.
5. rdf_comment_storage_load() tries to call methods of the commented entity and fatals because the commented entity is NULL.

With tests, and my compliments!

There's a bit of patch noise in the passing patch because I removed a few deprecated function calls from MigrateCommentTest.

vancode strikes again!

Escalating. This will cause failures on any site with a substantial number of comments if the D8 RDF module is enabled -- which, in the default install profile, it is. So this is likely to be encountered by a lot of people.

Whoops. This affects D6 and D7.

I think this is a duplicate of #2340401: Fill commented entity during stub comment saving

This patch doesn't change the stubbing at all -- it works around the problem by introducing better ordering into the comment sources. I think we should commit this now and continue work on that patch, since:

• This patch is ready now;
• It doesn't change or affect the stubbing code in any way;
• It will prevent fatal errors for people migrating even a modest amount of comments

The other patch is good and work should continue on it; when it lands it will reinforce this one.

This patch is more of a workaround where as the other patch fixes the root cause of the problem. Some people might import comments from a CSV where they can't change the order in the source for example.

And I think the comments in this patch that mention ordering to avoid fatals are a bit weird because i'd read those and think, hmm, that's the wrong fix.

However, if this issue helps the upgrade path fine, but I think the comments in the patch should be re-done to reference the issue with the "correct" fix and mention that the order is a temporary work around.

That seems like an exceedingly fair compromise to me. If you're OK with the workaround as it is, I'll reroll the patch tomorrow, fix the comments, and get it committed.

Yeah that's good for me.

why not both? :)

Comments fixed.

@mikeryan says he's close to getting #2340401: Fill commented entity during stub comment saving completed, so I think it makes sense to close this and pursue that patch full-throttle.

Hello,

I got this error in Drupal 8.2.5 and I have fixed it. :)

Please see the details as below:
Issue: If comment created date is zero then fatal error come.
Solution: I have added any date in "created" column in "comment_field_data" in database using query and clear the cache. like that:

SELECT * FROM comment_field_data WHERE created = 0
UPDATE comment_field_data SET created = 1222056000, changed = 1222056000 WHERE cid = 7277;

I have used SQL because I have to perform for bulk records, you can try with edit comment.

If any query, please let me know.

Thanks,
Deepesh