Skip to content

Conversation

andreblanke
Copy link
Contributor

@andreblanke andreblanke commented May 30, 2024

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

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
jhoeller added a commit that referenced this pull request Jun 3, 2024
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
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
Development

Successfully merging this pull request may close these issues.

4 participants