Unload a element on Operation hook loaded

[FelipeAguayo]

I wish I could unload an item in 'loaded' hook, example:

result from datasource: [{id:1, name: 'example'}, {id:2, name: 'example2'}]

Model.observe('loaded', function (ctx, next, unload){
   if(ctx.instance.id === 2){
     unload();
   }else{
     next();
   }
});

result for request: [{id:1, name: 'example'}]

is possible?

[superkhau]

@bajtos ^

[bajtos]

Hi @FelipeAguayo, thank you for reporting this use case. Could you please provide more details on the filtering criteria you would like to apply?

If these criteria are static, then you can use the access hook to add extra filter conditions to all requests.

// mock-up, not tested
Model.observe('access', function(ctx, next) {
  var extra = { id: { neq: 2 } };
  if (ctx.query.where) {
    ctx.query.where = { and: [ctx.query.where, extra] };
  } else {
    ctx.query.where = extra;
  }
  next();
});

[FelipeAguayo]

the criteria are dynamic, depend on variables that are external to the loaded model, but this can't be validate in Operation Hook 'access'. An example of this would be:

datasource: mysql.

Model:

{
  "name":"Model",
  "plural":"Models",
  "idInjection":true,
  "properties":{
     "name":"string"
  },
  "relations":{
    "relOtherModel":{
      "type": "belongsTo",
      "model": "OtherModel",
      "foreignKey": "idOtherModel"
    }
  }
}

values Model:

[ {id:1, idOtherModel:1, name:'one'}
, {id:2, idOtherModel:1, name:'two'}
, {id:3, idOtherModel:2, name:'three'}
, {id:4, idOtherModel:3, name:'four'}]

OtherModel:

{
  "name":"OtherModel",
  "plural":"OtherModels",
  "idInjection":true,
  "properties":{
     "name":"string",
     "state":"string"
  }
}

values OtherModel:

[ {id:1, name:'otherOne', state: 'created'}
, {id:2, name:'otherTwo', state: 'deleted'}
, {id:3, name:'otherThree', state: 'created'}]

after load (get models):

Model.observe('loaded', function (ctx, next, unload){
   var otherModel = ctx.instance.relOtherModel();
   if(!otherModel || otherModel.state === 'deleted'){
     unload();
   }else{
     next();
   }
});

Result values:

[ {id:1, idOtherModel:1, name:'one'}
, {id:2, idOtherModel:1, name:'two'}
, {id:4, idOtherModel:3, name:'four'}]

[bajtos]

@FelipeAguayo thank you for the example, now I see why you cannot use access hook. I have one more question, what should happen when the client tries to access the model called three, e.g. by calling findById(3)? Do you expect such call to return 404?

[FelipeAguayo]

yes, i do.

[bajtos]

Good, that's what I would expect too.

Ideally, LoopBack should allow filtering using nested/related model properties - see strongloop/loopback#517. In which case your access hook could build a query like

{
   'relOtherModel.deleted': { '$neq': true } 
}

However, because #517 is not very likely to get implemented soon, I think it makes sense to add a new hook, something like "after access" that would allow you to filter the result array.

Note that a naive hook implementation calling ctx.instance.relOtherModel() will lead to a high performance overhead - each item in the result array will trigger extra database query. This is known as select n+1 problem.

A better approach is to load all related models in one query using a filter like { id: { '$inq': results.map(getOtherModelIdProperty)) } }. In order to support that, the new hook must be invoked only once and pass the full array of models in the context object. That's why I don't think the "loaded" hook is suitable in this case.

[FelipeAguayo]

A better approach is to load all related models in one query using a filter like { id: { '$inq': results.map(getOtherModelIdProperty)) } }. In order to support that, the new hook must be invoked only once and pass the full array of models in the context object. That's why I don't think the "loaded" hook is suitable in this case.

@bajtos can you explain this better please?, Does that would use 'after access'?

[bajtos]

@bajtos can you explain this better please?, Does that would use 'after access'?

Yes, it would use a new hook we need to implement. I am arguing that the new hook should be called only once even if multiple models are loaded/access, and therefore we should come up with a different name than "after access" to prevent confusion.

I have one more question. What is the intended behaviour of this new restriction for write operations like "create", "updateAttributes" and "updateAll"?

[FelipeAguayo]

In my opinion, it should serve to any method for access to information.

[FelipeAguayo]

@bajtos Is there any progress on this?

[bajtos]

I have one more question. What is the intended behaviour of this new restriction for write operations like "create", "updateAttributes" and "updateAll"?

In my opinion, it should serve to any method for access to information.

Please be more specific.

Case 1 - user invokes create. Note that this operation does not load any data from the database. What context properties and behaviour do you propose for this new hook?

Case 2 - user invokes updateAll. Note that this operation does not load any data from the database. What context properties and behaviour do you propose for this the new hook?

[bajtos]

Is there any progress on this?

Frankly, this is not a high priority for us right now.

If you are willing to contribute this new hook yourself, then perhaps it will be best to wait with further discussion until you have a partial implementation, so that we can discuss specific code as opposed to abstract ideas.

[denkan]

I too am looking for a way to deny a loaded object to proceed to a potential array result (as find() generates).

My use-case is similar to #issuecomment-148072721, except that the logic to grant/deny is even more dynamic and not based on any static property but rather on a external calculation of its properties.

Illustrated as such:

Model.observe('loaded', function (ctx, next, unload){
   var data = ctx.instance
   if(!LibWithBusinessLogic.isValidModelData(data)){
     unload();
   }else{
     next();
   }
});

I'm able to pass an error to next(new Error('Invalid, mate!')), which makes it work fine when requesting single data. But will throw the same error to whole request when the result would be array, even if 5 out of 6 objects are valid.

Any thoughts or guidelines are appreciated.

[stale]

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.