Affects: 6.0.11

I have some read-only code paths with important processing time mainly caused by JPA flushes. According to my tests, Spring read-only transactions disables autoflush mechanism while marking any fetched entity as read-only on the Hibernate side.

Applying @Transactional(readonly = true) makes Spring:

  • calls Session.setDefaultReadOnly(true) on Hibernate
  • tells Hibernate to use FlushMode.MANUAL (instead of AUTO)

Session.setDefaultReadOnly(true) makes Hibernate consider any fetched entity as a readonly entity:

  • EntityManager#merge on readonly entity will be silently ignored. Calling EntityManager#flush after the merge will have no influence on the behaviour.
  • EntityManager#persist is still functional. But, since Spring enables FlushMode.MANUAL, it is mandatory to call EntityManager#flush to send the persist command to DB.

Spring read-only transactions look like the perfect fit to improve performances of my case but I think it is not as secure as it could be. A developper could make the mistake of trying to merge a read-only entity during a read-only transaction. According to my tests, calling Session#merge during a read-only transaction is just ignored by Hibernate. The code neither fails nor sends the modifications to the database.

At first, I thought this feature should be implemented in the Hibernate project. But given complexity that could be incured by a situation where a read-write entity cascades on a read-only entity, I think it makes more sense to implement it in the context of Spring read-only transactions. IMHO, in a Spring read-only transaction, we can consider any entity as read-only and therefore implement a safeguard behind the EntityManager interface without the need to consider the cascading case.

I already have implemented this on my side as follow:

/**
 * @author Réda Housni Alaoui
 */
public class ExtendedLocalContainerEntityManagerFactoryBean
    extends LocalContainerEntityManagerFactoryBean {

  @Override
  protected EntityManagerFactory createNativeEntityManagerFactory() throws PersistenceException {
    EntityManagerFactory entityManagerFactory = super.createNativeEntityManagerFactory();
    ClassLoader classLoader =
        Optional.of(entityManagerFactory)
            .filter(EntityManagerFactoryInfo.class::isInstance)
            .map(EntityManagerFactoryInfo.class::cast)
            .map(EntityManagerFactoryInfo::getBeanClassLoader)
            .orElseGet(ExtendedLocalContainerEntityManagerFactoryBean.class::getClassLoader);

    return createExtendedEntityManagerFactory(classLoader, entityManagerFactory);
  }

  private static EntityManagerFactory createExtendedEntityManagerFactory(
      ClassLoader classLoader, EntityManagerFactory entityManagerFactory) {
    return (EntityManagerFactory)
        Proxy.newProxyInstance(
            classLoader,
            ClassUtils.getAllInterfaces(entityManagerFactory),
            new ExtendedEntityManagerFactory(classLoader, entityManagerFactory));
  }

  private record ExtendedEntityManagerFactory(ClassLoader classLoader, EntityManagerFactory target)
      implements InvocationHandler {

    @Override
    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
      switch (method.getName()) {
        case "equals":
          return (proxy == args[0]);
        case "hashCode":
          return hashCode();
        case "toString":
          return "com.cos.framework.data_jpa_data_source.entity_manager.ExtendedEntityManagerFactory";
        case "unwrap":
          Class<?> targetClass = (Class<?>) args[0];
          if (targetClass != null && targetClass.isInstance(proxy)) {
            return proxy;
          }
          break;
        default:
          break;
      }

      Object result;
      try {
        result = method.invoke(target, args);
      } catch (InvocationTargetException e) {
        throw e.getTargetException();
      }

      if ("createEntityManager".equals(method.getName())) {
        return wrapEntityManager(proxy, (EntityManager) result);
      }

      return result;
    }

    private EntityManager wrapEntityManager(
        Object entityManagerFactory, EntityManager entityManager) {
      return (EntityManager)
          Proxy.newProxyInstance(
              classLoader,
              ClassUtils.getAllInterfaces(entityManager),
              new ExtendedEntityManager(entityManagerFactory, entityManager));
    }
  }

  private record ExtendedEntityManager(Object entityManagerFactory, EntityManager target)
      implements InvocationHandler {

    private static final Set<String> WRITE_METHOD_NAMES =
        Set.of("persist", "merge", "remove", "flush");

    @Override
    public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
      switch (method.getName()) {
        case "equals":
          // Only consider equal when proxies are identical.
          return (proxy == args[0]);
        case "hashCode":
          return hashCode();
        case "toString":
          return "com.cos.framework.data_jpa_data_source.entity_manager.ExtendedEntityManager";
        case "unwrap":
          Class<?> targetClass = (Class<?>) args[0];
          if (targetClass != null && targetClass.isInstance(proxy)) {
            return proxy;
          }
          break;
        case "getEntityManagerFactory":
          return entityManagerFactory;
        case "getDelegate":
          return target;
        default:
          break;
      }

      if (WRITE_METHOD_NAMES.contains(method.getName())) {
        assertNotInAReadOnlyTransaction(method);
      }
      try {
        return method.invoke(target, args);
      } catch (InvocationTargetException e) {
        throw e.getTargetException();
      }
    }

    private void assertNotInAReadOnlyTransaction(Method invokedWriteMethod) {
      if (!TransactionSynchronizationManager.isCurrentTransactionReadOnly()) {
        return;
      }
      throw new UnexpectedReadOnlyTransactionException(
          "%s cannot be invoked in a read-only transaction.".formatted(invokedWriteMethod));
    }
  }

  public static class UnexpectedReadOnlyTransactionException extends RuntimeException {

    public UnexpectedReadOnlyTransactionException(String message) {
      super(message);
    }
  }
}

I think the safeguard should be part of the spring-framework. This improvement would allow many framework users to use @Transactional(readonly = true) to improve their performances on read-only cases with a safeguard preventing them from adding silent bugs to their applications.

Comment From: snicoll

There is some discussion in https://github.com/spring-projects/spring-framework/issues/21494 about this. The readOnly flag is also a hint so I wonder how far we can go there.

I think @jhoeller will have to chime in.

Comment From: snicoll

So we've discussed this. JPA doesn't have the notion of a read-only session (as your change above confirms). And there are other use cases where JPA entities can be modified without calling one of those four methods (the most straightforward being loading an entity and modifying it).

More generally, it would be impossible for us to go further than an read-only hint if the underlying infrastructure does not enforce it.