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 IndexCoordinates in repositories #2505

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -79,11 +79,13 @@ public Object execute(Object[] parameters) {

Query query = createQuery(parameters);

IndexCoordinates index = elasticsearchOperations.getIndexCoordinatesFor(clazz);
IndexCoordinates index = parameterAccessor
.getIndexCoordinatesOrDefaults(elasticsearchOperations.getIndexCoordinatesFor(clazz));

Object result = null;

if (isDeleteQuery()) {
index = elasticsearchOperations.getIndexCoordinatesFor(clazz);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need this line, index is already set above

result = countOrGetDocumentsForDelete(query, parameterAccessor);
elasticsearchOperations.delete(query, clazz, index);
elasticsearchOperations.indexOps(index).refresh();
Expand Down
Expand Up @@ -98,7 +98,7 @@ private Object execute(ElasticsearchParametersParameterAccessor parameterAccesso
queryMethod.addMethodParameter(query, parameterAccessor, elasticsearchOperations.getElasticsearchConverter());

String indexName = queryMethod.getEntityInformation().getIndexName();
IndexCoordinates index = IndexCoordinates.of(indexName);
IndexCoordinates index = parameterAccessor.getIndexCoordinatesOrDefaults(IndexCoordinates.of(indexName));

ReactiveElasticsearchQueryExecution execution = getExecution(parameterAccessor,
new ResultProcessingConverter(processor));
Expand Down
Expand Up @@ -16,6 +16,7 @@
package org.springframework.data.elasticsearch.repository.query;

import org.springframework.core.MethodParameter;
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
import org.springframework.data.elasticsearch.core.query.RuntimeField;
import org.springframework.data.elasticsearch.core.query.ScriptedField;
import org.springframework.data.repository.query.Parameter;
Expand All @@ -42,7 +43,8 @@ class ElasticsearchParameter extends Parameter {

@Override
public boolean isSpecialParameter() {
return super.isSpecialParameter() || isScriptedFieldParameter() || isRuntimeFieldParameter();
return super.isSpecialParameter() || isScriptedFieldParameter() || isRuntimeFieldParameter()
|| isIndexCoordinatesParameter();
}

public Boolean isScriptedFieldParameter() {
Expand All @@ -52,4 +54,7 @@ public Boolean isScriptedFieldParameter() {
public Boolean isRuntimeFieldParameter() {
return RuntimeField.class.isAssignableFrom(getType());
}
public Boolean isIndexCoordinatesParameter() {
return IndexCoordinates.class.isAssignableFrom(getType());
}
}
Expand Up @@ -15,7 +15,9 @@
*/
package org.springframework.data.elasticsearch.repository.query;

import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
import org.springframework.data.repository.query.ParameterAccessor;
import org.springframework.lang.NonNull;

/**
* @author Christoph Strobl
Expand All @@ -29,4 +31,6 @@ public interface ElasticsearchParameterAccessor extends ParameterAccessor {
* @return
*/
Object[] getValues();

IndexCoordinates getIndexCoordinatesOrDefaults(@NonNull IndexCoordinates defaults);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add javadoc to the new method

}
Expand Up @@ -29,10 +29,11 @@
* @since 3.2
*/
public class ElasticsearchParameters extends Parameters<ElasticsearchParameters, ElasticsearchParameter> {

private final List<ElasticsearchParameter> scriptedFields = new ArrayList<>();
private final List<ElasticsearchParameter> runtimeFields = new ArrayList<>();

private final int indexCoordinatesIndex;

public ElasticsearchParameters(Method method, TypeInformation<?> domainType) {

super(method, parameter -> new ElasticsearchParameter(parameter, domainType));
Expand All @@ -50,17 +51,35 @@ public ElasticsearchParameters(Method method, TypeInformation<?> domainType) {
runtimeFields.add(parameter);
}
}
this.indexCoordinatesIndex = initIndexCoordinatesIndex();
}

private int initIndexCoordinatesIndex() {
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);
Comment on lines +58 to +69
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

}

private ElasticsearchParameter parameterFactory(MethodParameter methodParameter, TypeInformation<?> domainType) {
return new ElasticsearchParameter(methodParameter, domainType);
}


private ElasticsearchParameters(List<ElasticsearchParameter> parameters) {
super(parameters);
this.indexCoordinatesIndex = initIndexCoordinatesIndex();
}


@Override
protected ElasticsearchParameters createFrom(List<ElasticsearchParameter> parameters) {
return new ElasticsearchParameters(parameters);
Expand All @@ -73,4 +92,12 @@ List<ElasticsearchParameter> getScriptedFields() {
List<ElasticsearchParameter> getRuntimeFields() {
return runtimeFields;
}

public boolean hasIndexCoordinatesParameter() {
return this.indexCoordinatesIndex != -1;
}

public int getIndexCoordinatesIndex() {
return indexCoordinatesIndex;
}
}
Expand Up @@ -15,7 +15,13 @@
*/
package org.springframework.data.elasticsearch.repository.query;

import java.util.Arrays;
import java.util.List;

import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
import org.springframework.data.repository.query.Parameters;
import org.springframework.data.repository.query.ParametersParameterAccessor;
import org.springframework.lang.NonNull;

/**
* @author Christoph Strobl
Expand All @@ -25,6 +31,7 @@ class ElasticsearchParametersParameterAccessor extends ParametersParameterAccess
implements ElasticsearchParameterAccessor {

private final Object[] values;
private final ElasticsearchParameters eleasticSearchParameters;

/**
* Creates a new {@link ElasticsearchParametersParameterAccessor}.
Expand All @@ -36,10 +43,20 @@ class ElasticsearchParametersParameterAccessor extends ParametersParameterAccess

super(method.getParameters(), values);
this.values = values;
this.eleasticSearchParameters = method.getParameters();
}

@Override
public Object[] getValues() {
return values;
}


@Override
public IndexCoordinates getIndexCoordinatesOrDefaults(@NonNull IndexCoordinates defaults) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (!eleasticSearchParameters.hasIndexCoordinatesParameter()) {
return defaults;
}
return (IndexCoordinates) getValues()[eleasticSearchParameters.getIndexCoordinatesIndex()];
}
}
Expand Up @@ -340,6 +340,11 @@ private String[] mapParameters(String[] source, ParameterAccessor parameterAcces
return fieldNames.toArray(new String[0]);
}

@Override
public ElasticsearchParameters getParameters() {
return (ElasticsearchParameters) super.getParameters();
}

// region Copied from QueryMethod base class
/*
* Copied from the QueryMethod class adding support for collections of SearchHit instances. No static method here.
Expand Down
Expand Up @@ -142,11 +142,6 @@ public boolean isStreamQuery() {
return true;
}

@Override
public ElasticsearchParameters getParameters() {
return (ElasticsearchParameters) super.getParameters();
}

@Override
protected boolean isAllowedGenericType(ParameterizedType methodGenericReturnType) {
return super.isAllowedGenericType(methodGenericReturnType)
Expand Down
Expand Up @@ -17,12 +17,13 @@

import java.util.Arrays;
import java.util.Iterator;
import java.util.Optional;

import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.ScrollPosition;
import org.springframework.data.domain.Sort;
import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates;
import org.springframework.data.repository.query.ParameterAccessor;
import org.springframework.lang.NonNull;

/**
* Simple {@link ParameterAccessor} that returns the given parameters unfiltered.
Expand Down Expand Up @@ -97,6 +98,11 @@ public Object[] getValues() {
return this.values;
}

@Override
public IndexCoordinates getIndexCoordinatesOrDefaults(@NonNull IndexCoordinates defaults) {
return defaults;
}

/*
* (non-Javadoc)
* @see org.springframework.data.repository.query.ParameterAccessor#findDynamicProjection()
Expand Down