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

Introduce ObjectUtils.nullSafeConciseToString() #30286

Closed
1 task done
sbrannen opened this issue Apr 4, 2023 · 7 comments
Closed
1 task done

Introduce ObjectUtils.nullSafeConciseToString() #30286

sbrannen opened this issue Apr 4, 2023 · 7 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@sbrannen
Copy link
Member

sbrannen commented Apr 4, 2023

Overview

org.springframework.util.ObjectUtils.nullSafeToString(Object) exists for generating a toString() representation of various objects in a "null-safe" manner, including support for object graphs, collections, etc.

However, there are times when we would like to generate a more "concise" null-safe toString() representation that does not include an entire object graph (or potentially a collection of object graphs).

org.springframework.beans.BeanUtils.isSimpleValueType(Class<?>) already provides a check for an opinionated list of "simple types"; however, that resides in spring-beans and therefore cannot be used in spring-core.

We should introduce ObjectUtils.nullSafeConciseToString(Object) that generates a concise, null-safe string representation of the supplied object relying on logic similar to BeanUtils.isSimpleValueType() to decide when to use the standard toString() implementation for the supplied object and when to replace the standard implementation with a more concise format -- for example, something along the lines of what Object#toString generates instead of including the toString() representation of an entire object graph recursively.

Deliverables

  • Introduce nullSafeConciseToString(Object) in ObjectUtils
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Apr 4, 2023
@sbrannen sbrannen added this to the 6.0.8 milestone Apr 4, 2023
@sbrannen sbrannen self-assigned this Apr 4, 2023
@sbrannen sbrannen added the for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x label Apr 4, 2023
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x labels Apr 4, 2023
@sbrannen sbrannen changed the title Introduce nullSafeSimpleToString(Object) in ObjectUtils Introduce nullSafeConciseToString(Object) in ObjectUtils Apr 5, 2023
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Apr 5, 2023
StringUtils.truncate() serves as central, consistent way for truncating
strings used in log messages and exception failure messages, for
immediate use in LogFormatUtils and ObjectUtils.

See spring-projectsgh-30286
Closes spring-projectsgh-30290
sbrannen added a commit that referenced this issue Apr 5, 2023
@sbrannen sbrannen changed the title Introduce nullSafeConciseToString(Object) in ObjectUtils Introduce ObjectUtils.nullSafeConciseToString() Apr 5, 2023
sbrannen added a commit that referenced this issue Apr 5, 2023
StringUtils.truncate() serves as central, consistent way for truncating
strings used in log messages and exception failure messages, for
immediate use in LogFormatUtils and ObjectUtils.

See gh-30286
Closes gh-30290
sbrannen added a commit that referenced this issue Apr 5, 2023
ObjectUtils.nullSafeToString(Object) exists for generating a string
representation of various objects in a "null-safe" manner, including
support for object graphs, collections, etc.

However, there are times when we would like to generate a "concise",
null-safe string representation that does not include an entire object
graph (or potentially a collection of object graphs).

This commit introduces ObjectUtils.nullSafeConciseToString(Object) to
address this need and makes use of the new feature in FieldError and
ConversionFailedException.

Closes gh-30286
sbrannen added a commit that referenced this issue Apr 5, 2023
@gebhardt
Copy link

This change in the FieldError.toString() method makes our bean validation unusable. If we validate a File or an Optional<String> (e.g Optional<@Pattern(regexp="...") String> the resulting error Message is useless, as it does not contain the Filename or the content of the optional String any more.
I highly suggest to rollback the changes.

@bclozel
Copy link
Member

bclozel commented Jun 15, 2023

@gebhardt This was done in #30661, see above in the linked issues. The release is already out.

@gebhardt
Copy link

@bclozel Unfortunately only UUID and neither File nor Optional was added to the classes where the toString() method gets called. I assume that there will be a very long list of classes in the future that are regarded as simple type.
I'd suggest to rollback the changes in the Validation output, i.e. call ObjectUtils.nullSafeToString(...) in the org.springframework.validation.FieldError.toString() method.

@bclozel
Copy link
Member

bclozel commented Jun 15, 2023

Can you create a new issue and provide a sample showing the issue?

mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
StringUtils.truncate() serves as central, consistent way for truncating
strings used in log messages and exception failure messages, for
immediate use in LogFormatUtils and ObjectUtils.

See spring-projectsgh-30286
Closes spring-projectsgh-30290
mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
ObjectUtils.nullSafeToString(Object) exists for generating a string
representation of various objects in a "null-safe" manner, including
support for object graphs, collections, etc.

However, there are times when we would like to generate a "concise",
null-safe string representation that does not include an entire object
graph (or potentially a collection of object graphs).

This commit introduces ObjectUtils.nullSafeConciseToString(Object) to
address this need and makes use of the new feature in FieldError and
ConversionFailedException.

Closes spring-projectsgh-30286
mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Jul 3, 2023

@gebhardt we're currently considering what to do about this for the upcoming releases in mid July. It would be great if you could create a separate issue for the FieldError.toString() regression for which we would potentially revert to the original behavior, while still using the concise representation for ConversionFailedException and potentially also for BindException.

Could you please clarify how you are consuming the FieldError.toString() output? Are you calling the method directly (like in #30799) or indirectly via Errors/BindingResult/BindException?

@gebhardt
Copy link

gebhardt commented Jul 4, 2023

We are using the FieldError indirectly via Bean Validation in our REST API or BindExeptions when validating configuration files.

We are using Optional fields in our Rest Patch requests, and the validation annotations look like this:

private Optional<@Pattern(regexp="..." String> prop1;
private Optional<@Size(min = 1, max = 255) String> prop2;

Now I get exception messages like this, that do not contain the rejected value in a readable form anymore.

org.springframework.web.bind.MethodArgumentNotValidException: Validation failed for argument ..  rejected value [java.util.Optional@79004b3f]
    at org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor.resolveArgument(RequestResponseBodyMethodProcessor.java:141)
    at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:122)

Another example is a configuration file where we have created an @ExistingFile annotation:

@Validated
public class MyConfiguration {

    @ExistingFile
    private File myFile;
}

If I try to initialize an ApplicationContext, I get the following Exception:

Caused by: org.springframework.boot.context.properties.bind.validation.BindValidationException: Binding validation errors on abc.def
   - Field error in object 'abc.def' on field 'myFile': rejected value [java.io.File@566806a]; codes [ExistingFile.abc.def.myFile,ExistingFile.myFile,ExistingFile.java.io.File,ExistingFile]; arguments [org.springframework.context.support.DefaultMessageSourceResolvable: codes [abc.def.plugins[0].myFile,myFile]; arguments []; default message [myFile]]; default message [must be an exiting file]; origin "abc.def.myFile" from property source "class path resource [config/my.properties]"

However, if we use the Spring Boot FailureAnalyzer I get a meaningful output:

Binding to target org.springframework.boot.context.properties.bind.BindException: Failed to bind properties under 'abc.def' to com.exmaple.MyConfiguration failed:

    Property: com.exmaple.myFile
    Value: "/path/to/my/file.txt"
    Origin: " com.exmaple.myFile" from property source "URL [file:/path/to/my.properties]"
    Reason: must be an exiting file.

@sbrannen
Copy link
Member Author

sbrannen commented Jul 4, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants