Spring Service, Repository, Entity design pattern advice needed

Question

I've got a (hopefully) simple software design question: I want to make my entities (= domain objects which get persisted to DB) to be kinda immutable. Means: Entities should only get created by a service and every other part of the application works with an interface which only has getter methods.

Example:

1. MyController want to retrieve MyEntity with id=5

2. MyController has to ask MyService in order to get the object: myService.getMyEntityById(5)

3. MyService will ask the MyEntityRepository to fetch the Object from DB

4. MyService returns the MyEntityInterface to MyController

Package design:

root
  |--- service   
  |         |--- MyService.java
  |         |--- MyServiceImpl.java
  |         |
  |         |--- MyEntity.java
  |         |--- MyEntityImpl.java
  |         |
  |         |--- MyEntityRepository.java
  |
  |
  |------- web
            |--- MyController.java

Ideas:

My first idea was to simply use a package-protected constructor in MyEntityImpl, but this doesn't work some other libraries I'm using (i.e. Orika). So it must be public.

The next idea was to use the MyEntity interface. But now I've got some problem:

Problems:

The MyService(Impl) has a method called: updateMyEntityData(MyEntity e, Data data). Now I can't be sure inside my service that this MyEntity object is really an instance of MyEntityImpl. Of course I could do an if(e instanceof MyEntityImpl) ... but that's exactly what I don't want to do.

The next problem is: This service method uses the MyEntityRepository which can save and retrieve MyEntityImpl objects, but can't handle the MyEntity interface. As a workaround I could do an additional DB query, but again that's not what I want:

void updateMyEntityData(MyEntity e, Data data) {
  MyEntityImpl impl = repo.findOne(e.getId());
  impl.setData(data);
  repo.saveToDB(impl);
}

This is an unnecessary DB query, because I know that MyEntity is an instance of MyEntityImpl and it's been created by this service, so it must be an object from DB. The other possibility is to use a cast:

void updateMyEntityData(MyEntity e, Data data) {
  MyEntityImpl impl = (MyEntityImpl) e;
  impl.setData(data);
  repo.saveToDB(impl);
}

Summary:

• Only the service is allowed to construct MyEntityImpl
• MyService(Impl) must be able to modify fields of MyEntityImpl afterwards (means: there must be setters)
• Avoid unnecessary DB queries

Thank you in advance!

Answer 1

I've now used a more simple approach:

public class MyEntity {

  MyEntity() {

  }

  @Id
  private ObjectId id;
  public ObjectId getId() { return id; }

  private String someOtherField;
  public String getSomeOtherField() { return someOtherField; }
  setSomeOtherField(String someOtherField) { this.someOtherField = someOtherField; }

}

If the Entity has some "final" fields, it get's a second constructor, because Spring Data throws an exception if it can't map the field names to the constructor parameter names, and this way always works:

public class MyEntity {

  protected MyEntity() {} // this one is for Spring Data,
                          // because it can't map
  MyEntity(Integer i) {   // this constructor param "i"
    this.finalInt = i;    // to a field named "i". (The
  }                       // field is called "finalInt")

  @Id
  private ObjectId id;
  public ObjectId getId() { return id; }

  private Integer finalInt;
  public Integer getFinalInt() { return finalInt; }

  private String someOtherField;
  public String getSomeOtherField() { return someOtherField; }
  setSomeOtherField(String someOtherField) { this.someOtherField = someOtherField; }

}

Package layout is like this:

root
  |--- service   
  |         |--- MyService.java (public interface)
  |         |--- MyServiceImpl.java (package protected class implements MyService)
  |         |
  |         |--- MyEntity.java (public class)
  |         |
  |         |--- MyEntityRepository.java (package protected)
  |
  |
  |------- web
            |--- MyController.java

Now the Controller can't construct own Entity objects (at least not while using the constuctor) and it has to use the Service (which get's wired by Spring to ServiceImpl).

The Repository can't be accessed by the Controller because it's package protected, and thus can only be used by the Service.

It's also only possible for the Service (and Repository) to modify the contents of an Entity because all setters are package protected.

I think this is a pretty nice solution, which can prevent a lot of bad code like

• Repository access inside the Controller code

• Entity modification in Controller and saving to DB without the Service having control over that

• Passing invalid (self-constructed) objects through the application, which have for example no ID.

Of course it's still possible to use reflection to bypass it, BUT that's not the point. The whole thing is not about security, it's about clean code and a well structured application where the data and control flow is clearly defined.

Answer 2

I think you need to get over the public constructor. Given only objects retrieved from the repository/DB can have a valid identity assigned, you can use this to control updates.

Yes, you could guess identities, but you could do a whole load of daft things to work round whatever protection you think you're putting in place - utlimately, I can create an instance and assign the fields with relfection if I choose.

Now, immutability is a more noble goal, at least in a multithreaded environment (if you're not in a environment where multiple threads perform updates then the benefits are less obvious and, imho, not worth the cost).

The problem is immutablity conflicts with domain entities that are typically be mutated. A common approach to this problem is to include a timestamp indicating when the last mutation was committed and to use mutated copy. Here's an example of a clean way to create a mutated copy using the builder pattern:

public MyEntity
{
  private Object identity;
  private long mutated;
  private Data data;

  public MyEntity(Object identity, long mutated, Data data)
  {
    this.identity = identity;
    this.mutated= mutated;
    this.data = data;        
  }

  public Object getIdentity()
  {
    return this.identity;
  }

  public Data getData()
  {
    return this.data;
  }

  public Builder copy()
  {
    return new Builder();
  }

  public class Builder
  {
    private Data data = MyEntity.this.data;

    public Builder setData(Data data)
    {
      this.data = data;  
    }

    public MyEntity build()
    {
      return new MyEntity(MyEntity.this.identity, MyEntity.this.mutated, this.data);
    }
  }
}

The calling code would look like this:

MyEntity mutatedMyEntity = myEntity.copy().setData(new Data()).build();

While this approach keeps things relatively clean, it introduces the problem of multiple threads creating mulitple mutated copies at the same time.

Depending on you exact requirements, this means you'll need to detect conflicts when the changes are committed (your saveToDB method) by checking that the mutated timestamp with the latest version (to avoid two database hits, it's best to do the lot in a stored procedure, though alternatives would be to keep a cache of identities to mutated timestamps in the class performing the writes). The conflict resolution will again be down to your specific requirements as will propogating changes to the other instances of the same entity.