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

Add routing parameter to ElasticsearchOperations [DATAES-644] #1218

Closed
spring-projects-issues opened this issue Aug 27, 2019 · 7 comments · Fixed by #562
Closed

Add routing parameter to ElasticsearchOperations [DATAES-644] #1218

spring-projects-issues opened this issue Aug 27, 2019 · 7 comments · Fixed by #562
Labels
in: core Issues in core support type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

sothawo opened DATAES-644 and commented

The IndexRequest should be able to be configure with a routing argument (Update, delete and search requests as well)


Issue Links:

Referenced from: pull request #562

3 votes, 6 watchers

@spring-projects-issues
Copy link
Author

sothawo commented

This supersedes DATAES-439 as I do not think that having a @Routing annotation will be enough. The annotation is alright for indexing a document, but the routing parameter in the requests is also necessary when updating or getting a document (see The same routing value needs to be provided when getting, deleting, or updating the document.).

So tying the routing to a document helps in indexing, on updating or deleting it is error-prone if the routing property of the document is changed and for getting there normally is no document to be passed in and the routing must be provided in a different way.

So I think we should look for a solution how to provide the routing information to all these calls in a common way

@spring-projects-issues
Copy link
Author

wangqinghuan commented

Sorry, I ignored  find and delete case. The @Routing  annotation can still when delete(T entity).  For the case deleteById and findById, the option I think is add serveral methods such deleteByIdWithRouting(String id,String routing).  And we should suggest user to use deleteByIdWithRouting method  when user want to execute deleteById in the case of @Routing annotation has been declared in entity. What do you think? sothawo

@spring-projects-issues
Copy link
Author

sothawo commented

wangqinghuan: thanks for your contribution. I agree in all cases where we can provide an entity like in index, update, delete; Adding overloaded methods with WithRouting is a solution, but we have to check how this fits not only to the *Template classes, but this must work with *Repository classes as well - and we need to check the reactive classes as well.

I'll have a call with the Spring Data project lead later this week, we'll discuss this then.

What you could do in the meantime:

@spring-projects-issues
Copy link
Author

sothawo commented

Hello, I had a call with the Spring Data Project Lead last week, we also discussed the topic of this issue. Let me shortly summarize this:

  • this routing will be definietelyintegrated
  • for the first step, it's ok, if only the *Template classes support this, support in the *Repository classes can come later.
  • we don't want to have the routing being defined by a annotation on a field, as this is not flexible enough. What if the routing should depend on more than one field of the document? You either have then to be able to handle multiple @Routing annotations on different fields (in which order are they processed) or  need to add a synthesized field to your class. What we think would be better, would be to add a parameter to the @Document annotation of the class that defines the routing. This may then be a single field of the object, or by using SpEL it even could be a separate bean calculating the routing value.

I think I will have time on Monday evening (central european time) to elaborate on the last point and show you will sample code what we mean and how this can be implemented

@spring-projects-issues
Copy link
Author

sothawo commented

Routing definition

Let me outline the idea for defining the routing for an object:

  • We define an interface RoutingAccessor which has a method String getRouting(Object o) and implement a DefaultRoutingAccessor implementing this interface.
  • The ElasticsearchPersistentProperty gets an additional method getRoutingAccessor, which might be defaulting to return null
  • The @Document annotation gets an additional parameter routing of type String which defaults to null
  • When the SimpleElasticsearchPersistentEntity is initialized, the @Document is checked for this attribute. If present and not null or empty, an DefaultRoutingAccessor is created with the parameter value and added to the PersistentEntity.
  • When there routing for an object needs to be determined, the RoutingAccessor from the PersistentEntity is retrieved, and the object is passed in to get the routing

What can the RoutingAccessor do?

  • Basic implementation would take the argument from creation as a field name and get the corresponding value from the object
  • If the argument is a SpEL expression then this expression is evaluated (question: how is the current object used in that?)

What could be done additionally?

  • Only use a DefaultRoutingAccessor, if no bean of type RoutingAccessor is present, this would enable users to just inject their own RoutingAccessor

@spring-projects-issues
Copy link
Author

utkarshupendra commented

Landed here from a  related google search. Is this being worked upon currently? I see that the PR is closed and sothawo has added the outline of the idea to define routing, however, I am not sure if this has been implemented as a part of a separate thread. Can someone please confirm?

@spring-projects-issues
Copy link
Author

sothawo commented

No, this is still an open issue

@spring-projects-issues spring-projects-issues added type: enhancement A general enhancement in: core Issues in core support labels Dec 31, 2020
@sothawo sothawo linked a pull request Jan 2, 2021 that will close this issue
@sothawo sothawo added this to the 4.2 M3 (2021.0.0) milestone Jan 18, 2021
sothawo added a commit that referenced this issue Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core support type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants