search

mtedone / podam

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

Enable defining ArributeStrategy that will be applicable for all annotation classes implementing the same interface #279

Closed zonkeydonkey closed 4 years ago

zonkeydonkey commented 4 years ago

Podam allows to set custom AttributeStrategies for specific, explicitly mentioned annotation types:

val factory = PodamFactoryImpl()                                                            
(factory.strategy as RandomDataProviderStrategy).addOrReplaceAttributeStrategy(MyCustomAnnotation::class.java, MyCustomAttributeStrategy())

but it is not possible to define "fallback" strategy that will be used for all annotations that implement the same base interface (for example in Kotlin every annotation class implements kotlin.Annotation):

(factory.strategy as RandomDataProviderStrategy).addOrReplaceAttributeStrategy(Annotation::class.java, MyCustomAttributeStrategy())

All above examples are written in Kotlin.

daivanov commented 4 years ago

Hi,

Thank you for the contribution. I will look into it. We definitely need a test for this.

Thanks, Daniil

zonkeydonkey commented 4 years ago

@daivanov in the test for this issue you are assuming that when strategy for generic Annotation class is defined, the number of calls of that strategy is equal to 0. Is it only for the test to pass before this PR gets merged or is it some internal requirement for Podam you wish to preserve?

daivanov commented 4 years ago

This is just a placeholder test, which I made to succeed otherwise Travis will be failing and everybody will be unhappy. The idea was that @Version and @Basic will be triggering generic AnnotationStrategy, which doesn't happen at the moment.

Thanks, Daniil

daivanov commented 4 years ago

And it looks like the whole enterprise has failed because each field has multiple annotations and at least one of them is supported by Podam :)

Thanks, Daniil

daivanov commented 4 years ago

I think I need to investigate your branch more thoroughly.

Thanks, Daniil

daivanov commented 4 years ago

What do you think about the latest changes?

Thanks, Daniil

zonkeydonkey commented 4 years ago

Hi, I agree with all your changes. Thanks for applying them, could you release a new version of Podam? Thanks, Sylwia

daivanov commented 4 years ago

Ok, thank you. Fixed in Podam 7.2.4

Thanks, Daniil

daivanov commented 4 years ago

It's already there, but will be visible in Maven UI, when they will synchronize everything https://repo1.maven.org/maven2/uk/co/jemos/podam/podam/7.2.4.RELEASE/

Thanks, Daniil

daivanov commented 4 years ago

https://mvnrepository.com/artifact/uk.co.jemos.podam/podam :man_shrugging:

zonkeydonkey commented 4 years ago

@daivanov could you reopen the issue? I have noticed that the logging still needs to be conformed to the changes that have been done for this issue, example: supposing you have your custom validation annotation and you did not registered an attribute strategy for this annotation itself, but you registered a fallback strategy for Annotation class (the interface for all annotation classes), Podam still logs: Please, register AttributeStratergy for custom constraint (...) Better would be to log the above message when no strategy for certain annotation can be applied (neither strategy for the annotation nor for Annotation class) and when fallback strategy can be applied, then log some message in this manner: "A strategy for $annotation was not found, $baseClassOfAnnotation strategy was used."

daivanov commented 4 years ago

True, the warning is logged too early.

Thanks, Daniil

daivanov commented 4 years ago

Sorry for a delay. Not it's merged.

Thanks, Daniil