loopback-datasource-juggler icon indicating copy to clipboard operation
loopback-datasource-juggler copied to clipboard

v3.31.0 introduces bug

Open ataft opened this issue 6 years ago • 10 comments

The following code works fine in v3.30.0:

const query = {
    where: {
        or: [
            { username: username },
            { email: username }
        ]
    }
};
self.userModel.findOne(query, function (err, user) {

As of v.3.31.0, I get the following error:

Error: The where clause "[email protected]" is not an object
    at Function.ModelUtils._coerce (C:\apps\flexit\node_modules\loopback-datasource-juggler\lib\model-utils.js:369:11)
    at Function.ModelUtils._coerce (C:\apps\flexit\node_modules\loopback-datasource-juggler\lib\model-utils.js:552:20)
    at Function.ModelUtils._coerce (C:\apps\flexit\node_modules\loopback-datasource-juggler\lib\model-utils.js:394:14)
    at Function.ModelUtils._normalize (C:\apps\flexit\node_modules\loopback-datasource-juggler\lib\model-utils.js:177:8)
    at Function.find (C:\apps\flexit\node_modules\loopback-datasource-juggler\lib\dao.js:1529:10)
    at Function.findOne (C:\apps\flexit\node_modules\loopback-datasource-juggler\lib\dao.js:1782:8)
    at Strategy._verify (C:\apps\flexit\server\boot\authentication.js:288:36)
    at Strategy.authenticate (C:\apps\flexit\node_modules\passport-local\lib\strategy.js:90:12)
    at attempt (C:\apps\flexit\node_modules\passport\lib\middleware\authenticate.js:361:16)
    at authenticate (C:\apps\flexit\node_modules\passport\lib\middleware\authenticate.js:362:7)

Looks like something broke in the _coerce function.

ataft avatar Jun 27 '19 19:06 ataft

It seems to be related to https://github.com/strongloop/loopback-datasource-juggler/commit/4f31db1b64efa384e2c46c73bd34b97463db78f1.

raymondfeng avatar Jun 27 '19 19:06 raymondfeng

@ataft Can you check #1755 to see if I'm missing something for a test I wrote using your scenario? I am not able to reproduce your issue with it:

biniams-mbp:loopback-datasource-juggler badmike$ npm t

> [email protected] test /Users/badmike/loopback/loopback-datasource-juggler
> nyc mocha



  ․

  1 passing (46ms)

Here is the User model definition that I used.

b-admike avatar Jun 27 '19 20:06 b-admike

@ataft can you share a sandbox app to reproduce the failure if that's easier? Please also include the steps you took to see the error.

b-admike avatar Jun 27 '19 20:06 b-admike

Test looks OK, but it does differ from mine in that it doesn't include "username". Perhaps if you tried:

or: [{username: '[email protected]'}, {email: '[email protected]'}],

I doubt this will make it fail though. It's probably some other difference between our environments (I'm on loopback version 3.17.1). I'll try to reproduce the issue with a sandbox app, but for now I'm just going to stick on juggler v3.30.0.

ataft avatar Jun 27 '19 21:06 ataft

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Aug 26 '19 22:08 stale[bot]

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

stale[bot] avatar Sep 09 '19 23:09 stale[bot]

OK, I finally figured out what was causing this error. I'm using the "defineProperty" function, like this: app.models.User.defineProperty('email', { required:false });

In the loopback-datasource-juggler, this causes the DataType to be type: [Function: AnonymousModel_6], which makes isNestedModel true, here: https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/model-utils.js#L544

Then, it throws an error any time "email" is used in the where clause, e.g.

    var whereClause = {
      where: {
        email: '[email protected]'
      },
    };
    app.models.User.findOne(whereClause, function (err, user) {
      console.log('err: ', err);
      console.log('user: ', user);
    });

It has nothing to do with using "OR", like I originally thought.

Here's a sandbox that shows the issue: https://github.com/ataft/loopback-sandbox

ataft avatar Oct 29 '19 21:10 ataft

@b-admike I see that @raymondfeng comment about it being related to https://github.com/strongloop/loopback-datasource-juggler/commit/4f31db1b64efa384e2c46c73bd34b97463db78f1 is spot on

ataft avatar Oct 29 '19 21:10 ataft

@ataft thank you for the update. Would you like to contribute the fix yourself? I am not sure if you are using defineProperty correctly, shouldn't you specify type too?

app.models.User.defineProperty('email', {
  type: 'string',
  required: false,
});

bajtos avatar Nov 01 '19 16:11 bajtos

Thanks @bajtos , adding type: 'string' fixes the problem. I tried to find where type was being set to [Function: AnonymousModel_6], but could not find this and don't know whether it's intended or not, so I'll let you decide that. I'm satisfied with just adding type: 'string'.

I'm not sure I'm doing this properly, but I want to use the built-in User model without email for auth providers like LDAP and Active Directory. Let me know if there's a better way.

ataft avatar Nov 01 '19 16:11 ataft