[meta] Find out what config schemas are still missing and add them

Here are all the exceptions (just grepped the HTML of the qa results page)

  50 config_test.noschema
  48 migrate.migration.d6_action_settings
  30 language.settings
  18 content_translation.settings
  10 form_test.object
  10 color.bartik
   8 language.config.de.locale_test.translation
   6 language.config.xx.system.site
   4 language.config.xx.system.date_format.fallback
   4 language.config.xx.image.style.medium
   4 language.config.xx.contact.category.feedback
   4 config_test.query.1
   4 config_events_test.test
   4 color_test_theme.settings
   4 breakpoint.breakpoint_group.atestset
   2 views_test_data.tests
   2 language.config.xx.system.maintenance
   2 language.config.nl.menu_test.menu_item
   2 language.config.it.tour.tour.tour-test
   2 language.config.fr.system.site
   2 language.config.de.block.block.stark_admin
   2 foo.bar
   2 config_test.query.entity1
   2 config_test.crud
   2 breakpoint.breakpoint.custom.user.

Note the trailing . on that last breakpoint one

Here they are per namespace, maybe an issue per each?

  66 language
  58 config_test
  48 migrate
  18 content_translation
  10 form_test
  10 color
   6 breakpoint
   4 config_events_test
   4 color_test_theme
   2 views_test_data
   2 foo

Wowza. That's a lot of missing data model.

Discussed with @alexpott. We really kinda need to make this beta-blocking. =/

Why does #2167623: Add test for all default configuration to ensure schema exists and is correct not catch this? Does this config have partial, but incomplete, schemata?

We aware few of them in the list and omitted schema with reason when getting #2167623: Add test for all default configuration to ensure schema exists and is correct in. Here is an update on each item:

We have Drupal\config\Tests\DefaultConfigTest to test incomplete schema.

1. config_test.noschema - Has to be skipped as it tests TypedConfigManagerInterface::hasConfigSchema() method.
2. migrate.* - #2183957: Provide configuration schema for Migration module
3. language.config.*.*.* - #2168609: Move config translations (language.config.[langcode]) in to new location but it is closed as duplicate of #2224887: Language configuration overrides should have their own storage

Yet to fix:

Language:
  30 language.settings
  18 content_translation.settings

Config Test:
   4 config_test.query.1
   4 config_events_test.test
   2 config_test.query.entity1
   2 config_test.crud

Breakpoint:
   4 breakpoint.breakpoint_group.atestset
   2 breakpoint.breakpoint.custom.user.
Color:
  10 color.bartik
   4 color_test_theme.settings

Views:
   2 views_test_data.tests

System:
  10 form_test.object
   2 foo.bar

#2246557: Missing schema for entity area plugin

Made this a parent for #2183957: Provide configuration schema for Migration module and #2246557: Missing schema for entity area plugin too.

I wanted re re-run the patch to see the current errors. but it didn't apply.

conflict in core/lib/Drupal/Core/Config/Config.php was:

    if ($this->typedConfigManager->hasConfigSchema($this->name)) {
      // Ensure that the schema wrapper has the latest data.
      $this->schemaWrapper = NULL;
      foreach ($this->data as $key => $value) {
        $this->data[$key] = $this->castValue($key, $value);
      }
    }
<<<<<<< HEAD
    else {
      foreach ($this->data as $key => $value) {
        $this->validateValue($key, $value);
      }
=======
    // @todo Why are these schema not recognized during installation?
    elseif (!in_array($this->name, array('system.module'))) {
      throw new ConfigException(format_string('Configuration file @name cannot be saved due to lack of schema.', array(
        '@name' => $this->name,
      )));
>>>>>>> errors on april1
    }

resolved by putting the elseif from the patch in before the else...

    if ($this->typedConfigManager->hasConfigSchema($this->name)) {
      // Ensure that the schema wrapper has the latest data.
      $this->schemaWrapper = NULL;
      foreach ($this->data as $key => $value) {
        $this->data[$key] = $this->castValue($key, $value);
      }
    }
    // @todo Why are these schema not recognized during installation?
    elseif (!in_array($this->name, array('system.module'))) {
      throw new ConfigException(format_string('Configuration file @name cannot be saved due to lack of schema.', array(
        '@name' => $this->name,
      )));
    }
    else {
      foreach ($this->data as $key => $value) {
        $this->validateValue($key, $value);
      }
    }

but I'm not sure. validate sounds like it might be trying to catch problems also?

--
needs review just for the testbot, can put it back to active.

viewing the testbot results error was:

WD php: Drupal\Core\Config\ConfigException: Configuration file [error]
update.settings cannot be saved due to lack of schema. in
Drupal\Core\Config\Config->save() (line 220 of
/var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Config/Config.php).
Drupal\Core\Config\ConfigException: Configuration file update.settings cannot be saved due to lack of schema. in Drupal\Core\Config\Config->save() (line 220 of /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Config/Config.php).

To clarify #19, Drupal no longer installs with this test patch applied, so we want to look at a different way of finding them. :)

we could run it, and patches on a git checkout from "April 2, 2014" ... not conclusive, but should provide some kind of automation of testing.

That information is right here, yep: https://qa.drupal.org/pifr/test/762278

So given that schema were introduce to solve the issue of knowing what to translate (#1866610: Introduce Kwalify-inspired schema format for configuration, #1648930: Introduce configuration schema and use for translation) and are now also used to ensure that data types are consistent regardless of how the config is edited (#2130811: Use config schema in saving and validating configuration form to ensure data is consistent and correct ). Before we decide to do all the related issues we need to decide whether or not config schema are mandatory. Given the two use cases I don't think that we need schema for all test config. However I do think we need schema for all deployable config in core.

So far we did not make schemas mandatory BUT we introduced them as best practice to educate contribs. Adding schemas to tests is not something we need contribs to do either, but I think even if we don't technically need those schemas for tests, for contribs copy-pasting example code from the tests, it may set an example. Not sure how realistic is that, but that may be the only reason left to write schemas for those too?

Discussed again on the Drupal 8 Multilingual meeting with Alex Pott attending. We'll not do schemas for tests since they are not mandatory and they have very marginal benefits. Will close sub-issues that are test specific.

Discussed with @alexpott and @Gábor Hojtsy. Not all of the child issues here are beta blocking as some are just related to tests, so I've explicitly tagged two beta blockers and am untagging the meta.

Added #2259525: system.action.user_add_role_action.ROLE_ID does not match any config schema as sub-issue. That is closed and only #2183957: Provide configuration schema for Migration module is left now, so we can close this down I believe. There is no point in having a META for one other issue, is there? Closing as duplicate of #2183957: Provide configuration schema for Migration module on principle then because this is not fixed per say.

I am quite convinced that there is A LOT of views stuff which does not have yet proper schema.

Hum, hum. How do we find them then? :)

I am quite convinced that there is A LOT of views stuff which does not have yet proper schema.

/me worried to hear the word 'LOT' from @dawehner :)

