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

Rebuild bundle config #41

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Rebuild bundle config #41

wants to merge 11 commits into from

Conversation

jdreesen
Copy link
Member

@jdreesen jdreesen commented Feb 6, 2023

When I wanted to implement #39, I realized that the current bundle configuration does not really allow configuring different converter types (as already described in #40). It is possible to specify any converter class, but it must have the exact same constructor parameters as the class GenericConverter. This is not the case with the StrategicConverter, so that the current configuration is not suitable for this type.

Therefore, here is the proposal of an implementation that allows arbitrary converter types. Instead of defining the class, the type is now specified by means of a keyword and the corresponding configuration is nested underneath.

The advantage is that the configuration per type is freely definable. In addition, this allows the configuration to be extended by other bundles or the app.

Current:

neusta_converter:
  converter:
    person.converter:
      converter: Neusta\ConverterBundle\Converter\GenericConverter
      target_factory: Neusta\ConverterBundle\Tests\Fixtures\Model\PersonFactory
      populators:
        - Neusta\ConverterBundle\Tests\Fixtures\Populator\PersonNamePopulator

Proposed:

neusta_converter:
  converters:
    person.converter:
      generic:
        target_factory: Neusta\ConverterBundle\Tests\Fixtures\Model\PersonFactory
        populators:
          - Neusta\ConverterBundle\Tests\Fixtures\Populator\PersonNamePopulator

However, it is not yet entirely clear how decorating converters fit into this scheme. Currently, only exactly one converter type keyword is allowed. One consideration would be to create a new interface for decorating converters and to allow several decorating converters in addition to the one main converter, like:

neusta_converter:
  converters:
    person.converter:
      generic:
        target_factory: Neusta\ConverterBundle\Tests\Fixtures\Model\PersonFactory
        populators:
          - Neusta\ConverterBundle\Tests\Fixtures\Populator\PersonNamePopulator
      cached:
        key_factory: YourNamespace\UserKeyFactory

@jdreesen jdreesen added the bug Something isn't working label Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants