Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Epic: Validation at Model/ORM level #1872

Closed
David-Mulder opened this issue Oct 18, 2018 · 13 comments
Closed

Epic: Validation at Model/ORM level #1872

David-Mulder opened this issue Oct 18, 2018 · 13 comments
Labels
epic Repository Issues related to @loopback/repository package stale Validation

Comments

@David-Mulder
Copy link
Contributor

Feel a bit guilty about asking this like this, but considering this is currently "released", how are we supposed to do validation?

As far as I can tell the model doesn't support any validation (per #1624) and frankly I am at a loss at how to use this without validation? Of course you could manually do the validation in the controller, but that destroys the point of an ORM 😅 . What am I missing?

Or is this not yet supposed to be used for CRUD API's?

@bajtos bajtos changed the title How are we supposed to implement validation at the moment? Validation at Model/ORM level Oct 30, 2018
@bajtos
Copy link
Member

bajtos commented Oct 30, 2018

@David-Mulder could you be more specific please? What kind of validations do you have in mind?

I acknowledge that the current set of validations offered by the data-access layer of LB4 is way too small. In LoopBack 3.x, it was possible to use one of the few built-in validation rules like "validatesPresenceOf", "validatesLengthOf", etc. In our experience, these rules were not sufficient enough and were difficult to use & extend anyways.

Let's discuss the use cases and requirements for a better validation in LB4.

@bajtos bajtos added epic Repository Issues related to @loopback/repository package and removed question labels Oct 30, 2018
@bajtos bajtos changed the title Validation at Model/ORM level Epic: Validation at Model/ORM level Oct 30, 2018
@bajtos bajtos added the TOB label Oct 30, 2018
@bajtos
Copy link
Member

bajtos commented Oct 30, 2018

(FWIW, the discussion in #1624 has some interesting bits to be carried over to this epic.)

@bajtos
Copy link
Member

bajtos commented Nov 1, 2018

I was thinking about ORM-level validations in the past few days and want to post few ideas and opinions.

  • I feel it's critically important do design these validations in such way that extensions can easily contribute custom validations.
  • I think it's also very important to make validations robust and free of race conditions. For certain validations like uniqueness constraint and foreign keys, only the database can enforce them in a reliable way and LB4 should make it possible to write such validations.
  • The design of our validation framework should allow us to perform partial validations too. For example, when PATCH /customers/123 wants to change the phone field only, then we should be able to run only validations for the phone property and skip other validations (e.g. email is required and must be a valid email address).

I am thinking about the following high-level design:

  1. A model-level mixin to add validation-related APIs:

    • Register a validation step. Few examples of a validation steps: check that property email is a valid email address; check that endDate is always greater than startDate.
    • Run all validation steps. (Optionally, perform a partial validation.)
    • Describe the validation steps as constraints that connectors can use to define database schema. For example, a string property with the constraint maxLength: 50 can be mapped to NVARCHAR(50). A validation rule "end date must be greater or equal than start date" can be mapped into a CHECK constraint CHECK (endDate >= startDate).
    • Describe the validation steps as JSON schema constraints.
  2. Each validation rule is implemented as a decorator, under the hood this decorator calls validation APIs provided by the mixin above to define a new validation step. By using decorators, we put app developers in control of how to discover validation rules and which version of validation rule to use. There is no need for a central registry of all validation rules. (We may need to introduce such registry later, to support declarative declaration of validation steps in JSON files - see Declarative Support #565 - but let's not worry about that right now.)

    Example usage:

    @check(
      // which properties are required to perform this check
      // this allows the validation framework to skip this rule
      // when doing a partial update that's not touching these properties
      ['startDate', 'endDate'],
    
      // the validation check
      data => data.endDate >= data.startDate,
    )
    class Event {
      @unique()
      @minLength(5)
      @maxLength(20)
      @property()
      urlSlug: string;
    }
  3. Repository implementations are expected to call validation APIs as part of the data-access operations. The repositories provided by LoopBack (DefaultCrudRepository, DefaultKeyValueRepository) should implement validation out of the box. This is based on the following assumption:

  4. All Entity classes should come with the validation mixin applied. For example, if the mixin is called Validated, then the Entity class should be defined as class Entity extends Validated(Model). At the same time, the underlying Model class should not come with validation baked in, to allow app developers to define light-weight model classes used as a type of nested properties and also to describe input parameters and return values of remote (controller) methods.

Additional ideas/requirements:

  • All validation checks should be asynchronous (returning a promise) by default. To improve performance, we can introduce support for synchronous validation checks too: if a validation check returns a thenable instance, then we await it, otherwise we treat the returned value as the validation outcome. (We already follow this approach in @loopback/context when resolving dependencies.)

  • When a model instance (data) is not valid, the validation error should return all validation errors and include machine-readable details that will allow the clients to understand which property values are invalid (to render validation errors next to appropriate UI input elements) and offer the user localized error messages in their language.

  • For validations that can be provided by the database only, e.g. uniqueness constraint or foreign keys, the framework should check at app startup and/or before the first database command is executed whether the database schema is configured to enforce these constraints.

  • To be investigated: how to support auto-discovery of constraints, i.e. map database-specific constraints like NVARCHAR(50) or CHECK(start <= end) into LB4 validations.

@raymondfeng @strongloop/loopback-maintainers what's your opinion? Do you agree with my proposal? Is there anything you would like to add?

@KalleV
Copy link
Contributor

KalleV commented Jun 19, 2019

I would like to add a related issue to the mix. Performing a CRUD operation using a relation is also unnecessarily throwing validation errors.

For example: myRepo.users(tenantId).create(new User(...)) fails unless "tenantId" is explicitly passed in the request body even though the relation automatically attaches the "tenantId" property. This is a change in behavior from LB3 where I could set "tenantId" as a required property and it would pass validation in the above scenario.

@bajtos
Copy link
Member

bajtos commented Jun 21, 2019

myRepo.users(tenantId).create(new User(...)) fails unless "tenantId" is explicitly passed in the request body even though the relation automatically attaches the "tenantId" property

Good catch! I think that we should modify the controller to remove the foreign-key property from the OpenAPI schema describing the input parameter. Let's make this improvement a part of #2653.

@mastermunj
Copy link
Contributor

Hi @bajtos, any visibility on when the model validation will be available?

Or maybe you can point me towards an interim solution to achieve custom business validation, similar to that of LB3 Model.validateAsync which is applied every time the model is created/updated.

@mastermunj
Copy link
Contributor

Sorry to bump in again, however, it's important to have model validation. Is this being planned for the near future? If not, kindly suggest an interim approach to do custom business logic validation at the model level.

@dhmlau
Copy link
Member

dhmlau commented Feb 13, 2020

I think one of the ways is to use interceptor. You can create a class level interceptor for the controller so that all requests need to go through your validation before further processing. I have a sample app in this blog post: https://strongloop.com/strongblog/loopback4-interceptors-part2/

@mastermunj
Copy link
Contributor

Controller level validators work only for requests, however, model level validations work for every data-access on the model.

As of now, I am thinking to use ajv in the repository for doing custom async validation for data-access methods of models.

@bajtos
Copy link
Member

bajtos commented Jul 17, 2020

Yesterday in a Maintainers Call, @raymondfeng proposed to refactor the current AJV-based validation layer in REST to make it more flexible and allow us to use it at ORM level too.

AJV validation can be extended via custom keywords. That's a pretty nice programming model because it allows you to extend JSON schema with additional constraints. Works for instance-level validation, but not for collection-level (uniqueness).
We can use decorators to hook up additional validators at property or model level. Validation service can leverage this metadata.
Validation deserves first-class attention, it's one of the reasons why people choose a framework (not just Express routes). We should do a spike on a validation service, refactor validation out of REST. Use AJV as the default, but allow users to use joi or other custom validator instead.

@mitsos1os
Copy link

@bajtos Do you believe that property value validation that adheres to some business logic is also valid for ORM level validation ?
For example in LB3 you could set a custom validation function that would not allow a relation field to be set in a model if the target model had some specific content.

This required some async operations (querying the database) and also provided a proper invalid value message with 422 status code.

I found this really useful and we used it extensively in our implentations, because we could easily provide feedback to the users by using the ValidationError format from loopback.

What are your thoughts on that?

Yesterday in a Maintainers Call, @raymondfeng proposed to refactor the current AJV-based validation layer in REST to make it more flexible and allow us to use it at ORM level too.

AJV validation can be extended via custom keywords. That's a pretty nice programming model because it allows you to extend JSON schema with additional constraints. Works for instance-level validation, but not for collection-level (uniqueness).
We can use decorators to hook up additional validators at property or model level. Validation service can leverage this metadata.
Validation deserves first-class attention, it's one of the reasons why people choose a framework (not just Express routes). We should do a spike on a validation service, refactor validation out of REST. Use AJV as the default, but allow users to use joi or other custom validator instead.

@stale
Copy link

stale bot commented Jul 14, 2021

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 stale bot added the stale label Jul 14, 2021
@stale
Copy link

stale bot commented Aug 13, 2021

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.

@stale stale bot closed this as completed Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Repository Issues related to @loopback/repository package stale Validation
Projects
None yet
Development

No branches or pull requests

6 participants