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

Certain validation failures cause further injection points not to be checked #409

Closed
Azquelt opened this issue Oct 2, 2020 · 7 comments
Closed
Milestone

Comments

@Azquelt
Copy link
Contributor

Azquelt commented Oct 2, 2020

In ConfigExtension.validate, there is clearly an attempt to detect all validation issues and add each of them as a deployment problem.

However, there are cases which will cause the validate method to throw an exception or return, meaning that it won't validate further injection points.

In particular:

(line numbers based on the mpconfig20 branch)

radcortez added a commit to radcortez/smallrye-config that referenced this issue Oct 2, 2020
…f returning after a property that should not be validated is found.
radcortez added a commit to radcortez/smallrye-config that referenced this issue Oct 5, 2020
…f returning after a property that should not be validated is found.
@radcortez
Copy link
Member

#412 Should fix this.

On io.smallrye.config.inject.ConfigProducerUtil#getConfigKey, I believe it should be fine. Did you have any trouble with it?

@Azquelt
Copy link
Contributor Author

Azquelt commented Oct 6, 2020

io.smallrye.config.inject.ConfigProducerUtil#getConfigKey will throw an exception if the name isn't set for the ConfigProperty annotation and it can't work out a default name (e.g. because the injection point is a method parameter and the parameter name is not available).

However, ConfigExtension.validate won't catch this exception and therefore won't validate any further injection points.

@radcortez
Copy link
Member

Ah, right! Forgot about the method parameter injection. I'll fix it. Thank you!

radcortez added a commit to radcortez/smallrye-config that referenced this issue Oct 7, 2020
…f returning after a property that should not be validated is found.
@radcortez
Copy link
Member

Actually, it should be fine. Any exception thrown in the AfterDeploymentValidation observer event is added as a deployment problem.

@radcortez radcortez added this to the 1.9.2 milestone Oct 7, 2020
@Azquelt
Copy link
Contributor Author

Azquelt commented Oct 15, 2020

This is true, and the error will be reported, but no further validation will be done. I think it would be preferable if we could report all the validation issues, rather than stopping if we hit this one. I'll make a PR for this.

@radcortez
Copy link
Member

Sure.

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

No branches or pull requests

2 participants