Skip to content

Commit

Permalink
Issue 1197 add memberof operator (#1291)
Browse files Browse the repository at this point in the history
* Hasmember operation added for attribute collection

* Allow memberof operation for only attributes and collection types

* make tags to datastore

* Add exclude tags to test

* import error fix

* Refactor Operator.java

* refactor OperatorTest

* member of accepts all type

* Add extra test case

* Update FilterIT.java

* Add unit test

* unit test for filter dialect

* new unist test for member of filter translator

* compilation fix

* Update pom.xml

Updated CVSS limit to 7 to unblock release.

* Bug fix

Co-authored-by: Chandrasekar Rajasekar <[email protected]>
Co-authored-by: Aaron Klish <[email protected]>
  • Loading branch information
3 people authored May 8, 2020
1 parent 1ce3b3c commit 21a8332
Show file tree
Hide file tree
Showing 21 changed files with 622 additions and 146 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ public static boolean toManyInPathExceptLastPathElement(EntityDictionary diction
.anyMatch(RelationshipType::isToMany);
}

public static boolean isLastPathElementAssignableFrom(EntityDictionary dictionary, Path path, Class<?> clz) {
return path.lastElement()
.map(last ->
clz.isAssignableFrom(
dictionary.getType(last.getType(), last.getFieldName())
))
.orElse(false);
}

public FilterPredicate(PathElement pathElement, Operator op, List<Object> values) {
this(new Path(Collections.singletonList(pathElement)), op, values);
}
Expand Down Expand Up @@ -145,7 +154,7 @@ public <T> T accept(FilterExpressionVisitor<T> visitor) {

@Override
public Predicate apply(RequestScope dictionary) {
return operator.contextualize(getFieldPath(), values, dictionary);
return operator.contextualize(path, values, dictionary);
}

public boolean isMatchingOperator() {
Expand Down
173 changes: 106 additions & 67 deletions elide-core/src/main/java/com/yahoo/elide/core/filter/Operator.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.yahoo.elide.utils.coerce.CoerceUtil;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -111,12 +112,20 @@ public FilterExpression parseGlobalExpression(String path, MultivaluedMap<String
throw new ParseException(String.format("Invalid predicate: %s", filterPredicate));
}

if ((filterPredicate.getOperator().equals(Operator.HASMEMBER)
|| filterPredicate.getOperator().equals(Operator.HASNOMEMBER))
&& !FilterPredicate.isLastPathElementAssignableFrom(
dictionary, filterPredicate.getPath(), Collection.class)) {
throw new ParseException("Invalid Path: Last Path Element has to be a collection type");
}

if (joinedExpression == null) {
joinedExpression = filterPredicate;
} else {
joinedExpression = new AndFilterExpression(joinedExpression, filterPredicate);
}
}

return joinedExpression;
}

Expand Down Expand Up @@ -201,6 +210,10 @@ private void validateFilterPredicate(FilterPredicate filterPredicate) throws Par
case NOTEMPTY:
emptyOperatorConditions(filterPredicate);
break;
case HASMEMBER:
case HASNOMEMBER:
memberOfOperatorConditions(filterPredicate);
break;
default:
if (FilterPredicate.toManyInPath(dictionary, filterPredicate.getPath())) {
throw new ParseException("Invalid toMany join: " + filterPredicate);
Expand All @@ -221,4 +234,14 @@ private void emptyOperatorConditions(FilterPredicate filterPredicate) throws Par
+ filterPredicate);
}
}

private void memberOfOperatorConditions(FilterPredicate filterPredicate) throws ParseException {
if (FilterPredicate.toManyInPath(dictionary, filterPredicate.getPath())) {
throw new ParseException("Invalid toMany join: member of operator cannot be used for toMany relationships");
}

if (!FilterPredicate.isLastPathElementAssignableFrom(dictionary, filterPredicate.getPath(), Collection.class)) {
throw new ParseException("Invalid Path: Last Path Element has to be a collection type");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import cz.jirutka.rsql.parser.ast.RSQLVisitor;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -60,6 +61,8 @@ public class RSQLFilterDialect implements SubqueryFilterDialect, JoinFilterDiale
private static final Pattern TYPED_FILTER_PATTERN = Pattern.compile("filter\\[([^\\]]+)\\]");
private static final ComparisonOperator ISNULL_OP = new ComparisonOperator("=isnull=", false);
private static final ComparisonOperator ISEMPTY_OP = new ComparisonOperator("=isempty=", false);
private static final ComparisonOperator HASMEMBER_OP = new ComparisonOperator("=hasmember=", false);
private static final ComparisonOperator HASNOMEMBER_OP = new ComparisonOperator("=hasnomember=", false);

/* Subset of operators that map directly to Elide operators */
private static final Map<ComparisonOperator, Operator> OPERATOR_MAP =
Expand All @@ -68,6 +71,8 @@ public class RSQLFilterDialect implements SubqueryFilterDialect, JoinFilterDiale
.put(RSQLOperators.GREATER_THAN, Operator.GT)
.put(RSQLOperators.GREATER_THAN_OR_EQUAL, Operator.GE)
.put(RSQLOperators.LESS_THAN_OR_EQUAL, Operator.LE)
.put(HASMEMBER_OP, Operator.HASMEMBER)
.put(HASNOMEMBER_OP, Operator.HASNOMEMBER)
.build();


Expand All @@ -90,6 +95,8 @@ private static Set<ComparisonOperator> getDefaultOperatorsWithIsnull() {
Set<ComparisonOperator> operators = RSQLOperators.defaultOperators();
operators.add(ISNULL_OP);
operators.add(ISEMPTY_OP);
operators.add(HASMEMBER_OP);
operators.add(HASNOMEMBER_OP);
return operators;
}

Expand Down Expand Up @@ -291,7 +298,7 @@ public FilterExpression visit(ComparisonNode node, Class entityType) {
Path path = buildPath(entityType, relationship);

//handles '=isempty=' op before coerce arguments
// ToMany Association is allowed if the operation in Is Empty
// ToMany Association is allowed if the operation is IsEmpty
if (op.equals(ISEMPTY_OP)) {
if (FilterPredicate.toManyInPathExceptLastPathElement(dictionary, path)
&& !allowNestedToManyAssociations) {
Expand All @@ -302,6 +309,16 @@ public FilterExpression visit(ComparisonNode node, Class entityType) {
return buildIsEmptyOperator(path, arguments);
}

if (op.equals(HASMEMBER_OP) || op.equals(HASNOMEMBER_OP)) {
if (FilterPredicate.toManyInPath(dictionary, path)) {
throw new RSQLParseException(
"Invalid toMany join: member of operator cannot be used for toMany relationships");
}
if (!FilterPredicate.isLastPathElementAssignableFrom(dictionary, path, Collection.class)) {
throw new RSQLParseException("Invalid Path: Last Path Element has to be a collection type");
}
}

if (FilterPredicate.toManyInPath(dictionary, path) && !allowNestedToManyAssociations) {
throw new RSQLParseException(String.format("Invalid association %s", relationship));
}
Expand Down Expand Up @@ -406,5 +423,6 @@ private FilterExpression buildIsEmptyOperator(Path path, List<String> arguments)
throw new RSQLParseException(String.format("Invalid value for operator =isempty= '%s'", arg));
}
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ public final boolean ok(T object, RequestScope requestScope, Optional<ChangeSpec
*/
public boolean applyPredicateToObject(T object, FilterPredicate filterPredicate, RequestScope requestScope) {
try {
String fieldPath = filterPredicate.getFieldPath();
com.yahoo.elide.core.RequestScope scope = coreScope(requestScope);
Predicate<T> fn = filterPredicate.getOperator()
.contextualize(fieldPath, filterPredicate.getValues(), scope);
.contextualize(filterPredicate.getPath(), filterPredicate.getValues(), scope);
return fn.test(object);
} catch (Exception e) {
log.error("Failed to apply predicate {}", filterPredicate, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ public String toString() {
assertFalse(isComputed(Editor.class, "badfield"));

assertEquals(
ImmutableSet.of("genre", "language", "title"),
ImmutableSet.of("awards", "genre", "language", "title"),
getFieldsOfType(Book.class, String.class));

assertTrue(isRelation(Book.class, "editor"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -91,23 +92,26 @@ public InMemoryStoreTransactionTest() {
"English",
System.currentTimeMillis(),
Sets.newHashSet(author),
publisher1);
publisher1,
Arrays.asList("Prize1"));

book2 = new Book(2,
"Book 2",
"Science Fiction",
"English",
System.currentTimeMillis(),
Sets.newHashSet(author),
publisher1);
publisher1,
Arrays.asList("Prize1", "Prize2"));

book3 = new Book(3,
"Book 3",
"Literary Fiction",
"English",
System.currentTimeMillis(),
Sets.newHashSet(author),
publisher2);
publisher2,
Arrays.asList());

books.add(book1);
books.add(book2);
Expand Down
Loading

0 comments on commit 21a8332

Please sign in to comment.