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

AbstractFallbackTransactionAttributeSource should consider repository interface transaction settings as fallback [DATACMNS-1468] #1900

Closed
spring-projects-issues opened this issue Jan 14, 2019 · 4 comments
Assignees
Labels
in: repository status: declined type: enhancement

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Jan 14, 2019

MyeongHyeonLee opened DATACMNS-1468 and commented

AbstractFallbackTransactionAttributeSource inspects the repository interface to allow cutomization of transaction settings for CRUD methods.

If i annotate @Transactional on the repository interface as shown below, Transaction is applied to the overridden delete method and not to the save method of CrudRepository.

 

 

 

@Transactional
public interface SampleRepository extends CrudRepository<Sample, Serializable> {
    @Override
    void delete(Sample entity);
}

 

 

I think,
It would be nice to AbstractFallbackTransactionAttributeSource should consider repository interface transaction settings with RepositoryInformation as fallback

 

related: https://jira.spring.io/browse/DATACMNS-464

 


Affects: 2.1.4 (Lovelace SR4)

Referenced from: pull request #333

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 14, 2019

Oliver Drotbohm commented

I'm not sure I want to follow that argument. If @Transactional is used, why would any of the methods of a super interface be affected by that? As in Spring Data, interface methods trump implementation methods that would override the default transactional settings declared in implementation classes like SimpleJpaRepository, i.e methods like findById(…) would lose their read only flag without that being obvious by any means. We simply cannot do that for backwards compatibility reasons.

In the test case you added in your pull request, why don't you just annotate the implementation method or class with @Transactional?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 15, 2019

MyeongHyeonLee commented

Oliver Drotbohm

I agree that the transaction setting of the implementation should have the highest priority.

I added additional test code to verify this.

https://github.com/spring-projects/spring-data-commons/pull/333/files#diff-529b9719847a4c109fe21d902bb23b8cR94

In my PR, the @Transactional of the interface has the last priority.

I propose this PR for the following reasons.

 

  1. I use the CrudRepository with spring-data-jdbc
  2. The default implementation of CrudRepository in spring-data-jdbc, SimpleJdbcRepository, has no Transaction setting. (SimpleJpaRepository annotated @Transactional(readOnly=true))
  3. if saveAll() is called as below, it is not bound to Transaction.
@Transactional
public interface SampleRepository extends CrudRepository<Sample, String> {
}

List<Sample> sampels = List.of(....);
this.sampleRepository.saveAll(samples);   // No Transaction Bound

     4. CrudRepository is expected to have similar results because Spring's general @Transactional policy applies Transaction.

// Spring Transaction Policy

public interface GenericService<T> {
    void saveAll(List<T> list);
}

@Transactional
public interface SampleService extends GenericService<Sample> {
}

@Service
public class SampleServiceImpl implements SampleService {
    @Override
    public void saveAll(List<Sample> list) {
       // save all
    }
}

List<Sample> sampels = List.of(....);
this.sampleRepository.saveAll(samples); // TransactionBound

 

 

 

@spring-projects-issues spring-projects-issues added status: waiting-for-feedback in: repository type: enhancement labels Dec 30, 2020
@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 6, 2021

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder label Jan 6, 2021
@mp911de mp911de removed status: feedback-reminder status: waiting-for-feedback labels Jan 6, 2021
@mp911de mp911de assigned mp911de and unassigned odrotbohm Feb 15, 2021
@mp911de mp911de added the status: declined label Feb 15, 2021
@mp911de
Copy link
Member

@mp911de mp911de commented Feb 15, 2021

Meanwhile, Spring Data JDBC has annotated their implementation with @Transactional. We'd like to keep the transaction definition lookup as-is and rather fix this kind of issues where they apply.

@mp911de mp911de closed this as completed Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: repository status: declined type: enhancement
Projects
None yet
Development

No branches or pull requests

3 participants