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

Add test ACustomerMessageTest (shows Size min not defaulting) #32

Merged
merged 3 commits into from
May 21, 2023

Conversation

rbygrave
Copy link
Contributor

@rbygrave rbygrave commented May 21, 2023

Size min expected to default to 0 according to the error messages we have.

So given:

    @NotBlank @Size(max = 5)
    final String name;

Generates:

    this.nameValidationAdapter = 
        ctx.<String>adapter(Size.class, Map.of("max",5))
            .andThen(ctx.adapter(NotBlank.class,Map.of()));

And we can get the validation message where the {min} is not replaced (with say 0 or indeed 1):

size must be between {min} and 5

The default value for Size min is 0, so we reasonably expect the {min} to be replaced by 0.

I note that we know in this case that there is also the @NotBlank and technically that means that the Size min should really be 1 (because we know with the NotBlank that 0 isn't a valid minimum number of chars).

Perhaps ... we could ensure the ordering such that NotBlank, NotEmpty, NotNull are higher precedence and the generated code always puts those first and IF that happened and the context knew that a NotBlank proceeded the Size THEN ... it could deduce that the default min should be 1.

Regardless it looks like the way SizeAdapter is created needs adjustment to allow for the min defaulting to 0.

@SentryMan
Copy link
Collaborator

yeah, with the exception of Pattern (since its values are accessed via prisms), there are no default values added to the attribute map.

@rob-bygrave rob-bygrave merged commit bfe5955 into main May 21, 2023
@rob-bygrave rob-bygrave deleted the feature/messageNotNull branch May 21, 2023 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants