Loopback: Model JSON default scope declaration breaks update

Created on 16 Jan 2015  路  25Comments  路  Source: strongloop/loopback

Assuming a model definition like so:

(test-model.json)

{
  "name": "TestModel",
  "plural": "TestModels",
  "base": "PersistedModel",
  "idInjection": false,
  "mysql": {
    "schema": "test_schema",
    "table": "test_model"
  },
  "properties": {
     "id": {
      "type": "Number",
      "id": true,
      "required": true,
      "index": true,
      "length": null,
      "precision": 11,
      "scale": 0,
      "mysql": {
        "columnName": "id",
        "dataType": "int",
        "dataLength": null,
        "dataPrecision": 11,
        "dataScale": 0,
        "nullable": "N"
      },
      "_selectable": true
    },
    "name": {
      "type": "String",
      "required": true,
      "length": 100,
      "precision": null,
      "scale": null,
      "mysql": {
        "columnName": "name",
        "dataType": "varchar",
        "dataLength": 100,
        "dataPrecision": null,
        "dataScale": null,
        "nullable": "N"
      },
      "_selectable": true
    },
    "deleted": {
        "type": "Boolean",
        "required": true,
        "length": null,
        "precision": null,
        "scale": null,
        "mysql": {
            "columnName": "deleted",
            "dataType": "tinyint",
            "dataLength": null,
            "dataPrecision": 1,
            "dataScale": 0,
            "nullable": "N"
        },
        "_selectable": false
    }
  },
  "scope": {
      "where": {
          "deleted": false
      }
  },
  "validations": [],
  "relations": {},
  "acls": [],
  "methods": []
}

and having overridden the TestModel.deleteById method:

(test-model.js)

module.exports = function(TestModel) {
    TestModel.on('attached', function() {
        TestModel.deleteById = function(id, cb) {
            console.log('Overridden TestModel.deleteById');

            TestModel.update({'id': id}, {'deleted': true}, cb);
        }
    });
};

the query produced is the following (datasource "debug": true):

--> ComQueryPacket
{ command: 3,
  sql: 'UPDATE `test_model` SET `deleted` = 0 WHERE (`id` = 1) AND (`deleted` = 0)' }

Seems that the default scope column "deleted" value is assigned as the SET value of the "deleted" param.

Note that if one is to remove the default scope declaration from the .json file, the update works as expected.

Regards.

doc

Most helpful comment

@fabien Are you sure this is not a bug? Why would you not allow to update a property that you used to define a scope?

All 25 comments

Default scope is applied to operations that have a where filter.

Hi Raymond,

Surely I understand that, my point is that the default scope actually changes the value of the attribute that is to be set in the update clause (i.e. converts the SQL string from 'UPDATE test_model SET deleted = 1 WHERE (id = 1) AND (deleted = 0)' to 'UPDATE test_model SET deleted = 0 WHERE (id = 1) AND (deleted = 0)').

I see what you mean now. The default scope idea is from Ruby On Rails (http://api.rubyonrails.org/classes/ActiveRecord/Scoping/Default/ClassMethods.html#method-i-default_scope).

The default_scope is also applied while creating/building a record. It is not applied while updating a record.

It would be tricky for upsert though.

@fabien What's your take?

@raymondfeng I think the current behavior is correct, and you can fine-tune the behavior by overriding the defaultScope and possibly applyProperties methods of your model. It is actually encouraged to alter these methods for cases like this.

Note that defaultScope came about to fully support the new context handling within Loopback. That is the most useful use-case. Otherwise you'd usually be better off using normal scopes instead.

Closing the issue per @fabien

@fabien Are you sure this is not a bug? Why would you not allow to update a property that you used to define a scope?

@smehrbrodt +1

Took me a while to figure out why this wasn't working. And I agree with @spirospolitis and @smehrbrodt : setting a default scope shouldn't affect updating properties used for that scope. If this won't be fixed, then please add some clarification to the docs.

@markleusink +1

Please let us know how to get around this problem. Thanks!

This is the workaround I used:

Report.trash = function(reportId, cb) {

  // Need to reset the default scope because of https://github.com/strongloop/loopback/issues/1018
  var defaultScope = Report.defaultScope;
  Report.defaultScope = function(){};

  Report.upsert({id: reportId, 'deleted': true}, function(err, obj){
    cb(null, '');
  });

  // Restore the default scope
  Report.defaultScope = defaultScope;
};

@smehrbrodt thanks for posting your workaround! I couldn't get this to work for the built-in remote methods though... what I was trying to do is I have 2 models: Project and Status. Project model has "status_id", and I wish to include the entire status object in the response after I update the "status_id" on a project. Does that mean I have to override the default update or upsert remote method as well?

This bug should be reopened.

The default_scope is also applied while creating/building a record. It is not applied while updating a record.

The above, as @raymondfeng quoted, is currently _not_ the case. The defaultScope is currently applied when updating a record.

The docs still make absolutely no mention of this that I can find (either on create or update). From reading the docs, it just sounds like the default scope works on queries (not on creates or updates). It took me 3 hours of pulling my hair out to figure out what was going on.

To make things further confusing, the model definition JSON file format doesn't refer to this as defaultScope, but rather just as scope (vs. scopes for the named scopes). So, I could never have figured out on my own (without pouring over the code) that I just needed to override/stub out the defaultScope method to get the behavior I was expecting.

And it never dawned on me for a second that a default scope filter with a where clause would force the values of upserts to match the where filter. That really shocked me when I finally discovered that on my own (after hours of wasted time).

I read the docs thoroughly, and was just looking at them going, "but why?" Then, I found this thread. I wish I'd found this thread first. :-(

Anyway -- the docs should really simply say something like this:


Default Scopes with where Filters

Adding a scope to a model definition (e.g., in model.json) causes a method to be generated
for the model, called defaultScope(). This method is called automatically whenever a model
is created, updated, or queried.

But it is important to note that default scopes with a where filter may not work as you expect!

Each time a model instance is created or updated, the generated defaultScope() method will
modify the model's properties matching the where filter to enforce the values specified.

If you do not wish to have the default scope applied in this way, we recommend using named
scopes where possible.

If you must use a default scope, but don't want it to affect upsert(), for example, you simply need to override the model's defaultScope() method prior to calling upsert(), as in this example:

  var defaultScope = Report.defaultScope;
  Report.defaultScope = function(){};
  Report.upsert({id: reportId, 'deleted': true}, ...);
  Report.defaultScope = defaultScope;

Since the model definition calls it scope, despite the docs referring to it as defaultScope, this might also help clarify the difference between these two ways of referring to the same thing.

Note that for _create_, I agree it makes sense (and seems like a powerful feature) to be able to enforce constraints on inserted objects (but again, it should be documented if it's doing that).

But for _update_, the case is not nearly as obvious why this is the behavior. ( @raymondfeng @fabien ) Especially if that is different than the way it works in Rails from which the feature draws its inspiration. You can't expect people to know about the Rails model and then deviate from it, and in neither case documenting it! :-(

Either way, it'd be fine if only the docs explained this. Expecting people to infer how it works based solely on the inspiration the feature drew from Rails seems a bit of a stretch.

Please update the docs and consider reopening the bug to perhaps change update to not trash our models :-) without at least making us explicitly ask it to (e.g. via an option).

Thanks.

Steve

@crandmck ^

Thanks @slgoldberg

Default scope _was_ previously mentioned in https://docs.strongloop.com/display/LB/Model+definition+JSON+file#ModeldefinitionJSONfile-Defaultscope but obviously this did not explain enough.

I added your text above (slightly edited) in two places, so hopefully no one will miss it:

I hope that fits the bill. Thanks for the input.

(Sorry for the delay getting back to you; tough week at my day job.)

Yes -- @crandmck it looks great, thanks so much for putting it in both places. Hopefully it will help others avoid the hair-pulling. :-)

Steve

@smehrbrodt, will this workaround not affect the queries that are fired before the callback for the given upsert query is received ?

I am pretty sure (as a user, so take it for what it's worth) that the value for defaultScope only matters at the time you call upsert(). So, it should not interfere with other invocations (JS closures being what they are).

Though that does bring up a minor potential bug in my sample code above. When I just went back to confirm, I did actually wait to restore defaultScope until the callback was fired, by moving the restore logic inside the callback, thusly:

  var defaultScope = Report.defaultScope;
  Report.defaultScope = function(){};
  Report.upsert({id: reportId, 'deleted': true}, function(...) {
    Report.defaultScope = defaultScope;
    ...
  });

I think that's the safer way to do it, since it ensures the defaultScope isn't prematurely removed from the model in case it's copied for some reason instead of scoped within that particular call.

Hope that's helpful.

Steve

I went ahead and made that change in the docs as well.

would love to see this reopened. i can understand why the scope definition would be used in all where filters, but modifying the update portion (in addition to the where portion) is VERY unintuitive and quite a few people have been bitten by this behaviour.

dao.js
screenshot 2017-04-10 14 26 39

utils.js
screenshot 2017-04-10 14 25 32

// setScopeValuesFromWhere(data, scope.where, this);
works

Solution
server.js before boot

screenshot 2017-04-10 15 53 58

Can't loopback just make a feature to set a scope option to include or exclude specific calls

LoopBack 3 has reached end of life, we are not going to add any new features.

@bajtos Also LoopBack 4 has the same problem, the scope interferes everywhere on every crud operation
According to the loopback overview logic it shouldn't
loopback-overview

Anyway good luck, i extended to my models to another repository that extended to default crud repository, that way i could just remove the scope completely and override the field i needed on every crud operation method.

Was this page helpful?
0 / 5 - 0 ratings