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

MONGOID-5067: Fix Storable.add_field_expression() when adding a nested String operator equivalent to an existing nested Symbol operator #4966

Conversation

dalton-braze
Copy link
Contributor

Summary

Storable.add_field_expression() doesn't correctly handle adding a nested String operator that is equivalent to an existing nested Symbol operator. This PR includes both the fix and a test reproducing the bug in the old code.

This is the second piece of a Symbol-String bug manifesting in a chained Query.where().where(). The first piece is addressed in MONGOID-5066 and its accompanying PR.

Original Repro

query.where(:f => { :$in => [...] }).where(:f => { "$in" => [...] })

Test Output (No Changes)

% rake spec

  1) Mongoid::Criteria::Queryable::Storable#add_field_expression when a String equivalent of an existing Symbol operator is added connects the equivalent operators with $and
     Failure/Error:
       modified.selector.should == {
         '$and' => [
           {
             "foo"=> {
               "$in"=>[2, 3]
             }
           }
         ],
         "foo"=> {
           :$in=> [1, 2]
     
       expected: {"$and"=>[{"foo"=>{"$in"=>[2, 3]}}], "foo"=>{:$in=>[1, 2]}}
            got: {"foo"=>{:$in=>[1, 2], "$in"=>[2, 3]}} (using ==)
       Diff:
       @@ -1,3 +1,2 @@
       -"$and" => [{"foo"=>{"$in"=>[2, 3]}}],
       -"foo" => {:$in=>[1, 2]},
       +"foo" => {:$in=>[1, 2], "$in"=>[2, 3]},

XYZ examples, 1 failure

Test Output (Fix Implemented)

% rake spec

XYZ examples, 0 failures

@dalton-braze dalton-braze changed the title Fix Storable.add_field_expression() when adding a nested String operator equivalent to an existing nested Symbol operator MONGOID-5067: Fix Storable.add_field_expression() when adding a nested String operator equivalent to an existing nested Symbol operator Mar 22, 2021
@alcaeus alcaeus requested a review from p-mongo March 24, 2021 10:08
@p-mongo
Copy link
Contributor

p-mongo commented Apr 9, 2021

@p-mongo p-mongo closed this Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants