Content dependencies lost on export and importing again if the content entities do not exist

+1 from me assuming that this doesn't prevent importing (but I think all content entity dependencies are soft, so we're good)

Something about this feels wrong. Can @benjy can you post such a module so I can see what's going on.

So here is a better fix - i think. I think given the fact that all content dependencies in configuration are soft we can't remove them on recalculation as the whole point is that don't have to be met.

This is a major bug - dependencies shouldn't go missing like this.

+++ b/core/modules/block_content/tests/modules/block_content_test/config/install/block.block.testblock.yml
@@ -0,0 +1,24 @@
+provider: null
...
+  provider: block_content

This looks weird

Is it possible to import block_content to import? This is found that after full import the block content unable to import in the db, the import block is missing/showing broken in the original region see http://drupal.stackexchange.com/questions/146037/block-error-on-cmi

Updated issue summary

Whilst trying to explain some of the test fails @tstoeckler and I discovered #2625216: LocaleConfigManager::getTranslatableDefaultConfig() reads config from uninstalled modules

1. +++ b/core/lib/Drupal/Core/Block/Plugin/Block/Broken.php
   @@ -49,4 +49,14 @@ protected function brokenMessage() {
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function calculateDependencies() {
   +    // Broken blocks should preserve the provider of the broken plugin.
   +    $dependencies = parent::calculateDependencies();
   +    $dependencies['module'][] = $this->configuration['provider'];
   +    return $dependencies;
   +  }
   +
    }
   

   In that case fixing \Drupal\views\Plugin\views\BrokenHandlerTrait would be neat.

2. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
   @@ -354,7 +354,7 @@ public function preSave(EntityStorageInterface $storage) {
      public function calculateDependencies() {
        // All dependencies should be recalculated on every save apart from enforced
        // dependencies. This ensures stale dependencies are never saved.
   -    $this->dependencies = array_intersect_key($this->dependencies, ['enforced' => '']);
   +    $this->dependencies = array_intersect_key($this->dependencies, ['enforced' => '', 'content' => '']);
   

   The comment is now missing some information

3. +++ b/core/modules/block_content/tests/src/Kernel/BlockContentDependencyTest.php
   @@ -0,0 +1,59 @@
   +/**
   + * @group block_content
   + */
   +class BlockContentDependencyTest extends KernelTestBase {
   

   Is that valid? I would be totally happy about that.

Thanks @dawehner. I think 1 should be addressed in a followup since it will require tests of its own.

Hmmm... I think there might be a problem with the approach. If you create an entity reference field to content and then change the default value I don't think the dependency to the initial node will be removed :( testing this theory...

Unfortunately... #23 is right...

uuid: 0f936f13-18cb-4940-ad00-a9764e1d9177
langcode: en
status: true
dependencies:
  config:
    - field.storage.node.field_ref
    - node.type.article
  content:
    - 'node:article:19b0f140-4e17-4858-8400-e78a83076e0b'
    - 'node:article:e6ccb833-139f-4c90-9000-b6345b9caceb'
id: node.article.field_ref
field_name: field_ref
entity_type: node
bundle: article
label: ref
description: ''
required: false
translatable: false
default_value:
  -
    target_uuid: 19b0f140-4e17-4858-8400-e78a83076e0b
default_value_callback: ''
settings:
  handler: 'default:node'
  handler_settings:
    target_bundles:
      article: article
    sort:
      field: _none
field_type: entity_reference

So one option would be to only preserve during a configuration sync. However this would mean that any post update that saved all config entities to fix something would risk losing these dependencies too. So going back to first principles - is it right for configuration to have dependencies listed for things that are not there? I don't think so. It is okay for configuration to be broken is something is missing. (By broken I mean report to the user that something is missing).

So is this a bug? As long as the missing content dependency event is fired with the correct information and the system has the chance to create the content then everything is working as expected. Also if there is missing content dependencies after an import the user should be informed so they know stuff is broken.

So testing the ConfigImporter reveals that importing a block through the config importer does indeed fire the ConfigEvents::IMPORT_MISSING_CONTENT event missing the correct missing content - and the dependencies are not removed during sync because of the following code:

    if (!$this->isSyncing()) {
      // Ensure the correct dependencies are present. If the configuration is
      // being written during a configuration synchronization then there is no
      // need to recalculate the dependencies.
      $this->calculateDependencies();
    }

So actually everything is behaving as expected for config sync. The question is more complicated in terms of install profiles but I think the answer there is to use a config sync based installer instead of our current installer (see the config_installer project). A further question is what do we do for module installation where the config depends on missing content - this is the test added by the current patch. Also given that content_block placements can not exist at all when the content block is missing should the content block module listen and stub if the content is missing - and should be apply this generally?

Here's what I think we should do:

1. Inform users that certain configuration is missing content dependencies after importing
2. Prevent installation of configuration from modules if content dependencies are missing (this is the simplest thing to do) - at the moment they are ignored. This will fix install profiles for now whilst the installer is using regular module install
3. Discuss whether or not we want to provided the ability to create stubs for the missing content in core

Also I just tested importing and re-exporting a block with a missing content dependency and is it maintained.

#28 works for me.

28.2 will break one of the primary uses of default_content module
Pretty sure agov and Berdir's/md-systems' profile are using it that way.

Yes, I don't think that is an option, how are modules supposed to create that content then. There's nothing that runs before config import AFAIK and content might depend on other config. So the config import must happen first (think custom block type > custom block > plugin > block placement) and then modules can create content, e.g. with default_content or hook_install or whatever they want.

That definitely seems like an API change if we'd change that now?

@Berdir thinking custom block type > custom block > plugin > block placement during a module install - in HEAD we have a problem. The block placement will be saved during the module install and this will recalculate the dependencies and at that point the content does not exist - so the plugin does not exist - so the block is not saved correctly.

I don't think BlockContentBlock even adds a dependency in the first place? It's the *plugin* that depends on the content entity, not the config entity, because it's a derivative (which I still think is bad architecture, but that's a different topic).

But that's kind of OK, we have that covered. You'd simply get that fallback plugin as long as the content entity doesn't exist.

So that example isn't actually really related, but there could be others that do involve config dependencies like that.

IIRC #2407853: TaxonomyIndexTid Views plugin stores selected terms with the ID instead of UUID is also related to this, that's a patch I need for our install profile.

@alexpott, @catch, and I agreed that this is a major bug because of the lost dependencies.

Based on the conversation looks like more work is neede.d