I wanted to extend config inspector to help debug missing schemas (see #2266427: Add more info on schema validity to configuration inspector for debugging for current state) and found on the way that the core schema tester is not complete, it misses several cases where it should fail. Also if it could be reusable for testing config with schemas, I would not need to copy the code in the config inspector issue. Yeah.

So I opened #2266501: Array level schema errors are not reported, fix wrong schemas identified which found several legit missing schemas for empty container elements that the tests did not find before. They are not THAT many so I think we can fix them there, so adding that as a child issue of this one.

We do have a test which creates views for all available fields https://api.drupal.org/api/drupal/core!modules!views!lib!Drupal!views!Te...

We would just have to combine this with the way how schema test validates its available schemas.

@dawehner: my current idea is to start throwing exceptions form castValue() once we have #2266501: Array level schema errors are not reported, fix wrong schemas identified, #2264179: Clarify use of property/undefined and add an ignore type in configuration schema and #2268975: Fix named schema elements missing when installing Drupal which will uncover those things in ALL the tests. That will likely be a long list, so we'll need ways to partition those fails as well. Or its a short list because we fix all the things ahead... We'll see.

Thankfully one of those 3 is RTBC, one is I think at RTBC but needs confirmation/review form @vijaycs85 and the third one actually @vijaycs85 is working on :)

Opened #2274175: Block plugin schema should be defined separately from the entity

Found more schema mismatch issues and fixing them at #2293063: Config schema mapping parsing is inconsistently forgiving, does not conform to the interface. Will probably obsolete #2183983: Find hidden configuration schema issues.

Also @vijaycs85 is finding and fixing lots of config and test issues on migrations in #2293419: Add config schema test to all configuration test in migration, fix bugs all found by schema :)

#2293063: Config schema mapping parsing is inconsistently forgiving, does not conform to the interface landed. Next kid on the block is #2295737: Not all shipped configuration passes validation even with all modules enabled.

#2293063: Config schema mapping parsing is inconsistently forgiving, does not conform to the interface also spawned #2293773: Field allowed values use dots in key names - not allowed in config which is a beta blocker.

So current list of outstanding issues here is:

- #2293773: Field allowed values use dots in key names - not allowed in config (beta blocker)
- #2293419: Add config schema test to all configuration test in migration, fix bugs (close to done)
- #2295737: Not all shipped configuration passes validation even with all modules enabled (just started)

UPDATE:

#2295737: Not all shipped configuration passes validation even with all modules enabled is almost ready, and now adds tests wholesale, hopefully going to land soon.

#2301045: Standard profile has views which include elements dependent on uninstalled modules, not valid in config was opened out of that one to resolve optional views elements.

#2293773: Field allowed values use dots in key names - not allowed in config is still under work.

#2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields and its extension #2224761: Add a generic way to add third party configuration on configuration entities and implement for field configuration will cover the translation_sync settings.

So still plenty to do :/

Of the ones from the prior comment #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields and #2224761: Add a generic way to add third party configuration on configuration entities and implement for field configuration are definitely in the works still. Now that the standard profile fix is in, I could continue with #2183983: Find hidden configuration schema issues, which uncovered more problems. So will need to open more issues for those. The first one is for a whole lot of the fails: #2309247: Views do not depend on modules providing their displays.

So, plenty got done :)

The only issue left is #2183983: Find hidden configuration schema issues. Should we close this as there is nothing actionable left to do here except that issue, which is a major task as well?

I think that is fine if we elevate that to critical? :)

one more #2380457: Some fixes of the views config schema