Skip to content

Commit

Permalink
Introduce case insensitive equality rsql operator. Default is case se…
Browse files Browse the repository at this point in the history 
…nsitive (#1519)

* introduce case insensitive equality rsql operator. Default is case sensitive

* refactoring

Co-authored-by: Chandrasekar Rajasekar <[email protected]>
  • Loading branch information
@Chandrasekar-Rajasekar
Chandrasekar-Rajasekar and Chandrasekar Rajasekar authored Sep 1, 2020
1 parent be2d25b commit d80e181
Show file tree
Hide file tree
Showing 12 changed files with 174 additions and 67 deletions.
52 changes: 36 additions & 16 deletions elide-core/src/main/java/com/yahoo/elide/core/filter/dialect/RSQLFilterDialect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.yahoo.elide.core.Path;
import com.yahoo.elide.core.exceptions.InvalidValueException;
import com.yahoo.elide.core.filter.FilterPredicate;
import com.yahoo.elide.core.filter.InInsensitivePredicate;
import com.yahoo.elide.core.filter.InPredicate;
import com.yahoo.elide.core.filter.IsEmptyPredicate;
import com.yahoo.elide.core.filter.IsNullPredicate;
Expand Down Expand Up @@ -59,6 +60,8 @@ public class RSQLFilterDialect implements SubqueryFilterDialect, JoinFilterDiale
private static final String SINGLE_PARAMETER_ONLY = "There can only be a single filter query parameter";
private static final String INVALID_QUERY_PARAMETER = "Invalid query parameter: ";
private static final Pattern TYPED_FILTER_PATTERN = Pattern.compile("filter\\[([^\\]]+)\\]");
private static final ComparisonOperator INI = new ComparisonOperator("=ini=", true);
private static final ComparisonOperator NOT_INI = new ComparisonOperator("=outi=", true);
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);
Expand All @@ -81,7 +84,7 @@ public class RSQLFilterDialect implements SubqueryFilterDialect, JoinFilterDiale
private final CaseSensitivityStrategy caseSensitivityStrategy;

public RSQLFilterDialect(EntityDictionary dictionary) {
this(dictionary, new CaseSensitivityStrategy.FIQLCompliant());
this(dictionary, new CaseSensitivityStrategy.UseColumnCollation());
}

