loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.97k stars 1.07k forks source link

Better docs for Repository.execute #5264

Open bajtos opened 4 years ago

bajtos commented 4 years ago

I noticed that we don't have a good documentation on how to use Repository.execute method. The API docs is very vague:

  /**
   * Execute a query with the given parameter object or an array of parameters
   * @param command - The query string or command object
   * @param parameters - The object with name/value pairs or an array of parameter
   * values
   * @param options - Options
   */

Let's improve the docs to show examples how to use execute for SQL databases using parameterized queries (to avoid SQL injection attacks).

If https://github.com/strongloop/loopback-next/issues/3342 is not implemented yet, then add a mention about NoSQL flavors not being supported yet, link to that GitHub issue and update it's acceptance criteria to request a MongoDB example to be added to the API docs. If the issue is already implemented, then

Add a MongoDB example as part of this story.

Prior work:

Acceptance criteria

hazimdikenli commented 4 years ago

I could not find a case where command is an object, is it connector specific, if this is the case, which connector? I checked out mysql, postgres and mongodb connectors, have I missed something? And again I think parameters is mostly an array, I found out that mysql connector lets you use objects, postgres will let you use numbered parameters like $1, $2, but mysql does not support that, you can just use ?, and there is no way to reuse parameters like postgres connector. mysql also supports ?? to specify like which fields to select or what table to query, again I dont think pg supports this. Regarding options,I found out that postgres and mysql connectors let you pass in a transaction object in there.

Can you guys provide any tips regarding this issue, in the end this function's documentation is very much based on the underlying connector, documentation may be very dry.

bajtos commented 4 years ago

Thank you @hazimdikenli for investigating the implementation of execute in different connectors.

Yes, the functionality of Repository.execute will be always connector specific, our documentation should make this clear.

I could not find a case where command is an object, is it connector specific, if this is the case, which connector? I checked out mysql, postgres and mongodb connectors, have I missed something?

Cross-posting a comment from the PR that introduced dataSource.execute API (https://github.com/strongloop/loopback-datasource-juggler/pull/1671#discussion_r239743392):

Any particular reason to allow an object?

Some community connectors are using object value for the comman and I want to preserve support for those connectors.

See e.g. https://www.npmjs.com/package/loopback-connector-neo4j-graph

var cypher = {
    query: "MATCH (u:User {email: {email}}) RETURN u",
    params: {
        email: 'alice@example.com',
    }
};
// ...
ModelClass.dataSource.connector.execute(
    cypher,
    [],
    options,
    callback
);

I think the connector authors misunderstood how command & args are meant to be used together. I think the following would be a better usage:

ModelClass.dataSource.connector.execute(
  `MATCH (u:User {email: {email}}) RETURN u`,
   {email: 'alice@example.com'},
   options,
   callback);

Based on this analysis, I am going to change the type of args from array to object. (Note that arrays are objects too.)


And again I think parameters is mostly an array, I found out that mysql connector lets you use objects, postgres will let you use numbered parameters like $1, $2, but mysql does not support that, you can just use ?, and there is no way to reuse parameters like postgres connector. mysql also supports ?? to specify like which fields to select or what table to query, again I dont think pg supports this.

The trick is to keep the type descriptions generic enough to support different flavors implemented by connectors, but specific enough to provide at least basic type checks.

One option is to create multiple overloads for different connectors. For example:

// PostgreSQL
execute(command: string, params: any[], options: Options & {transaction: object});

// neo4j-graph
execute(query: object, unused: any[], options: Options);

// generic variant
execute(command: string|object, params: any[], options: Options);

Regarding options,I found out that postgres and mysql connectors let you pass in a transaction object in there.

That looks correct to me :+1:

bajtos commented 4 years ago

Please note that most of this work has been already done as part of #3342, see #6030 and #6047 in particular.