From d0883ae138105686b98aca4482f164ff25c7c56e Mon Sep 17 00:00:00 2001 From: Aleksandr Date: Fri, 2 Feb 2018 13:58:02 +0700 Subject: Fixed SearchFilterParser handling NotIn operation; Improved Binary/NArt operations; Implemented tests covering NotIn --- .../query/SearchFilterBinaryOperation.scala | 16 +++++ .../query/SearchFilterNAryOperation.scala | 11 +++ .../rest/parsers/SearchFilterParser.scala | 84 ++++++++++++---------- .../rest/parsers/SearchFilterParserSuite.scala | 83 ++++++++++++++++----- 4 files changed, 139 insertions(+), 55 deletions(-) diff --git a/src/main/scala/xyz/driver/restquery/query/SearchFilterBinaryOperation.scala b/src/main/scala/xyz/driver/restquery/query/SearchFilterBinaryOperation.scala index 30d210c..923140d 100644 --- a/src/main/scala/xyz/driver/restquery/query/SearchFilterBinaryOperation.scala +++ b/src/main/scala/xyz/driver/restquery/query/SearchFilterBinaryOperation.scala @@ -12,4 +12,20 @@ object SearchFilterBinaryOperation { case object Lt extends SearchFilterBinaryOperation case object LtEq extends SearchFilterBinaryOperation + val All: Set[SearchFilterBinaryOperation] = Set( + Eq, + NotEq, + Like, + Gt, + GtEq, + Lt, + LtEq + ) + + val binaryOperationToName: Map[SearchFilterBinaryOperation, String] = + All.map(a => a -> a.toString.toLowerCase).toMap + + val binaryOperationsFromString: Map[String, SearchFilterBinaryOperation] = + for ((k, v) <- binaryOperationToName) yield (v, k) + } diff --git a/src/main/scala/xyz/driver/restquery/query/SearchFilterNAryOperation.scala b/src/main/scala/xyz/driver/restquery/query/SearchFilterNAryOperation.scala index a388597..51d18b1 100644 --- a/src/main/scala/xyz/driver/restquery/query/SearchFilterNAryOperation.scala +++ b/src/main/scala/xyz/driver/restquery/query/SearchFilterNAryOperation.scala @@ -7,4 +7,15 @@ object SearchFilterNAryOperation { case object In extends SearchFilterNAryOperation case object NotIn extends SearchFilterNAryOperation + val All: Set[SearchFilterNAryOperation] = Set( + In, + NotIn + ) + + val nAryOperationToName: Map[SearchFilterNAryOperation, String] = + All.map(a => a -> a.toString.toLowerCase).toMap + + val nAryOperationsFromString: Map[String, SearchFilterNAryOperation] = + for ((k, v) <- nAryOperationToName) yield (v, k) + } diff --git a/src/main/scala/xyz/driver/restquery/rest/parsers/SearchFilterParser.scala b/src/main/scala/xyz/driver/restquery/rest/parsers/SearchFilterParser.scala index 52a31bf..13f82a6 100644 --- a/src/main/scala/xyz/driver/restquery/rest/parsers/SearchFilterParser.scala +++ b/src/main/scala/xyz/driver/restquery/rest/parsers/SearchFilterParser.scala @@ -18,7 +18,7 @@ object SearchFilterParser { val (dimensionName, (strOperation, value)) = input val updatedValue = trimIfString(value) - parseOperation(strOperation.toLowerCase).map { op => + parseBinaryOperation(strOperation.toLowerCase).map { op => SearchFilterExpr.Atom.Binary(dimensionName, op, updatedValue.asInstanceOf[AnyRef]) } } @@ -30,13 +30,10 @@ object SearchFilterParser { val (dimensionName, (strOperation, xs)) = input val updatedValues = xs.map(trimIfString) - if (strOperation.toLowerCase == "in") { - Some( + parseNAryOperation(strOperation.toLowerCase).map( + op => SearchFilterExpr.Atom - .NAry(dimensionName, SearchFilterNAryOperation.In, updatedValues.map(_.asInstanceOf[AnyRef]))) - } else { - None - } + .NAry(dimensionName, op, updatedValues.map(_.asInstanceOf[AnyRef]))) } } @@ -46,21 +43,11 @@ object SearchFilterParser { case a => a } - private val operationsMapping = { - import xyz.driver.restquery.query.SearchFilterBinaryOperation._ - - Map[String, SearchFilterBinaryOperation]( - "eq" -> Eq, - "noteq" -> NotEq, - "like" -> Like, - "gt" -> Gt, - "gteq" -> GtEq, - "lt" -> Lt, - "lteq" -> LtEq - ) - } + private def parseBinaryOperation: String => Option[SearchFilterBinaryOperation] = + SearchFilterBinaryOperation.binaryOperationsFromString.get - private def parseOperation(x: String): Option[SearchFilterBinaryOperation] = operationsMapping.get(x) + private def parseNAryOperation: String => Option[SearchFilterNAryOperation] = + SearchFilterNAryOperation.nAryOperationsFromString.get private val whitespaceParser = P(CharPred(Utils.isSafeWhitespace)) @@ -76,14 +63,28 @@ object SearchFilterParser { } private val commonOperatorParser: Parser[String] = { - P(IgnoreCase("eq") | IgnoreCase("like") | IgnoreCase("noteq")).! + import xyz.driver.restquery.query.SearchFilterBinaryOperation.binaryOperationToName + val eq = binaryOperationToName(SearchFilterBinaryOperation.Eq) + val like = binaryOperationToName(SearchFilterBinaryOperation.Like) + val noteq = binaryOperationToName(SearchFilterBinaryOperation.NotEq) + P(IgnoreCase(eq) | IgnoreCase(like) | IgnoreCase(noteq)).! } private val numericOperatorParser: Parser[String] = { - P(IgnoreCase("eq") | IgnoreCase("noteq") | ((IgnoreCase("gt") | IgnoreCase("lt")) ~ IgnoreCase("eq").?)).! + import xyz.driver.restquery.query.SearchFilterBinaryOperation.binaryOperationToName + val eq = binaryOperationToName(SearchFilterBinaryOperation.Eq) + val noteq = binaryOperationToName(SearchFilterBinaryOperation.NotEq) + val gt = binaryOperationToName(SearchFilterBinaryOperation.Gt) + val lt = binaryOperationToName(SearchFilterBinaryOperation.Lt) + P(IgnoreCase(eq) | IgnoreCase(noteq) | ((IgnoreCase(gt) | IgnoreCase(lt)) ~ IgnoreCase(eq).?)).! } - private val naryOperatorParser: Parser[String] = P(IgnoreCase("in")).! + private val naryOperatorParser: Parser[String] = { + import xyz.driver.restquery.query.SearchFilterNAryOperation.nAryOperationToName + val in = nAryOperationToName(SearchFilterNAryOperation.In) + val notin = nAryOperationToName(SearchFilterNAryOperation.NotIn) + P(IgnoreCase(in) | IgnoreCase(notin)).! + } private val isPositiveParser: Parser[Boolean] = P(CharIn("-+").!.?).map { case Some("-") => false @@ -92,12 +93,13 @@ object SearchFilterParser { private val digitsParser: Parser[String] = P(CharIn('0' to '9').rep(min = 1).!) // Exclude Unicode "digits" - private val numberParser: Parser[String] = P(isPositiveParser ~ digitsParser.! ~ ("." ~ digitsParser).!.?).map { - case (false, intPart, Some(fracPart)) => s"-$intPart.${fracPart.tail}" - case (false, intPart, None) => s"-$intPart" - case (_, intPart, Some(fracPart)) => s"$intPart.${fracPart.tail}" - case (_, intPart, None) => s"$intPart" - } + private val numberParser: Parser[String] = + P(isPositiveParser ~ digitsParser.! ~ ("." ~ digitsParser).!.?).map { + case (false, intPart, Some(fracPart)) => s"-$intPart.${fracPart.tail}" + case (false, intPart, None) => s"-$intPart" + case (_, intPart, Some(fracPart)) => s"$intPart.${fracPart.tail}" + case (_, intPart, None) => s"$intPart" + } private val nAryValueParser: Parser[String] = P(CharPred(_ != ',').rep(min = 1).!) @@ -110,16 +112,21 @@ object SearchFilterParser { private val uuidParser: Parser[UUID] = P( - hexDigit.rep(8).! ~ "-" ~ hexDigit.rep(4).! ~ "-" ~ hexDigit.rep(4).! ~ "-" ~ hexDigit.rep(4).! ~ "-" ~ hexDigit + hexDigit.rep(8).! ~ "-" ~ hexDigit.rep(4).! ~ "-" ~ hexDigit + .rep(4) + .! ~ "-" ~ hexDigit.rep(4).! ~ "-" ~ hexDigit .rep(12) .!).map { - case (group1, group2, group3, group4, group5) => UUID.fromString(s"$group1-$group2-$group3-$group4-$group5") + case (group1, group2, group3, group4, group5) => + UUID.fromString(s"$group1-$group2-$group3-$group4-$group5") } private val binaryAtomParser: Parser[SearchFilterExpr.Atom.Binary] = P( dimensionParser ~ whitespaceParser ~ ((numericOperatorParser.! ~ whitespaceParser ~ (longParser | numberParser.!) ~ End) | - (commonOperatorParser.! ~ whitespaceParser ~ (uuidParser | booleanParser | AnyChar.rep(min = 1).!) ~ End)) + (commonOperatorParser.! ~ whitespaceParser ~ (uuidParser | booleanParser | AnyChar + .rep(min = 1) + .!) ~ End)) ).map { case BinaryAtomFromTuple(atom) => atom } @@ -127,7 +134,8 @@ object SearchFilterParser { private val nAryAtomParser: Parser[SearchFilterExpr.Atom.NAry] = P( dimensionParser ~ whitespaceParser ~ ( naryOperatorParser ~ whitespaceParser ~ - ((longParser.rep(min = 1, sep = ",") ~ End) | (booleanParser.rep(min = 1, sep = ",") ~ End) | + ((longParser.rep(min = 1, sep = ",") ~ End) | (booleanParser + .rep(min = 1, sep = ",") ~ End) | (nAryValueParser.!.rep(min = 1, sep = ",") ~ End)) ) ).map { @@ -149,8 +157,9 @@ object SearchFilterParser { case head :: Nil => atomParser.parse(head) match { - case Parsed.Success(x, _) => x - case e: Parsed.Failure[_, _] => throw new ParseQueryArgException("filters" -> formatFailure(1, e)) + case Parsed.Success(x, _) => x + case e: Parsed.Failure[_, _] => + throw new ParseQueryArgException("filters" -> formatFailure(1, e)) } case xs => @@ -172,7 +181,8 @@ object SearchFilterParser { } private def formatFailure(sectionIndex: Int, e: Parsed.Failure[_, _]): String = { - s"section $sectionIndex: ${fastparse.core.ParseError.msg(e.extra.input, e.extra.traced.expected, e.index)}" + s"section $sectionIndex: ${fastparse.core.ParseError + .msg(e.extra.input, e.extra.traced.expected, e.index)}" } } diff --git a/src/test/scala/xyz/driver/restquery/rest/parsers/SearchFilterParserSuite.scala b/src/test/scala/xyz/driver/restquery/rest/parsers/SearchFilterParserSuite.scala index e0a1696..92c1953 100644 --- a/src/test/scala/xyz/driver/restquery/rest/parsers/SearchFilterParserSuite.scala +++ b/src/test/scala/xyz/driver/restquery/rest/parsers/SearchFilterParserSuite.scala @@ -9,7 +9,7 @@ import org.scalatest.FreeSpecLike import org.scalatest.prop.Checkers import xyz.driver.restquery.query.SearchFilterBinaryOperation.Eq import xyz.driver.restquery.query.SearchFilterExpr.Dimension -import xyz.driver.restquery.query.SearchFilterNAryOperation.In +import xyz.driver.restquery.query.SearchFilterNAryOperation.{In, NotIn} import xyz.driver.restquery.query.{SearchFilterExpr, SearchFilterNAryOperation} import xyz.driver.restquery.rest.parsers.TestUtils._ import xyz.driver.restquery.utils.Utils @@ -42,8 +42,10 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { filter === Success(SearchFilterExpr.Intersection(List( SearchFilterExpr.Atom .NAry(Dimension(None, "status"), In, Seq("Summarized", "ReviewCriteria", "Flagged", "Done")), - SearchFilterExpr.Atom.Binary(Dimension(None, "previous_status"), NotEq, "New"), - SearchFilterExpr.Atom.Binary(Dimension(None, "previous_status"), NotEq, "ReviewSummary") + SearchFilterExpr.Atom + .Binary(Dimension(None, "previous_status"), NotEq, "New"), + SearchFilterExpr.Atom + .Binary(Dimension(None, "previous_status"), NotEq, "ReviewSummary") )))) } "dimensions" - { @@ -89,8 +91,9 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { SearchFilterParser .parse(Seq("filters" -> query)) .map { - case SearchFilterExpr.Atom.Binary(_, Eq | NotEq | Like, _) => true - case x => throw new UnexpectedSearchFilterExprException(x) + case SearchFilterExpr.Atom.Binary(_, Eq | NotEq | Like, _) => + true + case x => throw new UnexpectedSearchFilterExprException(x) } .successProp } @@ -115,16 +118,21 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { "actual recordId" - { "should not be parsed with numeric values" in { - val filter = SearchFilterParser.parse(Seq("filters" -> "recordId EQ 1")) - assert(filter === Success(SearchFilterExpr.Atom.Binary(Dimension(None, "record_id"), Eq, Long.box(1)))) + val filter = + SearchFilterParser.parse(Seq("filters" -> "recordId EQ 1")) + assert( + filter === Success(SearchFilterExpr.Atom + .Binary(Dimension(None, "record_id"), Eq, Long.box(1)))) } } "actual isVisible boolean" - { "should not be parsed with boolean values" in { - val filter = SearchFilterParser.parse(Seq("filters" -> "isVisible EQ true")) + val filter = + SearchFilterParser.parse(Seq("filters" -> "isVisible EQ true")) assert( - filter === Success(SearchFilterExpr.Atom.Binary(Dimension(None, "is_visible"), Eq, Boolean.box(true)))) + filter === Success(SearchFilterExpr.Atom + .Binary(Dimension(None, "is_visible"), Eq, Boolean.box(true)))) } } @@ -160,14 +168,26 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { "n-ary" - { "actual record Ids" - { - "should not be parsed with text values" in { + "should not be parsed with text values on 'IN'" in { val filter = SearchFilterParser.parse(Seq("filters" -> "id IN 1,5")) filter match { case Success(_) => () case Failure(t) => t.printStackTrace() } assert( - filter === Success(SearchFilterExpr.Atom.NAry(Dimension(None, "id"), In, Seq(Long.box(1), Long.box(5))))) + filter === Success(SearchFilterExpr.Atom + .NAry(Dimension(None, "id"), In, Seq(Long.box(1), Long.box(5))))) + } + "should not be parsed with text values on 'NOTIN'" in { + val filter = + SearchFilterParser.parse(Seq("filters" -> "id NOTIN 1,5")) + filter match { + case Success(_) => () + case Failure(t) => t.printStackTrace() + } + assert( + filter === Success( + SearchFilterExpr.Atom.NAry(Dimension(None, "id"), NotIn, Seq(Long.box(1), Long.box(5))))) } } @@ -182,8 +202,27 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { SearchFilterParser .parse(Seq("filters" -> query)) .map { - case SearchFilterExpr.Atom.NAry(_, SearchFilterNAryOperation.In, _) => true - case x => throw new UnexpectedSearchFilterExprException(x) + case SearchFilterExpr.Atom.NAry(_, SearchFilterNAryOperation.In, _) => + true + case x => throw new UnexpectedSearchFilterExprException(x) + } + .successProp + } + } + + "not in" in check { + val testQueryGen = queryGen( + dimensionGen = Gen.identifier, + opGen = Gen.const("notin"), + valueGen = inValuesGen + ) + + Prop.forAllNoShrink(testQueryGen) { query => + SearchFilterParser + .parse(Seq("filters" -> query)) + .map { + case SearchFilterExpr.Atom.NAry(_, SearchFilterNAryOperation.NotIn, _) => true + case x => throw new UnexpectedSearchFilterExprException(x) } .successProp } @@ -211,7 +250,9 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { } Prop.forAllNoShrink(intersectionsGen) { queries => - SearchFilterParser.parse(queries.map(query => "filters" -> query)).successProp + SearchFilterParser + .parse(queries.map(query => "filters" -> query)) + .successProp } } } @@ -222,8 +263,10 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { private val allBinaryOpsGen: Gen[String] = Gen.oneOf(CommonBinaryOps ++ NumericBinaryOps).flatMap(randomCapitalization) - private val commonBinaryOpsGen: Gen[String] = Gen.oneOf(CommonBinaryOps).flatMap(randomCapitalization) - private val numericBinaryOpsGen: Gen[String] = Gen.oneOf(NumericBinaryOps).flatMap(randomCapitalization) + private val commonBinaryOpsGen: Gen[String] = + Gen.oneOf(CommonBinaryOps).flatMap(randomCapitalization) + private val numericBinaryOpsGen: Gen[String] = + Gen.oneOf(NumericBinaryOps).flatMap(randomCapitalization) private val inValueCharsGen: Gen[Char] = arbitrary[Char].filter(_ != ',') @@ -231,9 +274,13 @@ class SearchFilterParserSuite extends FreeSpecLike with Checkers { !Utils.safeTrim(s).isEmpty } - private val numericBinaryAtomValuesGen: Gen[String] = arbitrary[Long].map(_.toString) + private val numericBinaryAtomValuesGen: Gen[String] = + arbitrary[Long].map(_.toString) private val inValueGen: Gen[String] = { - Gen.nonEmptyContainerOf[Seq, Char](inValueCharsGen).map(_.mkString).filter(s => Utils.safeTrim(s).nonEmpty) + Gen + .nonEmptyContainerOf[Seq, Char](inValueCharsGen) + .map(_.mkString) + .filter(s => Utils.safeTrim(s).nonEmpty) } private val inValuesGen: Gen[String] = Gen.choose(1, 5).flatMap { size => Gen.containerOfN[Seq, String](size, inValueGen).map(_.mkString(",")) -- cgit v1.2.3