From a59d6910eaccf9b5540fb41648a7d4e67cc241b5 Mon Sep 17 00:00:00 2001 From: Jakob Odersky Date: Tue, 8 Aug 2017 00:11:13 -0700 Subject: Fix parsers and renable fatal warnings --- .../pdsuicommon/parsers/ListRequestParser.scala | 20 --- .../pdsuicommon/parsers/PagiationParser.scala | 27 --- .../pdsuicommon/parsers/PaginationParser.scala | 29 ++++ .../parsers/PaginationParserSuite.scala | 95 +++++++++++ .../parsers/SearchFilterParserSuite.scala | 185 +++++++++++++++++++++ .../pdsuicommon/parsers/SortingParserSuite.scala | 91 ++++++++++ .../xyz/driver/pdsuicommon/parsers/TestUtils.scala | 52 ++++++ 7 files changed, 452 insertions(+), 47 deletions(-) delete mode 100644 src/main/scala/xyz/driver/pdsuicommon/parsers/ListRequestParser.scala delete mode 100644 src/main/scala/xyz/driver/pdsuicommon/parsers/PagiationParser.scala create mode 100644 src/main/scala/xyz/driver/pdsuicommon/parsers/PaginationParser.scala create mode 100644 src/test/scala/xyz/driver/pdsuicommon/parsers/PaginationParserSuite.scala create mode 100644 src/test/scala/xyz/driver/pdsuicommon/parsers/SearchFilterParserSuite.scala create mode 100644 src/test/scala/xyz/driver/pdsuicommon/parsers/SortingParserSuite.scala create mode 100644 src/test/scala/xyz/driver/pdsuicommon/parsers/TestUtils.scala (limited to 'src') diff --git a/src/main/scala/xyz/driver/pdsuicommon/parsers/ListRequestParser.scala b/src/main/scala/xyz/driver/pdsuicommon/parsers/ListRequestParser.scala deleted file mode 100644 index c3146ce..0000000 --- a/src/main/scala/xyz/driver/pdsuicommon/parsers/ListRequestParser.scala +++ /dev/null @@ -1,20 +0,0 @@ -package xyz.driver.pdsuicommon.parsers - -import xyz.driver.pdsuicommon.db.{Pagination, SearchFilterExpr, Sorting} -import play.api.mvc._ - -import scala.util.Try - -final case class ListRequestParameters(filter: SearchFilterExpr, sorting: Sorting, pagination: Pagination) - -class ListRequestParser(validSortingFields: Set[String]) { - - def tryParse(request: Request[AnyContent]): Try[ListRequestParameters] = { - for { - queryFilters <- SearchFilterParser.parse(request.queryString) - sorting <- SortingParser.parse(validSortingFields, request.queryString) - pagination <- PaginationParser.parse(request.queryString) - } yield ListRequestParameters(queryFilters, sorting, pagination) - } - -} diff --git a/src/main/scala/xyz/driver/pdsuicommon/parsers/PagiationParser.scala b/src/main/scala/xyz/driver/pdsuicommon/parsers/PagiationParser.scala deleted file mode 100644 index dd492e4..0000000 --- a/src/main/scala/xyz/driver/pdsuicommon/parsers/PagiationParser.scala +++ /dev/null @@ -1,27 +0,0 @@ -package xyz.driver.pdsuicommon.parsers - -import xyz.driver.pdsuicommon.db._ -import scala.util._ - -object PaginationParser { - - @deprecated("play-akka transition", "0") - def parse(query: Map[String, Seq[String]]): Try[Pagination] = - parse(query.toSeq.flatMap { - case (key, values) => - values.map(value => key -> value) - }) - - def parse(query: Seq[(String, String)]): Try[Pagination] = { - val IntString = """(\d+)""".r - def validate(field: String) = query.collectFirst { case (`field`, size) => size } match { - case Some(IntString(x)) => x.toInt - case Some(str) => throw new ParseQueryArgException((field, s"must be an integer (found $str)")) - case None => throw new ParseQueryArgException((field, "must be defined")) - } - - Try { - Pagination(validate("pageSize"), validate("pageNumber")) - } - } -} diff --git a/src/main/scala/xyz/driver/pdsuicommon/parsers/PaginationParser.scala b/src/main/scala/xyz/driver/pdsuicommon/parsers/PaginationParser.scala new file mode 100644 index 0000000..b59b1a5 --- /dev/null +++ b/src/main/scala/xyz/driver/pdsuicommon/parsers/PaginationParser.scala @@ -0,0 +1,29 @@ +package xyz.driver.pdsuicommon.parsers + +import xyz.driver.pdsuicommon.db._ +import scala.util._ + +object PaginationParser { + + @deprecated("play-akka transition", "0") + def parse(query: Map[String, Seq[String]]): Try[Pagination] = + parse(query.toSeq.flatMap { + case (key, values) => + values.map(value => key -> value) + }) + + def parse(query: Seq[(String, String)]): Try[Pagination] = { + val IntString = """(\d+)""".r + def validate(field: String, default: Int) = query.collectFirst { case (`field`, size) => size } match { + case Some(IntString(x)) if x.toInt > 0 => x.toInt + case Some(IntString(x)) => throw new ParseQueryArgException((field, s"must greater than zero (found $x)")) + case Some(str) => throw new ParseQueryArgException((field, s"must be an integer (found $str)")) + case None => default + } + + Try { + Pagination(validate("pageSize", Pagination.Default.pageSize), + validate("pageNumber", Pagination.Default.pageNumber)) + } + } +} diff --git a/src/test/scala/xyz/driver/pdsuicommon/parsers/PaginationParserSuite.scala b/src/test/scala/xyz/driver/pdsuicommon/parsers/PaginationParserSuite.scala new file mode 100644 index 0000000..48fc99b --- /dev/null +++ b/src/test/scala/xyz/driver/pdsuicommon/parsers/PaginationParserSuite.scala @@ -0,0 +1,95 @@ +package xyz.driver.pdsuicommon.parsers + +import xyz.driver.pdsuicommon.db.Pagination +import xyz.driver.pdsuicommon.parsers.TestUtils._ +import org.scalatest.{FreeSpecLike, MustMatchers} + +import scala.util.{Failure, Try} + +class PaginationParserSuite extends FreeSpecLike with MustMatchers { + + "parse" - { + "pageSize" - { + "should parse positive value" in { + val pagination = PaginationParser.parse(Seq( + "pageSize" -> "10", + "pageNumber" -> "1" + )) + pagination must success + pagination.get.pageSize mustBe 10 + } + + "should return a default value if there is no one" in { + val pagination = PaginationParser.parse(Seq( + "pageNumber" -> "1" + )) + pagination must success + pagination.get.pageSize mustBe 100 + } + + "should return a error for zero value" in { + val pagination = PaginationParser.parse(Seq( + "pageSize" -> "0", + "pageNumber" -> "1" + )) + + checkFailedValidationOnlyOn(pagination, "pageSize") + } + + "should return a error for negative value" in { + val pagination = PaginationParser.parse(Seq( + "pageSize" -> "-10", + "pageNumber" -> "1" + )) + + checkFailedValidationOnlyOn(pagination, "pageSize") + } + } + + "pageNumber" - { + "should parse positive value" in { + val pagination = PaginationParser.parse(Seq( + "pageSize" -> "1", + "pageNumber" -> "1" + )) + pagination must success + pagination.get.pageSize mustBe 1 + } + + "should return a default value if there is no one" in { + val pagination = PaginationParser.parse(Seq( + "pageSize" -> "1" + )) + pagination must success + pagination.get.pageNumber mustBe 1 + } + + "should return a error for zero value" in { + val pagination = PaginationParser.parse(Seq( + "pageSize" -> "1", + "pageNumber" -> "0" + )) + + checkFailedValidationOnlyOn(pagination, "pageNumber") + } + + "should return a error for negative value" in { + val pagination = PaginationParser.parse(Seq( + "pageSize" -> "1", + "pageNumber" -> "-1" + )) + + checkFailedValidationOnlyOn(pagination, "pageNumber") + } + } + } + + private def checkFailedValidationOnlyOn(pagination: Try[Pagination], key: String): Unit = { + pagination must failWith[ParseQueryArgException] + + val Failure(e: ParseQueryArgException) = pagination + e.errors.size mustBe 1 + e.errors.head._1 mustBe key + } + +} diff --git a/src/test/scala/xyz/driver/pdsuicommon/parsers/SearchFilterParserSuite.scala b/src/test/scala/xyz/driver/pdsuicommon/parsers/SearchFilterParserSuite.scala new file mode 100644 index 0000000..5cd2dc9 --- /dev/null +++ b/src/test/scala/xyz/driver/pdsuicommon/parsers/SearchFilterParserSuite.scala @@ -0,0 +1,185 @@ +package xyz.driver.pdsuicommon.parsers + +import xyz.driver.pdsuicommon.db.SearchFilterExpr.Dimension +import xyz.driver.pdsuicommon.db.{SearchFilterBinaryOperation, SearchFilterExpr, SearchFilterNAryOperation} +import xyz.driver.pdsuicommon.utils.Implicits.toStringOps +import xyz.driver.pdsuicommon.parsers.TestUtils._ +import fastparse.core.Parsed +import org.scalacheck.Arbitrary.arbitrary +import org.scalacheck.{Gen, Prop} +import org.scalatest.FreeSpecLike +import org.scalatest.prop.Checkers + +object SearchFilterParserSuite { + + class UnexpectedSearchFilterExprException(x: SearchFilterExpr) extends Exception(s"unexpected $x") + +} + +class SearchFilterParserSuite extends FreeSpecLike with Checkers { + + import SearchFilterParserSuite._ + + "parse" - { + "dimensions" - { + "with table name" in check { + val dimensionGen = { + for (left <- Gen.identifier; right <- Gen.identifier) + yield left -> right + } + Prop.forAllNoShrink(dimensionGen) { + case (left, right) => + val raw = s"$left.$right" + SearchFilterParser.dimensionParser.parse(raw) match { + case Parsed.Success(Dimension(Some(`left`), `right`), _) => true + case res => false + } + } + } + "just with field name" in check { + Prop.forAllNoShrink(Gen.identifier) { s => + SearchFilterParser.dimensionParser.parse(s) match { + case Parsed.Success(Dimension(None, `s`), _) => true + case _ => false + } + } + } + } + "atoms" - { + "binary" - { + "common operators" - { + "should be parsed with text values" in check { + import SearchFilterBinaryOperation._ + + val testQueryGen = queryGen( + dimensionGen = Gen.identifier, + opGen = commonBinaryOpsGen, + valueGen = nonEmptyString + ) + + Prop.forAllNoShrink(testQueryGen) { query => + SearchFilterParser.parse(Seq("filters" -> query)) + .map { + case SearchFilterExpr.Atom.Binary(_, Eq | NotEq | Like, _) => true + case x => throw new UnexpectedSearchFilterExprException(x) + } + .successProp + } + } + } + + "numeric operators" - { + "should not be parsed with text values" in check { + val testQueryGen = queryGen( + dimensionGen = Gen.identifier, + opGen = numericBinaryOpsGen, + valueGen = nonEmptyString.filter { s => !s.matches("^\\d+$") } + ) + + Prop.forAllNoShrink(testQueryGen) { query => + SearchFilterParser.parse(Seq("filters" -> query)).failureProp + } + } + } + + "all operators" - { + "should be parsed with numeric values" in check { + val testQueryGen = queryGen( + dimensionGen = Gen.identifier, + opGen = allBinaryOpsGen, + valueGen = numericBinaryAtomValuesGen + ) + + Prop.forAllNoShrink(testQueryGen) { query => + SearchFilterParser.parse(Seq("filters" -> query)) + .map { + case _: SearchFilterExpr.Atom.Binary => true + case x => throw new UnexpectedSearchFilterExprException(x) + } + .successProp + } + } + } + } + + "n-ary" - { + "in" in check { + val testQueryGen = queryGen( + dimensionGen = Gen.identifier, + opGen = Gen.const("in"), + valueGen = inValuesGen + ) + + Prop.forAllNoShrink(testQueryGen) { query => + SearchFilterParser.parse(Seq("filters" -> query)) + .map { + case SearchFilterExpr.Atom.NAry(_, SearchFilterNAryOperation.In, _) => true + case x => throw new UnexpectedSearchFilterExprException(x) + } + .successProp + } + } + } + } + + "intersections" - { + "should be parsed" in check { + val commonAtomsGen = queryGen( + dimensionGen = Gen.identifier, + opGen = commonBinaryOpsGen, + valueGen = nonEmptyString + ) + + val numericAtomsGen = queryGen( + dimensionGen = Gen.identifier, + opGen = numericBinaryOpsGen, + valueGen = numericBinaryAtomValuesGen + ) + + val allAtomsGen = Gen.oneOf(commonAtomsGen, numericAtomsGen) + val intersectionsGen = Gen.choose(1, 3).flatMap { size => + Gen.containerOfN[Seq, String](size, allAtomsGen) + } + + Prop.forAllNoShrink(intersectionsGen) { queries => + SearchFilterParser.parse(queries.map(query => "filters" -> query)).successProp + } + } + } + } + + private val CommonBinaryOps = Seq("eq", "noteq", "like") + private val NumericBinaryOps = Seq("gt", "gteq", "lt", "lteq") + + 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 inValueCharsGen: Gen[Char] = arbitrary[Char].filter(_ != ',') + + private val nonEmptyString = arbitrary[String].filter { s => !s.safeTrim.isEmpty } + + private val numericBinaryAtomValuesGen: Gen[String] = arbitrary[BigInt].map(_.toString) + private val inValueGen: Gen[String] = { + Gen.nonEmptyContainerOf[Seq, Char](inValueCharsGen).map(_.mkString).filter(_.safeTrim.nonEmpty) + } + private val inValuesGen: Gen[String] = Gen.choose(1, 5).flatMap { size => + Gen.containerOfN[Seq, String](size, inValueGen).map(_.mkString(",")) + } + + private def queryGen(dimensionGen: Gen[String], opGen: Gen[String], valueGen: Gen[String]): Gen[String] = for { + dimension <- dimensionGen + op <- opGen + value <- valueGen + } yield s"$dimension $op $value" + + private def randomCapitalization(input: String): Gen[String] = { + Gen.containerOfN[Seq, Boolean](input.length, arbitrary[Boolean]).map { capitalize => + input.view.zip(capitalize).map { + case (currChar, true) => currChar.toUpper + case (currChar, false) => currChar + }.mkString + } + } + +} diff --git a/src/test/scala/xyz/driver/pdsuicommon/parsers/SortingParserSuite.scala b/src/test/scala/xyz/driver/pdsuicommon/parsers/SortingParserSuite.scala new file mode 100644 index 0000000..e46015c --- /dev/null +++ b/src/test/scala/xyz/driver/pdsuicommon/parsers/SortingParserSuite.scala @@ -0,0 +1,91 @@ +package xyz.driver.pdsuicommon.parsers + +import xyz.driver.pdsuicommon.parsers.TestUtils._ +import org.scalacheck.Arbitrary.arbitrary +import org.scalacheck.{Gen, Prop} +import org.scalatest.prop.Checkers +import org.scalatest.{FreeSpecLike, MustMatchers} + +class SortingParserSuite extends FreeSpecLike with MustMatchers with Checkers { + + "parse" - { + "single dimension" - commonTests(singleSortingQueryGen) + "multiple dimensions in one query" - commonTests(multipleSortingQueryGen) + "multiple queries" in { + val r = SortingParser.parse(Set("foo", "bar"), Seq("sort" -> "foo", "sort" ->"bar")) + r must failWith[ParseQueryArgException] + } + } + + private def commonTests(queryGen: Set[String] => Gen[String]): Unit = { + "valid" in check { + val inputGen: Gen[(Set[String], String)] = for { + validDimensions <- dimensionsGen + sorting <- queryGen(validDimensions) + } yield (validDimensions, sorting) + + Prop.forAllNoShrink(inputGen) { + case (validDimensions, query) => + SortingParser.parse(validDimensions, Seq("sort" -> query)).successProp + } + } + + "invalid" in check { + val inputGen: Gen[(Set[String], String)] = for { + validDimensions <- dimensionsGen + invalidDimensions <- dimensionsGen.filter { xs => xs.intersect(validDimensions).isEmpty } + sorting <- queryGen(invalidDimensions) + } yield (validDimensions, sorting) + + Prop.forAllNoShrink(inputGen) { + case (validDimensions, query) => + SortingParser.parse(validDimensions, Seq("sort" -> query)).failureProp + } + } + } + + private val dimensionsGen: Gen[Set[String]] = for { + unPrefixedSize <- Gen.choose(0, 3) + prefixedSize <- Gen.choose(0, 3) + if (unPrefixedSize + prefixedSize) > 0 + + unPrefixedDimensions <- Gen.containerOfN[Set, String](unPrefixedSize, Gen.identifier) + + prefixes <- Gen.containerOfN[Set, String](prefixedSize, Gen.identifier) + dimensions <- Gen.containerOfN[Set, String](prefixedSize, Gen.identifier) + } yield { + val prefixedDimensions = prefixes.zip(dimensions).map { + case (prefix, dimension) => s"$prefix.$dimension" + } + unPrefixedDimensions ++ prefixedDimensions + } + + private def multipleSortingQueryGen(validDimensions: Set[String]): Gen[String] = { + val validDimensionsSeq = validDimensions.toSeq + val indexGen = Gen.oneOf(validDimensionsSeq.indices) + val multipleDimensionsGen = Gen.nonEmptyContainerOf[Set, Int](indexGen).filter(_.size >= 2).map { indices => + indices.map(validDimensionsSeq.apply) + } + + for { + dimensions <- multipleDimensionsGen + isAscending <- Gen.containerOfN[Seq, Boolean](dimensions.size, arbitrary[Boolean]) + } yield { + isAscending.zip(dimensions) + .map { + case (true, dimension) => dimension + case (false, dimension) => "-" + dimension + } + .mkString(",") + } + } + + private def singleSortingQueryGen(validDimensions: Set[String]): Gen[String] = for { + isAscending <- arbitrary[Boolean] + dimensions <- Gen.oneOf(validDimensions.toSeq) + } yield isAscending match { + case true => dimensions + case false => "-" + dimensions + } + +} diff --git a/src/test/scala/xyz/driver/pdsuicommon/parsers/TestUtils.scala b/src/test/scala/xyz/driver/pdsuicommon/parsers/TestUtils.scala new file mode 100644 index 0000000..4892b95 --- /dev/null +++ b/src/test/scala/xyz/driver/pdsuicommon/parsers/TestUtils.scala @@ -0,0 +1,52 @@ +package xyz.driver.pdsuicommon.parsers + +import org.scalacheck.Prop +import org.scalacheck.Prop.BooleanOperators +import org.scalatest.matchers.{MatchResult, Matcher} +import xyz.driver.pdsuicommon.utils.Utils + +import scala.reflect.ClassTag +import scala.util.{Failure, Success, Try} + +object TestUtils { + + object success extends Matcher[Try[Any]] { + override def apply(left: Try[Any]) = { + MatchResult(left.isSuccess, s"$left did not fail", s"did fail with $left") + } + } + + class FailWith[ThrowableT <: Throwable](implicit ct: ClassTag[ThrowableT]) extends Matcher[Try[Any]] { + override def apply(left: Try[Any]): MatchResult = { + MatchResult( + left.isFailure && left.failed.get.getClass == ct.runtimeClass, + left match { + case Success(x) => s"$left did not fail" + case Failure(e) => s"$left did fail with ${Utils.getClassSimpleName(e.getClass)}, " + + s"not ${Utils.getClassSimpleName(ct.runtimeClass)}" + }, + left match { + case Success(_) => s"$left failed with ${Utils.getClassSimpleName(ct.runtimeClass)}" + case Failure(e) => s"$left failed with ${Utils.getClassSimpleName(e.getClass)}" + } + ) + } + } + + def failWith[ThrowableT <:Throwable](implicit ct: ClassTag[ThrowableT]) = new FailWith[ThrowableT] + + final implicit class TryPropOps(val self: Try[Any]) extends AnyVal { + + def successProp: Prop = self match { + case Success(_) => true :| "ok" + case Failure(e) => false :| e.getMessage + } + + def failureProp: Prop = self match { + case Success(x) => false :| s"invalid: $x" + case Failure(e) => true + } + + } + +} -- cgit v1.2.3