I wanted to ask about having the equals method on entities. Im running a setup where Im only comparing entites based on Id and thought this is best way to do things. However I realized that when saving new UserRoles with cascaded persist then I run into problem where they are not unique initally since they dont have id yet. And so I cant add them to the set.
So my question is, is using only id field for equals a good approach? And is using cascades to persist related entitys a good approach? In my case it seems like I have to either add user and role fields to Equals (which I dont want to do beacause I dont want to have to load them eagerly all the time) or just persist the userRoles seperatly.
Context:
@Entity
@Getter
@Setter
@ToString(exclude = {"user", "role"})
@EqualsAndHashCode(onlyExplicitlyIncluded = true, callSuper = false)
@NoArgsConstructor
@AllArgsConstructor
@Table(name="kasutaja_roll")
public class UserRole extends BaseEntity {
@Id
@SequenceGenerator(name = "kasutaja_roll_id_seq", sequenceName = "kasutaja_roll_id_seq", allocationSize = 1)
@GeneratedValue(generator = "kasutaja_roll_id_seq", strategy = GenerationType.SEQUENCE)
@EqualsAndHashCode.Include
private Long id;
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "kasutaja_id")
private User user;
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "roll_id")
private Role role;
@Column(name = "kehtiv_alates")
private Instant validFrom;
@Column(name = "kehtiv_kuni")
private Instant validUntil;
public UserRole(User user, Role role) {
super();
this.validFrom = Instant.now();
this.user = user;
this.role = role;
}
}
@Entity
@Getter
@Setter
@ToString(exclude = {"roles"})
@EqualsAndHashCode(onlyExplicitlyIncluded = true, callSuper = false)
@NoArgsConstructor
@AllArgsConstructor
@Table(name = "kasutaja")
public class User extends BaseEntity{
@Id
@SequenceGenerator(name = "kasutaja_id_seq", sequenceName = "kasutaja_id_seq", allocationSize = 1)
@GeneratedValue(generator = "kasutaja_id_seq", strategy = GenerationType.SEQUENCE)
@EqualsAndHashCode.Include
private Long id;
@OneToMany(mappedBy = "user", cascade={CascadeType.PERSIST, CascadeType.MERGE})
private Set<UserRole> roles;
...
and for example adding them like this:
for (Long roleId : requestDto.getRoles()) {
UserRole existingUserRole = existingUserRolesByRoleId.get(roleId);
if (existingUserRole == null) {
// No existing userRoll - create new one
Role role = roleRepository.findById(roleId)
.orElseThrow(() -> new IllegalArgumentException("Role not found with id: " + roleId));
UserRole newUserRole = new UserRole(user, role);
user.getRoles().add(newUserRole); //!!!here adding multiple ones fails because they are equal, since id = null!!!
} else if (existingUserRole.getValidUntil() != null) {
// Existing userRole is inactive - reactivate it
existingUserRole.setValidUntil(null);
existingUserRole.setValidFrom(now);
}
//existing userRole is active, do nothing
}
So my question is, is using only id field for equals a good approach?
The short answer is no it isn't, the longer answer is it depends and you need to account for new instances.
There are many articles out there (like this one or more in-depth even this) that explain how to write a proper equals
/hashCode
method for JPA. There are some particularities you need to take into account.
Next to that JPA and Lombok are generally not friends is my experience (see this post of mine) and should be avoided. But that might be a personal vendetta.
All that being said you probably should write the equals
/hashCode
yourself. And a basic JPA implementation (with new Java constructs) could be something like this.
@Override
public boolean equals(Object that) {
if (that instanceof UserRole ur) {
return this.id != null && Objects.equals(this.id, that.id)
}
return false;
}
@Override
public int hashCode() {
return getClass().hashCode();
}
Something like this should work. The instanceof
check can be made smarter and you might even provide something like this in your BaseEntity
.