loopback-next icon indicating copy to clipboard operation
loopback-next copied to clipboard

updateById is not triggering DB validaiton

Open patelmayankce opened this issue 4 years ago • 5 comments

Steps to reproduce

Current Behavior

My Field configuration

@property({
    type: 'string',
    required: true,
    jsonSchema: {
      minLength: 2,
      transform: ['trim'],
    },
  })
  name: string;

this.repository.create({ name: ' ' }) = > This throw an error which is good

this.repository.updateById(id, { name: ' ' }) = > This function directly bypass the validation configured in model

Expected Behavior

this.repository.updateById(id, { name: ' ' }) = > This method call should throw an error based on model configuration.

Additional information

Commands

  1. node -e 'console.log(process.platform, process.arch, process.versions.node)'
  2. npm ls --prod --depth 0 | grep loopback

Output

  1. darwin x64 12.16.1

├── @loopback/[email protected] ├── @loopback/[email protected] ├── @loopback/[email protected] ├── @loopback/[email protected] ├── @loopback/[email protected] ├── @loopback/[email protected] ├── @loopback/[email protected] ├── @loopback/[email protected] ├── @loopback/[email protected] ├── [email protected] ├── [email protected]

Related Issues

See Reporting Issues for more tips on writing good issues

patelmayankce avatar Apr 14 '21 11:04 patelmayankce

AFAIK, jsonSchema is only honoured by at REST layer by AJV; Connectors will only honour connector-specific fields (i.e. mongodb: {}) or root-level fields (i.e. id: true).

I don't have a public record on that, but quoting from Slack thread:

IMO, connectors should not take into account jsonSchema and should provide their own fields to allow users to tweak the definition for the particular database. Once a custom field is supported by a large number of different components, we may want to promote it into a shared/generic field defined at root level.

So errors occurring in the create() function may be from the database engine itself instead of LoopBack 4's jsonSchema validation.

This method call should throw an error based on model configuration.

As per above, jsonSchema is, AFAIK, only passed to AJV for REST-level validation. Any connector-level validation need be within the connector-specific field (i.e. mongodb) or at the root level (i.e. outside jsonSchema).

This throw an error which is good

Could you provide the error? This would help confirm/disprove the above statement.

achrinza avatar Apr 15 '21 14:04 achrinza

Thanks for quick answer @achrinza , Appreciated!!

{
  "error": {
    "statusCode": 422,
    "name": "ValidationError",
    "message": "The `OrgHasFields` instance is not valid. Details: `key` can't be blank (value: \"\"); `name` can't be blank (value: \"\").",
    "details": {
      "context": "OrgHasFields",
      "codes": {
        "key": [
          "presence"
        ],
        "name": [
          "presence"
        ]
      },
      "messages": {
        "key": [
          "can't be blank"
        ],
        "name": [
          "can't be blank"
        ]
      }
    }
  }
}

Below is my API request body:

@requestBody({
      content: {
        'application/json': {
          schema: {
            type: 'object',
            properties: {
              name: {
                require: true,
                type: 'string',
                minLength: 2,
                maxLength: 500,
                transform: ['trim'],
              },
            },
          },
        },
      },
    })

Let me know if you need any more details

Thanks again!!

patelmayankce avatar Apr 15 '21 16:04 patelmayankce

This issue has been marked stale because it has not seen activity within six months. 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. This issue will be closed within 30 days of being stale.

stale[bot] avatar Oct 12 '21 17:10 stale[bot]

Hi @achrinza ,

Is there any plan to fix this issue? IMO this is a bug from loopback-datasource-juggler

Juggler validation which happens after AJV is enabled for Create and replaceById. create: https://github.com/loopbackio/loopback-datasource-juggler/blob/master/lib/dao.js#L371 replaceById: https://github.com/loopbackio/loopback-datasource-juggler/blob/master/lib/dao.js#L2926

In the case of replaceById it is true by default and then changed as per the model settings.

However in the case of updateAll which is used by LB updateById: https://github.com/loopbackio/loopback-datasource-juggler/blob/master/lib/dao.js#L2517

doValidate set to false by default and can be enabled by only setting it to true manually in each model setting.

If I am not wrong it should be set to true by default.

PS: I will be happy to contribute to the fix if approved

BR, Harshal

harshal619 avatar Aug 25 '23 13:08 harshal619

Just ran into the same error. It seems like some part of the validation is not triggered when passing an Object to updateById(), but it is triggered by create().

Example of Model:

import { Entity, model, property } from '@loopback/repository';

@model()
export class TestModel extends Entity {
  @property({
    type: 'number',
    id: true,
    generated: true
  })
  id?: number;

  @property({
    type: 'string',
    required: true
  })
  name: string;

  @property({
    type: 'string',
    required: true,
  })
  value: string;

  constructor(data?: Partial<TestModel >) {
    super(data);
  }
}

Some example code (only the relevant lines):

const testObject = new TestModel({name "test", value: ""});

// This works
await repo.updateById(id, testObject);

// This throws an error as mentioned in https://github.com/loopbackio/loopback-next/issues/7390#issuecomment-820559138
await repo.create(testObject);

Is there a fix in sight?

yiev avatar Feb 01 '24 11:02 yiev