customizable doctrine mappings #214
Conversation
| @@ -34,7 +36,9 @@ public function warmUp($cacheDir): void | |||
| $filesystem->mkdir($target = $cacheDir.'/'.$this->dirName); | |||
|
|
|||
| foreach ($this->mappingFiles as $file) { | |||
| $filesystem->copy($file, $target.'/'.basename($file)); | |||
| $contents = file_get_contents($file); | |||
| $contents = str_replace('%msgphp.doctrine.key_max_length%', $this->keyMaxLength, $contents); | |||
There was a problem hiding this comment.
let's move this to Doctrine\MappingConfig e.g. MappingConfig::interpolate(string $contents) which replaces its own property placeholders.
There was a problem hiding this comment.
I've created such a class in commit: ae59c6c. Was this what you meant?
As a side effect I had to enable the autowireing for the make command as well as the CacheWarmer. IMO this should be fine. Any objection against it?
|
I thought a bit about it and I would reduce the default limit from 255 characters to 191 characters to ensure every one will have a "it works out of the box" experience. Additionally, I would suggest to add a receipt to https://github.com/symfony/recipes-contrib/tree/master/msgphp which adds the config option (with the new default 191 character limit), so people will be able to simply change it, if they like. What do you think? |
| $container->register(DoctrineInfra\MappingConfig::class) | ||
| ->setPublic(false) | ||
| ->setArgument('$keyMaxLength', '%msgphp.doctrine.mapping.key_max_length%'); | ||
|
|
||
| if (FeatureDetection::hasFrameworkBundle($container)) { | ||
| $container->register(DoctrineInfra\MappingCacheWarmer::class) |
There was a problem hiding this comment.
$container->autowire(...) and remove setAutowired below
| public static function provideObjectFieldMappings(): iterable | ||
| private $keyMaxLength; | ||
|
|
||
| public function __construct(int $keyMaxLength = 255) |
There was a problem hiding this comment.
correct, I'll remove it
| $this->keyMaxLength = $keyMaxLength; | ||
| } | ||
|
|
||
| public static function provideObjectFieldMappings(int $keyMaxLength = 255): iterable |
There was a problem hiding this comment.
so we update the contract to ObjectFieldMappingsProviderInterface::provideObjectFieldMappings(MappingConfig $config)
i was also thinking to rename this part to simply MappingProviderInterface
|
@ro0NL most of the changes are done. Could you elaborate on where exactly I'll have to put As far as I can see, so far no config file for this exists. Would I need to make the modification in https://github.com/symfony/recipes-contrib/tree/master/msgphp? Another (related) question about: #214 (review) |
|
@ro0NL all right, if I understood all your changes correctly it should be ready now. The changeable parameters are: Questions:
|
|
About the unit tests, I think there is a caching issue or it tries to pull some things from the master branch. See: https://travis-ci.org/msgphp/msgphp/jobs/446701263 As in https://github.com/pascalwacker/msgphp/blob/key_length/src/Domain/Infra/Doctrine/ObjectFieldMappingsProviderInterface.php#L18 the interface should be: but it seams that the unit test is not seeing the Interface correctly. Anything we can do about that? |
There was a problem hiding this comment.
@pascalwacker can you rebase please? Master is green again :)
2c8489f to
126a78a
Compare
|
@ro0NL I had to move the config param to And there's a new As far as I can tell, this exception is thrown because of the changes here: 5cd0beb#diff-aa27d06b004f798c679f353e887025c6 |
| /** | ||
| * @author Pascal Wacker <hello@pascalwacker.ch> | ||
| * | ||
| * @internal |
There was a problem hiding this comment.
i think we should remove it, as we pass it as a service now
There was a problem hiding this comment.
the @internal annotation?
Where do I have to do this? Sorry, I've never worked on a flex receipt enabled project before. |
f709e5a to
843b414
Compare
|
@ro0NL like this? pascalwacker/recipes-contrib@103e4e6 (I haven't created a pull request yet) I'm guessing, that I need to enable |
|
I'm sorry, but I don't get it, what do I have to change at the annotation? Change |
|
All right, pull request is created at symfony/recipes-contrib#550 |
|
As requested, I've moved the However I'm not sure if it works as expected. As soon as I have in a symfony project a parameter in the All I get in Can you confirm this behaviour? And if so, any idea, how to fix it? |
There was a problem hiding this comment.
all good on 4.1 :) thanks @pascalwacker this works really nice.
|
no problem, it was cool working on it! At times I felt like an absolute beginner having no idea what I'm actually doing :P Do you have any reads you would recommend on domain/message driven applications, besides the ones listed in https://msgphp.github.io/ (or on the inner workings of Symfony, like all the compile pass magic going on)? |
based on the previous pull #213 if've also integrated the
key_max_lengthparam for the*.orm.xmlfiles as well as theRole.phpfile. Since I didn't know if this is the road to go down, I've decided to create a separate pull request, in case you want to merge 213 but no this one.