vt-password: remove edu.vt.middleware.password.MessageResolver's direct dependency on java.util.Properties

[GoogleCodeExporter]

I'm new to vt-password, and I'm liking it so far. It's definitely going to save 
me some work. But I have encountered one thing which I consider an 
inconvenience, and I think it would be nice to have it addressed in a future 
release. (I can't seem to select what sort of issue this is, but I'm sure that 
you'll understand it's under the heading of "Enhancement" rather than being a 
real defect/bug.)

It is unfortunate that MessageResolver requires a Properties object to resolve 
error message keys to error messages. It seems like MessageResolver should be 
just an abstract class which contains only an implementation of the resolve 
method, with the code to resolve a key to a message factored out. In that case, 
it would make sense and would be quite easy to have an implementation that 
could get the messages from some other source. As it stands now, though, 
extending MessageResolver is awkward, at least because:

a) subclasses don't get access to the Properties field, so even if all they 
wanted to do was to change the way resolve works, they couldn't really do that 
very easily except by capturing their own copy of the Properties object; and

b) if the subclass author didn't want to change the way the resolve method 
performs the formating operation, they would have to copy that code. You all 
(i.e., vt-middleware developers) may, in the future, choose to change the 
resolve method's implementation. Subclasses that have their own resolve method 
would not pick up such changes automatically, meaning maintainers must manually 
copy the updated code in order to take advantages of any improvements you may 
make.

The only other option for a developer who does not wish to use the default 
scheme for resolving messages seems to be manually populating a Properties 
object with the appropriate key-value pairs so they have something to pass it 
to the MessageResolver constructor -- or to simply avoid the PasswordValidator 
class entirely, and use the Rule classes and the individual RuleResults they 
generate, requiring probably even more work or at least more duplication of 
code.

There are surely several solid refactorings that would address this issue. But 
here's my suggestion, which I feel is a very simple and straightforward one:

1. Create an interface that does nothing but resolve messages from an error 
key, e.g.:

package edu.vt.middleware.password;

public interface Messages {

  public String getMessage(String key);

}

2. In MessageResolver, get rid of the private field Properties 
messageProperties and add a new private field MessageResolver messages. Then:

  2a. Make a method in MessageResolver that returns a Messages implementation in terms of a Properties object using an adapter class (I did this one inline but you could just as easily declare the class separately):

private static final Messages messagesFromProperties(final Properties 
properties) {
  if (properties == null) {
    throw new IllegalArgumentException("Properties cannot be null.");
  }
  return new Messages() {
    @Override
    public final String getMessage(String key) {
      return properties.getProperty(key);
    }
  };
}

  2b. Add a new MessageResolver constructor that takes a Messages object.

  2c. Chain the constructor that takes a Properties object to the one that takes a Messages object, using the messagesFromProperties method to create the Messages instance.

  2d. Where the resolve method now invokes messageProperties.getProperty(key), invoke messages.getMessage(key) instead.

This addresses the current inconvenience of the current dependency of 
MessageResolver on Properties without breaking the existing API.

Thanks in advance for reading this suggestion. Please keep up the good work.

Original issue reported on code.google.com by dshepper...@gmail.com on 8 Aug 2014 at 12:07

[GoogleCodeExporter]

Of course in step 2, I meant add a new private field Messages message (not 
MessageResolver messages).

Original comment by dshepper...@gmail.com on 8 Aug 2014 at 4:53

[GoogleCodeExporter]

Original comment by dfis...@gmail.com on 19 Aug 2014 at 1:30

• Changed state: Accepted
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Thanks for the detailed issue.
I went with a slightly different solution, but hopefully achieved the same goal.
I made MessageResolver an interface, then provided abstract and properties 
based implementations.

See https://github.com/vt-middleware/passay/issues/8

Note that vt-password is in maintenance, and only receiving security patches.

Original comment by dfis...@gmail.com on 9 Oct 2014 at 8:45

• Changed state: Fixed

[GoogleCodeExporter]

Looks to me like a fine solution. Thank you.

Original comment by dshepper...@gmail.com on 9 Oct 2014 at 10:10