I would like to know if the solution I came up with for managing relationships between entities is sensible. Guys in my team feel it is transparent, but maybe I'm missing a pitfall somewhere?
Consider the following situation:
@Entity
public class User {
@OneToMany(mappedBy = "user", cascade = ALL, orphanRemoval = true)
private List<Permission> permissions = new Arraylist();
// getters and setters...
}
@Entity
public class Permission {
@ManyToOne(fetch = LAZY, cascade = {PERSIST, MERGE, REFRESH})
@JoinColumn(name = "user_id", nullable = false)
private User user;
// getters and setters...
}
@Service
public class UserService {
// Just an interface extending JpaRepository<User, Long>
private final UserRepository repository;
private final PermissionService permissionService;
@Inject
public UserService(UserRepository repository, PermissionService permissionService) {
this.repository = repository;
this.permissionService = permissionService;
}
// Represents an update scenario
public User save(User user) {
// Find managed copy of the user
User managed = repository.findOne(user.getId());
// Ensure the user entity's permissions are consistent
if(isNotEmpty(user.getPermissions()) {
List<Permission> detached = user.getPermissions();
List<Permission> attached = new ArrayList();
for(Permission permission : detached) {
attached.add(permissionService.get(permission.getId());
}
managed.setPermissions(attached);
}
user = managed;
return repository.save(user);
}
// other service methods...
}
@Service
public class PermissionService {
private final PermissionRepository repository;
private final UserService userService;
@Inject
public PermissionService(PermissionRepository repository, UserService userService) {
this.repository = repository;
this.userService = userService;
}
public User save(Permission permission) {
// Similar code to UserService.save(User) employing UserService.get(Long id) to find the required user
}
// other service methods...
}
As you can see there is a circular dependency between the services. It is necessary for updating a user, for example, because the user to be saved must have permission entities which are managed by the entity manager. One could argue that setter injection could "solve" this, but it is not a real solution as the service should be constructed completely.
Here is the idea: using property change listeners to manage entity attached/detached state in a transparent manner.
Step 1: Add property support to the Entity:
@Entity
public class User {
// same as before plus:
@Transient
private PropertyChangeSupport support = new PropertyChangeSupport(this);
public synchronized void addPropertyChangeListener(PropertyChangeListener listener) {
this.support.addPropertyChangeListener(listener);
}
public void setPermissions(List<UserPermission> permissions) {
List<UserPermission> old = new ArrayList<>(this.permissions);
this.permissions.clear();
for(UserPermission permission : permissions) {
addPermission(permission);
}
support.firePropertyChange("permissions", old, permissions);
}
public void addPermission(Permission permission) {
this.permissions.add(permission);
if(permission.getUser() != this) {
permission.setUser(this);
}
}
}
In UserService:
@Service
public class UserService {
// Just an interface extending JpaRepository<User, Long>
private final UserRepository repository;
private final ApplicationContext context;
@Inject
public UserService(UserRepository repository, ApplicationContext context) {
this.repository = repository;
this.context = context;
}
// Represents an update scenario
public User save(User user) {
// Find managed copy of the user
User managed = repository.findOne(user.getId());
// Add a property listener for Permissions
managed.addPropertyChangeListener(context.getBean(PermissionChangeListener.class));
managed.setPermissions(user.getPermissions());
user = managed;
return repository.save(user);
}
// other service methods...
}
And the new PermissionChangeListener class which will be responsible with managing whether the entity is managed by the entity manager:
// An incomplete example which simply checks if there are any unmanaged Permissions in the source User, if there are then retrieve those and attach them to the user
@Component
public class PermissionChangeListener implements PropertyChangeListener {
private static final Logger logger = LoggerFactory.getLogger(PermissionChangeListener.class);
@PersistenceContext
private final EntityManager manager;
private final PermissionService service;
@Inject
public PermissionChangeListener(EntityManager manager, PermissionService service) {
this.manager = manager;
this.service = service;
}
@Override
public void propertyChange(PropertyChangeEvent event) {
if("permissions".equals(event.getPropertyName())) {
setPermissions(event);
}
}
private void setPermissions(PropertyChangeEvent event) {
List<Permission> values = (List<Permission>) event.getNewValue();
User user = (User) event.getSource();
if(user != null && CollectionUtils.isNotEmpty(values) && !isManaged(values)) {
// Set the permissions to empty to clear permissions in ORM
user.setPermissions(new ArrayList<>(0));
List<Permission> managed = new ArrayList<>(values.size());
for(int i = 0; i < values.size(); i++) {
Permission value = values.get(i);
logger.debug("[{}]:{}", (manager.contains(value)) ? "attached" : "detached", value);
if(!isManaged(value)) {
value = service.get(value.getId());
}
managed.add(value);
}
user.setPermissions(managed);
}
}
private Boolean isManaged(Permission permission) {
return permission != null && manager.contains(permission);
}
private Boolean isManaged(List<Permission> permissions) {
for(Permission permission : permissions) {
if(!isManaged(permission)) {
return false;
}
}
return true;
}
}
As you can see we seem to have a few wins:
- The service code becomes much simpler;
- The management of ORM functionality is collected in a single place;
- Different relationships are encapsulated into their own ChangeListeners;
- We solve the circular dependecy Constructor Injection on the service level by following the advise here: Circular Dependency in constructors and Dependency Injection;
- We avoid setter injection which is completely unecessary here as the service should not be reconfigured at runtime as discussed here: Why I Changed My Mind About Field Injection?;
- PermissionChangeListener can be further improved to deal with single addition of permissions (User.addPermission(Permission)) and other property changes (removing permissions, etc...). It could also create Permissions on-the-fly if the permission does not exist in the database.
- The
ChangeListenercan be further improved with Typed Change Events.
I would like to know if the idea is coherent and makes sense (from my tests it seems to be working), if I should be synchronizing the User's setters as I'm now updating properties in the listener and if there would be a problem with multiple entity updates (they should all be happening in transactions I guess).
