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

FMWK-315 Add query methods validation, refactor creating combined repository queries #701

Merged
merged 12 commits into from
Feb 20, 2024

Conversation

agrgr
Copy link

@agrgr agrgr commented Feb 13, 2024

  • Add validation for standard repository query methods
  • API change: add a special QueryParam for unambiguous transferring of arguments when using combined queries
  • API change: combined queries now require explicitly given arguments (e.g., "findBy...Or..." requires arguments for both parts of the query)

Iterator<Object> paramIterator = iterator;
if (isCombinedQuery && iterator.hasNext()) {
Object nextParam = iterator.next();
if (!(nextParam instanceof CombinedQueryParam)) {
Copy link
Member

Choose a reason for hiding this comment

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

if (!(nextParam instanceof CombinedQueryParam) nextQueryParam) ...

qb.setValue3(Value.get(nextParam)); // contains upper limit (inclusive)
} else {
if (op == FilterOperation.EQ) {
throw new IllegalArgumentException("Unsupported arguments '" + value1 + "' and '" + nextParam +
Copy link
Member

Choose a reason for hiding this comment

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

Use String.format

Comment on lines 641 to 649
// PropertyPath prev;
// while (propertyPath.hasNext()) {
// prev = propertyPath;
// propertyPath = propertyPath.next();
// if (propertyPath == null) {
// propertyPath = prev;
// }
// }
// return propertyPath;
Copy link
Member

Choose a reason for hiding this comment

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

Consider removing


private Qualifier getIdEqualsQualifier(Object value1) {
Qualifier qualifier;
if (value1 instanceof String) {
Copy link
Member

Choose a reason for hiding this comment

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

The id is handled as Object downstream. Consider having only the public static Qualifier idEquals(Object id) factory method and remove this if-else chain.

Copy link
Author

Choose a reason for hiding this comment

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

The idEquals() and idIn() methods that require specific type of arguments are public, we can discuss changes within another PR

}
return qualifier;
}

private Qualifier processCollection(Part part, Object value1, Object value2, Iterator<?> parameters,
private Qualifier getIdInQualifier(List<?> ids) {
Qualifier qualifier;
Copy link
Member

Choose a reason for hiding this comment

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

Same as for getIdEqualsQualifier

private Qualifier processMap2Params(Part part, Object value1, Object value2, List<Object> params,
FilterOperation op, String fieldName) {
Object nextParam = params.size() > 0 ? convertIfNecessary(params.get(0)) : null; // nextParam is de facto the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Object nextParam = params.size() > 0 ? convertIfNecessary(params.get(0)) : null; // nextParam is de facto the
Object nextParam = !params.isEmpty() ? convertIfNecessary(params.get(0)) : null; // nextParam is de facto the

}

private boolean queryHasOnlyBothValues(Object value1, Object value2, List<Object> params) {
return params.size() == 0 && value1 != null && value2 != null;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return params.size() == 0 && value1 != null && value2 != null;
return params.isEmpty() && value1 != null && value2 != null;

}
}

private void validateSimplePropertyQueryIn(Object value1, Object value2, String queryPartDescription) {
Copy link
Member

Choose a reason for hiding this comment

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

The method is not in use

Copy link
Member

Choose a reason for hiding this comment

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

  • Use String.format building exception messages
  • Normalize line length

package org.springframework.data.aerospike.query;

/**
* This class stores arguments passed to each part of a combined repository query,
Copy link
Member

Choose a reason for hiding this comment

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

Add code example with a combined query.

@agrgr agrgr marked this pull request as ready for review February 18, 2024 15:21
@agrgr agrgr merged commit bf374b5 into main Feb 20, 2024
9 checks passed
@agrgr agrgr deleted the FMWK-315-add-query-methods-validation branch February 20, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants