Partial reference support (without $db property)

[sokac]

This PR introduces new annotation property ##partial## for ReferenceOne and ReferenceMany which, when set to true, will create a reference with no $db key.

[sokac]

Does commit look good? It doesn't break old code and brings new functionality.

[jwage]

Remove space before empty(

[jwage]

This PR is missing support for the other mapping drivers (xml, yaml, etc.)

[jwage]

Also, could you explain the reasoning behind wanting to do this?

[sokac]

Sorry for the delay. The reason behind this PR is our previous code base had partial db reference (i.e. not having db in DBRef). When you issue a query with DBRef with $db it yields different result than without one. Note that $db parameter is optional (http://docs.mongodb.org/manual/reference/database-references/#dbrefs )

[solocommand]

@sokac the $db parameter is optional at the server level for the reference to be valid, however it's currently required for the odm to support references across databases.

While I can see the advantage of allowing yourself to bypass that behavior, my approach would be to update the existing data to include the $db parameter in all DBRefs.

It's forward compatible if you are going to be moving forward with mongodb-odm, and should? be backwards compatible with your existing system. Not arguing for/against this PR, just pointing out another option.

[sokac]

Makes sense completely. Maybe documentation could be update to reference this potential issue.

[jmikola]

Sorry, just noticed this after @malarzm linked me. Indeed, this is something we'd like to do, as $db is basically useless for single-database applications. We have a tracking ticket for this in #802.

I think we can re-open this for later review. Thanks for the initial PR!

[malarzm]

LGTM, just need to mention this in the docs. @jmikola @jwage @alcaeus thoughts?

[alcaeus]

Not always setting a $db key can cause errors in Expr::references() and Expr::includesReferenceTo() when using the methods with a reference that has the partial flag enabled and no targetDocument set. I'm not sure how much of a use case this is, but either the Expr class or the mapping drivers need to make sure to handle this case.

[malarzm]

definitely 👍 for catching this

[alcaeus]

I'm not quite sure about the partial parameter, especially since we already have simple. With this PR we'd have three different kinds of references:

• "simple" references that are simply just an identifier and contain no other mapping types (currently mapped using simple = true),
• "non-simple" but "partial" references that create a DBRef object but don't include a $db key (currently mapped using partial = true), and
• "full" references that create a complete DBRef object (currently mapped using simple = false and partial = false)

In my opinion it might be smarter to deprecate the simple parameter and introduce a new parameter that takes the values simple, partial and full (with full being the default value). Unfortunately, type is already taken, so we'd need to find another wording that clearly indicates what we're trying to do here.

[coudenysj]

I agree with the approach of @alcaeus.

type would be the obvious option, maybe we could introduce dbRefType?

[malarzm]

I'd throw storeAs to the proposition pool and ultimately ask @jmikola for a word proposition/judgment :)

[coudenysj]

Sounds good too! :)

[alcaeus]

👍 to using storeAs as it's more descriptive than type.

[malarzm]

Superseded by #1349 which is now available in 1.1.0 :)