Fix updateAll and deleteAll implementations in all connectors

[bajtos]

• 1. Add a test to juggler's test suite run by all connectors to check that updateAll is implemented and return the number of updated records.
• 2. Fix Memory connector to provide the number of updated records
• 3. Run the new test in all DB connectors and fix any test failures
  • Verify and fix MongoDB connector changes
  • Verify and fix MySQL connector changes
  • Verify and fix PostgreSQL connector changes
  • Verify and fix MSSQL connector changes
  • Verify and fix Oracle connector changes

[chandadharap]

Prioritize these connectors:
Mongo, Mysql, Postgress, MSSQL, Oracle

[superkhau]

@bajtos Should I write unit tests for each connector individually? ie) Add two more unit tests directly toloopback-connector-mongodb? One that verifies updateAll is implemented` and another to verify a count is returned when updating records?

[bajtos]

For number 3, should I write unit tests for each connector individually?

No, add your new tests to test/manipulation.test.js. This file is run by all connectors as part of their test suite (or at least it should be) via test/common.batch.js.

[bajtos]

See also https://github.com/strongloop/loopback-datasource-juggler/blob/274a5c778ae0e76403fbb1480c7b144b898c3e18/test/memory.test.js#L404-L427, I am not sure if dao.js provides a fallback implementation for connectors that don't implement updateAll. If it does, then we should test it too.

[bajtos]

It's ok to add the new tests to test/basic-querying.test.js as you did in loopbackio/loopback-datasource-juggler#502, considering that other updateAll tests are already living there.

[superkhau]

@bajtos After speaking with @raymondfeng on hangouts, it was decided that we can just modify the second argument (get rid of data.rows) which allows me to use the second arg for all connectors. I am going to modify all the third arg updates to do that instead.

[bajtos]

After speaking with @raymondfeng on hangouts, it was decided that we can just modify the second argument (get rid of data.rows) which allows me to use the second arg for all connectors. I am going to modify all the third arg updates to do that instead.

I am not convinced this is not a BC, but I'll go with whatever decision you have made so that we can get this finished ASAP.

[superkhau]

@bajtos Had another meeting with @raymondfeng explaining the BC issue you just mentioned. Gonna update the PostgreSQL connector to return data.rows if selecting and {count: affectedRows} on destroyAll/updateAll. We can discuss any further changes directly there.

[bajtos]

@superkhau could you please describe the callback arguments in the documentation explaining how to build a connector? http://docs.strongloop.com/display/LB/Building+a+connector

I am not sure if the API documentation in juggler's lib/dao.js was updated to match the changes we made to update* and delete* callbacks, could you please check that too?

[bajtos]

@superkhau for example, updateAll still says that a scalar "count" value is returned: https://github.com/strongloop/loopback-datasource-juggler/blob/e13c0cdc12467aff5f347fa1112c03ba1dcdbb8b/lib/dao.js#L1634 It would be nice to fix the implementation too, it's using a property called count instead of info now.

What is the final loopback/juggler API that you have arrived at with @raymondfeng? Is the description below correct?

MyModel.updateAll -> cb(null, { count: X });
MyModel.deleteAll -> cb(null, { count: X });

[bajtos]

I am not sure if the API documentation in juggler's lib/dao.js was updated to match the changes we made to update* and delete* callbacks, could you please check that too?

I made the changes myself, see loopbackio/loopback-datasource-juggler#540 and #1252

[superkhau]

@superkhau could you please describe the callback arguments in the documentation explaining how to build a connector? http://docs.strongloop.com/display/LB/Building+a+connector

Done.