Fix embedded filter silently getting ignored (#2133)

When the embedded resource is not included in the request

CHANGELOG.md

@@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2107, Clarify error for failed schema cache load. - @steve-chavez
    + From `Database connection lost. Retrying the connection` to `Could not query the database for the schema cache. Retrying.`
  - #2120, Fix reading database configuration properly when `=` is present in value - @wolfgangwalther
+ - #1771, Fix silently ignoring filter on a non-existent embedded resource - @steve-chavez
 
 ## [9.0.0] - 2021-11-25
 

src/PostgREST/Error.hs

@@ -68,6 +68,7 @@ instance PgrstError ApiRequestError where
   status NoRpc{}                 = HTTP.status404
   status (UnacceptableSchema _)  = HTTP.status406
   status (ContentTypeError _)    = HTTP.status415
+  status (NotEmbedded _)         = HTTP.status400
 
   headers _ = [ContentType.toHeader CTApplicationJSON]
 
@@ -111,6 +112,9 @@ instance JSON.ToJSON ApiRequestError where
     "message" .= ("The schema must be one of the following: " <> T.intercalate ", " schemas)]
   toJSON (ContentTypeError cts)    = JSON.object [
     "message" .= ("None of these Content-Types are available: " <> T.intercalate ", " (map T.decodeUtf8 cts))]
+  toJSON (NotEmbedded resource) = JSON.object [
+    "hint"    .= ("Verify that '" <> resource <> "' is included in the 'select' query parameter." :: Text),
+    "message" .= ("Cannot apply filter because '" <> resource <> "' is not an embedded resource in this request" :: Text)]
 
 compressedRel :: Relationship -> JSON.Value
 compressedRel Relationship{..} =

src/PostgREST/Request/DbRequestBuilder.hs

@@ -64,9 +64,12 @@ readRequest schema rootTableName maxRows allRels apiRequest  =
   mapLeft ApiRequestError $
   treeRestrictRange maxRows =<<
   augmentRequestWithJoin schema rootRels =<<
-  addFiltersOrdersRanges apiRequest (initReadRequest rootName sel)
+  addLogicTrees apiRequest =<<
+  addRanges apiRequest =<<
+  addOrders apiRequest =<<
+  addFilters apiRequest (initReadRequest rootName qsSelect)
   where
-    sel = QueryParams.qsSelect $ iQueryParams apiRequest
+    QueryParams.QueryParams{..} = iQueryParams apiRequest
     (rootName, rootRels) = rootWithRels schema rootTableName allRels (iAction apiRequest)
 
 -- Get the root table name with its relationships according to the Action type.
