mc_worker_logic:make_request was being called with the wrong DB

[stephb9959]

Although the code was properly figuring out the right DB, it would always pass the Connection DB to the make_request.

[comtihon]

Hi,
Can you please fix the tests?

[stephb9959]

Hi Val! So I tried to add a dumb test: ok = mongo_api:ensure_index(Pid, Collection, #{<<"key">> => {<<"cid">>, 1, <<"ts">>, 1}} , <<"test2">>), ok = mongo_api:ensure_index(Pid, Collection, {<<"key">>, {<<"z_first">>, 1, <<"a_last">>, 1}}), That makes the tests fail. I think the problem is in the protocol. Here is what I am finding, and please correct me if I am wrong. - If you connect to a specific DB, you cannot switch DBs afterward on the same connection. - If you do not connect with a DB, you may specify a DB on any subsequent call, and you may change DBs. So in order to make this test, I would need to add a second connection and write a test using that connection, and not setting the database in that connection. I came up with this fix after trying to create indexes for about 6 hours and no being able to. After looking at more traces than I care to, and massive printf() debugging, I followed the code into mc_worker.erl. Where ar line 76 it is not using the database name that was obtained earlier. So in the case where you do not connect with a DB, that line would always try to use "undefined". The fix was to use the calculated value of DB, which turns out to be right. Let me know if you still want me to create the test. It would be a large undertaking. Q: The operation to create an index does not seem to return an error, ever. I followed the code and it never looks for a return value from Mongo. Am I mistaken? Cheers!

…

On Mon, Sep 2, 2019 at 11:40 AM Val ***@***.***> wrote: Hi, Can you please fix the tests? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#217?email_source=notifications&email_token=AAOF7RCPZFSV2OS3VXI4H7TQHVM25A5CNFSM4ISZ3IIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5WMGSI#issuecomment-527221577>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOF7RANI3MWDQ3CAOVLWGLQHVM25ANCNFSM4ISZ3IIA> .

-- Stéphane Bourque

[comtihon]

Hi,
mc_worker checks only ok when sending ensure index request.

{ok, _, _} =
    mc_worker_logic:make_request(
      Socket,
      NetModule,
      ConnState#conn_state.database,
      #insert{collection = mc_worker_logic:update_dbcoll(Coll, <<"system.indexes">>), documents = [Index]}),

mongo_api:ensure_index doesn't send anything to the database. It just prepares the request. You can rename it in this pr, if you find it confusing.

As far as I remember you always pass authentication against the specific database, so you can't just switch database in the same connection.
I am not talking about creating a new test, but all the old tests are failing for some reason. Although I am not sure it is because of your changes, as the latest master is also red.

[stephb9959]

I think my application for Mongo is a little different. The DB is in a private cloud behind firewalls and applications marshaling all access. So I have no security on Mongo at all. That might be why this is working for me.

…

On Tue, Sep 3, 2019 at 12:28 AM Val ***@***.***> wrote: Hi, mc_worker checks only ok when sending ensure index request. {ok, _, _} = mc_worker_logic:make_request( Socket, NetModule, ConnState#conn_state.database, #insert{collection = mc_worker_logic:update_dbcoll(Coll, <<"system.indexes">>), documents = [Index]}), mongo_api:ensure_index doesn't send anything to the database. It just prepares the request. You can rename it in this pr, if you find it confusing. As far as I remember you always pass authentication against the specific database, so you can't just switch database in the same connection. I am not talking about creating a new test, but all the old tests are failing for some reason. Although I am not sure it is because of your changes, as the latest master is also red. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#217?email_source=notifications&email_token=AAOF7RGUQCACL6P25HQCH5LQHYGZXA5CNFSM4ISZ3IIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5XJBLY#issuecomment-527339695>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOF7RGGZ6F3HPW2RBGKTELQHYGZXANCNFSM4ISZ3IIA> .

-- Stéphane Bourque