Skip to content
This repository has been archived by the owner on May 12, 2020. It is now read-only.

RAVE should support multiple validator factories per compilation unit #39

Open
ZacSweers opened this issue May 22, 2017 · 2 comments
Open
Milestone

Comments

@ZacSweers
Copy link
Contributor

Currently RAVE limits this, though I'm not sure why. It's indirectly enforced by checking for multiple factories during verification, but this seems to be imposed by its inverted approach of using @Validated annotations to determine the factory to generate. I'm not sure if there was any other intention.

I actually ran into this in an unrelated bug where compilation with multiple modules that have duplicate package-info.java files will actually include multiple modules' models in the same processing round. Obviously this is a separate bug and would be really wasteful, but it got me thinking.

My suggestion would be this: Follow the approach used by auto-value-moshi/gson's factory processors and just collect all the @Validated and @Validator elements up front, then tie them together during processing. This would be api-invisible and not be a major breaking change to support.

I don't know that there's much use case for multiple validators per module, but if we're going to support/require specifying a validator per @Validated annotation it seems like we should at least support it. Otherwise I don't see why we have that specification requirement on @Validated annotations if they're all implicitly limited to one anyway.

@ZacSweers
Copy link
Contributor Author

Didn't realize RAVE didn't have @Validator before, and now remember @behroozkhorashadi and I having a discussion about this before. I guess this is another reason why I think we should switch to that style for Rave 2.0 :)

@naturalwarren
Copy link
Contributor

If we require the @Validator annotation on ValidatorFactory implementations it can find all @Validated models and we could:

  1. Conceivably error out with a descriptive message to address If no validated models exist, RAVE should either fail with a descriptive error message or generate an empty implementation #9
  2. Refactor AnnotationVerifier to not resolve ValidatorFactory classes to add RAVE should support multiple validator factories per compilation unit #39. Resolution of ValidatorFactory classes should occur when @Validated annotations are processed.

@behroozkhorashadi is there some reason why we only allow one ValidatorFactory per compilation unit?

@naturalwarren naturalwarren added this to the RAVE 3 milestone May 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants