Loopback-datasource-juggler: Versions >=3.25.0 are breaking include behavior for remote models using a non database connector

[lkappeler]

Description

Up to version 3.24.1 the described behavior works fine and is returning the queried data.

In version 3.25.0 the datasource jugglers findWithForeignKeysByPage function calls the given models _sanitize (in later versions _sanitizeQuery) and _coerce functions. This works fine as long as the given model is attached to a database connector.

Link to the change

If the model instead is attached to a non database connector, for example the remote-connector the code fails as the _sanizite or _sanitizeQuery function is not defined on the model with the remote datasource. (The _coerce functions as well.) I came along this Issue while including a remote model with the include filter. Only tested this with the remote connector. According to the documentation, I expect the code to fail for every non database connector as the functions are not mixed in to the model.

Error with stack trace

 TypeError: model._sanitizeQuery is not a function
globalize.ts:285
    at findWithForeignKeysByPage (/myapp/node_modules/loopback-datasource-juggler/lib/include.js:208:13)
    at throughFetchHandler (/myapp/node_modules/loopback-datasource-juggler/lib/include.js:484:9)
    at /myapp/node_modules/loopback-datasource-juggler/lib/dao.js:2193:9
    at /myapp/node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:1140:9
    at /myapp/node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:473:16
    at iteratorCallback (/myapp/node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:1064:13)
    at /myapp/node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:969:16
    at /myapp/node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:1137:13
    at buildResult (/myapp/node_modules/loopback-datasource-juggler/lib/dao.js:2159:11)
    at /myapp/node_modules/loopback-datasource-juggler/lib/dao.js:2173:13
    at doNotify (/myapp/node_modules/loopback-datasource-juggler/lib/observer.js:155:49)
    at doNotify (/myapp/node_modules/loopback-datasource-juggler/lib/observer.js:155:49)
    at doNotify (/myapp/node_modules/loopback-datasource-juggler/lib/observer.js:155:49)
    at doNotify (/myapp/node_modules/loopback-datasource-juggler/lib/observer.js:155:49)
    at Function.ObserverMixin._notifyBaseObservers (/myapp/node_modules/loopback-datasource-juggler/lib/observer.js:178:5)
    at Function.ObserverMixin.notifyObserversOf (/myapp/node_modules/loopback-datasource-juggler/lib/observer.js:153:8)

Steps to reproduce / Test case setup

You need two loopback applications, calling them App1 and App2. One with a DatabaseModel containing a relation to a RemoteModel. The RemoteModel can be any model in another loopback application.

Query for the DatabaseModel on App1 with an include filter on the RemoteModel.

GET http://app1.com/api/DatabaseModel?filter={"include": "remoteModel"}

The request will result in the Error described above.

Setup App1:

DatabaseModel.json

{
    "name": "DatabaseModel",
    "base": "PersistedModel",
    "properties": {
        ....
    },
    "relations": {
        "remoteModel": {
            "type": "ANY",
            "model": "RemoteModel",
            "foreignKey": "remote_id"
        }
    }
}

datasources.json

{

    "db": {
        "name": "db",
        "connector": "postgresql",
       ....
    },
    "remote": {
        "url": "http://app2.com/api",
        "name": "remote",
        "connector": "loopback-connector-remote"
    }
}

model-config.json

{
    "DatabaseModel": {
        "dataSource": "db",
        "public": true
    },
    "RemoteModel": {
        "dataSource": "remote",
        "public": true
    }
}

Setup App2:

RemoteModel.json

{
    "name": "RemoteModel",
    "base": "PersistedModel",
    "properties": {
        ....
    }
}

datasources.json

{

    "db": {
        "name": "db",
        "connector": "postgresql",
       ....
    }
}

model-config.json

{
    "RemoteModel": {
        "dataSource": "db",
        "public": true
    }
}

Needed Fixes

To make the change work together with the remote-connector, the remote connector needs to mixin the following functions of the DAO to its models. ( To test if it would work I imported the dao.js in to the remote-connector.js and mixed in the following functions)

remote-connector.js

const DataAccessObject = require('loopback-datasource-juggler/lib/dao');


function RemoteConnector(settings) {

  ....

  this.MissingMixins = function() {};
  MissingMixins._sanitizeQuery = DataAccessObject._sanitizeQuery;
  MissingMixins._coerce = DataAccessObject._coerce;
  MissingMixins._getSetting = DataAccessObject._getSetting;
  MissingMixins._getHiddenProperties = DataAccessObject._getHiddenProperties;
  MissingMixins._getProtectedProperties = DataAccessObject._getProtectedProperties;
  MissingMixins._allowExtendedOperators = DataAccessObject._allowExtendedOperators;
 // MissingMixins._sanitizeData = DataAccessObject._sanitizeData;

}


RemoteConnector.prototype.define = function(definition) {
  ....

  jutil.mixin(Model, this.MissingMixins);

  ....

}

The following code would fix the issue. However I don't think this is a proper solution.
The required functions should be moved to a mixin file, similar to the include.js so other connectors can import it and mixing it into their models.

In addition, may there should be a check if the function is present on a model to not break the support for any 3rd-pary connectors in a minor version. There could be a fallback case to uses the DAO's functions instead as they are already defined.

Further on the remote-connector needs to import and mixin the required functions. Probably other non database connectors as well.
Therefore the fallback implementation is potentially the better option.

[dhmlau]

@lkappeler, would you be able to move to the latest version of juggler? i.e. 3.27.0 or 4.3.0?
cc @raymondfeng

[lkappeler]

@dhmlau

I tested all the 3.x versions after 3.24.1 the problem is affecting all versions >= 3.25
The only thing changing is the name of the undefined function with the version 3.26.0 form model._sanitize to model._sanitizeQuery.

As you suggested I tested it also with the version 4.3.0, but unfortunately I ran into the same error. (Was not sure if it is compatible)

[raymondfeng]

I'm in the process of refactoring such methods into ModelBase.

[raymondfeng]

Please verify with [email protected] and [email protected].

[lkappeler]

@raymondfeng
Works with 3.28.0 and with 4.4.0 as well.
Thanks for the quick reaction time for fixing the issue.