cloudant / nodejs-cloudant

Cloudant Node.js client library
Apache License 2.0
255 stars 90 forks source link

Bad types for MangoSelector interface #428

Closed Heydan83 closed 4 years ago

Heydan83 commented 4 years ago

Please read these guidelines before opening an issue.

Bug Description

Bad types for MangoSelector interface. Unable to write proper interface which is allowed by CouchDB and MongoDb references. http://docs.couchdb.org/en/latest/api/database/find.html#selector-syntax https://docs.mongodb.com/manual/reference/operator/query/

1. Steps to reproduce and the simplest code sample possible to demonstrate the issue

Once you have your instance for cloudant and then point to the database, in this example we are using a database called 'reservations', when we use the method 'find' it receives and object and one of those properties is selector, which receives the mangoquery to execute, there is where we cannot use reserved words from mango like $and, $eq, $lte, it throws an error when specifying them: image

2. What you expected to happen

User should be able to use those operators without receiving an error message from them. I found that this is issue was reported and corrected directly on the Nano library, this is the reported issue: https://github.com/apache/couchdb-nano/issues/202 So if Im not wrong it would be just a matter of updating the Nano dependency on the Cloudant project in order to resolved it, right now Im applying the changes described on that issue manually in order to let me run the queries and it works. On the nano.d.ts file we need to change the declaration of the MangoSelector interface for this:

Old code: interface MangoSelector { [key: string]: MangoSelector | MangoValue | MangoValue[]; }

New code: type MangoOperator = '$regex' | '$eq' | '$gt' | '$gte' | '$lt' | '$lte' | '$ne' | '$and' | '$or' | '$not' | '$nor' | '$all' | '$elemMatch' | '$allMatch' | '$in'

interface MangoSelector { [K in MangoOperator | string]: MangoSelector | MangoValue | MangoValue[]; }

3. What actually happened

User receives the next error message when using the operators: image

Environment details

Heydan83 commented 4 years ago

Im not sure if I added all the mango operators, please make sure of that.

emlaver commented 4 years ago

@Heydan83 I believe your cloudantDB variable needs to be written in this order: const cloudantDB = cloudant.db.use(dbname); You'll also need to update your selector and fields to string types:

  const response = db.find({
    'selector': {
      '$and': [
        {
          'name': { '$eq': 'test'}
        }
      ]
    }
  });

There's another example in the couchdb-nano docs: https://github.com/apache/couchdb-nano#dbfindselector-callback

Heydan83 commented 4 years ago

Hi @emlaver thanks for the quick response, I try your suggestions and here are my results:

For this example I decided to create a new db connection to cloudant, because I was using a custom class in order to make the connection, this way it can be more clear.

By changing selector and fields to strings it still sending me the same error message described above: image

I though maybe it as to do with using the "and" operator which is and array, with just 1 value, so I did this: image But same error message keeps showing.

And actually by removing the "and" operator I was able to passed the syntax check, with and without strings: With strings: image

Without strings: image

If I try what comes in the Nano documentation by putting the operators as string and the fields like properties I also received the same error message: image

But if I modify the file I mention before (nano.d.ts): From this: image

To this: image

All of the queries above are now valid, here's an example of the last query: image

And it is important for us to be able to use operators like "and" or "or" so we can construct complex queries.

Please let me know if you have any other ideas, thanks!

emlaver commented 4 years ago

@Heydan83 thanks for sending the results. I was able to successfully run a Typescript sample using the and operator in the selector:

const Cloudant = require('@cloudant/cloudant');

const cloudant = Cloudant({ url: 'https://account.cloudant.com'})
const cloudantDb = cloudant.db.use('users')

const query = { 'selector': { 
  '$and': [
    { 
      'email_verified': {'$eq': true}
    },
    {
      'active': {'$eq': true}
    }
  ]
}};

cloudantDb.find(query, function(err, data) {
  console.log(err);
  console.log(data);
})

To run in Node, I executed the commands tsc sample.ts then node sample.js (sample.ts contains the code above).

Either way, I believe this request should be filed in https://github.com/apache/couchdb-nano/issues since that's where MangoSelector would need to be updated: https://github.com/apache/couchdb-nano/blob/master/lib/nano.d.ts#L1305

Heydan83 commented 4 years ago

Hi @emlaver, I did a similar test as you did and everything goes ok, I didnt have any issues, which made me think that another dependency in my project was causing the issue and I did found it. It seems like when you configured your typescript project with ESlint and Prittier it forces a stronger type in the code causing the issue with the MangosSelector interface. I create an example of this in this project so you can check it: https://github.com/Heydan83/cloudant-types-error If you want to reproduce it from scratch, here are the steps: -initiate node: npm init --y -install cloudant: npm install --save @cloudant/cloudant -install typescript as a dev dependency: npm install -D typescript -install eslint: npm install eslint --save-dev -install plugin for eslint for typscript: npm i --save-dev typescript @typescript-eslint/parser and npm i --save-dev @typescript-eslint/eslint-plugin -install prettier: npm install --save-dev --save-exact prettier -generate the .eslintrc.json configuration file as in the example I put above -create ts file with test code also in the example above.

Now as you said before, this is not an issue with the cloudant library, but also as I said from the beginning, this issue was already reported and resolved on the Nano library with this issue: apache/couchdb-nano#202, but what Im asking you guys is to update (if there's any issues with the update) to the most recent version of the Nano project. The issues was reported on Nano v"8.1.0", which is the one you guys have as a dependency, and the most recent version is 8.2.2 which makes me think that there would be not issues with the update.

Please let me know you comments, thanks!

emlaver commented 4 years ago

@Heydan83 I'm going to verify that our tests pass after updating the nano dependency to 8.2.2. I'll update this ticket with my findings.

Heydan83 commented 4 years ago

@emlaver thanks!