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

AbstractMessageSource does not support null as default message anymore [SPR-16127] #20675

Closed
spring-projects-issues opened this issue Oct 28, 2017 · 3 comments
Assignees
Labels
in: core type: regression
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Oct 28, 2017

Thomas Heigl opened SPR-16127 and commented

I just encountered some issues in my application due to a subtle change in AbstractMessageSource between Spring 4.x and 5.x.

In Spring 4.x is was possible to pass null as defaultMessage, Spring 5.x always returns an empty string.

Spring 4.x:

public final String getMessage(String code, Object[] args, String defaultMessage, Locale locale) {
	String msg = getMessageInternal(code, args, locale);
	if (msg != null) {
		return msg;
	}
	if (defaultMessage == null) {
		String fallback = getDefaultMessage(code);
		if (fallback != null) {
			return fallback;
		}
	}
	return renderDefaultMessage(defaultMessage, args, locale);
}

Spring 5.x:

public final String getMessage(String code, @Nullable Object[] args, @Nullable String defaultMessage, Locale locale) {
	String msg = getMessageInternal(code, args, locale);
	if (msg != null) {
		return msg;
	}
	if (defaultMessage == null) {
		String fallback = getDefaultMessage(code);
		return (fallback != null ? fallback : "");
	}
	return renderDefaultMessage(defaultMessage, args, locale);
}

If the default message is null and the fallback is null Spring now returns "".

This makes it hard to distinguish between properties that are defined but empty and properties that are undefined.


Affects: 5.0.1

Issue Links:

  • #20596 AbstractMessageSource does not properly interact with DelegatingMessageSource parent
  • #20099 Introduce null-safety of Spring Framework API

Referenced from: commits e5c8dc0

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 29, 2017

Juergen Hoeller commented

Good point, the three-arg getMessage variant doesn't expose such a distinction anymore. We may have to revisit this, doing the empty String adaptation at the caller's level instead.

Generally speaking, you could consider usung getMessage(code, args, locale which throws a NoSuchMessageException in the undefined case. This is definitely the clearer contract for such a purpose, not affected by useCodeAsDefaultMessage and the like either.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 29, 2017

Thomas Heigl commented

Juergen, thanks for the fast feedback!

I briefly though about using the throwing implementation instead, but it isn't ideal in my case. I'm using the AbstractMessageSource in a chain of property locators. If one locator doesn't return a result (i.e. returns null, I move on to the next. It is the expected case, that a single locator doesn't contain all application codes and try/catching isn't the best way to express this.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 31, 2017

Juergen Hoeller commented

For 5.0.2, I've revised MessageSource towards returning null for a null default message again. This also includes a revision of 5.0.1's fix for #20596, now streamlined towards a null check like it originally was in 4.x.

Higher-level accessors such as MessageSourceAccessor.getMessage or RequestContext.getMessage declare their default message argument as non-null to begin with, so also consistently return a non-null message outcome. Internal null-to-empty-String adaptation happens at that level now, not within AbstractMessageSource anymore, which keeps our @Nullable checks and our corresponding Kotlin callers happy. This should be a good enough compromise going forward.

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

No branches or pull requests

2 participants