[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Use getMessageSource internally in RequestContext #32926

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Use getMessageSource internally in RequestContext #32926

merged 1 commit into from
Jun 3, 2024

Conversation

andreblanke
Copy link
Contributor
@andreblanke andreblanke commented May 30, 2024 •
edited
Loading

When subclassing RequestContext, providing a custom MessageSource to be used by all getMessage methods should be as easy as overriding getMessageSource:

class CustomRequestContext extends RequestContext {

    private MessageSource messageSource;

    CustomRequestContext(HttpServletRequest request, MessageSource messageSource) {
        super(request);

        this.messageSource = messageSource;
    }

    @Override
    public MessageSource getMessageSource() {
        return messageSource;
    }
}

Unfortunately, the RequestContext.getMessage methods internally use this.webApplicationContext instead of getMessageSource() which hurts the extensibility of the class. Thus in order to provide a custom MessageSource, three additional getMessage methods must be overridden, duplicating functionality from the super methods:

class CustomRequestContext extends RequestContext {

    private MessageSource messageSource;

    CustomRequestContext(HttpServletRequest request, MessageSource messageSource) {
        super(request);

        this.messageSource = messageSource;
    }

    @Override
    public MessageSource getMessageSource() {
        return messageSource;
    }

    @Override
    public String getMessage(String code, @Nullable Object[] args, String defaultMessage, boolean htmlEscape) {
	String msg = getMessageSource().getMessage(code, args, defaultMessage, getLocale());
	if (msg == null) {
		return "";
	}
	return (htmlEscape ? HtmlUtils.htmlEscape(msg) : msg);
    }

    @Override
    public String getMessage(String code, @Nullable Object[] args, boolean htmlEscape) {
	String msg = getMessageSource().getMessage(code, args, getLocale());
	return (htmlEscape ? HtmlUtils.htmlEscape(msg) : msg);
    }

    @Override
    public String getMessage(MessageSourceResolvable resolvable, boolean htmlEscape) {
	String msg = getMessageSource().getMessage(resolvable, getLocale());
	return (htmlEscape ? HtmlUtils.htmlEscape(msg) : msg);
    }
}

This PR changes those three RequestContext.getMessage methods to use getMessageSource() internally (defaulting to this.webApplicationContext unless overridden) to allow providing a custom MessageSource more easily.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 30, 2024
@snicoll
Copy link
Member
snicoll commented May 31, 2024

@pivotal-cla This is an Obvious Fix

Sorry, but it isn't. That is code change that requires the CLA to be signed. Please move forward before we can consider reviewing this.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label May 31, 2024
@andreblanke
Copy link
Contributor Author

Sorry about that. I thought it wasn't necessary due to the simplicity of the changes, but I've signed the CLA.

@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 Jun 1, 2024
@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 3, 2024
@jhoeller jhoeller self-assigned this Jun 3, 2024
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 3, 2024
@jhoeller jhoeller added this to the 6.2.0-M4 milestone Jun 3, 2024
@jhoeller
Copy link
Contributor
jhoeller commented Jun 3, 2024 •
edited
Loading

This generally makes sense, it just needs another change to go with it: namely removing the final declaration from getMessageSource() to make it actually overridable. At the opportunity, I've also removed final from getLocale() for consistency, as well as checking the Servlet 3.0+ ServletRequest.getServletContext() method in the request-only constructors (being able to fall back to the root WebApplicationContext even without an explicit ServletContext reference provided).

@jhoeller jhoeller merged commit a582e48 into spring-projects:main Jun 3, 2024
8 checks passed
jhoeller added a commit that referenced this pull request Jun 3, 2024
@jhoeller
Remove final declaration from getMessageSource() and getLocale() methods
34eccbe 
Includes checking the Servlet 3.0+ ServletRequest.getServletContext() method in the request-only constructors, being able to fall back to the root WebApplicationContext.

See gh-32926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jhoeller jhoeller

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Milestone
6.2.0-M4
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@andreblanke @snicoll @jhoeller @spring-projects-issues