EDIT: I am not worried about being called in the wrong order since this is enforced through using multiple interfaces, I am just worried about the terminal method getting called at all.

I am using a builder pattern to create permissions in our system. I chose a builder pattern because security is so important in our product (it involves minors so COPPA et al), I felt it was imperative that permissions be readable, and felt that the readability was of the utmost importance (i.e. use a fluent-style builder pattern rather than a single function with 6 values).

The code looks like this:

 permissionManager.grantUser( userId ).permissionTo( Right.READ ).item( docId ).asOf( new Date() );

The methods populate a private backing bean, that upon having the terminal method (i.e. asOf ) commit the permission to the database; if that method doe开发者_JAVA技巧s not get called nothing happens. Occasionally developers will forget to call the terminal method, which does not cause a compiler error and is easy to miss on a quick reading/skimming of the code.

What could I do to prevent this problem? I would not like to return a Permission object that needs to get saved since that introduces more noise and makes permission code harder to read, follow, track, and understand.

I have thought about putting a flag on the backing which gets marked by the terminal command. Then, check the flag in the finalize method and write to the log if the object was created without persisting. (I know that finalize is not guaranteed to run, but it's the best I can think of.)

Solution

A good way to structure this fluent API pattern is instead of just returning this from each method, return an instance of a Method Object Pattern that implements an Interface that only supports the method that should be next in the list and have the last method call return the actual object you need.

If that is the only way to get an instance of that object, the last method will always have to be called.

Q6613429.java

package com.stackoverflow;

import javax.annotation.Nonnull;
import java.util.Date;

public class Q6613429
{
    public static void main(final String[] args)
    {
        final Rights r = PermissionManager.grantUser("me").permissionTo("ALL").item("EVERYTHING").asOf(new Date());
        PermissionManager.apply(r);
    }

    public static class Rights
    {
        private String user;
        private String permission;
        private String item;
        private Date ofDate;

        private Rights() { /* intentionally blank */ }
    }

    public static class PermissionManager
    {
        public static PermissionManager.AssignPermission grantUser(@Nonnull final String user)
        {
            final Rights r = new Rights(); return new AssignPermission() {

                @Override
                public AssignItem permissionTo(@Nonnull String p) {
                    r.permission = p;
                    return new AssignItem() {
                    @Override
                    public SetDate item(String i) {
                        r.item = i;
                        return new SetDate()
                    {
                        @Override
                        public Rights asOf(Date d) {
                            r.ofDate = d;
                            return r;
                        }
                    };}
                };}
            };
        }

        public static void apply(@Nonnull final Rights r) { /* do the persistence here */ }

        public interface AssignPermission
        {
            public AssignItem permissionTo(@Nonnull final String p);
        }

        public interface AssignItem
        {
            public SetDate item(String i);
        }

        public interface SetDate
        {
            public Rights asOf(Date d);
        }
    }
}

This enforces the chain of construction calls, and is very friendly with code completion as it shows what the next interface is and it only method available.

Here is a more complete example with optional things in the middle:

UrlBuilder.java

This provides a foolproof checked exception free way to construct URL objects.

Mixing the persistence with the construction is mixing concerns:

Creating the object and storing it are different concerns and should not be mixed. Considering that .build() does not imply .store() and vice-versa and buildAndStore() points out the mixing of concerns immediately do the different things in different places and you get the guarantees you want.

Put your call to your persistence code in another method that only accepts a fully constructed instance of Rights.

You could write a rule for PMD or Findbugs if you really want to enforce it in the code. This would have the advantage that it is already available at compile time.

Runtime: If you only want to make sure the users call your builder in the correct order then use separate interfaces for each step.

grantUser() will return ISetPermission which has the method permissionTo(), which will return an IResourceSetter which has the method item()...

You can add all those interfaces to one builder, just make sure that the methods return the correct interface for the next step.

There is now an annotation processing based compiler plugin, that will check that for you, and throw compilation error, if the method is not present: Fluent API sentence end check.

You can either annotate your final method with @End annotation, or if you do not control the class, you can still provide a text file with fully qualified method name(s), and get the check working.

Then Maven can check during compilation.

It only works in Java 8, upwards, because it uses new compiler plugin mechanism, introduced there.

public class MyClass {
    private final String first;
    private final String second;
    private final String third;

    public static class False {}
    public static class True {}

    public static class Builder<Has1,Has2,Has3> {
        private String first;
        private String second;
        private String third;

        private Builder() {}

        public static Builder<False,False,False> create() {
            return new Builder<>();
        }

        public Builder<True,Has2,Has3> setFirst(String first) {
            this.first = first;
            return (Builder<True,Has2,Has3>)this;
        }

        public Builder<Has1,True,Has3> setSecond(String second) {
            this.second = second;
            return (Builder<Has1,True,Has3>)this;
        }

        public Builder<Has1,Has2,True> setThird(String third) {
            this.third = third;
            return (Builder<Has1,Has2,True>)this;
        }
    }

    public MyClass(Builder<True,True,True> builder) {
        first = builder.first;
        second = builder.second;
        third = builder.third;
    }

    public static void test() {
        // Compile Error!
        MyClass c1 = new MyClass(MyClass.Builder.create().setFirst("1").setSecond("2"));

        // Compile Error!
        MyClass c2 = new MyClass(MyClass.Builder.create().setFirst("1").setThird("3"));

        // Works!, all params supplied.
        MyClass c3 = new MyClass(MyClass.Builder.create().setFirst("1").setSecond("2").setThird("3"));
    }
}

There is the step builder pattern that does exactly what you needed : http://rdafbn.blogspot.co.uk/2012/07/step-builder-pattern_28.html

Apply the new permission in a separate step that first validates that the Builder was constructed correctly:

PermissionBuilder builder = permissionManager.grantUser( userId ).permissionTo( Right.READ ).item( docId ).asOf( new Date() );
permissionManager.applyPermission(builder); // validates the PermissionBuilder (ie, was asOf actually called...whatever other business rules)

Apart from using Diezel to generate the whole set of interfaces, is to force them getting the "token" object:


    Grant.permissionTo( permissionManager.User( userId ).permissionTo( Right.READ ).item( docId ).asOf( new Date() ) );

users won't be able to finish the statement until the last/exit method returns the right type. The Grant.permissionTo can be a static method, statically imported, a simple constructor. It will get all it needs to actually register the permission into the permissionManager, so it doesn't need to be configured, or obtained via configuration.

Folks at Guice uses another pattern. They define a "callable", that is used to configure permission (in Guice it's all about binding instead).


    public class MyPermissions extends Permission{

    public void configure(){
    grantUser( userId ).permissionTo( Right.READ ).item( docId ).asOf( new Date() );
    }

    }

    permissionManager.add(new MyPermissions() );

grantUser is a protected method. permissionManager can ensure that MyPermissions only contains fully qualified permissions.

For a single permission this is worst than the first solution, but for a bunch of permission it's cleaner.