phpro / zf-doctrine-hydration-module

Configurable Doctrine hydrators for ZF2
18 stars 33 forks source link

Feature/cascade naming strategy to embedded documents #33

Open matwright opened 8 years ago

matwright commented 8 years ago

When a naming strategy is set for an object, the strategy is not applied to embedded objects.

example object:

$name //mapped to @String
$embedMany //mapped to  @EmbedMany

if I set a naming strategy for the above object to prefix names with an underscore I would get the following back when extracting:

{
"_name" : "jane doe","_embedMany" : [{ "someFieldName" : "some value"}]                         
}

Note the naming strategy has been applied to the main object field names but not the embedded document field name (someFieldName). There is currently no means of configuring a naming strategy for embedded documents and if there were such an option I don't think it would be very practical considering that documents may have many embedded documents. I assume that the namingStrategy should always be cascaded for consistency. This PR provides a feature to always cascade naming strategies to embedded documents.

Although I haven't included one, a config option may be required for backward compatibility, for example a 'cascade=false' on the naming strategy config.

I have included tests for both hydration and extraction, the tests confirm that the naming strategy cascades and can therefore also be used to confirm that this is not the case currently when ran with the current v2 release.

matwright commented 8 years ago

This PR also includes my previous bug fix https://github.com/phpro/zf-doctrine-hydration-module/pull/30

veewee commented 8 years ago

Thanks for the PR. I'll try to review asap.

veewee commented 8 years ago

HI @matwright,

Can you rebase against 2.0.X? The changes of your other PR got merged in. This feature does break BC so I guess it should be configurable and disabled by default.

I would try to avoid passing the naming strategy from the ODM hydrator all the way down the strategy classes. Is there another way we could attach the naming strategy to the hydration strategy? Maybe something like a wrapper strategy or an interface we can apply to a strategy? Maybe the strategies can be configured with a new naming straegy based on the configuration? Then we only need the naming stragy in the embed strategies?

matwright commented 8 years ago

Hi @veewee yes I'll rebase and spend some more time on the feature to add the config option and see what can be done to attache the naming strategy to the hydrator less intrusively.