carloea2 commented on code in PR #8151:
URL: https://github.com/apache/texera/pull/8151#discussion_r3936634219
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala:
##########
@@ -158,9 +158,10 @@ object DashboardResource {
case "Asc" => value.asc()
case "Desc" => value.desc().nullsLast()
})
- case None => List()
+ case None =>
+ throw new BadRequestException(s"Unknown orderBy:
${searchQueryParams.orderBy}")
Review Comment:
Made the mapping total and moved the impossible default to an
IllegalStateException inside getColumnField.
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/DashboardResourceSpec.scala:
##########
@@ -101,17 +102,12 @@ class DashboardResourceSpec extends AnyFlatSpec with
Matchers {
// -- getOrderFields: everything the grammar rejects
-------------------------
- it should "fall back to no ordering at all when orderBy does not match the
pattern" in {
- // Consequence of the empty list: the query runs unordered while
- // offset/limit still apply, so pagination silently becomes
- // non-deterministic. Pin the inputs that take this branch.
- orderSql("") shouldBe empty // the frontend can send an empty query param
- orderSql("Bogus") shouldBe empty
- orderSql("NameSideways") shouldBe empty // known column, junk direction
- orderSql("Name") shouldBe empty // direction missing entirely
- orderSql("nameAsc") shouldBe empty // the regex is case-sensitive
- orderSql("SortByNameAsc") shouldBe empty // Regex.unapplySeq anchors the
- orderSql("NameAscending") shouldBe empty // whole string, both ends
+ it should "reject values outside the orderBy grammar" in {
+ Seq("", "Bogus", "NameSideways", "Name", "nameAsc", "SortByNameAsc",
"NameAscending")
+ .foreach { orderBy =>
+ val thrown = the[BadRequestException] thrownBy orderSql(orderBy)
+ thrown.getMessage shouldBe s"Unknown orderBy: $orderBy"
+ }
}
// Two branches in this area are unreachable and are therefore deliberately
Review Comment:
Updated the stale note and restored the purpose of each invalid fixture.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
