0

I'm working on a legacy codebase and saw something unsettling in our custom repositories. EntityManager is autowired into the repos directly. For example:

@Repository
public class OperationRepository {

    // Yikes
    private final EntityManager entityManager;

    private QOperation operation = QOperation.operation;

    @Autowired
    public OperationRepositoryImpl(EntityManager entityManager) {
        this.entityManager = entityManager;
    }

    @Override
    public Optional<Operation> findOneByIdAndType(Long operationId, OperationType type) {
        JPAQuery<Operation> query = new JPAQuery<>(entityManager);
        QOperation qOperation = QOperation.operation;
        query.select(operation).from(operation)
                .where(
                        operation.type.eq(type),
                ...
    }
}

My understanding is that this is dangerous since EntityManager is not threadsafe and is being managed here as a field. I don't see an explicit definition in any of our configuration files to provide an EntityManager bean so am not sure what Spring does in this case.

I am going to start on the fix for using @PersistenceContext here instead but wanted to understand what the impact is on this being in production for so long and what consequences might have come from it.

Would we be limiting our connection pool size to the number of single entitymanagers in our repository beans? Are there any other dangers it could have caused? Or am I making too much of it?

Jens Schauder
  • 77,657
  • 34
  • 181
  • 348
IcedDante
  • 6,145
  • 12
  • 57
  • 100

1 Answers1

0

It really depends on the configuration of your application context, which might get configured by Spring Boot if you are using that, but in general and with a typicals setup this is not a problem.

In such a setup the application context doesn't contain a single (or a limited set of) EntityManager instance but an EntityManagerFactory which can create EntityManager instances on demand. And you don't get injected a normal EntityManager either, but a proxy which will point to different instances upon different calls, typically so that each thread has its own EntityManager and after a web request is completed the next request handled by the same thread gets a fresh instance.

If you are still concerned, I recommend to use a debugger to inspect the EntityManager and confirm that it is indeed a proxy and that it delegates to different actual instances for different threads.

You might also want to take a look at this question about Spring Scopes which is the underlying abstraction for this: Spring Bean Scopes

Jens Schauder
  • 77,657
  • 34
  • 181
  • 348
  • I was not aware of the proxy pattern. Debugger shows: `shared entitymanager proxy for target factory` for the EntityManager – IcedDante Jan 27 '21 at 16:55
  • very interesting. So `OperationRepository` is still a singleton but the EntityManager is a proxy. In that case is this a valid pattern or should the switch still be made to `PersistenceContext` and why? – IcedDante Jan 27 '21 at 16:58
  • This is perfectly valid. I'm not aware of any benefit of using `PersistenceContext` – Jens Schauder Jan 27 '21 at 17:36