Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing TypedFieldMapper config #1595

Closed
michnovka opened this issue Dec 19, 2022 · 11 comments
Closed

Missing TypedFieldMapper config #1595

michnovka opened this issue Dec 19, 2022 · 11 comments

Comments

@michnovka
Copy link

michnovka commented Dec 19, 2022

Now that 2.14.0 is released, lets agree on the best syntax of doctrine bundle config

Since I believe most people will use the DefaultTypedFieldMapper and maybe supply it few additional specific mappings (possible via its constructor), Id like to have a simple option to do this:

doctrine:
    orm:
        typed_field_mapper:
            mapper_class: 'default'
            additional_mappings:
                - App\Objects\CustomObject: App\DBAL\Types\CustomTypeFQCN
                - App\Objects\CustomObject2: custom_type2_using_name_from_dbal

I would like this syntax to be special for the default mapper, as I really believe this will be the most commonly used application.

Another special case will be the ChainTypedFieldMapper and I propose this syntax:

doctrine:
    orm:
        typed_field_mapper:
            mapper_class: 'chain'
            mappers:
                - App\DBAL\TypedFieldMapper\CustomTypedFieldMapper
                    arguments:
                            $typedFieldMappings:
                                - App\Objects\CustomObject: App\DBAL\Types\CustomTypeFQCN
                                - App\Objects\CustomObject2: custom_type2_using_name_from_dbal
                - App\DBAL\TypedFieldMapper\CustomTypedFieldMapper2
                - 'default'

And of course, the general use of any other custom mapper can look like this:

doctrine:
    orm:
        typed_field_mapper:
            mapper_class: App\DBAL\TypedFieldMapper\CustomTypedFieldMapper
                arguments:
                        $typedFieldMappings:
                            - App\Objects\CustomObject: App\DBAL\Types\CustomTypeFQCN
                            - App\Objects\CustomObject2: custom_type2_using_name_from_dbal
                        $someOtherConstructorParam: true

In both above cases, the arguments part will be specified based on whatever is required by the constructor of any given TypedFieldMapper.

Lets discuss. @derrabus @greg0ire @beberlei @stof

@derrabus
Copy link
Member

derrabus commented Dec 31, 2022

Another special case will be the ChainTypedFieldMapper and I propose this syntax:

doctrine:
    orm:
        typed_field_mapper:
            mapper_class: 'chain'
            mappers:
                - App\DBAL\TypedFieldMapper\CustomTypedFieldMapper
                    arguments:
                            $typedFieldMappings:
                                - App\Objects\CustomObject: App\DBAL\Types\CustomTypeFQCN
                                - App\Objects\CustomObject2: custom_type2_using_name_from_dbal
                - App\DBAL\TypedFieldMapper\CustomTypedFieldMapper2
                - 'default'

This $arguments thing looks a lot like we're reinventing the DI component here. I believe the app should register those custom mappers as services so that we only deal with service IDs in the bundle configuration.

@michnovka
Copy link
Author

Im afraid this is too complicated for me to implement in the limited free time I will have in the next 3 months, is anybody ready to take over and implement this please?

I agree with @derrabus - lets register the typed field mappers as services. then Id allow for these 3 configs

Default mapper with optional additional mappings (this can be passed to its constructor). This will most likely be the most used config, as if you just want to add one or 2 custom typed field mappings, this is the way to go:

doctrine:
    orm:
        typed_field_mapper:
            mapper_class: 'default'
            additional_mappings:
                - App\Objects\CustomObject: App\DBAL\Types\CustomTypeFQCN
                - App\Objects\CustomObject2: custom_type2_using_name_from_dbal

chain linking of mapper services:

doctrine:
    orm:
        typed_field_mapper:
            mapper_class: 'chain'
            mappers:
                - ... array of service tags that represent the mappers

and single custom mapper service:

doctrine:
    orm:
        typed_field_mapper:
            mapper_class: 'typed field mapper service tag'

@stof
Copy link
Member

stof commented Jan 6, 2023

Custom mappers should not attempt to be configured in the bundle semantic config. Projects should define them as services (giving them the full capabilities of the DI component instead of a subset we would expose) and tag them (which could be done by autoconfiguration).

@ostrolucky
Copy link
Member

I agree. If you want this as part of doctrine.yaml, you are free to define services: section directly in doctrine.yaml and define it there. Let's close.

@ostrolucky ostrolucky closed this as not planned Won't fix, can't repro, duplicate, stale Jan 6, 2023
@michnovka
Copy link
Author

I dont understand - say I define the typedfieldmapper and tag it in services: section, what next? How do I make doctrine use this typed field mapper? At least such config option should be added to doctrine.yaml.

@ostrolucky
Copy link
Member

You should be able to do something like so

services:
  Doctrine\ORM\Mapping\TypedFieldMapper:
    arguments: - whatever you want here -
  doctrine.orm.configuration:
    class: Doctrine\ORM\Configuration
    calls: 
      - setTypedFieldMapper: ['@Doctrine\ORM\Mapping\TypedFieldMapper']

@michnovka
Copy link
Author

@ostrolucky thanks, where is the appropriate place to write docs about this? Should I mention it here: https://www.doctrine-project.org/projects/doctrine-orm/en/2.14/reference/typedfieldmapper.html ?

@ostrolucky
Copy link
Member

Honestly not sure if this should be documented, or what's the best way to document it. It shouldn't be specific to mappers, but more generic, doc for adjusting Doctrine\ORM\Configuration. Additionally, this example I posted will need to look different in case people are using multiple entity managers and so on, writing doc requires some more thought put into it.

@michnovka
Copy link
Author

I would like to reopen this and maybe propose alternative solution.

doctrine:
    orm:
        default_typed_field_mapper:
            additional_mappings:
                - App\Objects\CustomObject: App\DBAL\Types\CustomTypeFQCN
                - App\Objects\CustomObject2: custom_type2_using_name_from_dbal

I think it is important and has valid use-case to allow users to easily add custom typed field mappings on top of the default ones. If they want to use any of the funky logic or chain mappers, or whatever, they are free to make it and configure it using DI. But in lot of projects I worked with this is not needed, you only maybe want to auto-map one or two objects to some custom (or even existing) DBAL types. This would allow it. WDYT?

@ostrolucky ostrolucky reopened this Jan 9, 2023
@ostrolucky
Copy link
Member

Looks like this is not getting a traction so I'm going to close again. We won't implement thing if there is only one known case of this usage.

@ostrolucky ostrolucky closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2023
@michnovka
Copy link
Author

michnovka commented Oct 16, 2024

@ostrolucky

You should be able to do something like so

services:
  Doctrine\ORM\Mapping\TypedFieldMapper:
    arguments: - whatever you want here -
  doctrine.orm.configuration:
    class: Doctrine\ORM\Configuration
    calls: 
      - setTypedFieldMapper: ['@Doctrine\ORM\Mapping\TypedFieldMapper']

I know this was a long time ago, but this is broken. See doctrine/DoctrineMigrationsBundle#557 The problem is that when compiled, the setTypedFieldMapper defined like this is called before the DependencyInjection's setTypedFieldMapper (which uses DefaultTypedFieldMapper)

So there is no way to configure the TypedFieldMapper in Symfony currently without redefining custom EntityManager (where you can specify its TypedFieldMapper

But seems that we now have #1802 which allows us to configure it under doctrine.orm.typed_field_mapper

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

No branches or pull requests

4 participants