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

Fix the Collection::getIndexInfo method for missing "unique" option #117

Closed

Conversation

damienalexandre
Copy link
Contributor

I saw this issue when dealing with a bug in Doctrine ODM. They have a command handling indexes changes.

In that command, they check that the defined mapping is the same as the indexes returned by mongodb (code here for the curious). If not, unknown indexes are removed, and then "ensureIndexes" is called.

I got a very bad situation in production where some of my indices were removed, and re-created, which was taking a very long time and basically broke everything (and was doing so in total absence of error exit).

What is wrong

The isMongoIndexEquivalentToDocumentIndex from Doctrine was not returning true when comparing some of my indexes.

That's because the response from the old Mongo extension and what mongo-php-adapter returns is not always the same, especially when there is the unique option set on the index.

The fix

Adding back the unique key in the array was enough to fix the bug in the schema update command, but I took a look inside the old Mongo extension and server code and it looks like that the option is only returned if it's true (code here), so I added a small if to mimic this behavior too.

Thanks 🍻

$this->assertSame(
$expected,
$collection->getIndexInfo()
$this->assertTrue(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use assertEquals instead of assertSame which does the same thing here but is more explanatory.

@damienalexandre
Copy link
Contributor Author

Took care of your comment and squashed my commits 👍

@alcaeus alcaeus closed this in e93279c Jun 23, 2016
@alcaeus
Copy link
Owner

alcaeus commented Jun 23, 2016

Thanks! 🍻

Merged into 1.0.x branch and scheduled for inclusion in 1.0.5.

@alcaeus alcaeus added the bug label Jun 23, 2016
@alcaeus alcaeus added this to the 1.0.5 milestone Jun 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants