-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PropertyAccess] Allow different prefixes than "add" and "remove" #9336
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
Comments
ref #5013 |
@bschussek it should actually allow defining the full name for adders and removers, not only the prefix. This will allow people to customize it in case they write their method names in a different language than English, for whch the inflection rules will fail to find the singular. See #9205 for the report |
I would like to take on this ticket as my first active involvment with Symfony. In which branch should I make this contribution? |
After looking further into this and into Ticket #5013 I don't think this is such an Easy Pick. Adding the ability to the current setValue/getValue through an $options array would lead to a lot of changes just so that PropertyAccessor::findAdderAndRemover() can use those settings. That could call for a similar approach as the enableMagicCall(). Maybe something like enableCallbacks() which takes an array of callback names for add/remove/get/set. I really would like to get some feedback on this. |
@webmozart Is this still relevant? I've been using this component so this might be easy. What's the final decision? I would be for custom callbacks. Because there might be loads off different prefixes like has/join/leave/remove/attach/add/get/set etc... (just from the top of my head). Any relevant sources? |
Ping @webmozart |
@TomasVotruba Indeed this is not such an easy pick. This post in #9336 and the following posts explain how this could be solved in a nice manner. The difficulty is that, if we use annotations to configure property paths, we also have to provide XML and YAML mappings and their respective loaders, as we do in other places. That's quite a bit of work and duplicating what we already have in other components. A solution for this problem (at least for XML) would be native support for XML-based annotations in Doctrine, and as far as I know (ping @Ocramius) this is WIP. Second, we could refactor all the different metadata loaders into a separate component. There is a library that does that already, but it's not maintained anymore and I'm not sure whether it applies to our use cases. The benefit is that the PropertyAccess-based serializer would become more powerful, as would the Form component. |
@webmozart I've been studying how metadata gets loaded by other Symfony components (i.e. Validator, Routing, etc.) and you're right: there is a lot of overlapping. I'm about to try replicating the loader code from the Validator component while taking out any unneeded feature as a first approach. Later we can figure out how to extract all metadata loaders their own independent component. The library you pointed out, |
Just a quick note: The loader from the Validator component is the one that we need to get rid of. All the other components are already using a standardized way. |
It's not something for ORM 2.x anyway. We are crunching on it, but far from getting anywhere soon, sorry. |
https://github.com/rollerworks/metadata/ MIT licensed, documentation is missing currently. Feel free to reuse my work and move it Symfony 👍 |
…rface and implementation on reflection (joelwurtz, Korbeil) This PR was merged into the 5.1-dev branch. Discussion ---------- [PropertyInfo] Add accessor and mutator extractor interface and implementation on reflection | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #30248, partially: #22190, #18016, #5013, #9336, #5219, | License | MIT | Doc PR | TODO This PR brings accessor / mutator extraction on the PropertyInfo component, There is no link to existing code, as IMO it should be in another PR as this will add a dependency on property access to the property info component and not sure this is something wanted (although, it will reduce a lot of code base on the property access component as a lot of code seems to be duplicated) Code is extracted from #30248 also there is some new features (that can be removed if not wanted) * Allow extracting private accessor / mutator (will do a new PR that improve private extraction on reflection latter) * Allow extracting static accessor / mutators * Allow extracting constructor mutators Current implementation try to be as close as the PropertyAccess implementation and i did not reuse some methods already available in the class as there is some differences in implementation, but maybe it will be a good time to make this consistent (Looking forward to your input) ? Things that should be done in a new PR: * Linking property info to property access to remove a lot of duplicate code * Add a new system that allow adding Virtual Property based on this extractor Commits ------- 0a92dab Rebase, fix tests, review & update CHANGELOG fc25086 [PropertyInfo] Add accessor and mutator extractor interface and implementation on reflection
Yes, now you can instantiate PropertyAccessor with custom In CustomWriteInfoExtractor:
class: Symfony\Component\PropertyAccess\PropertyAccessor
arguments:
$arrayMutatorPrefixes: ['join', 'leave']
Symfony\Component\PropertyAccess\PropertyAccessorInterface:
arguments:
$writeInfoExtractor: '@CustomWriteInfoExtractor' |
…orbeil) This PR was merged into the 5.1 branch. Discussion ---------- [PropertyAccess] Non-standard adder/remover methods Will add documentation about non-standard adder/remover methods for PropertyAccessor. Related to #13023 and symfony/symfony#9336 This is not a complete documentation for related issues, but this will introduce one of the possible new use. Commits ------- bf6cca9 Non-standard adder/remover methods
Currently, adders and removers must be prefixed with "add" and "remove". When applying DDD, this sometimes doesn't make sense, for example:
Here it would make much more sense to prefix the methods with "join" and "leave":
The PropertyAccessor should provide a way to use these methods.
The text was updated successfully, but these errors were encountered: