Fatal error when call to getRevisionId()

Test coverage for this should be pretty easy.

In an existing test with an entity that references another, delete the referenced entity and then resave the parent, that should be enough to trigger this.

Comment in the test is no very clear, but Miro can improve that on commit I think if he has an idea for improving it?

Using devel_generate for creating nodes with paragraphs is still borken.

Got this message:

Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'field_paragraphs_target_revision_id' cannot be null: INSERT INTO {node__field_paragraphs} (entity_id, revision_id, bundle, delta, langcode, field_paragraphs_target_id, field_paragraphs_target_revision_id) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array ( [:db_insert_placeholder_0] => 8 [:db_insert_placeholder_1] => 8 [:db_insert_placeholder_2] => article [:db_insert_placeholder_3] => 0 [:db_insert_placeholder_4] => de [:db_insert_placeholder_5] => 1 [:db_insert_placeholder_6] => ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (Zeile 757 in /Users/christianfritsch/repositories/thunder/thunder-local/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

That's not the fault of this issue, if anything, we just change the error because it fails differently. ERR needs specific integration for devel_generate.

Could we add test coverage for PostgreSQL?

I open the bug about update paragraphs 8003 and we use PostgreSQL

I added a test with PostgreSQL for the patch. This has nothing to do with the database. If it fails then HEAD very likely already fails too.

@Miro: Can you enable on commit testing for PHP7 + mysql/postgresql/sqlite on https://www.drupal.org/node/2411903/qa? just php7 for those should be enough and it's faster.

Thx, fixed.

Add missing reference.

Still reproduces on 1.14, and there's a clear asymmetry that makes the fix a one-liner: EntityReferenceRevisionsItem::onChange() guards the target_id write for a new/unsaved target via isTargetNew(), but the sibling target_revision_id write calls $property->getValue()->getRevisionId() with no null-guard — so when the computed entity property is NULL, it fatals with Call to a member function getRevisionId() on null.

Real-world trigger seen in the wild: Drupal core's DefaultContent importer passes a failed dependency lookup (NULL) into an ERR field when recipe default content contains a _meta.depends cycle (filed against core as #3595546 — core shouldn't pass NULL, but ERR fataling here makes the authoring mistake much harder to diagnose). Any $item->set('entity', NULL) reproduces it.

Fix: mirror the target_id branch — guard the getRevisionId() call and write NULL to target_revision_id when there is no referenced entity. Also related: #2675076 (the adjacent onChange('entity') revision-id concern). Happy to roll a patch + test.

@alex ua: On such an old issue it maybe make sense to create a new one for providing MR / tests with your findings on 8.x-1.14 and relates this one to it.