1

Recently, I came across the following piece of code:

@Transactional
public MyEntity insert(MyEntity entity) {
    MyEntity merged = entityManager.merge(entity);
    return myEntityRepository.save(merged);
}

where entity manager is defined as follows:

@PersistenceContext private EntityManager entityManager;

and repository is Spring QueryDSL repository:

@Repository
public interface MyEntityRepository extends QueryDslRepository<MyEntity>{
}

My question is, whether is it really necessary to call entityManager.merge(entity) when we are persisting the entity using the myEntityRepository right after? Is there something the entityManager is doing that the repository cannot? Shouldn't calling the repository be enough?

Smajl
  • 7,555
  • 29
  • 108
  • 179
  • It shouldn't be used. The repository does that work if required: https://stackoverflow.com/a/38894500/1199132 – Aritz Jul 30 '18 at 07:53

2 Answers2

2

That looks like cargo cult programming to me. The implementation of save() already does a merge if necessary (and sometimes when it isn't necessary):

/*
 * (non-Javadoc)
 * @see org.springframework.data.repository.CrudRepository#save(java.lang.Object)
 */
@Transactional
public <S extends T> S save(S entity) {

    if (entityInformation.isNew(entity)) {
        em.persist(entity);
        return entity;
    } else {
        return em.merge(entity);
    }
}
Jens Schauder
  • 77,657
  • 34
  • 181
  • 348
  • Does it work when the inserted entity contains some nested entities that need to be updated as well? – Smajl Jul 30 '18 at 12:01
  • Not sure, what you mean by "it" nor, by "work". The intended behavior of `merge` is described in the JPA spec. – Jens Schauder Jul 30 '18 at 12:11
0

As for me this looks as a pretty dangerous code with a bit blurry intention and some over-engineering:

  1. Do you have 100% guarantee to share the same transaction manager between your layer and repository? If not you are in trouble.
  2. You are doing just double work (@Jens answer shows it).
  3. @Transactional here can only make things worse (if you have some non-standard flushing policy). Especially pay attention it will not work if you call method from within the same class as it works via proxy.
  4. If you really intend to insert() (new record) why do you need merge() at all?

My vote is - just use save() as @Jens pointed. If you really need insert() functionality then you may need real transaction with protection from update and in this case I'd do some custom code on repository layer. Hope you do not need it.

Roman Nikitchenko
  • 12,800
  • 7
  • 74
  • 110