Yeah I didn't look into it.. But reading further, it still would've prevented this bug. If you expect 1 or 0 results, just make sure it does.
The query here returned where id = xxx or renew_locked_at >= xxx or renew_locked_at is null. That will return more than 1 result. If find_one actually did what the name says it does, find 1. Not 0, not 2, not 475. This issue would've never existed.
Both mongoid and activerecord (don't remember if this was the case with hibernate / jpa) will not throw an exception if there are more than one records. They will check for zero results, although it would just result in an NPE if they didn't..
Besides this, I'd have made it more explicit by doing something like: where(xxx, or(yyy, yyy)). This is still an implicit 'and', but chaining always seems confusing to me, so I never use it like that.
Mongoid shouldn't have done such a change, even with a major upgrade. At least not in this way.
IMO, both the author and mongoid set themselves up for issues.
I like to throw around exceptions and asserts. I like to fail fast. Strictness is easy to handle for a programmer. Only external human input should/could be handled less strict. Unfortunately, many disagree..
It's different with other types of software (client side apps), but for applications that have no state, or a state that can easily be recovered (webapps), it seems plain stupid not to fail fast whenever something's wrong
The query here returned where id = xxx or renew_locked_at >= xxx or renew_locked_at is null. That will return more than 1 result. If find_one actually did what the name says it does, find 1. Not 0, not 2, not 475. This issue would've never existed.
Both mongoid and activerecord (don't remember if this was the case with hibernate / jpa) will not throw an exception if there are more than one records. They will check for zero results, although it would just result in an NPE if they didn't..
Besides this, I'd have made it more explicit by doing something like: where(xxx, or(yyy, yyy)). This is still an implicit 'and', but chaining always seems confusing to me, so I never use it like that.
Mongoid shouldn't have done such a change, even with a major upgrade. At least not in this way.
IMO, both the author and mongoid set themselves up for issues.
I like to throw around exceptions and asserts. I like to fail fast. Strictness is easy to handle for a programmer. Only external human input should/could be handled less strict. Unfortunately, many disagree..
It's different with other types of software (client side apps), but for applications that have no state, or a state that can easily be recovered (webapps), it seems plain stupid not to fail fast whenever something's wrong