Fix and use TypedConfigManagerInterface/TypedConfigManager

This looks pretty good to me.

Only one issue. The documentation of TypedConfingManagerInterface::createInstance() method doesn't match because of that $config array. But instead of fixing the documentation I'd rather update the code so it is still more consistent with TypedDataManager.

/**
   * Instantiates a typed configuration object.
   *
   * @param string $data_type
   *   The data type, for which a typed configuration object should be
   *   instantiated.
   * @param array $configuration
   *   The plugin configuration array, i.e. an array with the following keys:
   *   - data definition: The data definition array.
   *   - name: (optional) If a property or list item is to be created, the name
   *     of the property or the delta of the list item.
   *   - parent: (optional) If a property or list item is to be created, the
   *     parent typed data object implementing either the ListInterface or the
   *     ComplexDataInterface.
   *
   * @return \Drupal\Core\Config\Schema\Element
   *   The instantiated typed configuration object.
   */
  public function createInstance($data_type, array $configuration = array());

Also on the implementation itself there's one comment that is wrong, where it reads:

    // Allow per-data definition overrides of the used classes, i.e. take over
    // classes specified in the data definition.

it should say

    // Allow per-data definition overrides of the used classes, i.e. take over
    // classes specified in the type definition.

that is just the same as TypedDataManager.

I think the updated code also clears up the difference between 'data definition' and 'type definition'.

Attached updated patch and inter-diff.

There are other consistency issues, like that DataDefinition objects, and the 'list class' that were changed in TypedDataManager, but unfortunately not in TypedConfigManager, but I think that will better go in a follow up patch, since this one only changes internal parameters, but the other may have a bigger impact

@alexpott,

Right, updated patch with that change

This is the inter-diff.

--- a/core/lib/Drupal/Core/Config/TypedConfigManager.php
+++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
@@ -104,8 +104,6 @@ public function create(array $definition, $value = NULL, $name = NULL, $parent 
       $definition['type'] = $this->replaceName($definition['type'], $replace);
     }
     // Create typed config object.
-    $definition['name'] = $name;
-    $definition['parent'] = $parent;
     $wrapper = $this->createInstance($definition['type'], array(
       'data_definition' => $definition,
       'name' => $name,

Please review updated patch as the last patch #4 was not getting applied in current 8.x branch (2d3d33b48f901b1fedd9ad00f176e3bff17056ec). Below was the error,

$ git apply -v 2271967-4.patch
Checking patch core/lib/Drupal/Core/Config/Config.php...
Checking patch core/lib/Drupal/Core/Config/ConfigFactory.php...
Checking patch core/lib/Drupal/Core/Config/ConfigManager.php...
Hunk #1 succeeded at 36 (offset 1 lines).
error: while searching for:
   *   The entity manager.
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
   *   The configuration factory.
   * @param \Drupal\Core\Config\TypedConfigManager $typed_config_manager
   *   The typed config manager.
   * @param \Drupal\Core\StringTranslation\TranslationManager $string_translation
   *   The string translation service.
   */
  public function __construct(EntityManagerInterface $entity_manager, ConfigFactoryInterface $config_factory, TypedConfigManager $typed_config_manager, TranslationManager $string_translation, StorageInterface $active_storage) {
    $this->entityManager = $entity_manager;
    $this->configFactory = $config_factory;
    $this->typedConfigManager = $typed_config_manager;

error: patch failed: core/lib/Drupal/Core/Config/ConfigManager.php:60
error: core/lib/Drupal/Core/Config/ConfigManager.php: patch does not apply

I have made the given changes in core/lib/Drupal/Core/Config/ConfigManager.php and now it applies successfully.

@amitgoyal,
Not sure I get it, the patch was much bigger than #6.

Anyway, I'll be re-rolling the patch in #4, for any core updates, and also fixing the issues found by the test

@Jose - Oh! I just provided the changes in file core/lib/Drupal/Core/Config/ConfigManager.php in my last patch at #6.

Attaching here the complete patch (#4 + #6).

Not very sure about the issues found by automated test script. So please guide me or fix them. Thanks!

@amitgoyal,
I think the only issue with the patch is that it expects 'parent' and 'name' to be part of the definition array. But they are not anymore (because of #3), nor they need to be there,

So this is just the test making a wrong assumption, just need to fix the test.

@alexpott,
Yes, they shouldn't.

They used to be passed in the same parameters array , but not anymore, see
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21TypedData...

Note the only tests that fail are the ones that assume they are into the array, not the others, that actually test schema functionality.

Re-rolled the patch in #10 for PSR-4 update.

I think this is ready.

We need this committed to move on with #1928868: Typed config incorrectly implements Typed Data interfaces

I agree this is a much needed cleanup. It looks like API changes but its documenting the API better with the expected methods documented on the interface instead of just being there :D

15: 2271967-psr4.patch queued for re-testing.

Committed/pushed to 8.x, thanks!

nice improvement and the interface looks much better now.