Property Change Listener for Entity Management

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 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
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 detached = user.getPermissions();
List 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 = use

The problem is not in your Services. The problem is the circular dependency you have modeled from Permissions to Users:

if (permission.getUser() != this) {
    permission.setUser(this);
}

This circular dependency permeates into your Services, which is completely understandable. But to resolve it you'll need to go one level of abstraction deeper.

Overall what you seem to have here is a Many-to-Many relationship gone awry.

If you want allow a User to have multiple Permissions and allow tracing Permissions back to users you'll have to work with a mapping table. Unfortunately from reading your code I can't quite discern whether I'm correct, accordingly this is ... guesswork.

Nitpicks:

• You should return primitives instead of objects where possible, because it makes it abundantly clear that a method cannot return null (relevant to isManaged)

• You are using copious amounts of whitespace. I personally dislike code having overly much whitespace and I'd remove about two thirds of the empty lines you have there.

• Use an enhanced for-loop in PermissionChangeListener#setPermissions. you're not using the index for anything but retrieving the element. It's simpler to use higher-level constructs.

• You're not using isManaged(List) anywhere in the code you've shown.

StackExchange Code Review Q#81995, answer score: 3