public RSQLFilterDialect(EntityDictionary dictionary, CaseSensitivityStrategy caseSensitivityStrategy) {
Expand All @@ -93,6 +96,8 @@ public RSQLFilterDialect(EntityDictionary dictionary, CaseSensitivityStrategy ca
//add rsql isnull op to the default ops
private static Set<ComparisonOperator> getDefaultOperatorsWithIsnull() {
Set<ComparisonOperator> operators = RSQLOperators.defaultOperators();
operators.add(INI);
operators.add(NOT_INI);
operators.add(ISNULL_OP);
operators.add(ISEMPTY_OP);
operators.add(HASMEMBER_OP);
Expand Down Expand Up @@ -342,46 +347,61 @@ public FilterExpression visit(ComparisonNode node, Class entityType) {
.map((argument) -> (Object) CoerceUtil.coerce(argument, relationshipType))
.collect(Collectors.toList());


if (op.equals(RSQLOperators.EQUAL) || op.equals(RSQLOperators.IN)) {
return equalityExpression(arguments.get(0), path, values);
}
if (op.equals(RSQLOperators.NOT_EQUAL) || op.equals(RSQLOperators.NOT_IN)) {
return new NotFilterExpression(equalityExpression(arguments.get(0), path, values));
}
if (OPERATOR_MAP.containsKey(op)) {
return equalityExpression(arguments.get(0), path, values, true);
} else if (op.equals(INI)) {
return equalityExpression(arguments.get(0), path, values, false);
} else if (op.equals(RSQLOperators.NOT_EQUAL) || op.equals(RSQLOperators.NOT_IN)) {
return new NotFilterExpression(equalityExpression(arguments.get(0), path, values, true));
} else if (op.equals(NOT_INI)) {
return new NotFilterExpression(equalityExpression(arguments.get(0), path, values, false));
} else if (OPERATOR_MAP.containsKey(op)) {
return new FilterPredicate(path, OPERATOR_MAP.get(op), values);
}

throw new RSQLParseException(String.format("Invalid Operator %s", op.getSymbol()));
}

private FilterExpression equalityExpression(String argument, Path path, List<Object> values) {
private FilterExpression equalityExpression(String argument, Path path,
List<Object> values, boolean caseSensitive) {
boolean startsWith = argument.startsWith("*");
boolean endsWith = argument.endsWith("*");
if (startsWith && endsWith && argument.length() > 2) {
String value = argument.substring(1, argument.length() - 1);
return new FilterPredicate(path, caseSensitivityStrategy.mapOperator(Operator.INFIX),
Collections.singletonList(value));
Operator op = caseSensitive
? caseSensitivityStrategy.mapOperator(Operator.INFIX)
: Operator.INFIX_CASE_INSENSITIVE;
return new FilterPredicate(path, op, Collections.singletonList(value));
}
if (startsWith && argument.length() > 1) {
String value = argument.substring(1, argument.length());
return new FilterPredicate(path, caseSensitivityStrategy.mapOperator(Operator.POSTFIX),
Collections.singletonList(value));
Operator op = caseSensitive
? caseSensitivityStrategy.mapOperator(Operator.POSTFIX)
: Operator.POSTFIX_CASE_INSENSITIVE;
return new FilterPredicate(path, op, Collections.singletonList(value));
}
if (endsWith && argument.length() > 1) {
String value = argument.substring(0, argument.length() - 1);
return new FilterPredicate(path, caseSensitivityStrategy.mapOperator(Operator.PREFIX),
Collections.singletonList(value));
Operator op = caseSensitive
? caseSensitivityStrategy.mapOperator(Operator.PREFIX)
: Operator.PREFIX_CASE_INSENSITIVE;
return new FilterPredicate(path, op, Collections.singletonList(value));
}

Boolean isStringLike = path.lastElement()
.map(e -> e.getFieldType().isAssignableFrom(String.class))
.orElse(false);
if (isStringLike) {
return new FilterPredicate(path, caseSensitivityStrategy.mapOperator(Operator.IN), values);
Operator op = caseSensitive
? caseSensitivityStrategy.mapOperator(Operator.IN)
: Operator.IN_INSENSITIVE;
return new FilterPredicate(path, op, values);
}

return new InPredicate(path, values);
return caseSensitive
? new InPredicate(path, values)
: new InInsensitivePredicate(path, values);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions ...core/src/test/java/com/yahoo/elide/core/VerifyFieldAccessFilterExpressionVisitorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,11 +389,11 @@ public void testCustomFilterJoin() throws Exception {

assertEquals("Book", pathElement.getType().getSimpleName());
assertEquals(GENRE, filterPredicate.getField());
assertEquals("book.genre IN_INSENSITIVE [foo]", filterPredicate.toString());
assertEquals("book.genre IN [foo]", filterPredicate.toString());

// custom processing
return "Book".equals(pathElement.getType().getSimpleName())
&& filterPredicate.toString().matches("book.genre IN_INSENSITIVE \\[\\w+\\]")
&& filterPredicate.toString().matches("book.genre IN \\[\\w+\\]")
&& reason.getLoggedMessage().matches(".*Message=ReadPermission Denied.*")
? ExpressionResult.DEFERRED
: ExpressionResult.FAIL;
Expand Down
86 changes: 74 additions & 12 deletions elide-core/src/test/java/com/yahoo/elide/core/filter/dialect/RSQLFilterDialectTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,30 @@ public void testTypedExpressionParsing() throws Exception {
"books.title=in=(foo,bar,baz)"
);


Map<String, FilterExpression> expressionMap = dialect.parseTypedExpression("/author", queryParams, NO_VERSION);

assertEquals(2, expressionMap.size());
assertEquals(
"((book.title INFIX_CASE_INSENSITIVE [foo] AND NOT (book.title PREFIX_CASE_INSENSITIVE [bar])) "
+ "AND (book.genre IN_INSENSITIVE [sci-fi, action] OR book.publishDate GT [123]))",
"((book.title INFIX [foo] AND NOT (book.title PREFIX [bar])) "
+ "AND (book.genre IN [sci-fi, action] OR book.publishDate GT [123]))",
expressionMap.get("book").toString()
);

assertEquals(
"author.books.title IN [foo, bar, baz]",
expressionMap.get("author").toString()
);

queryParams.clear();
queryParams.add(
"filter[author]",
"books.title=ini=(foo,bar,baz)"
);

// Case Insensitive
expressionMap = dialect.parseTypedExpression("/author", queryParams, NO_VERSION);
assertEquals(1, expressionMap.size());
assertEquals(
"author.books.title IN_INSENSITIVE [foo, bar, baz]",
expressionMap.get("author").toString()
Expand All @@ -80,13 +95,13 @@ public void testGlobalExpressionParsing() throws Exception {

queryParams.add(
"filter",
"title==*foo*;authors.name==Hemingway"
"title==*foo*;authors.name=ini=Hemingway"
);

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals(
"(book.title INFIX_CASE_INSENSITIVE [foo] AND book.authors.name IN_INSENSITIVE [Hemingway])",
"(book.title INFIX [foo] AND book.authors.name IN_INSENSITIVE [Hemingway])",
expression.toString()
);
}
Expand All @@ -102,7 +117,7 @@ public void testEqualOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("book.title IN_INSENSITIVE [Hemingway]", expression.toString());
assertEquals("book.title IN [Hemingway]", expression.toString());
}

@Test
Expand All @@ -116,7 +131,7 @@ public void testNotEqualOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("NOT (book.title IN_INSENSITIVE [Hemingway])", expression.toString());
assertEquals("NOT (book.title IN [Hemingway])", expression.toString());
}

@Test
Expand All @@ -130,6 +145,20 @@ public void testInOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("book.title IN [Hemingway]", expression.toString());
}

@Test
public void testInInsensitiveOperator() throws Exception {
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();

queryParams.add(
"filter",
"title=ini=Hemingway"
);

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("book.title IN_INSENSITIVE [Hemingway]", expression.toString());
}

Expand All @@ -144,6 +173,20 @@ public void testOutOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("NOT (book.title IN [Hemingway])", expression.toString());
}

@Test
public void testOutInsensitiveOperator() throws Exception {
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();

queryParams.add(
"filter",
"title=outi=Hemingway"
);

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("NOT (book.title IN_INSENSITIVE [Hemingway])", expression.toString());
}

Expand Down Expand Up @@ -180,6 +223,25 @@ public void testSubstringOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals(
"((book.title INFIX [Hemingway] "
+ "OR book.title POSTFIX [Hemingway]) "
+ "OR book.title PREFIX [Hemingway])",
expression.toString()
);
}

@Test
public void testSubstringCIOperator() throws Exception {
MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();

queryParams.add(
"filter",
"title=ini=*Hemingway*,title=ini=*Hemingway,title=ini=Hemingway*"
);

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals(
"((book.title INFIX_CASE_INSENSITIVE [Hemingway] "
+ "OR book.title POSTFIX_CASE_INSENSITIVE [Hemingway]) "
Expand All @@ -194,14 +256,14 @@ public void testExpressionGrouping() throws Exception {

queryParams.add(
"filter",
"title==foo;(title==bar;title==baz)"
"title=ini=foo;(title==bar;title==baz)"
);

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals(
"(book.title IN_INSENSITIVE [foo] AND (book.title IN_INSENSITIVE [bar] "
+ "AND book.title IN_INSENSITIVE [baz]))",
"(book.title IN_INSENSITIVE [foo] AND (book.title IN [bar] "
+ "AND book.title IN [baz]))",
expression.toString()
);
}
Expand Down Expand Up @@ -273,7 +335,7 @@ public void testVisit() {
RSQLFilterDialect.RSQL2FilterExpressionVisitor visitor = dialect.new RSQL2FilterExpressionVisitor(true);

assertEquals(
"author.id INFIX_CASE_INSENSITIVE [20]",
"author.id INFIX [20]",
visitor.visit(comparisonNode, Author.class).toString()
);
}
Expand Down Expand Up @@ -303,7 +365,7 @@ public void testFilterOnCustomizedStringIdField() throws ParseException {

FilterExpression expression = dialect.parseGlobalExpression("/stringForId", queryParams, NO_VERSION);

assertEquals("stringId.surrogateKey INFIX_CASE_INSENSITIVE [identifier]", expression.toString());
assertEquals("stringId.surrogateKey INFIX [identifier]", expression.toString());
}

@Test
Expand All @@ -318,7 +380,7 @@ public void testInfixFilterOnPrimitiveIdField() throws ParseException {
FilterExpression expression = dialect.parseGlobalExpression("/primitiveTypeId", queryParams, NO_VERSION);

assertEquals(expression.toString(),
"primitiveId.primitiveId INFIX_CASE_INSENSITIVE [1]"
"primitiveId.primitiveId INFIX [1]"
);
}

Expand Down
22 changes: 11 additions & 11 deletions ...alectWithColumnCollationStrategyTest.java → ...DialectWithFIQLCompliantStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
/**
* Tests RSQLFilterDialect
*/
public class RSQLFilterDialectWithColumnCollationStrategyTest {
public class RSQLFilterDialectWithFIQLCompliantStrategyTest {
private static RSQLFilterDialect dialect;

@BeforeAll
Expand All @@ -33,7 +33,7 @@ public static void init() {

dictionary.bindEntity(Author.class);
dictionary.bindEntity(Book.class);
dialect = new RSQLFilterDialect(dictionary, new CaseSensitivityStrategy.UseColumnCollation());
dialect = new RSQLFilterDialect(dictionary, new CaseSensitivityStrategy.FIQLCompliant());
}

@Test
Expand All @@ -49,8 +49,8 @@ public void testTypedExpressionParsing() throws Exception {

assertEquals(1, expressionMap.size());
assertEquals(
"((book.title INFIX [foo] AND NOT (book.title PREFIX [bar])) "
+ "AND (book.genre IN [sci-fi, action] OR book.publishDate GT [123]))",
"((book.title INFIX_CASE_INSENSITIVE [foo] AND NOT (book.title PREFIX_CASE_INSENSITIVE [bar])) "
+ "AND (book.genre IN_INSENSITIVE [sci-fi, action] OR book.publishDate GT [123]))",
expressionMap.get("book").toString()
);
}
Expand All @@ -66,7 +66,7 @@ public void testGlobalExpressionParsing() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("(book.title INFIX [foo] AND book.authors.name IN [Hemingway])", expression.toString());
assertEquals("(book.title INFIX_CASE_INSENSITIVE [foo] AND book.authors.name IN_INSENSITIVE [Hemingway])", expression.toString());
}

@Test
Expand All @@ -80,7 +80,7 @@ public void testEqualOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("book.title IN [Hemingway]", expression.toString());
assertEquals("book.title IN_INSENSITIVE [Hemingway]", expression.toString());
}

@Test
Expand All @@ -94,7 +94,7 @@ public void testNotEqualOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("NOT (book.title IN [Hemingway])", expression.toString());
assertEquals("NOT (book.title IN_INSENSITIVE [Hemingway])", expression.toString());
}

@Test
Expand All @@ -108,7 +108,7 @@ public void testInOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("book.title IN [Hemingway]", expression.toString());
assertEquals("book.title IN_INSENSITIVE [Hemingway]", expression.toString());
}

@Test
Expand All @@ -122,7 +122,7 @@ public void testOutOperator() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("NOT (book.title IN [Hemingway])", expression.toString());
assertEquals("NOT (book.title IN_INSENSITIVE [Hemingway])", expression.toString());
}

@Test
Expand Down Expand Up @@ -159,7 +159,7 @@ public void testSubstringOperator() throws Exception {
FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals(
"((book.title INFIX [Hemingway] OR book.title POSTFIX [Hemingway]) OR book.title PREFIX [Hemingway])",
"((book.title INFIX_CASE_INSENSITIVE [Hemingway] OR book.title POSTFIX_CASE_INSENSITIVE [Hemingway]) OR book.title PREFIX_CASE_INSENSITIVE [Hemingway])",
expression.toString()
);
}
Expand All @@ -175,7 +175,7 @@ public void testExpressionGrouping() throws Exception {

FilterExpression expression = dialect.parseGlobalExpression("/book", queryParams, NO_VERSION);

assertEquals("(book.title IN [foo] AND (book.title IN [bar] AND book.title IN [baz]))", expression.toString());
assertEquals("(book.title IN_INSENSITIVE [foo] AND (book.title IN_INSENSITIVE [bar] AND book.title IN_INSENSITIVE [baz]))", expression.toString());
}

@Test
Expand Down
Loading

0 comments on commit d80e181

Please sign in to comment.