zymlabs / nswag-fluentvalidation

Use FluentValidation rules instead of ComponentModel attributes to define swagger schema.
MIT License
59 stars 13 forks source link

Doesn't Handle RuleSets well #2

Open mariohines opened 4 years ago

mariohines commented 4 years ago

While the use of RuleSets is probably more on the edge, this doesn't quite support them. I'm getting 2 distinct signs of what validation is, but without the context of what RuleSet is being applied. ex.

RuleSet("On Insert", () => );
RuleSet("On Update", () =>);

Having 2 different rulesets yields showing 2 different validations for the same property without the context which is slightly confusing. I'm looking at the code to see if I can figure out where to possibly apply this change.

t-mxcom commented 3 years ago

I have the same issue - two different rule sets that are selected using the CustomizeValidatorAttribute above the controller methods parameter. In the generated swagger.json there is only one description of the type that has different validation rule sets applied, so the generator would have to create different type descriptions for every rule set. I'm not sure if this is a good way to go... !?

geoffreytran commented 3 years ago

Yes, it currently doesn't handle RuleSets at the moment. I'm trying to think how it would be implemented.

From the NSwag schema, we should be able to back into the controller/action that has the CustomizeValidatorAttribute and apply the requirements for the correct rule set.

The challenge is with the swagger spec, the validation is located on the object itself and not on the endpoint, so even if we could look at the CustomizeValidatorAttribute, if there are two actions that use the same object, the rulesets would collide for this scenario.

class FooController 
{
    [CustomizeValidatorAttribute(RuleSet="create")]
    public void CreateFooAction(FooModel foo);

    [CustomizeValidatorAttribute(RuleSet="update")]
    public void UpdateFooAction(FooModel foo);
}

class FooModel
{}

NSwag would generate an endpoint for create and update; however, the endpoints in the swagger spec would reference the same FooModel, which is where the swagger spec defines the validation requirements.

"requestBody": {
    "x-name": "command",
    "description": "Create foo command.",
    "content": {
        "application/json": {
            "schema": {
              "$ref": "#/components/schemas/FooModel"
            }
        }
    },
    "required": true,
    "x-position": 2
},
"FooModel": {
    "type": "object",
    "description": "Fooo Result",
    "additionalProperties": false,
    "required": [
        "items"
    ],
    "properties": {
        "foo": {
            "type": "string",
            "description": "Foo",
            "minLength": 1, // Validations used by both create and update actions due to object reference from action request definition
        }
},

Thoughts?

t-mxcom commented 3 years ago

Thanks, Geoffrey, for your answer!

I added two more operations to your example:

class FooController 
{
    [CustomizeValidatorAttribute(RuleSet="create")]
    public void CreateFooAction(FooModel foo);

    [CustomizeValidatorAttribute(RuleSet="update")]
    public void UpdateFooAction(FooModel foo);

    [CustomizeValidatorAttribute(RuleSet="another,default")]
    public void AnotherFooAction(FooModel foo);

    [CustomizeValidatorAttribute(RuleSet="*")]
    public void UltimateFooAction(FooModel foo);
}

class FooModel
{}

I think it's necessary to generate multiple models/types to support different rule sets.

Idea 1 Append the type name with the rule set name(s) This could be FooModel_Create and FooModel_Update to cover CreateFooAction and UpdateFooAction. Both containing the same members but with different validation information assigned. It also is possible to specify more than one rule set in CustomizeValidatorAttribute by delimiting the names (including default) with a comma (see AnotherFooAction with will result in FooModel_AnotherDefault) or even applying all rule sets by typing * (see UltimateFooAction which will result in FooModel_All).

Idea 2 Define a type for every operation that needs it This will create CreateFooAction_FooModel, UpdateFooAction_FooModel, AnotherFooAction_FooModel and UltimateFooAction_FooModel in this case if we assume the type name is simply prefixed with the operations name.

Resume In my opinion, Idea 1 could keep the number of types defined as low as possible, as operations using the same rule set(s) could share the same type. But if multiple rule sets are specified, it will need a strategy to detect equally validated types (for example if the CustomizeValidatorAttribute is applied having the rule set names specified in different order) and using all specified rule set names to build the type names could lead to quite long names. Idea 2 will help to create clearly and understandable named types but just defining a new type for each operation would result in a lot of equal type definitions.

So will a combination of both be the best solution? What do you think about it?

Greetings from Austria! Max