Doctrine chokes on binary IDs #425

Closed
Bilge opened this Issue Oct 9, 2012 · 15 comments

Comments

Projects
None yet
4 participants
Contributor

Bilge commented Oct 9, 2012

After implementing a custom ID generator for BinUUID generation I get lots of warnings similar to the following.

Warning: Illegal offset type in isset or empty in mongodb-odm/lib/Doctrine/ODM/MongoDB/UnitOfWork.php on line 1525

Warning: Illegal offset type in mongodb-odm/lib/Doctrine/ODM/MongoDB/UnitOfWork.php on line 1528

Warning: Illegal offset type in isset or empty in mongodb-odm/lib/Doctrine/ODM/MongoDB/UnitOfWork.php on line 1655

Warning: Illegal offset type in mongodb-odm/lib/Doctrine/ODM/MongoDB/UnitOfWork.php on line 2512

Please add support for binary IDs. This is a big blocker for my project.

ianunruh commented Dec 8, 2012

+1.. I have the same problem when I try to use an object as an identifier

Owner

jmikola commented Jan 16, 2013

Please see if doctrine/mongodb-odm#444 addresses this issue. That PR changed some internal behavior where document IDs were being used as array indexes -- they're now serialized.

Contributor

Bilge commented Jan 27, 2013

@jmikola Sorry for taking so long to test this. Reading and writing the binary values appears to work fine, however, I cannot query on the ID.

$documentManager->find('User', $id);

If I try to query using a binary string I may receive this error:

PHP Fatal error: Uncaught exception 'MongoException' with message 'non-utf8 string' in mongodb/lib/Doctrine/MongoDB/Cursor.php:229

I therefore assume I must query using the MongoBinData type.

$documentManager->find('User', new \MongoBinData($id, \MongoBinData::UUID));

However, this causes its own error.

Warning: array_merge(): Argument #1 is not an array in mongodb-odm/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php on line 805

Contributor

Bilge commented Feb 12, 2013

@jmikola Can you confirm that reading and writing binary IDs works but querying them does not? I want to be sure this is still a bug and not just me doing something wrong!

Contributor

Bilge commented Apr 5, 2013

@jmikola ping

Owner

jmikola commented Apr 5, 2013

I'm going to need a commit hash of Doctrine/MongoDB/Cursor.php to look at for the non-utf8 string error on line 229. If I had to guess, that's an issue with the _id being cast to a string and used as an array index for iteration, but I can't be sure.

With regard to the second error, I'd expect that based on the code inside DocumentPersister::prepareQuery(). The $id argument is only wrapped in an array (i.e. array('_id' => <value>)) if it's a scalar or MongoId instance. For any other type, you'll need to manually form the query criteria yourself.

As for #444, there are still some outstanding todo items there that I haven't had a chance to get back to. But if the code above fixes your situation, you may want to just utilize that PR branch in the meantime (I imagine that's what you're already doing).

Contributor

Bilge commented Apr 10, 2013

@jmikola The non-utf8 string error is a result of trying to query a binary field using a string. I believe doing this is incorrect so we shouldn't need to worry about it, although it would be useful if doctrine would automatically wrap strings in \MongoBinData when querying.

When querying using find() and \MongoBinData I still see.

Warning: array_merge(): Argument #1 is not an array in mongodb-odm/lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php on line 805

Although the following does not work,

$dm->find('User', new \MongoBinData($id, \MongoBinData::UUID));

the following workaround does work.

$dm->getRepository('User')->findOneBy(['_id' => new \MongoBinData($id, \MongoBinData::UUID)]);

The commit hash is 4d0ae88 (preserve-id-types).

Contributor

Bilge commented Apr 27, 2013

@jmikola When can you continue working on preserve-id-types? I am stuck using this branch in production which, as you can imagine, is a big problem since I can't receive any updates from master.

Owner

jmikola commented May 1, 2013

I don't have a definite timeline for this or the collection persister issue. Currently focusing on the 1.4.0 release of the PHP driver.

Contributor

Bilge commented Aug 29, 2013

@jmikola As it's been four months, and nearly a year since the issue was first openend, I hope it's not too soon to be asking for an update? :)

Owner

jwage commented Sep 2, 2013

As far as I know nobody has had time to work on this issue yet. It hasn't been a problem for many people so unfortunately other issues that impact more people have been worked on instead.

Contributor

Bilge commented Sep 2, 2013

Hey Jwage!
Thanks for the update. I may not be many people but it's still a very big issue for me. I have been relegated to basing my live project off a buggy and ever ageing fork, currently 8 months old, because I designed parts of my application around binary IDs.

Would love for preserve-id-types to be merged into master so I can start receiving updates again.

Owner

jwage commented Sep 2, 2013

I agree, it would be awesome to have this fixed. However, people usually contribute things that are relative to them. I am a heavy user of the ODM but I don't need that feature so I only get a chance to work on things that are related to me. I think Jeremy will have time to finish the PR at some point, hopefully sooner than later.

Owner

jmikola commented Jan 16, 2014

@Bilge: #444 has finally been merged to master, and we have tests included for using BinData types for IDs. There are some related edge case issues that are pending, but @jwage and I didn't want to hold it up further. I'm going to close this out, but feel free to re-open if your problem persists.

jmikola closed this Jan 16, 2014

Contributor

Bilge commented Jan 17, 2014

@jmikola Thanks. I can finally stop using my ancient fork of this project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment