loopback.getCurrentContext() returns null in MyModel.getDataSource() override, when querying model relation with { include: ... } filter

[drywolf]

Hi,

I'm currently working on a way to integrate a multi-tenancy usecase for a project that uses loopback.
(see the following discussion for details about the approach: https://groups.google.com/forum/#!msg/loopbackjs/KIhO2_W5dF4/bmY2CNkrBwAJ)

Basically we are overriding certain models' getDataSource() method and we then switch data-sources between multiple different MS-SQL data-sources (pointing to different schemas in the same DB in our case).
The information which data-source to return from our MyModel.getDataSource() override is retrieved by using the loopback context functionality -> loopback.getCurrentContext()

In a request middleware we put a tenant-id into the context, later when the MyModel.getDataSource() is called we retrieve the context and look for the tenant-id, which tells us which data-source we should use for this request (we then attach the Model to the data-source that was determined).

This works fine for the most part, but when it comes to querying relation-properties of models this does not work properly, because loopback.getCurrentContext() returns null in the target model of the relation. So for the origin-model of the query our getDataSource() override works fine and uses the correct data-source, but in the getDataSource() override of the model that is included via a relation in the query, the getCurrentContext() call just returns null because it can't find the context.

I'm currently trying to debug into the depths of the loopback context functionality (specifically loopback/server/current-context.js, but I'm not really experienced with the CLS magic that is used to implement this functionality, so I'm not really progressing anywhere at the moment 😬

Can anyone please give some hints what I should look at to solve the bug ?
I tracked the callstack for the call to MyModel.getDataSource() that shows the bug, but I just don't know what to look out for to fix the issue and make the CLS/context functionality work for this case.

EDIT:
especially I'm looking at the chain(...) calls in loopback/server/current-context.js#L79/80.
I guess they are mixing in the context functionality into the data-juggler and remote-objects modules, but I can't grasp what exactly is happening there.

Thanks & Regards,
Wolfgang

[drywolf]

Here's are the server stack traces for the Model.find() function calls that are executed via the remote-connector from the browser to the server.
The query call to the model looks like follows:

Person.find({ include: 'pets' }, function(err, items) { ... });

Root-Query ... Model.find ... context is OK

at Function.module.exports.model.getDataSource (loopback-full-stack-tenancy\common\models\utils\switch-datasource-mixin.js:29:21)
at Function.find (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\dao.js:1348:48)
at SharedMethod.invoke (loopback-full-stack-tenancy\node_modules\strong-remoting\lib\shared-method.js:248:25)
at HttpContext.invoke (loopback-full-stack-tenancy\node_modules\strong-remoting\lib\http-context.js:384:12)
at loopback-full-stack-tenancy\node_modules\strong-remoting\lib\remote-objects.js:620:11
at execStack (loopback-full-stack-tenancy\node_modules\strong-remoting\lib\remote-objects.js:460:7)
at loopback-full-stack-tenancy\common\models\utils\switch-datasource-mixin.js:10:3
at Function.<anonymous> (loopback-full-stack-tenancy\node_modules\loopback\lib\model.js:196:11)
at execStack (loopback-full-stack-tenancy\node_modules\strong-remoting\lib\remote-objects.js:452:26)
at RemoteObjects.execHooks (loopback-full-stack-tenancy\node_modules\strong-remoting\lib\remote-objects.js:464:10)

Query Include-Filter ... { include: 'relation' } ... context is NULL

    at Function.module.exports.model.getDataSource (loopback-full-stack-tenancy\common\models\utils\switch-datasource-mixin.js:16:28)
    at Function.find (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\dao.js:1348:48)
    at includeHasManySimple (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\include.js:505:24)
    at processIncludeItem (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\include.js:275:16)
    at loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\include.js:173:5
    at loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\node_modules\async\lib\async.js:157:13
    at _each (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\node_modules\async\lib\async.js:57:9)
    at Object.async.each (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\node_modules\async\lib\async.js:156:9)
    at Function.Inclusion.include (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\include.js:172:9)

and also

    at Function.module.exports.model.getDataSource (loopback-full-stack-tenancy\common\models\utils\switch-datasource-mixin.js:16:28)
    at Function.DataAccessObject.getConnector (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\dao.js:133:15)
    at Function.find (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\dao.js:1377:24)
    at includeHasManySimple (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\include.js:505:24)
    at processIncludeItem (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\include.js:275:16)
    at loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\lib\include.js:173:5
    at loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\node_modules\async\lib\async.js:157:13
    at _each (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\node_modules\async\lib\async.js:57:9)
    at Object.async.each (loopback-full-stack-tenancy\node_modules\loopback-datasource-juggler\node_modules\async\lib\async.js:156:9)

the big difference that I can spot is the use of HttpContext.invoke and SharedMethod.invoke in the calls that are working.

[Nivl]

getCurrentContext() is pretty much deprecated and is highly unstable: #1676

[0candy]

You can use the alternative to inject remote context via options.
#1495

[0candy]

Duplicate of loopbackio/loopback-datasource-juggler#657

[drywolf]

Thanks for the replies.

@0candy

You can use the alternative to inject remote context via options.

Currently I am overriding the Model.getDataSource() function, I don't have access to the remote context there, so the workarround does not immediately solve my issue.

I will have a look to see if I can achieve the same functionality that I have now, but without overriding getDataSource(). But that's just to say that there are currently usecases of loopback.getCurrentContext() that might not be covered by the workarround.

@Nivl

getCurrentContext() is pretty much deprecated and is highly unstable: #1676

I see, it should really be marked as deprecated asap then in the code / docs ... I've seen so many threads/issues/workarrounds that all stem from getCurrentContext() not really being supported anymore. It would be helpful to let people know what they are getting into if they start using getCurrentContext() today.

Regards

[0candy]

I am closing this issue as its a duplicate of loopbackio/loopback-datasource-juggler#657

[eggerdo]

@drywolf
How did you solve this issue in the end? I have the same problem that I need to change the data sources of my models based on the authenticated user and that there is no context available in the getDataSouce function. What I came up with based on this issue and the work around is:

1. in the beforeRemote hook of my models I update the data source based on the access token in the context, using the above mentioned workaround, then I store the data source in a global variable.
2. in the getDataSource functions of my models, I retrieve the data source from the global variable and attach it to the model

I don't really like this approach though, most importantly, I don't think it will scale well, and if a second remote operation is started before the first completes, the second will overwrite the data source in the global variable, and the first will read data from the wrong data source.

So I would be really curious how you solved this problem,
Regards

[drywolf]

hi @eggerdo

I did basically the same, this is a small playpen project that I used to evaluate and test the approach...
https://github.com/drywolf/loopback-full-stack-tenancy/blob/replace-getcurrentcontext/common/models/utils/switch-datasource-mixin.js#L20

The difference to your approach is that I don't use a global variable and I also do not override the getDataSource function anymore, instead I switch the data-source in the beforeRemote() hook of the models directly.
Also I'm doing some very basic handling of the "include" filter, which is important if you want to fetch related models synchronously with a single API call.
The models that are related to the original model of the API call then also will get their data-sources switched.

About concurrency, I wrote a basic test that I can run from a browser. This basically just spams requests, switching between six data-sources repreatedly and checking if the returned data is consistent with what is expected to be returned for that data-source in the request header.

This works pretty well up to now, but I'm also not a 100% certain that this solution covers all situations about request-concurrency cases.

[eggerdo]

@drywolf
Thanks I will have a look at your implementation and see if I can adjust my part. Btw, for the remote operation PUT /Models/{id}, did you notice that the datasource is already accessed before the beforeRemote hook is triggered? More precisely, it calls findById on the datasource before the beforeRemote hook is triggered. So in this case, both our solutions fail!

[drywolf]

I did a very quick test in the example project from above, and I think you are right about the PUT/id call.

Client-Result-Log:

// format is [datasource, method, err, person => {name, age}]
@family2 GET/Person.find() null Simon 2
@family1 GET/Person.find() null John 1
@family2 PUT/p.updateAttributes() null New John 1 <---- instance call, looks like we got the age of John on the result, when it should be Simon's
@family1 GET/Person.find() null John 1
@family2 GET/Person.find() null New John 2 <---- the age is back to 2 with a fresh query

This is a corresponding debug log from the server where I logged out the relevant information in

• the Person.beforeRemote() hook
• directly from the monkey-patched DataAccessObject.findById() method

Server-Log:

beforeRemote     >   get Person.find                         @ data-source family2
beforeRemote     >   get Person.find                         @ data-source family1
DataAccessObject >   --- Person.findById(1)                  @ data-source family1
beforeRemote     >   put Person.prototype.updateAttributes   @ data-source family2
beforeRemote     >   get Person.find                         @ data-source family1
beforeRemote     >   get Person.find                         @ data-source family2

Strangely on the incorrect datasource does not produce a data inconsistency on the server, which I believe might be a special case because I am using the memory data-source for those tests. The behavior of the updateAttributes() call from the client's perspective is pretty off though. Since it uses an entity that was previously queried from datasource1 but uses this instance to update a different entity in datasource2. But from the perspective of the http-requests alone, it is still a consistent and valid transaction as far as I can tell.

It would be interesting to see what happens with a real database connector, I would asume that data would get falsely updated / leaked from one datasource to another one.

@eggerdo Which connector / datasource are you using ? Have you been able to create a test case where data actually breaks ?

Cheers

[drywolf]

I just ran the same test code with a SQL data-source, the result is exactly the same as above.
This means that if you are aware that you should not switch the data-source "mid-flight" and then call an instance CRUD method on a model in the client, at least you will not unintentionally corrupt any data I think.

One thing that I also figured out tho is that if in the original data-source there is no entity with the same ID as of the entity in the datasource that you want to update, then the update will fail silently and no data will be changed anywhere.

Client-Result-Log:

@sql_familyB GET/Person.find() null Simon 2
@sql_familyA GET/Person.find() null John 1
@sql_familyB PUT/p.updateAttributes() null New John 1
@sql_familyA GET/Person.find() null John 1
@sql_familyB GET/Person.find() null Simon 2

[eggerdo]

@drywolf
Thanks for the confirmation. I am using a mongodb connector but I am only in the development phase, so I have not done any further tests than manually trying to write/read with different users.
But yes the last point you wrote is exactly my main problem. It will be very likely in my use case that the different data sources will not have the same objects with the same ids. So checking for an object in one datasource although trying to update it in another will most likely break my system.
I already created an issue (#2064) to check if this is a bug and/or if they are aware of this case, so I hope I will get a solution for this as well.