Skip to content

Add dsnClassMap property to StaticConfigTrait#19257

Open
dereuromark wants to merge 2 commits into6.xfrom
add-static-config-trait-properties
Open

Add dsnClassMap property to StaticConfigTrait#19257
dereuromark wants to merge 2 commits into6.xfrom
add-static-config-trait-properties

Conversation

@dereuromark
Copy link
Member

Summary

  • Adds $dsnClassMap property directly to StaticConfigTrait using lazy initialization
  • Implementing classes override buildDsnClassMap() instead of declaring property
  • Removes duplicate property declarations from Cache, Log, ConnectionManager, TransportFactory, Mailer
  • Updates test app config classes to use new pattern

Why not $registry?

The $registry property is intentionally NOT added to the trait because implementing classes require specific registry types (CacheRegistry, LogEngineRegistry, ConnectionRegistry, etc.) that cannot be generalized. The trait continues to use @phpstan-ignore property.notFound for the optional $registry access in drop().

Breaking Changes

Classes extending or using StaticConfigTrait that declare their own $dsnClassMap property must:

  1. Remove the property declaration
  2. Override buildDsnClassMap() method instead

Refs https://github.com/cakephp/cakephp/wiki/6.0-Ideas

This change adds the `$dsnClassMap` property directly to StaticConfigTrait
using a lazy initialization pattern via `buildDsnClassMap()`. Classes that
need a custom DSN class map override `buildDsnClassMap()` instead of
declaring their own property.

Benefits:
- Property is now defined in the trait, eliminating PHPStan issues
- Implementing classes use `buildDsnClassMap()` for customization
- Maintains backwards compatibility via `getDsnClassMap()`/`setDsnClassMap()`

Note: The `$registry` property is NOT added to the trait because implementing
classes require specific registry types (CacheRegistry, LogEngineRegistry, etc.)
that cannot be generalized in the trait.

Refs cakephp/cakephp#wiki/6.0-Ideas
@josbeir
Copy link
Contributor

josbeir commented Feb 12, 2026

If this means not adding the $dsnClassMap property to classes anymore that don't need it I'm all for it!

@dereuromark dereuromark requested a review from ADmad February 12, 2026 06:02
@dereuromark dereuromark added this to the 6.0 milestone Feb 12, 2026
@dereuromark
Copy link
Member Author

Dont know who added it to the wiki ideas/roadmap, but since it was there, might as well look into it...

@josbeir
Copy link
Contributor

josbeir commented Feb 12, 2026

Dont know who added it to the wiki ideas/roadmap, but since it was there, might as well look into it...

Idk but noticed it's iffyness when I was creating my attribute facade factory.

It was also on my radar to do something about it, guess you beat me to it 💪

@dereuromark dereuromark marked this pull request as ready for review February 12, 2026 06:19
*
* @return array<string, class-string>
*/
protected static function buildDsnClassMap(): array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is required because PHP doesn't allow overriding the property with a different value if it's already declared in the trait?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a more explicit pattern would be to use an interface ? DsnAwareStaticConfigInterface ? Or is this overkill ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case probably a bit overkill. After all, this is also more internal, so not too much to be used from the outside.

Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me. Should dsn mapping become a utility class that gets used instead of trait properties being inherited?

@LordSimal
Copy link
Contributor

Should dsn mapping become a utility class that gets used instead of trait properties being inherited?

To find an answer for that I'd ask if that DSN parsing and mapping Utility would be used somewhere other than inside objects using the StaticConfigTrait.

Currently we have examples documented for ConnectionManager, Log, Email and Caching

I looked at the InstanceConfigTrait (and the classes using it) as it serves a similar purpose and haven't found a good usecase to use DSN mapping/parsing there.

But if there is a case I am not thinking of then sure, it can/should be extracted into a utility class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants