Where Should The Validation Logic Live?

Question

Question

How best can i manage construction of an object graph where complex validation logic is required?. I would like to retain dependency injected, do-nothing constructors for testability reasons.

Testability is very important to me, what does your suggestion do to maintain this attribute of the code?

Background

I have a plain-old-java-object which manages the structure of some business data for me:

class Pojo
{
    protected final String p;

    public Pojo(String p) {
        this.p = p;
    }
}

I want to make sure that p is of valid format because this business object really makes no sense without that guarantee; it should not even be created if p is nonsense. However, validating p is non-trivial.

The Catch

Really it requires complex enough validation logic that the logic should be fully testable in it's own right, and so i have that logic in a separate class:

final class BusinessLogic() implements Validator<String>
{
    public String validate(String p) throws InvalidFoo, InvalidBar {
        ...
    }
}

Possible Duplicate Questions

• Where Should Validation Logic Be Implemented? - The accepted answer is impenetrable to me. I read "be run in the class's native environment" as a tautology, how can validation rules be run in anything other than "the class's native environment"? Point 2 i fail to grok so i can't comment.
• Where To Provide Logic Rules for Validation? - Both answers suggest the responsibility lies with the client / data provider, which i like in principle. However, in my case the client may not be the originator of the data and cannot validate it.
• Where To Keep Validation Logic? - Suggests validation can be owned by the model, however i find this approach less than ideal for testing. Specifically, for every unit test i need to care about all the validation logic even while i am testing other parts of the model - i can never fully isolate what i want to test by following the suggested approach.

My Thinking So Far

Although the following constructor openly declares the dependencies of Pojo and preserves its simple testability, it is completely unsafe. There is nothing here to prevent the client providing a validator which claims every input is acceptable!

public Pojo(Validator businessLogic, String p) throws InvalidFoo, InvalidBar {
    this.p = businessLogic.validate(p);
}

So, i restrict visibility of the constructor somewhat, and i provide a factory method which ensures validation then construction:

@VisibleForTesting
Pojo(String p) {
    this.p = p;
}

public static Pojo createPojo(String p) throws InvalidFoo, InvalidBar {
    Validator businessLogic = new BusinessLogic();
    businessLogic.validate(p);
    return new Pojo(p);
}

Now i could refactor createPojo into a factory class, that would restore "single responsibility" to Pojo and simplify testing of the factory logic, not to mention the performance benefits of no longer wastefully creating a new (stateless) BusinessLogic on every new Pojo.

My gut feeling is that i should stop, ask for an outside opinion. Am i on the right track?

Answer 1

A few elements of response below... Let me know if it makes sense / answers your questions.

---

Introduction: I consider that your system can be either a simple library, a multi-tiered application, or a complex distributed system, it actually doesn't make much difference when it comes to validation:

• client: a remote client (e.g. HTTP client) or just another class calling your library.
• service: a remote service (e.g. REST service) or just what you expose of your API.

---

Where to validate?

You typically want to validate your input parameters:

• on the client side, before passing the parameters to the service, to ensure early on that objects will be valid down the line. This is especially desirable if it is a remote service or there is a complex flow between the generation of the parameters and the object creation.

• on the service side:

  • at the class-level, in your constructor, to ensure you create valid objects;
  • at a sub-system level, i.e. the layer managing these objects (e.g. DAL persisting your Pojos);
  • at the boundary of your service, e.g. the facade or external API of your library or your controller (as in MVC, e.g. REST endpoint, Spring controller, etc.).

---

How to validate?

Assuming the above, since you may have to reuse your validation logic in several places, it may be a good idea to extract it in an utility class. This way:

• you don't duplicate it (DRY!);
• you are sure that every layer of the system will validate the same way;
• you can easily unit test this logic separately (because it is stateless).

More concretely, you would at least call this logic in your constructor, to enforce the validity of the object (think of having valid dependencies as preconditions to your algorithms presents in Pojo's methods):

Utility class:

public final class PojoValidator() {
    private PojoValidator() {
        // Pure utility class, do NOT instantiate.
    }

    public static String validateFoo(final String foo) {
        // Validate the provided foo here.
        // Validation logic can throw:
        // - checked exceptions if you can/want to recover from an invalid foo, 
        // - unchecked exceptions if you consider these as runtime exceptions and can't really recover (less clutering of your API).
        return foo;
    }

    public static Bar validateBar(final Bar bar) {
        // Same as above...
        // Can itself call other validators.
        return bar;
    }
}

Pojo class: Notice the static import statement for better readability.

import static PojoValidator.validateFoo;
import static PojoValidator.validateBar;

class Pojo {
    private final String foo;
    private final Bar bar;

    public Pojo(final String foo, final Bar bar) {
        validateFoo(foo);
        validateBar(bar);
        this.foo = foo;
        this.bar = bar;
    }
}

---

How to test my Pojo?

1. You should add creational unit tests to ensure your validation logic is called at construction time (to avoid regressions as people "simplify" your constructor later by removing this validation logic for reason X, Y, Z).

2. Inline the creation of your dependencies if they are simple, as it makes your test more readable, since everything you use is local (less scrolling, smaller mental footprint, etc.)

3. However, if the setup of your Pojo's dependencies is complex/verbose enough that the test is no longer readable, you could factor this setup out in a @Before / setUp method, so that the unit tests testing Pojo's logic really focus on validating the behaviour of your Pojo.

In any case, I would agree with Jeff Storey:

1. write your tests with valid parameters,
2. don't have a constructor without validation just for your tests. It is indeed a code smell as you mix production and testing code, and will definitely be used inadvertently by someone at some point, comprising the stability of your service.

Finally, think of your tests as code samples, examples or executable specifications: you don't want to give a confusing example by:

• injecting invalid parameters;
• injecting the validator, which would clutter your API / reads "weird".

---

What if Pojo requires VERY complex dependencies?

[If this is the case, please let us know]

Production code: You could try to hide this complexity in a factory.

Testing code: Either:

• as mentioned above, factor this out in a different method; or
• use your factory; or
• use mock parameters, and configure them just so that they validate the requirements for your tests to pass.

---

EDIT: A few links about input validation in the context of security, which can also be useful:

• OWASP's Input Validation Cheat Sheet
• OWASP's wikipage about Data Validation

Answer 2

First, I would say that validation logic should not live solely in the client. This helps to ensure you don't end up putting invalid data in your data store. If you add additional clients (maybe you have a thick client and you add a web service client for example, you need to maintain the validation in both places).

I don't think that you should have a constructor for constructing an object that doesn't validate (with that @VisibleForTesting annotation). You should generally be testing with valid objects (unless you're testing error cases). Also, adding additional code in your production code that is only for testing is a code smell, since it's not really production code.

I think the appropriate place to put the validation logic is within the domain object itself. This will ensure you don't create an invalid domain object.

I don't really like the idea of passing a validator into the domain object. That puts a lot of work on clients of the domain object who need to know about the validators. If you want to create a separate validator, that may add benefits of reuse and modularization, but I wouldn't inject that. In testing, you can always use a mock object that completely bypasses validation.

Adding validation to the domain model is something common that web frameworks do (grails/rails). I think it's a good approach, and it shouldn't hurt testability.