Skip to content
This repository has been archived by the owner on May 29, 2024. It is now read-only.

Feature/cascade naming strategy to embedded documents #33

Open
wants to merge 10 commits into
base: 2.0.x
Choose a base branch
from

Conversation

matwright
Copy link
Contributor

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.

veewee and others added 10 commits June 14, 2016 20:22
the $value argument must be an object as per annotations in the
$hydrator->extract() method which is called within this method.
Passing other values, particularly null values causes exceptions to be
thrown.
test case to demonstrate that embed field hydration fails when the embedded file is not set (null or not-exists in DB)
updated param docbloc annotation
test if not object then return same value
'origin/hotfix/test-case-to-demonstrate-embed-field-break-when-not-set'
into hotfix/avoid-sending-null-embedded-objects-to-hydrators
@matwright
Copy link
Contributor Author

This PR also includes my previous bug fix #30

@veewee
Copy link
Contributor

veewee commented Jul 8, 2016

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

@veewee veewee added this to the 2.0.2 milestone Jul 8, 2016
@veewee
Copy link
Contributor

veewee commented Jul 8, 2016

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
Copy link
Contributor Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants