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

Provide a dedicated read-only repository interface [DATACMNS-1638] #2064

Closed
spring-projects-issues opened this issue Dec 15, 2019 · 8 comments
Assignees
Labels
in: repository Repositories abstraction status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

Vitor Carvalho opened DATACMNS-1638 and commented

Whenever a developer needs to create a read only repository, it has to extend the Repository interface and then add all the read methods from CrudRepository to it.

It would be useful to have a ReadRepository similar to CrudRepository with all the read only methods.

Optional<T> findById(ID id);

boolean existsById(ID id);

Iterable<T> findAll();

Iterable<T> findAllById(Iterable<ID> ids);

long count();

No further details from DATACMNS-1638

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

This has been discussed a couple of times already and so far we've shied away from introducing additional repository base interfaces as the situation is far from trivial. Let me elaborate a bit.

The hierarchy of repository interfaces as non-trivial already

The repository hierarchy already contains quite a few interfaces for users to choose from:

Repository<T, ID>
CrudRepository<T, ID>
PagingAndSortingRepository<T, ID>
– store-specific extensions

Providing even more of those complicates the picture significantly as we have to explain the tradeoffs for each interface we provide. The more we get opinionated about that the higher the chance that some of the interface almost meets a user's expectations but then again does not quite. We've had requests to remove the count() method as it's an expensive operation for some stores to implement. Or have a dedicated interface that prevents removal operations. So, where do we stop?

As a side note: interestingly we've seen your request from a different angle. Users asking for a repository not containing find operations other than findById(…) to make sure all but aggregate-by-id-lookups are declared using projection types, to essentially move towards a CQRS style application

Most applications craft their own base interface anyway

This is usually done for multiple purposes: one important one is to minimize the number of places with a fundamental Spring Data dependency in the codebase. Another one is aligning all repositories on a certain identifier type. A third one is slightly tweaking return values for common operations (e.g. returning Streamable instead of Iterable from the findAll… flavors available. If we introduce other base interfaces, chances are still high that users might (have to) do that as we cannot (and don't even necessarily want to) solve all those issues for them.

Changing the interface hierarchy is a breaking change

Introducing an interface in between Repository and CrudRepository breaks binary compatibility. I.e. all code currently compiled against e.g. CrudRepository.findById(…) would have to be recompiled. This naturally limits our ability to ship such a change to major revisions.

Also, we'd have to answer the question what interface PagingAndSortingRepository would extend? It only contains reading methods, which'd made it a natural candidate to extend ReadOnlyRepository. That however would break existing code as they'd all of a sudden miss the save(…) methods of CrudRepository. if we continue to let it extend CrudRepository it'd create some inconsistency of a repository interface primarily targeted at reading operations to include writing operations.

Concluding

I wonder how far we could get if we started with a mini library existing outside Spring Data that could try different arrangements based on Repository so that we can gain some experience in real world applications? As long as the methods stay signature compatible with the types we provide out of the box, the new interfaces should be usable without any friction

@spring-projects-issues
Copy link
Author

Vitor Carvalho commented

Thanks for the explanation and actually I agree with everything you said.

Regarding this part "Introducing an interface in between Repository and CrudRepository", I understand that problem, and my suggestion was to have the new ReadOnly side by side with CrudRepository (both extending Repository), not between Repository and CrudRepository.

I didn't quite understand the concluding part though. Are you suggesting to have a separate library to "pack" all these commonly requested dedicated repositories? Like, having a ReadOnlyRepository, and a NoFindRepository, and a WithoutCountRepository, and others that are commonly requested?

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

Yes, a separate library would allow us to explore which subsets of repositories might make sense, alter those as feedback is coming in. Once we gained a bit more confidence, we can then move them over into Spring Data proper

@spring-projects-issues
Copy link
Author

Vitor Carvalho commented

Looks like a good plan to me. How can we proceed?

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

Care to throw the interfaces you have into a repo at Github? Preferably accompanied by a few tests that make use of them to show that the CRUD methods still do what they're supposed to. We can then beat the drum for people to use them and see how it goes

@spring-projects-issues
Copy link
Author

Vitor Carvalho commented

Sorry for the delayed response. Didn't had much free time unfortunately.
Here it is.
Also, here, I've created an example on how to use it with some tests using the Crud and ReadOnly, as you suggested.
-I was trying to use GitHub Actions to publish directly in GitHub Packages repository, but had some issues, and it stopped being published. Not sure why. Do you have any idea/suggestion that can help? Thanks- This is now fixed

@spring-projects-issues spring-projects-issues added status: waiting-for-feedback We need additional information before we can continue in: repository Repositories abstraction type: enhancement A general enhancement labels Dec 30, 2020
@mp911de mp911de added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 30, 2020
@mp911de mp911de assigned mp911de and unassigned odrotbohm Jan 18, 2021
@mp911de mp911de added this to the 3.x milestone Jan 27, 2021
@gregturn
Copy link
Contributor

Duplicates #2610.

gregturn added a commit that referenced this issue Apr 20, 2022
Gather read-only, i.e. "safe" operations into a separate interface, giving users a clear ability to define such repositories themselves.

Closes #2610, #2064.
@gregturn
Copy link
Contributor

Based on discussions with the team, we have decided NOT to implement this. However, you are free to implement it locally in your own project.

Check out this part of the ref docs, where you can see how to define a base repository of your own, e.g. ReadOnlyRepository (or ReadingRepository) where you could have only read-based operations.

@gregturn gregturn added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided labels Apr 21, 2022
@gregturn gregturn self-assigned this Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: repository Repositories abstraction status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants