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

Debugging Validation Rules #10

Open
andrewwilkin opened this issue Jun 12, 2021 · 7 comments
Open

Debugging Validation Rules #10

andrewwilkin opened this issue Jun 12, 2021 · 7 comments

Comments

@andrewwilkin
Copy link

I've followed the steps as outlined and for some reason I'm not seeing the schema for the object change in the UI. I've confirmed that the validation is indeed being applied.

    public class CreateProfileRequestValidator : AbstractValidator<CreateProfileRequest>
    {
        // ToDo: Case for custom validation here?
        public CreateProfileRequestValidator()
        {
            RuleFor(c => c.FirstName).NotNull().NotEmpty();
            RuleFor(c => c.LastName).NotNull().NotEmpty();

            RuleFor(c => c.IndustryId).NotEmpty();
            RuleFor(c => c.JobTitleId).NotEmpty();

            RuleFor(c => c.CountryId).NotEmpty();
            RuleFor(c => c.City).NotNull().NotEmpty();
            RuleFor(c => c.TimezoneId).NotNull().NotEmpty();
        }
    }

What's the best way of debugging the FluentValidationSchemaProcessor, and seeing that it picked up the validation rule? Have my LogLevel set to debug, and not seeing anything in the output.

Thanks.

@andrewwilkin
Copy link
Author

The reason why this is not working is that the CreateProfileRequest inherits from a base class and IsObject and Properties don't work in that case.

I'm not looking to do this as would have lots:
https://github.com/RicoSuter/NJsonSchema/wiki/Inheritance

Would say the FluentValidationSchemaProcessor needs to cater for inheritance. Thoughts?

@geoffreytran
Copy link
Member

Hi Andrew,

If you set the following setting to settings.FlattenInheritanceHierarchy = true; for the NSwag config, it should include inherited fields as a workaround, but it will not show the base classes in the swagger doc. By default NJson schema includes it into AllOf according to the docs, but I'm not sure I see where it's actually putting it yet.

            serviceCollection.AddOpenApiDocument(
                (settings, serviceProvider) =>
                {
                    var fluentValidationSchemaProcessor = serviceProvider.CreateScope().ServiceProvider.GetService<FluentValidationSchemaProcessor>();
                    settings.SchemaProcessors.Add(fluentValidationSchemaProcessor);
                    
                    settings.SchemaProcessors.Add(new RequiredSchemaProcessor());
                    settings.FlattenInheritanceHierarchy = true; // This setting
               }

@geoffreytran
Copy link
Member

It looks like NJsonSchema is correctly generating the AllOf references; however, modifying the FluentValidation schema processor to add the validation rules to the base class in the child class validator would not be a good idea. The reason for this is the validation rule for a property in the base class could conflict with other rules in the base class for other OpenAPI endpoints.

@andrewwilkin
Copy link
Author

It's fairly common to use an abstract base class, and the flatten hierarchy would work fine for me in that situation. Though may be able to achieve similar with composition (e.g. having pagination and other properties available on list endpoints).

I think if you had multiple endpoints the validation would not conflict by having it on the base class, the calls are independent of one another and yet some implementation is common 🤔

@geoffreytran
Copy link
Member

Yes, I agree, inheritance is a fairly common use case to support without the flattened hierarchy. I'm trying to find a solution that would work with the OpenAPI spec and FluentValidation. Let me know if you have any ideas.

https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/
https://docs.fluentvalidation.net/en/latest/inheritance.html

In the case where you have the following models:

class Pet 
{
    public string Name { get; set; }
    public string Breed { get; set; }
}

class Dog : Pet
{

}

class Cat : Pet
{
}
    public class DogValidator : AbstractValidator<Dog>
    {
        public DogValidator()
        {
            RuleFor(c => c.Breed).MustBeDogBreed();
        }
    }

    public class CatValidator : AbstractValidator<Dog>
    {
        public DogValidator()
        {
            RuleFor(c => c.Breed).MustBeCatBreed();
        }
    }
paths:
  /pets:
    patch:
      requestBody:
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/Cat'
                - $ref: '#/components/schemas/Dog'
              discriminator:
                propertyName: pet_type
      responses:
        '200':
          description: Updated
components:
  schemas:
    Pet:
      type: object
      required:
        - pet_type
      properties:
        pet_type:
          type: string
      discriminator:
        propertyName: pet_type
    Dog:     # "Dog" is a value for the pet_type property (the discriminator value)
      allOf: # Combines the main `Pet` schema with `Dog`-specific properties 
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Dog`
          properties:
            bark:
              type: boolean
            breed:
              type: string
              enum: [Dingo, Husky, Retriever, Shepherd]
    Cat:     # "Cat" is a value for the pet_type property (the discriminator value)
      allOf: # Combines the main `Pet` schema with `Cat`-specific properties 
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Cat`
          properties:
            hunts:
              type: boolean
            age:
              type: integer

If you specify a validator on Cat and Dog for the parent Pet, we run into an issue with the OpenAPI spec on how to define the requirement on the Pet property for instance since Pet is shared by both Cat and Dog.

@andrewwilkin
Copy link
Author

Will loop back when I get the chance to take a proper look, think I understand the challenge.

@fretje
Copy link

fretje commented Nov 29, 2024

See #18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants