From b00742814216be685fddcd3ed61c2ce81894fbf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steve=20Ch=C3=A1vez?= Date: Wed, 21 Jun 2017 00:52:21 -0500 Subject: [PATCH] Allow more than two conditions in a single and/or, Fix #889 (#892) --- CHANGELOG.md | 4 ++++ src/PostgREST/Parsers.hs | 2 +- src/PostgREST/QueryBuilder.hs | 2 +- src/PostgREST/Types.hs | 2 +- test/Feature/AndOrParamsSpec.hs | 33 +++++++++++++++++++++++---------- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c1d95271..2261f7d16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### Added + +- #889, Allow more than two conditions in a single and/or - @steve-chavez + ### Fixed ## [0.4.2.0] - 2017-06-11 diff --git a/src/PostgREST/Parsers.hs b/src/PostgREST/Parsers.hs index 5ee75d9d7..9f59a3f70 100644 --- a/src/PostgREST/Parsers.hs +++ b/src/PostgREST/Parsers.hs @@ -177,7 +177,7 @@ pOrderTerm = pLogicTree :: Parser LogicTree pLogicTree = Stmnt <$> try pLogicFilter - <|> Expr <$> pNot <*> pLogicOp <*> (lexeme (char '(') *> pLogicTree) <*> (lexeme (char ',') *> pLogicTree <* lexeme (char ')')) + <|> Expr <$> pNot <*> pLogicOp <*> (lexeme (char '(') *> pLogicTree `sepBy1` lexeme (char ',') <* lexeme (char ')')) where pLogicFilter :: Parser Filter pLogicFilter = Filter <$> pField <* pDelimiter <*> pOperation pLogicVText pLogicVTextL diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index d01f317e1..35be26a4e 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -449,7 +449,7 @@ pgFmtFilter table (Filter fld (Operation hasNot_ ex)) = notOp <> " " <> case ex Nothing -> emptyValForIn op pgFmtLogicTree :: QualifiedIdentifier -> LogicTree -> SqlFragment -pgFmtLogicTree qi (Expr hasNot_ op lt rt) = notOp <> " (" <> pgFmtLogicTree qi lt <> " " <> show op <> " " <> pgFmtLogicTree qi rt <> ")" +pgFmtLogicTree qi (Expr hasNot_ op forest) = notOp <> " (" <> intercalate (" " <> show op <> " ") (pgFmtLogicTree qi <$> forest) <> ")" where notOp = if hasNot_ then "NOT" else "" pgFmtLogicTree qi (Stmnt flt) = pgFmtFilter qi flt diff --git a/src/PostgREST/Types.hs b/src/PostgREST/Types.hs index 3d2967bad..d63305edc 100644 --- a/src/PostgREST/Types.hs +++ b/src/PostgREST/Types.hs @@ -171,7 +171,7 @@ instance Show LogicOperator where / \ id.eq.1 id.eq.2 -} -data LogicTree = Expr Bool LogicOperator LogicTree LogicTree | Stmnt Filter deriving (Show, Eq) +data LogicTree = Expr Bool LogicOperator [LogicTree] | Stmnt Filter deriving (Show, Eq) type FieldName = Text type JsonPath = [Text] diff --git a/test/Feature/AndOrParamsSpec.hs b/test/Feature/AndOrParamsSpec.hs index b500fbac2..949d70731 100644 --- a/test/Feature/AndOrParamsSpec.hs +++ b/test/Feature/AndOrParamsSpec.hs @@ -105,6 +105,29 @@ spec = get "/entities?and=( and ( id.in.( 1, 2, 3 ) , id.eq.3 ) , or ( id.eq.2 , id.eq.3 ) )&select=id" `shouldRespondWith` [json|[{ "id": 3 }]|] { matchHeaders = [matchContentTypeJson] } + context "multiple and/or conditions" $ do + it "cannot have zero conditions" $ + get "/entities?or=()" `shouldRespondWith` + [json|{ + "details": "unexpected \")\" expecting field name (* or [a..z0..9_]), negation operator (not) or logic operator (and, or)", + "message": "\"failed to parse logic tree (())\" (line 1, column 4)" + }|] { matchStatus = 400, matchHeaders = [matchContentTypeJson] } + it "can have a single condition" $ do + get "/entities?or=(id.eq.1)&select=id" `shouldRespondWith` + [json|[{"id":1}]|] { matchHeaders = [matchContentTypeJson] } + get "/entities?and=(id.eq.1)&select=id" `shouldRespondWith` + [json|[{"id":1}]|] { matchHeaders = [matchContentTypeJson] } + it "can have three conditions" $ do + get "/grandchild_entities?or=(id.eq.1, id.eq.2, id.eq.3)&select=id" `shouldRespondWith` + [json|[{"id":1}, {"id":2}, {"id":3}]|] { matchHeaders = [matchContentTypeJson] } + get "/grandchild_entities?and=(id.in.(1,2), id.in.(3,1), id.in.(1,4))&select=id" `shouldRespondWith` + [json|[{"id":1}]|] { matchHeaders = [matchContentTypeJson] } + it "can have four conditions combining and/or" $ do + get "/grandchild_entities?or=( id.eq.1, id.eq.2, and(id.in.(1,3), id.in.(2,3)), id.eq.4 )&select=id" `shouldRespondWith` + [json|[{"id":1}, {"id":2}, {"id":3}, {"id":4}]|] { matchHeaders = [matchContentTypeJson] } + get "/grandchild_entities?and=( id.eq.1, not.or(id.eq.2, id.eq.3), id.in.(1,4), or(id.eq.1, id.eq.4) )&select=id" `shouldRespondWith` + [json|[{"id":1}]|] { matchHeaders = [matchContentTypeJson] } + context "used with POST" $ it "includes related data with filters" $ request methodPost "/child_entities?entities.or=(id.eq.2,id.eq.3)&select=id,entities{id}" @@ -145,21 +168,11 @@ spec = }|] { matchStatus = 400, matchHeaders = [matchContentTypeJson] } it "fails on malformed query params and provides meaningful error message" $ do - get "/entities?or=()" `shouldRespondWith` - [json|{ - "details": "unexpected \")\" expecting field name (* or [a..z0..9_]), negation operator (not) or logic operator (and, or)", - "message": "\"failed to parse logic tree (())\" (line 1, column 4)" - }|] { matchStatus = 400, matchHeaders = [matchContentTypeJson] } get "/entities?or=)(" `shouldRespondWith` [json|{ "details": "unexpected \")\" expecting \"(\"", "message": "\"failed to parse logic tree ()()\" (line 1, column 3)" }|] { matchStatus = 400, matchHeaders = [matchContentTypeJson] } - get "/entities?or=(id.eq.1)" `shouldRespondWith` - [json|{ - "details": "unexpected \")\" expecting \",\"", - "message": "\"failed to parse logic tree ((id.eq.1))\" (line 1, column 11)" - }|] { matchStatus = 400, matchHeaders = [matchContentTypeJson] } get "/entities?and=(ord(id.eq.1,id.eq.1),id.eq.2)" `shouldRespondWith` [json|{ "details": "unexpected \"d\" expecting \"(\"",