microtweak / conditional-validator

An extension package for Bean Validation 2.0 that adds conditional validations
MIT License
6 stars 1 forks source link

Support of @ValidWhen annotation #4

Closed Djaytan closed 1 year ago

Djaytan commented 1 year ago

Hello,

First of all thanks for the initiative with this project! It's clearly useful for corner cases not supported (at least yet) by Beans Validator API.

Unfortunately, I don't see any way to use "@ValidWhen" logic. Is it a limitation of the solution? Otherwise, would it be possible to add it?

I would be happy to help you with this new change if necessary

salvadormarcos commented 1 year ago

Hi,

Thanks for the feedback. I'm happy that my project is useful to other developers.

To be honest, when I developed the project I didn't think about the possibilities of an @ValidWhen logic. At that time I only thought about enabling/disabling the BV constraints if a boolean expression was true.

Unfortunately, I believe it is not possible to add due to the way @Valid works. This annotation is not a BV constraint. It only serves to mark a field that must be validated recursively.

Of course, I may not be seeing other implementation possibilities. So feel free to look into other approaches. If u get something promising, we can add to the project.

By the way, I recently started an internal project refactoring (branch fix-class-not-found) to reduce the use of HV and BVal internal APIs. My project broke whenever some HV internal API was changed. I'm working on using only the APIs from the Bean Validation specification. If you wish to make a contribution, please refrain from using the HV and BVal internal APIs.

salvadormarcos commented 1 year ago

Briefly, my project works as follows: 1) The only constraint in my project is @ConditionalValidate and it is validated by DelegatedConditionalConstraintValidator. This validator looks for all fields annotated with some @*When and invokes the corresponding ConstraintValidatorimplementation for the validated type. For example: private @NotEmptyWhen String name; will look for and call the existing ConstraintValidator<NotEmpty, String> or ConstraintValidator<NotEmpty, CharSequence> in Hibernate Validator or Apache BVal;

2) All @*When constraints annotations are not actual BV constraints. When the DelegatedConditionalConstraintValidator is invoked, my project instantiates a proxy of the actual BV annotation. As mentioned before, I get the appropriate ConstraintValidator and call the initialize() method providing the proxy. With this, I can reuse the ConstraintsValidators from HV or BVal;

3) When BV detects a field annotated with @Valid, it starts a new validation with the object that is in that field. If this other object is annotated with @ConditionalValidate, we're back to point 1;

Djaytan commented 1 year ago

Thanks for the details.

I was thinking about a potential solution but in fact I don't see simple way to solve the problem except one.

Let's take the following class as an illustration:

@ConfigSerializable
@Getter
@EqualsAndHashCode(callSuper = true)
@ToString(callSuper = true)
@NoArgsConstructor
@AllArgsConstructor(staticName = "of")
public final class DataSourceValidatingProperties
    extends ValidatingConvertibleProperties<DataSourceProperties> {

  @NotNull
  @Required
  private DataSourceType type;
  @NotBlank
  @Size(max = 128)
  @Required
  private String table;
  @NotNull
  @Valid
  @Required
  private DbmsServerValidatingProperties dbmsServer;
  @NotNull
  @Valid
  @Required
  private ConnectionPoolValidatingProperties connectionPool;

  @Override
  protected @NonNull DataSourceProperties convertValidated() {
    DbmsServerProperties dbmsServerProperties = dbmsServer.convertValidated();
    ConnectionPoolProperties connectionPoolProperties = connectionPool.convertValidated();
    return DataSourceProperties.of(type, table, dbmsServerProperties, connectionPoolProperties);
  }
}

What I would like to do here is to validate the dbmsServer field only if the type match an elligible type. An SQLite database doesn't need properties about DBMS server.

About what you explained to me and about what I understood from the BV and your project, a possible solution that I see would be to add @NotNullWhen annotation for the dbmsServer field... But I don't know the result: is the @Valid annotation doesn't applies if object is null? Or does it imply implicitly that object mustn't be null leading that @NotNull is redundant in my example and thus generating a constraint violation. Logically, I would say that the @Valid will not conflict with `@NotNull, but I can't give any confirmation since it seems to be a weird case.

If there is a potential solution, this is here imho. What do you think?

Djaytan commented 1 year ago

I have done the following test:

{
      // Given
      DataSourceValidatingProperties dataSourceValidatingProperties =
          DataSourceValidatingProperties.of(DataSourceType.SQLITE, "patch_place_break",
              DbmsServerValidatingProperties.of(
                  DbmsHostValidatingProperties.of("example.com", 1234, true),
                  CredentialsValidatingProperties.of("", "bar"), "patch_database"),
              ConnectionPoolValidatingProperties.of(60000, 10));

      // When
      Set<ConstraintViolation<DataSourceValidatingProperties>> constraintViolations =
          ValidatorTestWrapper.validate(dataSourceValidatingProperties);

      // Then
      assertThat(constraintViolations).isEmpty();
}

by putting the tag @NotNullWhen on the dbmsServer field:

  @NotNullWhen(expression = "type.equals(DataSourceType.MYSQL)")
  @Valid
  @Required
  private DbmsServerValidatingProperties dbmsServer;

And @ConditionalValidate on the class.

And it works when setting the dbmsField to null

Djaytan commented 1 year ago

Anyway it doesn't solve my issue finally.

I clearly need the @ValidWhen annotation even if theorically I get reach my goal without it.

salvadormarcos commented 1 year ago

According to the scenario you illustrated and the tests you did, @NotNullWhen worked but didn't fully meet the needs. Am I correct or did I misunderstand?

Could you give me more details why this approach doesn't completely solve your problem?

Djaytan commented 1 year ago

Finally I realized that in my case I can make the class mutable and use setter to make the dbmsServer field null. This is the only way I have since the whole data structure is generated from a deserialization process.

But ideally it would be possible to achieve the same thing with an immutable class

salvadormarcos commented 1 year ago

I think I now understand your need. You want "dbmsServer" to be null when in one situation and not null in another, correct?

Try using the following way:

@NullWhen(expression = "type.equals(DataSourceType.SQLITE)")
@NotNullWhen(expression = "type.equals(DataSourceType.MYSQL)")
@Valid
@Required
private DbmsServerValidatingProperties dbmsServer;

You can put two conditional validations with opposite boolean expressions

salvadormarcos commented 1 year ago

In Brazil, we have a similar situation with CPF (for people) and CNPJ (for organizations) documents. These two documents have their own formats and rules for validation.

It is very common for systems to use a customer record that accepts people and companies, including using the same field to store these documents. In fact, this situation was one of the motivations for creating this project.

I worked on a project we had something like this:

public class Cliente {

    @Valid
    @NotNull
    private InscricaoFederal inscricaoFederal

}

public class InscricaoFederal {

   @NotNull
   private TipoPessoa tipo;

   @NotBlank
   @CNPJWhen(expression = "tipo.equals(TipoPessoa.JURIDICA)") // When the enum indicates an organization
   @CPFWhen(expression = "tipo.equals(TipoPessoa.FISICA)")    // And when to indicates a person
   private String numero;

}
Djaytan commented 1 year ago

Alright I see!

It shows me several possibilities about using your library. I will definitively opt to your solution like in your last example. This is exactly my need! You make me happy :D

Thanks a lot for your time :)

salvadormarcos commented 1 year ago

That's great news. I'm glad I helped!

Can I close this issue?

Djaytan commented 1 year ago

Yeah sure :)