Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Support simple reference with discriminator maps #432

Open
Bilge opened this Issue · 17 comments

4 participants

Bilge Jeremy Mikola Dayson Pais Jonathan H. Wage
Bilge

Although not documented, currently the following attributes are mutually exclusive.

  • discriminatorMap
  • simple=true

Trying to use them together causes errors.

A typical discriminatorMap field must therefore be similar to the following.

"foo" : {
    "$ref" : "Bar",
    "$id" : ObjectId("123"),
    "$db" : "MyDb",
    "type" : 'bar'
}

However, Doctrine only needs to know the class and ID. Everything else it can infer from the class's metadata. Therefore there is no reason not to support simple references on discriminator fields.

"foo" : {
    "$id" : ObjectId("123"),
    "type" : 'bar'
}
Jeremy Mikola
Owner

The object with an $id and type field is an incomplete DBRef. I wouldn't consider that a simple reference, which is a single, mapped field containing an ObjectId.

If discrimination is to be supported with simple references, I think it'd be preferable to store the reference type in another field (at the same level of the simple ref's ObjectId). Using something that looks like a DBRef wouldn't follow an existing convention (to by knowledge) and might lead to confusion with users and other libraries.

Bilge

I'm not suggesting you should use DBRef at all. On the contrary, I'm suggesting you don't, when simple is set to true.

Jeremy Mikola
Owner

What did you mean with the following code snippet?

"foo" : {
    "$id" : ObjectId("123"),
    "type" : 'bar'
}
Bilge

Just that that should be the only data you'd need to store for Doctrine to work. Exactly how you choose to store it is not really any of my concern.

Dayson Pais

Has there been any progress on this? I am confused as to what is the recommended approach if I have all my references simple, but STILL need to resolve to the right type (as defined in the referenced collection's discriminator map annotations).

Jonathan H. Wage
Owner

If you want a discriminator stored don't use simple references.

Bilge

Why, jwage? Some said it wasn't possible, but for all my useless theorising, someone has even gone and made the #645 pull request that implements it, so what reason do you give for not permitting it now?

Jonathan H. Wage
Owner

Because it is not merged and I haven't had time to review it yet, so until then if you want to use discriminitor maps you can't use simple references.

Bilge

I'm well aware that it isn't merged. I thought you were still condemning the concept. If it remains feasible then this issue remains open.

Jonathan H. Wage
Owner

Yes, I was simply answering @epicwhale what he should do now given the current behavior.

Dayson Pais

@jwage ok.. hope to see support for this feature soon! And also a way to do it via annotations (for backwards compatibility with documents which don't have the additional field)

For now, I am just going to use the 'id' value from the simple reference and query the referenced collection's repository separately so I get an instance of the Document type I desire.. This strategy should be safe for now I hope?

Jonathan H. Wage
Owner

@epicwhale If it is possible for you to change, I would recommend not using simple references and store a full reference so that your discriminator value can be stored at the end of the DBRef.

Dayson Pais

@jwage I was always under the impression that Simple references is the best approach unless I have a very compelling reason to use DBRef :-S

Does DBRef with Doctrine store the database name? (I don't want it to)

Would be difficult for me to change to DBRef now, but I guess for all future Collections, I will use DBRef instead of Simple References..

Jonathan H. Wage
Owner

@epicwhale Simple references are fine but it's just an unfortunate limitation in the ODM that hasn't been fixed yet :( It does include the $db field right now.

Dayson Pais

@jwage I'll just live by writing extra code for now to query what I need separately.. Hopefully, you will find the time soon for this! Keep up the great work and I hope to see the ODM get out of beta someday (with this feature!). Thanks.

Jeremy Mikola
Owner

@Bilge: #645 never had a working implementation. That PR is just a failing test case for what the user would like to have supported. Furthermore, the annotation mappings in the test case appear to be incorrect (on the Stream.$action property).

@jwage: It may be relatively straightforward to support this for to-one relationships, since we could utilize an unmapped field in the document owning the relationship; however, it gets complicated with to-many relationships. Currently, a simple to-many relationship would be an array of identifiers. That makes it easy to write queries using $in to match IDs, and the link. If we instead store custom objects with a discriminator type and the referenced _id, we're essentially creating some hacky DBRef object that omits $ref and $db. Users would need to use $elemMatch to match to-many simple references, as they currently need to do for complex references.

As a compromise, what about having an option to stop storing $db in DBRef objects? It has always been optional, and most drivers/libraries don't use it.

Bilge

As a compromise, what about having an option to stop storing $db in DBRef objects? It has always been optional, and most drivers/libraries don't use it.

That would cure a large part of my OCD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.