beforeRemote hook executed after datasource access

[eggerdo]

For remote method PUT /Models/{id}, the beforeRemote('**') hook is triggered after the datasource is accessed. Is this a bug?

From the description of the beforeRemote hook I would expect that that hook is triggered before anything else happens. At least that's how it is with other remote operations like GET /Models/{id} or GET /Models/. I checked and the Problem is that it calls findById, probably to check if the item exists in the database, before the beforeRemote hook is triggered. But I need to modify my datasources depending on the current user, so I need a hook, which is triggered before any datasoure is accessed. However in this case, all the remote hooks, operation hooks and model hooks are triggered after the datasource is accessed.

[richardpringle]

Hey @eggerdo, I was unable to reproduce the issue. Can you point me to the spot where findById is called. And how are you modifying your datasource based on the current user? Do you have multiple datasources attached to the same model?

When I attempted to reproduce, the behavior was the same no matter what method I was calling. Do you have to most recent version LoopBack?

I need a little more detail to be able to help you out. It would be even better if you could fork loopback-sandbox and make an example of exactly what you are trying to do.

[eggerdo]

Hey @richardpringle, I might have been a bit too unclear. The findById that I was referring to is not the findById of the model, but that of the DataAccessObject in the loopback-datasource-luggler's dao.js.
My issue is not exactly coming from modifying the datasources, it is there even if only one datasource is used. The only difference is, that when using only one datasource, it doesn't matter if the datasource is accessed to verify if the object exists before the beforeRemote hook triggers.

I created a fork of the example project which you can find here where I show the issue, without switching the databases. I wrote the error description and the cases in the readme there.

If you want I can update the example to include the switching of the datasources based on the current user, but I don't think it is relevant to determine if this issue is a bug or not :) But for your information I will give you an overview:

1. I use the workaround described here to inject the context
2. In my models, I set the beforeRemote('**') hook to obtain the userId from the injected context/accessToken and update the datasource based on the userId
3. I overwrite the getDataSource of the models to return the updated datasource

This works so far except from the case described above: when I try to update an object using PUT /Model/{id} it first checks in the datasource which was last accessed. If that datasource does not have an object with this id, the call fails, even if the datasource that I actually want to query has it. And since there is no hook which triggers before the datasource is accessed, I cannot update it in time.

[richardpringle]

Hey @eggerdo, first of all, thanks forking and adding the detailed description. There are a bunch of us who are trying to help solve the long list of LoopBack issues, and this makes our job much easier!...Which in turn means you get help faster.

I believe that this remote hook is functioning as expected. A remote hook simply implies that the hook is triggered before the function associated with the request. For PUT models/{id}, the associated method is upsert or updateOrCreate. The remote hook is called before the associated remote method.

The issue with your scenario is that there is another method (find) that is called before updateOrCreate upon access to that endpoint. What you are really looking for is a 'before access' operation hook...which does not currently exist. I'm going to throw a feature tag on this issue so that we could potentially add that hook in.

In the meantime... I'm not too sure how to implement a workaround. We're currently working on a feature to make endpoints a little easier to customize, but I'm not sure when it will be ready for release. For now, I think you're going to have to create your own middleware that runs before the loopback#rest phase.

Another solution would be to implement your own 'before access' hook and submit a PR.

Sorry for the bad news... please post your solution here for any others who run into the same issue!

Also, if you are submitting a PR, we can help you along the way.

Good luck!

[eggerdo]

Ok thanks. Yes I was thinking one could argue for either case. I understand your point that the remote hook is associated with the updateAttributes, for which case it is working correctly, since the remote hook is triggered before the values are actually updated. But as the 'find' is implicit, and moreover an essential part of the PUT models/{id}, since the PUT models/{id} fails (and the updateAttributes is not even executed) if the find returns false, I would argue that find should be part of the updateAttributes and therefore be considered for the beforeRemote hook as well.

But to avoid this problem, I think the easiest solution for me is to disable the PUT models/{id} and instead use a HEAD models/{id} together with a PUT models/ to achieve the same result.

And if I have time I might even look into the 'before access' hook :)

But thanks for your help in any case.

[richardpringle]

@eggerdo, I have an update for you.

I was jumping to conclusions with that getDatasource logging. Although it is called multiple times before the hook, you should still be able to use the access hook to modify the datasouce (or whatever you are doing) before the database is actually queried. Make sense?

Let me know if that works for you!

[richardpringle]

I would also be interested to see your implementation of using multiple datasources on the same model if possible...

[eggerdo]

@richardpringle, sorry I had some other things on my plate and just managed to look into it again.

I had already checked the access hook previously, and it does not work with that either. The datasource is accessed before access hook is triggered. I updated the example on my github with context injection, access hook, and how I update the datasource based on the user.

[richardpringle]

@Amir-61, can you weigh in on this?

@eggerdo, even though getDatasource is called, it doesn't necessarily mean that it has been queried for the record in question. The access hook is called before any data is "accessed" (@Amir-61 can confirm). What I'm not sure is if you can change the datasource at this point or not, you would have to test it out.

In any case, the way you are going to achieve your desired functionality will be a bit of a workaround, so I want to make sure we have a well defined feature request.

[eggerdo]

@richardpringle, thanks I appreciate any input to this problem.

You are correct. I got misled by the fact that getDatasource is called and assumed it was connecting already. But I checked the debug log of the mongodb connector and it indeed does not access the datasource before the access hook.

However, there are still two problems with the access hook.

1. The remote context injected in the remote hooks is not available in the access hook. So I can't obtain the accessToken/userId to update my datasource
2. Even if I could update the datasource the way I showed in the example github, at the time that the access hook is triggered, the connector is already stored in a local variable. So updating the datasource on the model or changing the connector does not have any effect, it will still use the old connector, obtained before the access hook is triggered.

I can think of ways to solve item 2, by modifying the file dao.js in the loopback-datasource-juggler, although I would rather prefer avoiding that, but I have no idea how to solve item 1.

[richardpringle]

@eggerdo: I may have found a solution for you, middleware to the rescue!

DISCLAIMER: I am not sure what the security repercussions are. If @bajtos could comment, that would be great.

Also, before implementing anything, I suggest reading the middleware documentation

In your server/middleware.json file, you can add the built-in loopback#token middleware as follows:

"auth": {
  "loopback#token": {}
}

Now you will get the ctx.req.accessToken object with all your beforeRemote and afterRemote methods. Again, I'm not sure about "best practices" with regards to the access token remaining in your req object.

As for switching datasources, this can be accomplished in a custom middleware function. How you implement that is up to you. I placed the following code in the server.js file, but that is not the only way (or the most elegant way) you can do this:

app.middleware('routes:before', function(req, res, next) {
  console.log(req.accessToken);
  next();
});

obviously placing the code you have in your forked sandbox instead of a console.log.

Let me know how it goes!

[eggerdo]

@richardpringle: thanks you're awesome, this works like a charm! And as for a first impression, this looks to be much neater than I had it before in any case. Thanks a lot already :)

One more question I have: Is it possible that one REST API call may interrupt another? Meaning, if I get a call, then update my datasources in the middleware function, is it possible that a second call will modify the datasources before the first call is completed?

[bajtos]

Hi, for LB 3.0, we are thinking about replacing findById + updateAttributes with a single static method patchById, that would make the original approach with beforeRemote work. See
loopbackio/loopback-datasource-juggler#825

I think he approach described above, which uses a middleware to select the datasource, is reasonable. The security implication is that the middleware selection is performed before any authentication and authorization takes place, because auth & auth is performed in loopback.rest() middleware handler.

One more question I have: Is it possible that one REST API call may interrupt another? Meaning, if I get a call, then update my datasources in the middleware function, is it possible that a second call will modify the datasources before the first call is completed?

The calls will be interleaved. Because each model is a global singleton and each model can be attached to a single datasource, there is indeed a race condition in which an operation started with datasourceForUser1 can finish with datasourceForUser2.

See my older StackOverflow answer for alternative solutions.

[sachinhub]

Hi

For switching data source at runtime, we have overridden the
model.getDatasource () method which finds out the data source to be used
for that request using loopback.getCurrentcontext() and does
model.attach(data source);

The approach is working. We have done some testing with concurrent requests
and it seems to work very well.

We are just concerned about the overhead model.attach(data source) has in
every request. The model.getDatasource () method gets called multiple times
in a request.

Bajtos, do you see any concern with this approach?
On Feb 26, 2016 8:13 PM, "Miroslav Bajtoš" notifications@github.com wrote:

Hi, for LB 3.0, we are thinking about replacing findById +
updateAttributes with a single static method patchById, that would make
the original approach with beforeRemote work. See
loopbackio/loopback-datasource-juggler#825
loopbackio/loopback-datasource-juggler#825

I think he approach described above, which uses a middleware to select the
datasource, is reasonable. The security implication is that the middleware
selection is performed before any authentication and authorization takes
place, because auth & auth is performed in loopback.rest() middleware
handler.

One more question I have: Is it possible that one REST API call may
interrupt another? Meaning, if I get a call, then update my datasources in
the middleware function, is it possible that a second call will modify the
datasources before the first call is completed?

The calls will be interleaved. Because each model is a global singleton
and each model can be attached to a single datasource, there is indeed a
race condition in which an operation started with datasourceForUser1 can
finish with datasourceForUser2.

See my older StackOverflow answer
http://stackoverflow.com/a/28327323/69868 for alternative solutions.

—
Reply to this email directly or view it on GitHub
#2064 (comment)
.

[eggerdo]

Thank you. The race condition is a bit worrying, and I will need to discuss internally how we should handle that. I'm glad to hear about the update for LB 3.0, but I'm not sure that will solve my problems either. Querying related models with an include filter, does not trigger any remote hooks in the related model, so I think I will always end up with a race condition when switching my datasources. So I might need to think that over. And the solution in stackoverflow seems fine, except that I would have to update the LB server every time I need to add a new group of models using a different datasource. And I don't think it will work correctly with related models queried through include filters either. But thanks in any case for your help and valuable input!

[sachinhub]

Dominik,
Do you have any comments on the approach we are taking for data source
switch? I described that in my previous email
On Feb 29, 2016 4:20 PM, "Dominik Egger" notifications@github.com wrote:

Thank you. The race condition is a bit worrying, and I will need to
discuss internally how we should handle that. I'm glad to hear about the
update for LB 3.0, but I'm not sure that will solve my problems either.
Querying related models with an include filter, does not trigger any remote
hooks in the related model, so I think I will always end up with a race
condition when switching my datasources. So I might need to think that
over. And the solution in stackoverflow seems fine, except that I would
have to update the LB server every time I need to add a new group of models
using a different datasource. And I don't think it will work correctly with
related models queried through include filters either. But thanks in any
case for your help and valuable input!

—
Reply to this email directly or view it on GitHub
#2064 (comment)
.

[eggerdo]

I see the problem rather in the loopback.getCurrentcontext() call, since that can return null in many cases. Other than that, I am also overriding the model.getDataSource function to return the datasource and attach it there to the model. So far I haven't seen any performance issues with that.

[sachinhub]

So what's the alternative you used to know the context (which data source
to use) instead of using loopback. GetCurrentcontext ()

I was facing one issue with getCurrentcontext () but that was because of
incorrect use of it. Instead of calling .get() and .set() methods on
loopback. GetCurrentcontext () I was directly setting the value as a
property. After fixing it, no issues with it.
On Feb 29, 2016 18:29, "Dominik Egger" notifications@github.com wrote:

I see the problem rather in the loopback.getCurrentcontext() call, since
that can return null in many cases. Other than that, I am also overriding
the model.getDataSource function to return the datasource and attach it
there to the model. So far I haven't seen any performance issues with that.

—
Reply to this email directly or view it on GitHub
#2064 (comment)
.