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

Allow Repositories to be composed of an arbitrary number of implementation classes. [DATACMNS-102] #581

Closed
spring-projects-issues opened this issue Nov 24, 2011 · 6 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link

Michael Hunger opened DATACMNS-102 and commented

Instead of having the current way of needing a MyTypeRepositoryImpl for custom implementations it would be much simpler and flexible to support implementation classes per mixin-interface. This would simplify the core-implementations of repositories by factoring out each of the responsibilities (even for derived/annotated methods which could be implemented in marker interfaces).

And for the end-user it would be easy to understand and realize. (And probably rename the ~Impl suffix to ~Mixin)

so for:

interface PersonRepository extends Repository<Person>, CRUDRepository<Person>, MyRepository<Person> {
}

There would be implementation classes CRUDRepositoryMixin, MyRepositoryMixin with the concrete method implementations.


Issue Links:

  • DATAMONGO-1702 Switch repository implementation to use fragments
    ("is depended on by")

  • DATACMNS-1233 Add CDI support for repository fragments (mixins)

  • DATACMNS-1136 Update reference documentation for composite repositories

  • DATAJPA-1134 Adopt tests to composable repositories

  • DATAJPA-541 Provide ReadOnlyRepository as parent of CrudRepository

  • DATAGRAPH-130 DefaultRepositoryInformation.getQueryMethods() seems to add wrong methods
    ("supersedes")

Referenced from: pull request #222

3 votes, 8 watchers

@spring-projects-issues
Copy link
Author

Mark Paluch commented

Lookup scheme for custom repository implementation types would be:

  • MyRepositoryCustom -> MyRepositoryImpl (compatibility)
  • MyRepository -> MyRepositoryMixin
  • And MyRepositoryCustom -> MyRepositoryCustomMixin could work as well

Currently, custom repository implementation type is detected from the defined repository interface. With the change from above, implementation types would be detected from the mixin interfaces.

RepositoryConfiguration would hold a set of custom repository implementations/mixins and use those for validation/construction of the runtime repository instance.

Could this be a feasible way to support multiple implementation classes?

@spring-projects-issues
Copy link
Author

Sébastien Deleuze commented

I am wondering if we could not use this feature to provide a new way to customize easily builtin operations. For example, if I want to send a notification every time CrudRepository#deleteAll() is invoked, I could provide a mixin class like:

class NotificationRepositoryMixin implements NotificationRepository {
  public void deleteAll() {
    // Send notification ...
  }
}

Spring Data could automatically call this method after CrudRepository one if no exception is thrown.
That would be a nice alternative to event based extensibility and that would avoid to require to write a Spring MVC controller for Spring Data REST users (and I think this is a very common need) who just want to add this kind of small operation (notification, logging, send a message on broker, send an HTTP request) to regular CRUD operations.

We could also use @Order annotations to specify the order of invocation of multiple methods. Spring Data own methods could be considered as annotated with @Order(0) in order to allow custom implementation to be run after of before depending on there own @Order annotations (security is a nice use case for executing methods before Spring Data ones).

Any thoughts?

@spring-projects-issues
Copy link
Author

Greg Turnquist commented

I'm not sure how that is simpler than this:

@Component
@RepositoryEventHandler(Item.class)
public class SpringDataRestEventHandler {

	@HandleAfterDelete
	public void notifyAllClientsAboutItemDeletion(Item item) {
            // send notification
	}

}

The existing mechanism comes with ordering built in and hence no need to leverage @Order.

And exactly where is there any requirement to write a Spring MVC controller to make this work? This leverages Spring app context events, not Spring MVC. If you want your event to get piped over a websocket, then you have to pull in some other components. But I also use this event-based mechanism to publish events over Redis and RabbitMQ to other microservices.

See https://github.com/gregturn/spring-a-gram/blob/master/spring-a-gram-backend/src/main/java/com/greglturnquist/springagram/backend/SpringDataRestEventHandler.java for full details

@spring-projects-issues
Copy link
Author

Sébastien Deleuze commented

I used deleteAll() customization in my example because the need I have in mind is about customizing any kind of repository method implementation, not only resource changes like current event handling mechanism. In my example, I want to send a single notification when deleteAll() is called (to warn the administrator for example), not one notification for each resource deleted. I guess we could find a wide range of other needs where current resource event handling is not suitable (findAll() customization ...).

For these kind of use cases, it seems to me that people currently unplug Spring Data REST to write their own Spring MVC controllers.
So I am wondering if we could improve the level of customization we can apply to Spring Data REST in order to be able to keep Spring Data REST for the kind of use case I described.

That said the mixin mechanism is perhaps not what is needed, since it could not permit to use the return value of the CrudRepository implementation for example. An interceptor/delegate mechanism may be a better fit. Basically the need would be to provide a custom PagingAndSortingRepository or CrudRepository implementation where I could override the method I want (the overriden method should obviously call the super one provided by Spring Data) to add this kind of notification/logging customization.

Is it possible to handle that currently with Spring Data?
If not, should I create a separate issue, or are you aware of an existing one that could match (I had a look but did not find)?

@spring-projects-issues
Copy link
Author

Oliver Drotbohm commented

While I certainly see the connection to the use case in this ticket, there's nothing that makes the implementation of this ticket a prerequisite to implement what you're trying to achieve. The decision not to have any dedicated events on the repository is a deliberate one for the reason that providing them is not possible across all data stores. Stores like JPA support persistence by reachability and thus flush changes to a loaded domain object even if save(…) is not called explicitly.

Thus, to reliably get notified of these events you need to hook into the store specific means, which we usually provide (e.g. for MongoDB etc.). Also, you can already override the standard CRUD methods in the repository base implementation to trigger events, but if you do that you need to be aware of the above mentioned limitation

@spring-projects-issues
Copy link
Author

Sébastien Deleuze commented

Thanks for your feedback Oliver Drotbohm, it makes sense and I am perfectly fine with hooking into store specific events.
I guess I will submit some improvement requests for some (potentially store specific) additional events if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants