DataSource juggler is adding the model property with its default value in response

[sharathmuthu6]

Description/Steps to reproduce

I created a simple loopback application with model name testDefault with its model defintion as below :

{
  "name": "testDefault",
  "base": "PersistedModel",
  "idInjection": true,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "name": {
      "type": "string"
    },
    "age": {
      "type": "number",
      "default": 12
    }
  },
  "validations": [],
  "relations": {},
  "acls": [],
  "methods": {}
}

DataSource of testDefault is as follows :

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

Since it is an in-memory connector, i changed the .all method of memory connector (https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/connectors/memory.js#L504)
to return callback(null,[{"name":"john"}])

Once i start the app and do a GET on /testDefaults, i am getting the following response :

[
  {
    "name": "john",
    "age": 12
  }
]

Link to reproduction sandbox

Expected result

My connector response(from endpoint) doesn't have age property, so i am expecting an output as below

[
  {
    "name": "john"
  }
]

In dao.js, https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/dao.js#L1637
A new model is being created, thats where i could see that

ModelConstructor {
  __cachedRelations: {},
  __data: { name: 'john', age: 12 },
  __dataSource: undefined,
  __strict: false,
  __persisted: true }

age property is getting added.

Additional information

Task list

• Fix in master (4.x): fix: do not apply default values on data from database #1704, Improve tests for empty vs. default values #1707
• Fix in 3.x: Do not apply default values on data from database [3.x] #1705
• Fix in 2.x: Do not apply default values on data from database [2.x] #1706

[hacksparrow]

@sharathmuthu6 you are modifying the source code and expecting it to behave in a certain way, which is not a valid issue in terms of the user API.

I can help you achieve what you want to achieve, if you can describe your requirements with the use of the API exposed to the user.

[sharathmuthu6]

I have added the testcase in loopback-sandbox repo
https://github.com/sharathmuthu6/loopback-sandbox/tree/issue1692

[hacksparrow]

Hi @sharathmuthu6 you are overwriting the behavior of the connector with your own implementation.

Memory.prototype.all = function all(model, data, options, callback) {
  callback(null, [{'name': 'john'}]);
};

This code will have many other side effects, by the way.

If you really intent to re-write the memory connector to your liking, you should go all the way and modifying everything to suite your needs and make it work for you. We can't help you with that.

If you want to make the age property optional (since the response does not have the age property) just change the age property in the model definition file from:

"age": {
  "type": "number",
  "required": true,
  "default": 12
}

to:

"age": {
  "type": "number"
}

[bajtos]

@sharathmuthu6 IIUC, you would like to define a model property that is filled with a default value when the model data is coming from the user (e.g. from incoming HTTP request body), but that is not applied to data coming from your backend service/database via connector.

First of all, I think the current behavior is intentional. Even if it was not, we cannot easily change it - it would be a huge breaking change for existing LB applications.

I don't have bandwidth to debug through our codebase, but I suspect the problem is caused by the following.

• DataAccessObject invokes _initProperties to set model properties from the data returned by the connector. Some places call the method directly, e.g. here and here); other places call model constructor (which invokes _initProperties under the hood), see e.g. here for find method: https://github.com/strongloop/loopback-datasource-juggler/blob/70b3bb6aae8e93536076ff5ca6f742c5fb19bdda/lib/dao.js#L1629-L1640

• _initProperties are accepting different flag, one of them is called applyDefaultValues. This flag is set to true by the model constructor. https://github.com/strongloop/loopback-datasource-juggler/blob/bd5fd07d270084f8cc3d0cce1470431708f9b72a/lib/model.js#L59-L61

• When applyDefaultValues is set to true, known model properties with no value are set to their default value. https://github.com/strongloop/loopback-datasource-juggler/blob/bd5fd07d270084f8cc3d0cce1470431708f9b72a/lib/model.js#L280-L282 https://github.com/strongloop/loopback-datasource-juggler/blob/bd5fd07d270084f8cc3d0cce1470431708f9b72a/lib/model.js#L302-L304

[bajtos]

@sharathmuthu6 I think @hacksparrow may be correct that the problem is in your model definition.

First of all, how can the backend datasource return model instances with a missing age property, when this property is required? That looks like a violation of schema constraints to me. IMO, if there are records with no age property, then the property must not be defined as required.

As for applying the default value. I see how the current behavior, where the default value is applied both on data coming from the clients but also on the data coming from the database, can be problematic. As I am thinking about it, it would seem more natural to me if the default value was applied on the data coming from the client only (when creating new records).

To preserve backwards compatibility, the new behavior must be implemented via a new feature flag that's disabled by default. I think that implementation wise, the easiest option is to introduce the flag in model settings:

{
  "name": "MyModel",
  "properties": {
    "age": {
      "type": "number",
      "required": false,
      "default": 42,
    },
    "applyDefaultsOnReads": false,
  }
}

Example implementation in find - we will need to update all DAO operations accordingly:

        function buildResult(data, callback) {
          const ctorOpts = {
            fields: query.fields,
            applySetters: false,
            persisted: true,
          };
          if (Model.settings. applyDefaultsOnReads === false) {
            ctorOpts.applyDefaultValues = false;
          }
          let obj;
          try {
            obj = new Model(data, ctorOpts);
          } catch (err) {
            return callback(err);
          }

Thoughts?

[bajtos]

As a temporary workaround, can you try to rework your model as follows?

• Remove default flag
• Implement a before save hook to add default value when needed and wanted. (It may be possible to do that via beforeRemote too.)

[bajtos]

Possibly related - notice the commit is more than 4 years old:

• 342bc2f
• default values are being overwritten in upsert #616

[jannyHou]

I mostly agree with @hacksparrow 's comment in #1692 (comment), and still trying to understand what is the problem @sharathmuthu6 has...

[jannyHou]

If your model property has a default value, e.g. age is default to 6, then your created model instance(stored in db) always has value assigned to age.

If you want to exclude certain properties(fields) from the GET endpoints, you can use filter, see https://loopback.io/doc/en/lb3/Fields-filter.html

[bajtos]

We have further discussed this issue with Nagarjuna (@snarjuna). In their project, they are using PersistedModel to access a datasource that's not a database. As a result, the model is describing values that are not always returned back by the connector.

While this may seem like they are using PersistedModel incorrectly, there are actually use cases where LoopBack's current behavior is problematic for databases too:

• Let's say we have an existing table with some columns and applications that are already using this table.

• Now we add a new column with the following properties:

  • Existing records are updated to set the new column to NULL (or undefined).
  • In LB application, we want to set the new colum to a default value when no value was provided by the client. This is achieved by using default metadata in property definition.

• When we run the updated application and query the database, the new default value is incorrectly applied on old records.

I'll go ahead and implement a solution based on #1692 (comment) and #1692 (comment)

[bajtos]

Proposed fix: #1704

[bajtos]

Done.

The fix is available in the following juggler versions:

• [email protected]
• [email protected]
• [email protected]