Single config export UI exports the wrong entity properties and sometimes in the wrong format

Relevant IRC discussion:

12:36:57 WimLeers: alexpott: RE: https://www.drupal.org/node/2464427 + default views
12:36:59 Druplicon: https://www.drupal.org/node/2464427 => Replace CacheablePluginInterface with CacheableDependencyInterface #2464427: Replace CacheablePluginInterface with CacheableDependencyInterface => 110 comments, 12 IRC mentions
12:37:34 WimLeers: alexpott: why does the export UI do this:

  $form['export']['#value'] = Yaml::encode($this->configStorage->read($name));

and not respect

  \Drupal\Core\Config\Entity\ConfigEntityTypeInterface::getPropertiesToExport()

?
12:37:50 WimLeers: alexpott: the cacheability metadata in the View config entity should not be exported
12:37:52 WimLeers: dawehner: ^^
12:38:09 dawehner: WimLeers: i know that getPropertiesToExport got added later, maybe this is why
12:38:25 WimLeers: ah hmmm
12:38:33 WimLeers: that seems like it's at least major, potentially critical?
12:38:45 WimLeers: Because it means that all exported config as of today could contain things it should not contain?
12:39:02 dawehner: WimLeers: why is the export UI critical?
12:39:20 WimLeers: dawehner: because the export UI is how people create default views for example
12:39:34 dawehner: WimLeers: so?
12:39:37 WimLeers: dawehner: and because it exports things it should not export, https://www.drupal.org/node/2464427 disrupts default views
12:39:38 Druplicon: https://www.drupal.org/node/2464427 => Replace CacheablePluginInterface with CacheableDependencyInterface #2464427: Replace CacheablePluginInterface with CacheableDependencyInterface => 110 comments, 13 IRC mentions
12:39:53 dawehner: well then we have >100 criticals in the major queue
12:39:55 WimLeers: dawehner: i.e. data model changes in NON-EXPORTED things should not require re-exporting
12:40:02 dawehner: there was major issues way worse, just telling you
12:40:16 WimLeers: dawehner: I'm asking because this will affect every piece of exported config in the long run
12:40:25 WimLeers: dawehner: not every major has such long-term consequences as this one
12:40:40 alexpott: WimLeers: I think this is just a major bug
12:40:44 dawehner: well, I consider the export UI as pure debugging tool
12:40:53 WimLeers: alexpott: ok
12:41:01 dawehner: and config_devel the way how you actually export default views
12:41:07 WimLeers: dawehner: aha!
12:41:10 WimLeers: dawehner: I didn't know that

Seems to me this is important to land during the RC phase, because otherwise lots of people will end up exporting config incorrectly…

Per https://groups.drupal.org/node/484788, I cannot actually add that tag, but I did anyway for clarity. So, also added the "Needs committer feedback" tag and assigning to Alex Pott, who's both a committer and the authority on the configuration system, so he can decide whether to keep or remove that tag.

I'm fine with #20's course of action. But I'd like an answer to the original reason for reporting this issue:

The cacheability metadata in the View config entity should not be exported, but the above bug causes it to be exported anyway.

Consequently, all configuration exported through the config export UI until this is fixed will contain data that, when its schema changes, forces people to re-export or manually fix their exported config, rather than that being something that is fixed transparently.

I just checked, and View config entities' cache_metadata is still being exported at /admin/config/development/configuration/single/export. So, what I reported is still happening.

But then I started wondering… what is actually wrong with that? ConfigEntityType::getPropertiesToExport() looks at the config_export key in a config entity type's annotation. For the View config entity, that is:

 *   config_export = {
 *     "id",
 *     "label",
 *     "module",
 *     "description",
 *     "tag",
 *     "base_table",
 *     "base_field",
 *     "core",
 *     "display",
 *   }

… and since cache_metadata lives several levels under the display key, this is actually totally correct.

So, actually …

1. my bug report didn't make sense in the first place?
2. I wonder why I even expected cache_metadata not to be exported? I now wonder if I expected that because the patch in #2318377: Determine whether a view is cacheable and its required contexts, store this i/t config entity didn't update the exported View config entity YAML files? Which was something we did quite frequently back then, which is why we've re-exported all built-in configuration in one go at a later time: #2567183: Re-export all built-in configuration in core.

It's fine to re-close this with the simple statement that I was indeed completely wrong. But then… why didn't anybody call me out on that? :P