mongodb-erlang icon indicating copy to clipboard operation
mongodb-erlang copied to clipboard

mc_worker_api:update() broken for non-commands

Open zzantozz opened this issue 6 years ago • 5 comments

In mc_worker_api, an update goes like

update(Connection, Coll, Selector, Doc, Upsert, MultiUpdate) ->
  Converted = prepare(Doc, fun(D) -> D end),
  command(Connection, {<<"update">>, Coll, <<"updates">>,
    [#{<<"q">> => Selector, <<"u">> => Converted, <<"upsert">> => Upsert, <<"multi">> => MultiUpdate}]}).

All of the update tests in mc_worker_api_SUITE test this function with a {$set, _} document, which is treated specially by prepare/2. In that case, Converted is a single document. A non-command document, however, turns into a list, which turns into an invalid command. MongoDB responds with

#{
  <<"code">> => 9,
  <<"errmsg">> => <<"wrong type for 'u' field, expected object, found u: [ { _id: .....} ]
}

zzantozz avatar Mar 22 '18 22:03 zzantozz

Thanks, I'll check it out.

comtihon avatar Mar 23 '18 07:03 comtihon

I think changing the prepare() call to this takes care of it in this case, but I'm not sure yet if it breaks something else:

Converted = case prepare(Doc, fun(D) -> D end) of
    [D] -> D;
    D -> D
  end,

zzantozz avatar Mar 23 '18 15:03 zzantozz

Just rechecked with test. You can either update document with $set like in update test:

Command = #{
    <<"quantity">> => 500,
    <<"details">> => #{<<"model">> => "14Q3"},  %with flatten_map there is no need to specify non-changeble data
    <<"tags">> => ["coats", "outerwear", "clothing"]
  },
  mc_worker_api:update(Connection, Collection, #{<<"_id">> => 100}, #{<<"$set">> => bson:flatten_map(Command)}).

Or replace document without any command (added test update_non_command):

Update = #{
    <<"quantity">> => 500,
    <<"details">> => #{<<"model">> => "14Q3"},
    <<"tags">> => ["coats", "outerwear", "clothing"]
  },
  {true, #{<<"n">> := 1, <<"nModified">> := 1}} =
    mc_worker_api:update(Connection, Collection, #{<<"_id">> => 100}, Update)

Can you specify the request you have problems with?

comtihon avatar Mar 24 '18 10:03 comtihon

Sorry it took me a while to get back to this. I'm working on upgrading from a very old version of this library to the newest, and it's a looooong step in between. I should've mentioned before that I'm working from tag v3.1.9, and I see changes have been made since then. Here's a simple reproduction of the problem I see:

1> {ok, C} = mc_worker_api:connect([]).
{ok,<0.547.0>}

2> mc_worker_api:insert(C, <<"foo">>, {name, bob}).
{{true,#{<<"n">> => 1}},
 #{name => bob,
   <<"_id">> => {<<90,189,31,64,126,187,253,53,195,0,0,2>>}}}

3> mc_worker_api:update(C, <<"foo">>, {name, bob}, {name, joe}).
{false,#{<<"code">> => 9,
         <<"errmsg">> => <<"wrong type for 'u' field, expected object, found u: [ { name: \"joe\" } ]">>}}

zzantozz avatar Mar 29 '18 17:03 zzantozz

It works if the update doc is a map. It's broken for bson:

4> mc_worker_api:update(C, <<"foo">>, {name, bob}, #{name => joe}).
{true,#{<<"n">> => 1,<<"nModified">> => 1}}

5> mc_worker_api:update(C, <<"foo">>, {name, bob}, {name, joe}).   
{false,#{<<"code">> => 9,
         <<"errmsg">> => <<"wrong type for 'u' field, expected object, found u: [ { name: \"joe\" } ]">>}}

zzantozz avatar Mar 29 '18 17:03 zzantozz