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

Doctrine\MongoDB\Query\Expr::popFirst is incorrect #1830

Closed
juliusxyg opened this issue Jul 19, 2018 · 2 comments · Fixed by #1831
Closed

Doctrine\MongoDB\Query\Expr::popFirst is incorrect #1830

juliusxyg opened this issue Jul 19, 2018 · 2 comments · Fixed by #1831
Assignees
Labels
Milestone

Comments

@juliusxyg
Copy link

juliusxyg commented Jul 19, 2018

Bug Report

Q A
BC Break no
Version 1.6.2

Summary

Two methods
Doctrine\MongoDB\Query\Expr::popFirst()
Doctrine\MongoDB\Query\Expr::popLast()

use wrong $pop value.

Current behavior

according to mongodb manual

The $pop operator removes the first or last element of an array. Pass $pop a value of -1 to remove the first element of an array and 1 to remove the last element in an array.

pop first is -1, pop last is 1,

in the code is just reversed.

@malarzm malarzm added this to the 1.2.5 milestone Jul 19, 2018
@malarzm malarzm added the Bug label Jul 19, 2018
@alcaeus alcaeus modified the milestones: 1.2.5, 2.0.0 Jul 20, 2018
@alcaeus
Copy link
Member

alcaeus commented Jul 20, 2018

Moved to the 2.0 line since that's the only bug fix to do in ODM. For 1.x, the error is in doctrine/mongodb and will be fixed in a separate release today. Thanks for the report!

@alcaeus
Copy link
Member

alcaeus commented Jul 20, 2018

doctrine/mongodb 1.6.3 was just released containing the fix for this issue. For 2.0 (the current master branch, #1831 will port the fix to ODM.

Since I was bored this morning, I decided to dig into the history of this bug:

It was introduced 8 years ago (May 2010) when a young developer added update methods to the ODM query object and made a tiny mistake in the documentation (First is 1 and last is -1): 795dc99#diff-d9cea82dc18423811aa7d0cc84fdbe25R673
A few hours later, the same developer would then refactor the pop method into popFirst and popLast, incorporating their own documentation mistake and creating the wrong behavior you found: 0632861#diff-d9cea82dc18423811aa7d0cc84fdbe25R714.
This was released as ODM 1.0.0-BETA4 in August 2010, which would be the first version to contain the bug.
From then, the issue made its way to the new doctrine/mongodb library in December of 2010 later to be released as 1.0.0-BETA1: doctrine/mongodb@98e70d3#diff-0a4ad011a43c676e967fac2188a7ee0fR291
The bug had its big homecoming 7 years later in December 2017 when we dropped doctrine/mongodb in favour of the MongoDB PHP library: #1553 (I would link the exact line, but GitHub really doesn't like large commits and pull requests).

So, again, thank you for finding and reporting an 8 year old bug that caused wrong data to be removed - you were either the first person to hit it or the first to care enough about their data to report it as a bug. I hope you enjoyed that little trip down the long and colourful history of this bug.

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 a pull request may close this issue.

3 participants