Skip to content
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

Custom Identifier name isn't allowed to be custom. #2096

Closed
shehi opened this issue Nov 6, 2019 · 8 comments
Closed

Custom Identifier name isn't allowed to be custom. #2096

shehi opened this issue Nov 6, 2019 · 8 comments
Assignees
Labels
Milestone

Comments

@shehi
Copy link

shehi commented Nov 6, 2019

Bug Report

Q A
BC Break no idea
Version 2.0.1

Summary

In XML configuration, attempting to give custom field_name to IDs doesn't work. ClassMetadata fails.
<id field-name="uuid" strategy="UUID" type="bin_uuid" />

Current behavior

<id field-name="uuid" strategy="UUID" type="bin_uuid" />

Field-names both in document class, and in Db are is "uuid". Yet, code here:
https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php#L1953
resolves $mapping['fieldName'] value to id instead of intended uuid.

How to reproduce

Use this as ID field configuration in your XML document odm file:
<id field-name="uuid" strategy="UUID" type="bin_uuid" />

Expected behavior

For ClassMetadata to have $mapping['fieldName'] to actually be equal to $mapping['field_name'] (the latter having the correct value), being equal to uuid not id, as this example expects.

UPDATE: We can't have custom identifier name in Db, it is always _id. This was realized to be the case and the fact established in our chat below.

@shehi
Copy link
Author

shehi commented Nov 6, 2019

At https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php#L1953 the dumped value of $mapping variable is as below:

array(15) {
  'id' =>
  bool(true)
  'fieldName' =>
  string(2) "id"
  'field-name' =>
  string(4) "uuid"
  'strategy' =>
  string(4) "UUID"
  'type' =>
  string(8) "bin_uuid"
  'options' =>
  array(0) {
  }
  'name' =>
  string(3) "_id"
  'isCascadeRemove' =>
  bool(false)
  'isCascadePersist' =>
  bool(false)
  'isCascadeRefresh' =>
  bool(false)
  'isCascadeMerge' =>
  bool(false)
  'isCascadeDetach' =>
  bool(false)
  'nullable' =>
  bool(false)
  'isOwningSide' =>
  bool(true)
  'isInverseSide' =>
  bool(false)
}

@malarzm
Copy link
Member

malarzm commented Nov 6, 2019

Field-names both in document class, and in Db are "uuid".

IIRC field named uuid cannot be an identifier in the document as MongoDB expects document's identifier to be named _id and that's pretty much hardcoded in the db itself. I think we should rather block custom name for identifier fields.

@shehi
Copy link
Author

shehi commented Nov 6, 2019

After some research, I tend to agree with you @malarzm . I think this issue should be renamed appropriately into a task to disable custom fieldnames for IDs.

@alcaeus
Copy link
Member

alcaeus commented Nov 6, 2019

Identifier names in the database are fixed to the _id field - this is the identifier of a MongoDB document. There is a bug with handling the field-name attribute in XML mappings, which allows you to change the name of the class property the database field is mapped to. This bug is being fixed in #2097.

In your case, you can have a uuid field in the class and map it to the _id field by using the <id field-name="uuid" type="uuid" /> mapping. If you want to store this in a uuid field, then it is no longer an identifier.

@shehi
Copy link
Author

shehi commented Nov 6, 2019

@alcaeus , that IS the problem: I can't have field named $uuid in document class. It is being converted to $id. That might need fixing.

@shehi
Copy link
Author

shehi commented Nov 6, 2019

@alcaeus , I just checked your PR, and I believe change in XMLDriver is fixing the problem I described above.

@alcaeus
Copy link
Member

alcaeus commented Nov 6, 2019

I just wanted to make sure you don't have wrong expectations because of this:

Field-names both in document class, and in Db are "uuid".

With the id mapping you described, the database field will always be _id - the field-name attribute only affects the class property. If that's what you intended to do, I apologise for the confusion I caused.

@shehi
Copy link
Author

shehi commented Nov 6, 2019

Yea, updated the original post above, striking through some wordings. So yea, current expectation is the fix in document class.

@alcaeus alcaeus added this to the 1.3.3 milestone Nov 6, 2019
@alcaeus alcaeus self-assigned this Nov 6, 2019
@alcaeus alcaeus added the Bug label Nov 6, 2019
alcaeus added a commit that referenced this issue Nov 6, 2019
Fix wrong handling of custom identifier names
@alcaeus alcaeus closed this as completed Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants