Allow more than two conditions in a single and/or, Fix #889 (#892)

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

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

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
 

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]

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 \"(\"",