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

Rejected value is null when configuration property fails validation #19580

Closed
tazle opened this issue Jan 8, 2020 · 8 comments
Closed

Rejected value is null when configuration property fails validation #19580

tazle opened this issue Jan 8, 2020 · 8 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@tazle
Copy link

tazle commented Jan 8, 2020

After changing from Spring Boot 2.1.9 to Spring Boot 2.2.2, the error messages about constraint validation for some of our configuration properties started including null as the rejected value, rather than the actual value (-1 in this case).

The root cause seems to be that SpringValidationAdapter's getRejectedValue(ield, violation, bindingResult) has started returning null in certain cases with the bindingResult that's present in 2.2.2. Or rather, the getRawFieldValue(String) has started returning null.

Failing case in 2.2.2:
SpringValidationAdapter calls ValidationBindHandlers$ValidationResult.getRawFieldValue("intermediatefield.leafFieldInCamelCase")which returns null because it expects field names in form "intermediatefield.leaf-field-in-camel-case".

Working case in 2.1.9:
SpringValidationAdapter calls BeanPropertyBindingResult.getRawFieldValue("intermediatefield.leafFieldInCamelCase") which returns the actual field value.

I'll try to produce a minimal reproduction later.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 8, 2020
@wilkinsona wilkinsona changed the title rejectedValue in null when configuration property fails validation Rejected value is null when configuration property fails validation Jan 8, 2020
@mbhave
Copy link
Contributor

mbhave commented Jan 8, 2020

@tazle A sample that we can use to reproduce the issue would be very helpful, thanks.

@mbhave mbhave added the status: waiting-for-feedback We need additional information before we can continue label Jan 8, 2020
@tazle

This comment has been minimized.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 9, 2020
@snicoll
Copy link
Member

snicoll commented Jan 9, 2020

@tazle thank you for the sample but rather than code in text, please create a project (either a github project or a zip) as we'd have to copy paste all that in order to do something useful with it anyway.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 9, 2020
@tazle
Copy link
Author

tazle commented Jan 9, 2020

Zipped up:

repro_19580.zip

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 9, 2020
@nosan
Copy link
Contributor

nosan commented Jan 9, 2020

Looks like gh-17424 is a cause. ValidationBindHandler$ValidationResult.getActualFieldValue does not work at all if the provided value is in a camelCase format:

org.springframework.boot.context.properties.source.InvalidConfigurationPropertyNameException: Configuration property name 'camelCaseValue' is not valid
	at org.springframework.boot.context.properties.source.ConfigurationPropertyName.elementsOf(ConfigurationPropertyName.java:522)
	at org.springframework.boot.context.properties.source.ConfigurationPropertyName.probablySingleElementOf(ConfigurationPropertyName.java:495)
	at org.springframework.boot.context.properties.source.ConfigurationPropertyName.append(ConfigurationPropertyName.java:191)
	at org.springframework.boot.context.properties.bind.validation.ValidationBindHandler$ValidationResult.getName(ValidationBindHandler.java:190)
	at org.springframework.boot.context.properties.bind.validation.ValidationBindHandler$ValidationResult.getFieldType(ValidationBindHandler.java:168)
	at org.springframework.validation.AbstractBindingResult.resolveMessageCodes(AbstractBindingResult.java:325)
	at org.springframework.validation.beanvalidation.SpringValidatorAdapter.processConstraintViolations(SpringValidatorAdapter.java:175)
	at org.springframework.validation.beanvalidation.SpringValidatorAdapter.validate(SpringValidatorAdapter.java:109)
	at org.springframework.boot.context.properties.ConfigurationPropertiesJsr303Validator.validate(ConfigurationPropertiesJsr303Validator.java:51)
	at org.springframework.boot.context.properties.bind.validation.ValidationBindHandler.validateAndPush(ValidationBindHandler.java:132)
	at org.springframework.boot.context.properties.bind.validation.ValidationBindHandler.validate(ValidationBindHandler.java:110)
	at org.springframework.boot.context.properties.bind.validation.ValidationBindHandler.onFinish(ValidationBindHandler.java:102)
	at org.springframework.boot.context.properties.bind.Binder.handleBindResult(Binder.java:340)
	at org.springframework.boot.context.properties.bind.Binder.bind(Binder.java:321)

problem line:

Btw, I have found that

catches an exception but just swallows it, and that's why this issue was hidden from folks :( Maybe it makes sense at least to log it in DEBUG level.

This issue can be easily fixed with:

			return this.name.append(DataObjectPropertyName.toDashedForm(field));

But I am not sure about this and in addition DataObjectPropertyName is a package-private class.

@philwebb philwebb added type: bug A general bug and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Jan 9, 2020
@philwebb philwebb added this to the 2.2.x milestone Jan 9, 2020
@mbhave mbhave self-assigned this Jan 10, 2020
@mbhave mbhave closed this as completed in a017b89 Jan 10, 2020
@mbhave
Copy link
Contributor

mbhave commented Jan 10, 2020

Thanks for the analysis, @nosan. I was initially planning to change the code to skip validation if a previous validation exception was found and that would've fixed this issue as a side effect. I decided to create a separate issue for that.

@mbhave mbhave modified the milestones: 2.2.x, 2.2.3 Jan 10, 2020
snicoll pushed a commit to scottfrederick/spring-boot that referenced this issue Jan 10, 2020
@tazle
Copy link
Author

tazle commented Mar 2, 2020

This fix seems to have gotten rid of some of the issues, but others still persist. I'll create a test case.

@tazle
Copy link
Author

tazle commented Mar 2, 2020

This fix seems to have gotten rid of some of the issues, but others still persist. I'll create a test case.

Perhaps that was a transient issue with IDEA and outdated classpath before Maven reimport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants