Mixing PersistenceExceptionTranslator logic into FactoryBeans prevents using PersistenceExeptionTranslationPostProcessor with @Configuration classes [SPR-7387]

[spring-projects-issues]

Baruch Sadogursky opened SPR-7387 and commented

This is clear Single responsibility principle violation. The implementation logic of PersistenceExceptionTranslator resides into various FactoryBeans (LocalSessionFactoryBean, LocalEntityManagerFactoryBean, etc) and there is no way to introduce translators to ApplicationContext for PersistenceExeptionTranslationPostProcessor to find them without adding the FactoryBeans to ApplicationContext. It works fine for xml metadata, since we add the FactoryBeans to the context anyway. This is not the case when working with Java @Configuration classes. In them we don't expose FactoryBeans to ApplicationContext, but use them internally in @Bean methods, returning only the result of the getObject() call. This way there are no PersistenceExceptionTranslators in the ApplicationContext and PersistenceExeptionTranslationPostProcessor fails.

---

Affects: 3.0.3

Attachments:

• HibernateExceptionTranslator.java (3.69 kB)

Issue Links:

• 12076 Provide alternatives to using FactoryBean types within @Bean methods ("is superseded by")

Referenced from: commits https://github.com/spring-projects/spring-framework/commit/3bb01ee68b7f421f0d3ee5735a6c383a84483968

[spring-projects-issues]

Baruch Sadogursky commented

Sorry, it should be in CORE, not AOP.

[spring-projects-issues]

Baruch Sadogursky commented

Separated ExceptionTranslator for HibernateExceptions attached.

[spring-projects-issues]

Juergen Hoeller commented

Chris, what's your take on this? I would actually expect the FactoryBeans to be registered with the ApplicationContext even in case of @Bean methods... returning the FactoryBean instance itself rather than calling getObject(), leaving it up to the container to resolve the FactoryBean - and also allowing for PersistenceExceptionTranslator detection.

Juergen

[spring-projects-issues]

Chris Beams commented

Right. The workaround would be to return the FactoryBean from the Bean method, and then everything works in terms of detecting and registering the FactoryBean as an exception translator.

Of course the problem that Baruch is likely experiencing is that he wants to consume the object produced by that FactoryBean within another @Bean definition:

@Bean SessionFactory sessionFactory() {
   // create and configure LSBF
   lsfb.afterPropertiesSet();
   return lsfb.getObject();
}

@Bean FooRepository fooRepository() {
    return new HibernateFooRepository(sessionFactory());
}

It gets awkward if the sessionFactory() method returns type LSFB.

I see a couple of options:

1. (re-)introduce something like ConfigurationSupport, perhaps in the injectable form that Baruch suggested in #10687. Such an implementation could not only handle the dirty work of calling afterPropertiesSet() and other lifecycle methods, but could also run the detection routine to determine if the FactoryBean in question is a PersistenceExceptionTranslator, and if so, register it. This is questionable, though, because the instance in question won't be managed as a spring bean, so it would have to be explicitly registered as a singleton or otherwise hooked into the container. It all feels like a workaround.
2. Go the route that Baruch is suggesting here, and extract out discrete PersistenceExceptionTranslators implementations that can be used by the existing FactoryBeans via composition / delegation, but can also be used indepentently, allowing for the following:

@Bean SessionFactory sessionFactory() {
   // create and configure LSBF
   lsfb.afterPropertiesSet();
   return lsfb.getObject();
}

@Bean FooRepository fooRepository() {
    return new HibernateFooRepository(sessionFactory());
}

@Bean PersistenceExceptionTranslator exceptionTranslator() {
    return new HibernateExceptionTranslator();
}

Option (2) is probably the right way to go, but I don't yet have a sense of the scope. I'll leave it at this for the moment to solicit further comment.

Baruch, do you have a reasonable work-around for the time being? Are you actually using the HibernateExceptionTranslator that you posted?

[spring-projects-issues]

Juergen Hoeller commented

There is actually a whole bunch of implications when returning the FactoryBean's getObject result instead of the FactoryBean itself... Consider destroy methods or start/stop methods on the FactoryBean, none of which will be called. This applies to LocalSessionFactoryBean which will otherwise never call SessionFactory.close()... It's even worse with the Quartz SchedulerFactoryBean though, which has all sorts of lifecycle hooks on the FactoryBean class itself.

I'm afraid the only safe way of defining a FactoryBean is to return the actual FactoryBean instance. Anything else is brittle, with the existing FactoryBean impls, but also considering forward compatibility with changes in Spring-delivered FactoryBean classes. After all, FactoryBeans are not just dumb factories, they are designed to be fully managed bean instances with a fully managed lifecycle, just exposing some other reference for dependency injection...

I see the case where other @Bean methods want to consume the SessionFactory itself. However, I'm afraid we'll have to find some other solution for that problem. Returning FactoryBean.getObject() does not look like a recommendable option for the general case, and not for LocalSessionFactoryBean specifically either.

Juergen

[spring-projects-issues]

Baruch Sadogursky commented

Jurgen, Chris is right, the use-case is passing SessionFactory to, let's say, HibernateTemplate, defined in the same @Configuration class. Calling getObject() is, of course, not enough, it was just a figure of speech. I actually use the injectable ConfigurationSupport, I suggested in #10687. I handles the init method (I think). Considering the destroy method, I register it in destroyMethod attribute in @Bean annotation of the SessionFactory.

I feel very bad with returning FactoryBean. Correct me if I am wrong, but FactoryBeans are in essence,work-around for not being able to write code in XML (beyond constructor, properties and factory-method). The whole idea of Java @Configuration is to overcome this limitation, rendering FactoryBeans obsolete. They are still very useful, since they contain tons of great non-trivial configuration code (as in LSFB in question), but exposing them from my @Bean methods seems very wrong.

Chris, yap, I tried the attached HibernateExceptionTranslator, it works great. Others (LEMFB, etc.) seem simple as well.

[spring-projects-issues]

Baruch Sadogursky commented

P.S. Please move it to Core, I posted it to AOP by mistake.

[spring-projects-issues]

Chris Beams commented

See #12076