@@ -251,16 +254,11 @@ getJoinConditions previousAlias newAlias (Relationship Table{tableSchema=tSchema
     removeSourceCTESchema :: Schema -> TableName -> QualifiedIdentifier
     removeSourceCTESchema schema tbl = QualifiedIdentifier (if tbl == decodeUtf8 sourceCTEName then mempty else schema) tbl
 
-addFiltersOrdersRanges :: ApiRequest -> ReadRequest -> Either ApiRequestError ReadRequest
-addFiltersOrdersRanges ApiRequest{..} rReq = do
-  flip (foldr addLogicTree) qsLogic <$> (foldr addRange rOrds <$> ranges)
+addFilters :: ApiRequest -> ReadRequest -> Either ApiRequestError ReadRequest
+addFilters ApiRequest{..} rReq =
+  foldr addFilterToNode (Right rReq) flts
   where
-    rFlts = foldr addFilter rReq flts
-    rOrds = foldr addOrder rFlts qsOrder
     QueryParams.QueryParams{..} = iQueryParams
-    ranges :: Either ApiRequestError [(EmbedPath, NonnegRange)]
-    ranges = first QueryParamError $ QueryParams.pRequestRange `traverse` M.toList iRange
-    -- there can be no filters on the root table when we are doing insert/update/delete
     flts =
       case iAction of
         ActionInvoke InvGet  -> qsFilters
@@ -269,38 +267,51 @@ addFiltersOrdersRanges ApiRequest{..} rReq = do
         ActionRead _         -> qsFilters
         _                    -> qsFiltersNotRoot
 
-addFilterToNode :: Filter -> ReadRequest -> ReadRequest
-addFilterToNode flt (Node (q@Select {where_=lf}, i) f) = Node (q{where_=addFilterToLogicForest flt lf}::ReadQuery, i) f
+    addFilterToNode :: (EmbedPath, Filter) -> Either ApiRequestError ReadRequest ->  Either ApiRequestError ReadRequest
+    addFilterToNode =
+      updateNode (\flt (Node (q@Select {where_=lf}, i) f) -> Node (q{where_=addFilterToLogicForest flt lf}::ReadQuery, i) f)
 
-addFilter :: (EmbedPath, Filter) -> ReadRequest -> ReadRequest
-addFilter = addProperty addFilterToNode
-
-addOrderToNode :: [OrderTerm] -> ReadRequest -> ReadRequest
-addOrderToNode o (Node (q,i) f) = Node (q{order=o}, i) f
-
-addOrder :: (EmbedPath, [OrderTerm]) -> ReadRequest -> ReadRequest
-addOrder = addProperty addOrderToNode
-
-addRangeToNode :: NonnegRange -> ReadRequest -> ReadRequest
-addRangeToNode r (Node (q,i) f) = Node (q{range_=r}, i) f
-
-addRange :: (EmbedPath, NonnegRange) -> ReadRequest -> ReadRequest
-addRange = addProperty addRangeToNode
-
-addLogicTreeToNode :: LogicTree -> ReadRequest -> ReadRequest
-addLogicTreeToNode t (Node (q@Select{where_=lf},i) f) = Node (q{where_=t:lf}::ReadQuery, i) f
-
-addLogicTree :: (EmbedPath, LogicTree) -> ReadRequest -> ReadRequest
-addLogicTree = addProperty addLogicTreeToNode
-
-addProperty :: (a -> ReadRequest -> ReadRequest) -> (EmbedPath, a) -> ReadRequest -> ReadRequest
-addProperty f ([], a) rr = f a rr
-addProperty f (targetNodeName:remainingPath, a) (Node rn forest) =
-  case pathNode of
-    Nothing -> Node rn forest -- the property is silenty dropped in the Request does not contain the required path
-    Just tn -> Node rn (addProperty f (remainingPath, a) tn:delete tn forest)
+addOrders :: ApiRequest -> ReadRequest -> Either ApiRequestError ReadRequest
+addOrders ApiRequest{..} rReq =
+  foldr addOrderToNode (Right rReq) qsOrder
   where
-    pathNode = find (\(Node (_,(nodeName,_,alias,_,_, _)) _) -> nodeName == targetNodeName || alias == Just targetNodeName) forest
+    QueryParams.QueryParams{..} = iQueryParams
+
+    addOrderToNode :: (EmbedPath, [OrderTerm]) -> Either ApiRequestError ReadRequest -> Either ApiRequestError ReadRequest
+    addOrderToNode = updateNode (\o (Node (q,i) f) -> Node (q{order=o}, i) f)
+
+addRanges :: ApiRequest -> ReadRequest -> Either ApiRequestError ReadRequest
+addRanges ApiRequest{..} rReq =
+  foldr addRangeToNode (Right rReq) =<< ranges
+  where
+    ranges :: Either ApiRequestError [(EmbedPath, NonnegRange)]
+    ranges = first QueryParamError $ QueryParams.pRequestRange `traverse` M.toList iRange
+
+    addRangeToNode :: (EmbedPath, NonnegRange) -> Either ApiRequestError ReadRequest -> Either ApiRequestError ReadRequest
+    addRangeToNode = updateNode (\r (Node (q,i) f) -> Node (q{range_=r}, i) f)
+
+addLogicTrees :: ApiRequest -> ReadRequest -> Either ApiRequestError ReadRequest
+addLogicTrees ApiRequest{..} rReq =
+  foldr addLogicTreeToNode (Right rReq) qsLogic
+  where
+    QueryParams.QueryParams{..} = iQueryParams
+
+    addLogicTreeToNode :: (EmbedPath, LogicTree) -> Either ApiRequestError ReadRequest -> Either ApiRequestError ReadRequest
+    addLogicTreeToNode = updateNode (\t (Node (q@Select{where_=lf},i) f) -> Node (q{where_=t:lf}::ReadQuery, i) f)
+
+-- Find a Node of the Tree and apply a function to it
+updateNode :: (a -> ReadRequest -> ReadRequest) -> (EmbedPath, a) -> Either ApiRequestError ReadRequest -> Either ApiRequestError ReadRequest
+updateNode f ([], a) rr = f a <$> rr
+updateNode _ _ (Left e) = Left e
+updateNode f (targetNodeName:remainingPath, a) (Right (Node rootNode forest)) =
+  case findNode of
+    Nothing -> Left $ NotEmbedded targetNodeName
+    Just target ->
+      (\node -> Node rootNode $ node : delete target forest) <$>
+      updateNode f (remainingPath, a) (Right target)
+  where
+    findNode :: Maybe ReadRequest
+    findNode = find (\(Node (_,(nodeName,_,alias,_,_, _)) _) -> nodeName == targetNodeName || alias == Just targetNodeName) forest
 
 mutateRequest :: Schema -> TableName -> ApiRequest -> [FieldName] -> ReadRequest -> Either Error MutateRequest
 mutateRequest schema tName ApiRequest{..} pkCols readReq = mapLeft ApiRequestError $

src/PostgREST/Request/Types.hs

@@ -71,6 +71,7 @@ data ApiRequestError
   | UnacceptableSchema [Text]
   | ContentTypeError [ByteString]
   | UnsupportedVerb                -- Unreachable?
+  | NotEmbedded Text
 
 data QPError = QPError Text Text
 

test/spec/Feature/QuerySpec.hs

@@ -257,20 +257,34 @@ spec actualPgVersion = do
     it "cannot access a computed column that is outside of the config schema" $
       get "/items?always_false=is.false" `shouldRespondWith` 400
 
-    it "matches filtering nested items 2" $
-      get "/clients?select=id,projects(id,tasks2(id,name))&projects.tasks.name=like.Design*" `shouldRespondWith`
-        [json| {
-          "hint":"Verify that 'projects' and 'tasks2' exist in the schema 'test' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache.",
-          "message":"Could not find a relationship between 'projects' and 'tasks2' in the schema cache"}|]
-        { matchStatus  = 400
-        , matchHeaders = [matchContentTypeJson]
-        }
-
     it "matches filtering nested items" $
       get "/clients?select=id,projects(id,tasks(id,name))&projects.tasks.name=like.Design*" `shouldRespondWith`
         [json|[{"id":1,"projects":[{"id":1,"tasks":[{"id":1,"name":"Design w7"}]},{"id":2,"tasks":[{"id":3,"name":"Design w10"}]}]},{"id":2,"projects":[{"id":3,"tasks":[{"id":5,"name":"Design IOS"}]},{"id":4,"tasks":[{"id":7,"name":"Design OSX"}]}]}]|]
         { matchHeaders = [matchContentTypeJson] }
 
+    it "errs when the embedded resource doesn't exist and an embedded filter is applied to it" $ do
+      get "/clients?select=*&non_existent_projects.name=like.*NonExistent*" `shouldRespondWith`
+        [json|
+          {"hint":"Verify that 'non_existent_projects' is included in the 'select' query parameter.",
+          "message":"Cannot apply filter because 'non_existent_projects' is not an embedded resource in this request"}|]
+        { matchStatus  = 400
+        , matchHeaders = [matchContentTypeJson]
+        }
+      get "/clients?select=*,amiga_projects:projects(*)&amiga_projectsss.name=ilike.*Amiga*" `shouldRespondWith`
+        [json|
+          {"hint":"Verify that 'amiga_projectsss' is included in the 'select' query parameter.",
+           "message":"Cannot apply filter because 'amiga_projectsss' is not an embedded resource in this request"}|]
+        { matchStatus  = 400
+        , matchHeaders = [matchContentTypeJson]
+        }
+      get "/clients?select=id,projects(id,tasks(id,name))&projects.tasks2.name=like.Design*" `shouldRespondWith`
+        [json|
+          {"hint":"Verify that 'tasks2' is included in the 'select' query parameter.",
+           "message":"Cannot apply filter because 'tasks2' is not an embedded resource in this request"}|]
+        { matchStatus  = 400
+        , matchHeaders = [matchContentTypeJson]
+        }
+
     it "matches with cs operator" $
       get "/complex_items?select=id&arr_data=cs.{2}" `shouldRespondWith`
         [json|[{"id":2},{"id":3}]|]