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

JsfLabelMessageInterpolator is not called on @Param bean validation error #238

Closed
pvdlg opened this Issue Apr 14, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@pvdlg
Contributor

pvdlg commented Apr 14, 2016

The setting is as follow:

  • JsfLabelMessageInterpolator configured as in Showcase - JsfLabelMessageInterpolator with same custom message for @Size
  • Messages configured as Showcase - Messages i.e. using MessageFormat.format(message, params);
  • Managed Bean with an attribute annotated with @Param and @Size(min = 5, max = 10)

When the managed bean is called with a GET parameter value that doesn't match the validator constraint, RequestParameterProducer create a new JSF message.

The message The size of {jsf.label} must be between 5 and 10 characters ends up being processed by MessageFormat.format(message, params); and fail with NumberFormatException as the message still contains {jsf.label} while MessageFormat expect a number (i.e. {0}).

I was about to make a PR to modify RequestParameterProducer with context.addMessage(component.getClientId(context), createError(violation.getMessage().replace("{jsf.label}", label))); although the root cause of the issue might be the fact that JsfLabelMessageInterpolator is not called in that situation.

I would imagine in that situation javax.faces.validator.BeanValidator.JsfAwareMessageInterpolator should be called and delegate to JsfLabelMessageInterpolator. Not sure why it's not in that situation...

@pvdlg

This comment has been minimized.

Show comment
Hide comment
@pvdlg

pvdlg Apr 17, 2016

Contributor

Actually after some investigations, JsfAwareMessageInterpolator is called. But it's the method interpolate(String messageTemplate, Context context) that it is called rather than interpolate(String messageTemplate, Context context, Locale locale).

It turns out that interpolate(String messageTemplate, Context context) doesn't replace {jsf.label}.

Is there a particular reason for that ?

Contributor

pvdlg commented Apr 17, 2016

Actually after some investigations, JsfAwareMessageInterpolator is called. But it's the method interpolate(String messageTemplate, Context context) that it is called rather than interpolate(String messageTemplate, Context context, Locale locale).

It turns out that interpolate(String messageTemplate, Context context) doesn't replace {jsf.label}.

Is there a particular reason for that ?

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Apr 19, 2016

Member

@arjantijms This was an oversight, I guess? The JsfLabelMessageInterpolator#interpolate(String messageTemplate, Context context) should instead of delegating to the wrapped delegate to interpolate(String messageTemplate, Context context, Locale locale) with Faces#getLocale().

Member

BalusC commented Apr 19, 2016

@arjantijms This was an oversight, I guess? The JsfLabelMessageInterpolator#interpolate(String messageTemplate, Context context) should instead of delegating to the wrapped delegate to interpolate(String messageTemplate, Context context, Locale locale) with Faces#getLocale().

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 19, 2016

Member

@BalusC most likely indeed so. It's been a while since I worked on this though.

Member

arjantijms commented Apr 19, 2016

@BalusC most likely indeed so. It's been a while since I worked on this though.

@BalusC BalusC closed this in 2244a5e Apr 19, 2016

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Apr 19, 2016

Member

Fix is available in today's latest 2.4-SNAPSHOT.

Thank you for reporting and improving OmniFaces!

Member

BalusC commented Apr 19, 2016

Fix is available in today's latest 2.4-SNAPSHOT.

Thank you for reporting and improving OmniFaces!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment