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

Doesn't Handle RuleSets well #2

Open
mariohines opened this issue Nov 13, 2019 · 3 comments
Open

Doesn't Handle RuleSets well #2

mariohines opened this issue Nov 13, 2019 · 3 comments

Comments

@mariohines
Copy link

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
Copy link

t-mxcom commented Feb 12, 2021

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
Copy link
Member

geoffreytran commented Feb 12, 2021

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
Copy link

t-mxcom commented Feb 15, 2021

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

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