Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@EnableJpaRepositories activates exception translation for @Repository classes not in basePackages [DATAJPA-330] #737

Closed
spring-projects-issues opened this issue Apr 17, 2013 · 7 comments
Assignees
Labels

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Apr 17, 2013

Ed Belisle opened DATAJPA-330 and commented

I have @Repository DAO classes that are not Spring Data repositories and are not part of the basePackages passed to @EnableJpaRepsitories(basePackages={}). They are incorrectly being activated for Spring DataAcessException translation and fail to autowire.

@Repository annotated classes in the basePackages passed to @EnableJpaRepsitories(basePackages={}) are correctly being activated for Spring DataAcessException translation.

After reading DATAJPA-112 and DATAJPA-221, I understand that @Repository makes a class eligible for Spring DataAccessException translation, but the basePackages paremeter of @EnableJpaRepositories implies I have control over which classes are translated.


Affects: 1.3 GA

Issue Links:

  • DATACMNS-318 Introduce dedicated annotation to enable exception translation
    ("depends on")

  • DATAJPA-112 jpa:repositories changes @Repository objects into proxies. Autowiring fails

  • DATAJPA-221 <jpa:repositories base-package="..." /> hides legacy @Repository annotated beans

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 18, 2013

Ranie Jade Ramiso commented

@EnableJpaRepositories basePackages parameter does not control which classes are selected for translation. It only controls which packages to scan for annotated components

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 18, 2013

Oliver Drotbohm commented

As stated in the referenced tickets, annotating a class with @Repository expresses that it is eligible to be proxied for exception translation. Thus, you shouldn't refer to these classes directly but only through its interfaces.

Consider @Repository equivalent to @Component plus exception translation. Thus, if you don't want the class to be proxied, annotate them with @Component instead of @Repository.

The basePackage… attributes in @EnableJpaRepository do not deal with exception translation at all. They're solely responsible to detect repository interfaces (as documented). The activation of exception translation is also documented in the reference documentation

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 18, 2013

Ed Belisle commented

I've revised my Description about my expectation to change 'selected' to 'packages containing eligible'. I expanded my description of basePackages to clarify it selects packages containing @Repository classes. basePackages does not select @Repository classes directly.

We agree that a @Repository class not in basePackages should not have exception translation activated

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 18, 2013

Oliver Drotbohm commented

No, we don't :). We register a PersistenceExceptionTranslatorBeanPostProcessor that enables exception translation for beans annotated with @Repository application context wide (as documented in the Spring Data reference documentation as well as the JavaDoc of @Repository). Again, the sole purpose of @Repository over @Component is exception translation. If you don't want to get that, don't use @Repository

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 18, 2013

Ed Belisle commented

I know from testing basePackages is required because without it my Repository Dao classes doesn't register.

I know from API Documentation EnableJpaRepositories scans basePackages for SpringData repositories.

I know now that in addition to scanning package(s) for Spring Data repositories,
@EnableJpaRepositories has an additional side effect affecting classes outside of the basePackages. Specificly, it "registers a PersistenceExceptionTranslatorBeanPostProcessor that enables exception translation for beans annotated with @Repository application context wide."

Can you add something to the API Documentation for @EnableJpaRpositories? IMHO, "Enable JPA repositories" is not enough information. Suggestions for additions:

  • Registers a PersistenceExceptionTranslatorBeanPostProcessor.
  • PersistenceExceptionTranslatorBeanPostProcessor registered by @EnableJpaRepositories is not limited to basePackages specified for EnableJpaRepositories.
  • All your @Repository will belong to us (PersistenceExceptionTranslatorBeanPostProcessor) :)

I think with at least a reference to PersistenceExceptionTranslatorBeanPostProcessor in the API Documentation @EnableJpaRpositories, I may have stumbled upon PersistenceExceptionTranslatorBeanPostProcessor's greedy behavior. I saw PETBPP referenced in the @Reference description, but didn't follow up on it because I hadn't made the connection between PETBPP and @EnableJpaRpositories

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 19, 2013

Oliver Drotbohm commented

Okay, it's probably a bit of a reality distortion field from my side here. We documented the behavior in the XML namespace part of the documentation. But that might not be sufficient for developers starting with JavaConfig right away.

It turns out the PersistenceExceptionTranslationPostProcessor can be configured with a dedicated annotation to apply exception translation. So I've created DATACMNS-318 to introduce a special annotation so that we can then use the newly introduced annotation on the repository base class implementations and configure the PETPP to inspect only that annotation. That should essentially change the behavior to what you expected initially.

I'll use this ticket here then to refer to from changes made to adapt to the new annotation

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 24, 2013

Oliver Drotbohm commented

I've removed the registration of the PETPP entirely as we already apply exception translation on the repository proxy level. I've straightened out some of these bits in Spring Data Commons and fixed the issue here in ace028f. Feel free to give the latest snapshot a spin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants