Tests for #250 and #283 #284

merged 10 commits into from May 17, 2012


None yet

3 participants


Please check this out. #250 is resolved by you and now works (thousand thanks!). But second test (testAdvancedQueriesOnReferenceWithDiscriminatorMap) doesn't work (I think because of #283... or my mistake in test). Could you help me?

Adam Kusmierz added some commits Mar 23, 2012
Adam Kusmierz #250 8ca5a5a
Adam Kusmierz #250 and #283 tests 40779ef
@jmikola jmikola and 1 other commented on an outdated diff Mar 23, 2012
@@ -11,3 +11,6 @@ phpunit.xml
jmikola Mar 23, 2012 Doctrine member

This probably belongs in your local user's gitignore, not the project's file.

jwage commented Mar 23, 2012

It is hard to understand what you are doing in the tests. It looks odd. Can you write a unit test so we can see the query output? Are the wrong queries being set or what part exactly is failing?


First test I hope is obvious. I'm inserting 3 entries, 2 for SameCollection2 and one for SameCollection1. Physically all 3 are in the same collection (named "same_collection"), but they are separated by DiscriminatorMap in two Documents mentioned above. Next I'm searching for them by ID in SameCollection2 using $in closure. I put 3 ids in query, but only 2 should be found. Next I'm testing I received for sure my objects. It's in fact testing #250. It's passing fine (after your last commit).

Second test inserts entry with referenced entry (Comment with corresponding article). Here I'm testing again $in closure, but on reference field ("article"), so literally I'm testing #283. I don't know why, but it returns "0" as result. If I removed 226 and 227 lines it would work fine.


I've added second failing test because of #250 issue.

kusmierz commented Apr 3, 2012


kusmierz commented Apr 9, 2012

Attached another failing test related to nullable=true, reference(One|Many) and inversedBy/mappedBy functionality. I can't set it to null. I think it's because of https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php#L315-L316 - $new is null and nullable = true, so it should set it to null but instead it's trying to convert null value to database value.

Could you help me with it too?

kusmierz commented Apr 9, 2012

To fix last test you should change:

in https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php#L195

$updateData[$this->cmd . 'set'][$mapping['name']] = Type::getType($mapping['type'])->convertToDatabaseValue($new);
// to
$updateData[$this->cmd . 'set'][$mapping['name']] = (is_null($new) ? null : Type::getType($mapping['type'])->convertToDatabaseValue($new));

in https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php#L236

$updateData[$this->cmd . 'set'][$mapping['name']] = $this->prepareReferencedDocumentValue($mapping, $new);
// to
 $updateData[$this->cmd . 'set'][$mapping['name']] = (is_null($new) ? null :  $this->prepareReferencedDocumentValue($mapping, $new));

in https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php#L283

$updateData[$this->cmd . 'set'][$mapping['name']] = Type::getType($mapping['type'])->convertToDatabaseValue($new);
// to
$updateData[$this->cmd . 'set'][$mapping['name']] = (is_null($new) ? null : Type::getType($mapping['type'])->convertToDatabaseValue($new)); // copy'n'paste method? ;)

in https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php#L321

$updateData[$this->cmd . 'set'][$mapping['name']] = $this->prepareReferencedDocumentValue($mapping, $new);
// to
$updateData[$this->cmd . 'set'][$mapping['name']] = (is_null($new) ? null :$this->prepareReferencedDocumentValue($mapping, $new));
Adam Kusmierz added some commits Apr 9, 2012
kusmierz commented Apr 9, 2012

Ok, one of the tests had little mistake and now is passing.

kusmierz commented Apr 9, 2012

In second failing test there is something wrong with changing string id to correct-object id, so it can't find correct entry. Could you check it and fix it? Thanks in advance @jwage!

@jwage jwage merged commit 5f9f262 into doctrine:master May 17, 2012
@jmikola jmikola added a commit that referenced this pull request May 17, 2012
@rtens rtens Query preparation tests for simple references
This was originally included in PR #261. It tests the fixes for #250, #283 and #284.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment