mtedone / podam

PODAM - POjo DAta Mocker
https://mtedone.github.io/podam
MIT License
323 stars 750 forks source link

PODAM does not populate fields when constraint annotations not from javax.validation.constraints package present #189

Closed ignaciolarranaga closed 8 years ago

ignaciolarranaga commented 8 years ago

For example, a POJO with an string an a validation NotEmpty is not populated. A short field with a @Min annotation is neither populated.

mtedone commented 8 years ago

Podam does not honour all Hibernate validation annotations. I'm flagging this for future consideration unless, of course, you want to implement the remaining changes.

daivanov commented 8 years ago

org.hibernate.validator.constraints.NotEmpty is indeed not supported, but javax.validation.constraints.Min is supported for short type.

daivanov commented 8 years ago

In general Podam cannot support all imaginable validation annotations. If you face such need you always can extend DataProviderStrategy with adding custom AttributeStrategy implementing what you need.

maxdidato commented 7 years ago

Hello, I am facing the same problem. What I think @ignaciolarranaga was saying is that every time podam find a org.hibernate.validator.constraints annotation (or maybe any other type of annotation) it just ignore the attribute. So, for example, the following class

public class Test {

    @PodamStringValue(length = 3)
    @Length(min = 3, max = 3)
    String varA;

    @PodamStringValue(length = 3)
    String varB;
}

Will result in generating an object with only varB populated. varA will be ignored I think because the Hibernate annotation clash

daivanov commented 7 years ago

Can you use javax.validation.constraints.Size instead of I guess org.hibernate.validator.constraints.Length?

maxdidato commented 7 years ago

Hello. I think I could try but what I would like to understand is this part of code

if (annotation.annotationType().getAnnotation(Constraint.class) != null) {
    if (annotation instanceof NotNull ||
                    annotation.annotationType().getName().equals("org.hibernate.validator.constraints.NotEmpty")) {
            /* We don't need to do anything for NotNull constraint */
            iter.remove();
    } else if (!NotNull.class.getPackage().equals(annotationClass.getPackage())) {
        LOG.warn("Please, register AttributeStratergy for custom "
                + "constraint {}, in DataProviderStrategy! Value "
                + "will be left to null", annotation);
    }
} else {
    iter.remove();
}

It seems that any Constraint annotation used on attribute level will force the user to provide a specific AttributeStrategy or the attribute itself would be left empty. Is there a reason for that? As podam doesn't support any validation annotation (apart for Email as I could see), would not make sense removing the else if branch completely?

I was preparing a pull request with a failing test and I could easily fix the issue removing the else if branch. But I could not figure out the reason for it

daivanov commented 7 years ago

Hi!

Podam supports all constraint annotations from package javax.validation.constraints with notable exception of a javax.validation.constraints.Pattern. Also, as you can see from the snippet, Podam supports org.hibernate.validator.constraints.NotEmpty, which appears to be Podam's default strategy.

What Podam does not support, is custom Hibernate constraints from package org.hibernate.validator.constraints. Including org.hibernate.validator.constraints.Email.

So, if you want to limit size of a string, you have two options:

  1. Use javax.validation.constraints.Size
  2. Create custom AttributeStrategy for org.hibernate.validator.constraints.Length

What is valid here is that Podam selects only one constraint, if multiple constraints are attached to a field. Could you please file a bug for this or I will do it later myself.

maxdidato commented 7 years ago

Thanks for the explanation. In my case my pojo contains custom Hibernate validation tags but I don't want them to be considered by podam. That is why on top of the Hibernate annotation I have added a podam annotation, to instruct the generator on how I want my field populated. Since Hibernate validations are not supported why don't you ignore them completely Rather than applying the BeanValidation strategy?

daivanov commented 7 years ago

The reason for this is that org.hibernate.validator.HibernateValidator or whatever non JavaEE validator will report errors for default stuff injected by Podam, but null values are fine, unless there is also NotNull/NotEmpty annotation.

maxdidato commented 7 years ago

I understand. Thanks for making it clear On Wed, 11 Jan 2017 at 20:22, Daniil Ivanov notifications@github.com wrote:

The reason for this is that org.hibernate.validator.HibernateValidator or whatever non JavaEE validator will report errors for default stuff injected by Podam, but null values are fine, unless there is also NotNull/NotEmpty annotation.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/devopsfolks/podam/issues/189#issuecomment-271982894, or mute the thread https://github.com/notifications/unsubscribe-auth/ACwgWVH8Cr7supg-SyOk4YX57Cd-JCwlks5rRTnmgaJpZM4H81d8 .

daivanov commented 7 years ago

In general this is rather controversial topic as there are people, who do not want hibernate-validator as Podam dependency and there are people, who want Hibernate constraint annotations support.

maxdidato commented 7 years ago

In order to raise an error during the generation of random values,Podam should trigger the validation engine of Hibernate, is that right? On Wed, 11 Jan 2017 at 20:29, Daniil Ivanov notifications@github.com wrote:

In general this is rather controversial topic as there are people, who do not want hibernate-validator as Podam dependency and there are people, who want Hibernate constraint annotations support.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/devopsfolks/podam/issues/189#issuecomment-271984924, or mute the thread https://github.com/notifications/unsubscribe-auth/ACwgWZPiwLMkogeUGrkTTeXwD_ht0Fspks5rRTu9gaJpZM4H81d8 .

daivanov commented 7 years ago

Something will call the validation engine, not Podam. Please, file an issue regarding multiple annotations on the same field.

daivanov commented 7 years ago

I re-used Issue #205 to report this bug. Do you want to make a PR fixing it or should I do it?

maxdidato commented 7 years ago

I can do it if you want On Thu, 12 Jan 2017 at 07:50, Daniil Ivanov notifications@github.com wrote:

I re-used Issue #205 https://github.com/devopsfolks/podam/issues/205 to report this bug. Do you want to make a PR fixing it or should I do it?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/devopsfolks/podam/issues/189#issuecomment-272098666, or mute the thread https://github.com/notifications/unsubscribe-auth/ACwgWY-kEaZ2uP-ZiOTWaOyhASFIUb4Oks5rRdswgaJpZM4H81d8 .