Allow IndexCoordinates in repositories #2505
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spring-projects-issues
added
the
status: waiting-for-triage
sothawo
reviewed
Mar 31, 2023
| @@ -32,8 +30,6 @@ | |||
| import org.springframework.util.Assert; | |||
| import org.springframework.util.ClassUtils; | |||
|
|
|||
| import java.util.Collections; | |||
|
|
|||
| /** | |||
| * AbstractElasticsearchRepositoryQuery | |||
| * | |||
There was a problem hiding this comment.
add yourself to the authors in all the files you change
|
|
||
| Object result = null; | ||
|
|
||
| if (isDeleteQuery()) { | ||
| index = elasticsearchOperations.getIndexCoordinatesFor(clazz); |
There was a problem hiding this comment.
don't need this line, index is already set above
| */ | ||
| Object[] getValues(); | ||
|
|
||
| IndexCoordinates getIndexCoordinatesOrDefaults(@NonNull IndexCoordinates defaults); |
There was a problem hiding this comment.
add javadoc to the new method
| class ElasticsearchParameter extends Parameter { | ||
| static class ElasticsearchParameter extends Parameter { | ||
|
|
||
| private final boolean indexCoordinatesParameter; |
There was a problem hiding this comment.
Suggested change
| private final boolean indexCoordinatesParameter; | |
| private final boolean isIndexCoordinatesParameter; |
Comment on lines
+49
to
+69
| int index = 0; | ||
| List<Integer> foundIndices = new ArrayList<>(); | ||
| for (ElasticsearchParameter parameter : this) { | ||
| if (parameter.isIndexCoordinatesParameter()) { | ||
| foundIndices.add(index); | ||
| } | ||
| index++; | ||
| } | ||
| if (foundIndices.size() > 1) { | ||
| throw new IllegalArgumentException(this + " can only contain at most one IndexCoordinates parameter."); | ||
| } | ||
| return foundIndices.isEmpty() ? -1 : foundIndices.get(0); |
There was a problem hiding this comment.
alternate way without a list
int indexCoordinatesIndex = -1;
int index = 0;
for (ElasticsearchParameter parameter : this) {
if (parameter.isIndexCoordinatesParameter()) {
if(indexCoordinatesIndex != -1) {
throw new IllegalArgumentException(this + " can only contain at most one IndexCoordinates parameter.");
} else {
indexCoordinatesIndex = index;
}
}
index++;
}
return indexCoordinatesIndex;
| @Override | ||
| public Object[] getValues() { | ||
| return values.toArray(); | ||
| } | ||
|
|
||
| @Override | ||
| public IndexCoordinates getIndexCoordinatesOrDefaults(@NonNull IndexCoordinates defaults) { |
There was a problem hiding this comment.
Suggested change
| public IndexCoordinates getIndexCoordinatesOrDefaults(@NonNull IndexCoordinates defaults) { | |
| public IndexCoordinates getIndexCoordinates(@NonNull IndexCoordinates defaults) { |
I would not put that in the method name in addition to the parameter name
|
Hi, any progress with this one? I could help with integration tests if needed :) |
@Bartolg sitting in my inbox, contribution of tests would be welcome |
It is common to have multiple indexes with the same document structure. E.g. rolling indices over time and having a wildcard alias. Sometimes one wants to select different IndexCoordinates than specified in `@Document(indexName = "...", ...)`. Also you might want to use the same Repository to do cross cluster searches and for this need to specify the IndexCoordinates. It would therefore be helpful to support IndexCoordinates as an argument to repository methods the same way ScrollPosition, Sort and Pageable are handled. This PR is an attempt to introduce this change. I keep it as draft to get some comments I can add tests to it if we can agree on the change being valid.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is common to have multiple indexes with the same document structure. E.g. rolling indices over time and having a wildcard alias. Sometimes one wants to select different IndexCoordinates than specified in
@Document(indexName = "...", ...). Also you might want to use the same Repository to do cross cluster searches and for this need to specify the IndexCoordinates.It would therefore be helpful to support IndexCoordinates as an argument to repository methods the same way ScrollPosition, Sort and Pageable are handled.
This PR is an attempt to introduce this change. I keep it as draft to get some comments I can add tests to it if we can agree on the change being valid and something the spring-data-elasticsearch sees as useful.
com/spring-projects/spring-data-elasticsearch/issues). Add the issue number to the Closes #issue-number line below
Closes #2506