Calling TypedConfigManager::create() does not use the definitions of TypedConfigManager (config schema), but of TypedDataManager (field types)

Created on 23 November 2023, over 1 year ago

Problem/Motivation

Discovered in πŸ“Œ Configuration schema & required keys Fixed .

TypedConfigManager does not correctly override all of the methods of its base class, TypedDataManager.

The result is that sometimes it will get confused and NOT use the correct plugin definitions (*.schema.yml files' contents, surfaced by \Drupal\Core\Config\Schema\ConfigSchemaDiscovery), but instead those of the parent class (@DataType plugins).

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

11.0 πŸ”₯

Component
ConfigurationΒ  β†’

Last updated 4 days ago

Created by

πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @wim leers
  • Merge request !5521Add basic test coverage. β†’ (Open) created by wim leers
  • Status changed to Needs review over 1 year ago
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    I think we should add explicit test coverage for all methods on TypedConfigManager, because it's unclear which code paths work correctly and which ones fail in the way described in the issue summary and demonstrated in the basic test coverage already in the MR.

  • Pipeline finished with Failed
    over 1 year ago
    Total: 676s
    #54412
  • Status changed to Needs work over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    The test added seemed to work. Not sure if this issue is meant to include all test coverage or if that should be a follow up. Could that be noted in IS?

  • πŸ‡§πŸ‡ͺBelgium borisson_ Mechelen, πŸ‡§πŸ‡ͺ

    I think the plan is to include all test coverage in this issue, I think that would make the most sense?

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Bumping to given the data corruption/pollution nature of the problem.

  • heddn Nicaragua

    I think this is what is breaking πŸ“Œ Fix head tests or fix related functionality if necessary Active . Something between 10.2 and 10.3 changed dramatically enough that this happens all the time with that module.

  • heddn Nicaragua

    Something is overriding the typedDataManager. Adding a $definition->setTypedDataManager($this->container->get('config.typed')); seems to fix this for the contrib project. I'm not sure we actually want to do more than just do that on the contrib project so I'm going to remove that as a blocker. I've updated the test coverage. But we still want to make sure all the methods are tested i.e. #6.

  • Pipeline finished with Failed
    10 days ago
    Total: 308s
    #551516
Production build 0.71.5 2024