diff --git a/CHANGELOG.md b/CHANGELOG.md index b6e98711d2..d09d84481a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #3795, Clarify `Accept: vnd.pgrst.object` error message - @steve-chavez - #3779, Always log the schema cache load time - @steve-chavez - #3706, Fix insert with `missing=default` uses default value of domain instead of column - @taimoorzaeem + - #3697, Handle queries on non-existing table gracefully - @taimoorzaeem ### Changed diff --git a/docs/references/errors.rst b/docs/references/errors.rst index 4344f67a22..132d25ac62 100644 --- a/docs/references/errors.rst +++ b/docs/references/errors.rst @@ -294,6 +294,10 @@ Related to a :ref:`schema_cache`. Most of the time, these errors are solved by : | | | in the ``columns`` query parameter is not found. | | PGRST204 | | | +---------------+-------------+-------------------------------------------------------------+ +| .. _pgrst205: | 404 | Caused when the :ref:`table specified ` in | +| | | the URI is not found. | +| PGRST205 | | | ++---------------+-------------+-------------------------------------------------------------+ .. _pgrst3**: diff --git a/src/PostgREST/ApiRequest/Types.hs b/src/PostgREST/ApiRequest/Types.hs index 9bebcd2c8e..1ad31c8101 100644 --- a/src/PostgREST/ApiRequest/Types.hs +++ b/src/PostgREST/ApiRequest/Types.hs @@ -91,6 +91,7 @@ data ApiRequestError | UnacceptableSchema [Text] | UnsupportedMethod ByteString | ColumnNotFound Text Text + | TableNotFound Text | GucHeadersError | GucStatusError | OffLimitsChangesError Int64 Integer diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 8f386a8000..7a8235e9c5 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -86,6 +86,7 @@ instance PgrstError ApiRequestError where status UnsupportedMethod{} = HTTP.status405 status LimitNoOrderError = HTTP.status400 status ColumnNotFound{} = HTTP.status400 + status TableNotFound{} = HTTP.status404 status GucHeadersError = HTTP.status500 status GucStatusError = HTTP.status500 status OffLimitsChangesError{} = HTTP.status400 @@ -253,6 +254,9 @@ instance JSON.ToJSON ApiRequestError where toJSON (ColumnNotFound relName colName) = toJsonPgrstError SchemaCacheErrorCode04 ("Could not find the '" <> colName <> "' column of '" <> relName <> "' in the schema cache") Nothing Nothing + toJSON (TableNotFound relName) = toJsonPgrstError + SchemaCacheErrorCode05 ("Could not find relation '" <> relName <> "' in the schema cache") Nothing Nothing + -- | -- If no relationship is found then: -- @@ -640,6 +644,7 @@ data ErrorCode | SchemaCacheErrorCode02 | SchemaCacheErrorCode03 | SchemaCacheErrorCode04 + | SchemaCacheErrorCode05 -- JWT authentication errors | JWTErrorCode00 | JWTErrorCode01 @@ -689,6 +694,7 @@ buildErrorCode code = case code of SchemaCacheErrorCode02 -> "PGRST202" SchemaCacheErrorCode03 -> "PGRST203" SchemaCacheErrorCode04 -> "PGRST204" + SchemaCacheErrorCode05 -> "PGRST205" JWTErrorCode00 -> "PGRST300" JWTErrorCode01 -> "PGRST301" diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index 3a31d72a75..d4d0542c0a 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -52,7 +52,7 @@ import PostgREST.SchemaCache (SchemaCache (..)) import PostgREST.SchemaCache.Identifiers (FieldName, QualifiedIdentifier (..), RelIdentifier (..), - Schema) + Schema, dumpQi) import PostgREST.SchemaCache.Relationship (Cardinality (..), Junction (..), Relationship (..), @@ -152,14 +152,14 @@ dbActionPlan dbAct conf apiReq sCache = case dbAct of wrappedReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Bool -> Either Error CrudPlan wrappedReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{..},..} headersOnly = do - rPlan <- readPlan identifier conf sCache apiRequest + rPlan <- readPlan False identifier conf sCache apiRequest (handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest identifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan) if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right () return $ WrappedReadPlan rPlan SQL.Read handler mediaType headersOnly identifier mutateReadPlan :: Mutation -> ApiRequest -> QualifiedIdentifier -> AppConfig -> SchemaCache -> Either Error CrudPlan mutateReadPlan mutation apiRequest@ApiRequest{iPreferences=Preferences{..},..} identifier conf sCache = do - rPlan <- readPlan identifier conf sCache apiRequest + rPlan <- readPlan False identifier conf sCache apiRequest mPlan <- mutatePlan mutation identifier apiRequest sCache rPlan if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right () (handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest identifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan) @@ -173,7 +173,7 @@ callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferenc proc@Function{..} <- mapLeft ApiRequestError $ findProc identifier paramKeys (dbRoutines sCache) iContentMediaType (invMethod == Inv) let relIdentifier = QualifiedIdentifier pdSchema (fromMaybe pdName $ Routine.funcTableName proc) -- done so a set returning function can embed other relations - rPlan <- readPlan relIdentifier conf sCache apiRequest + rPlan <- readPlan True relIdentifier conf sCache apiRequest let args = case (invMethod, iContentMediaType) of (InvRead _, _) -> DirectArgs $ toRpcParams proc qsParams' (Inv, MTUrlEncoded) -> DirectArgs $ maybe mempty (toRpcParams proc . payArray) iPayload @@ -320,8 +320,8 @@ resolveQueryInputField ctx field = withTextParse ctx $ resolveTypeOrUnknown ctx -- | Builds the ReadPlan tree on a number of stages. -- | Adds filters, order, limits on its respective nodes. -- | Adds joins conditions obtained from resource embedding. -readPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Either Error ReadPlanTree -readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregates} SchemaCache{dbTables, dbRelationships, dbRepresentations} apiRequest = +readPlan :: Bool -> QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Either Error ReadPlanTree +readPlan fromCallPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregates} SchemaCache{dbTables, dbRelationships, dbRepresentations} apiRequest = let -- JSON output format hardcoded for now. In the future we might want to support other output mappings such as CSV. ctx = ResolverContext dbTables dbRepresentations qi "json" @@ -340,7 +340,8 @@ readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregate addLogicTrees ctx apiRequest =<< addRanges apiRequest =<< addOrders ctx apiRequest =<< - addFilters ctx apiRequest (initReadRequest ctx $ QueryParams.qsSelect $ iQueryParams apiRequest) + addFilters ctx apiRequest =<< + searchTable fromCallPlan qi dbTables (initReadRequest ctx $ QueryParams.qsSelect $ iQueryParams apiRequest) -- Build the initial read plan tree initReadRequest :: ResolverContext -> [Tree SelectItem] -> ReadPlanTree @@ -738,6 +739,14 @@ validateAggFunctions aggFunctionsAllowed (Node rp@ReadPlan {select} forest) | not aggFunctionsAllowed && any (isJust . csAggFunction) select = Left AggregatesNotAllowed | otherwise = Node rp <$> traverse (validateAggFunctions aggFunctionsAllowed) forest +-- We only search for the table when readPlan is not called from call plan. This +-- is because we reuse readPlan in callReadPlan to search for function name +searchTable :: Bool -> QualifiedIdentifier -> TablesMap -> ReadPlanTree -> Either ApiRequestError ReadPlanTree +searchTable fromCallPlan qi tbls readPlanTree = + case (fromCallPlan, HM.lookup qi tbls) of + (False, Nothing) -> Left (TableNotFound $ dumpQi qi) + _ -> Right readPlanTree + addFilters :: ResolverContext -> ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree addFilters ctx ApiRequest{..} rReq = foldr addFilterToNode (Right rReq) flts @@ -961,7 +970,7 @@ mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{ typedColumnsOrError = resolveOrError ctx tbl `traverse` S.toList iColumns resolveOrError :: ResolverContext -> Maybe Table -> FieldName -> Either ApiRequestError CoercibleField -resolveOrError _ Nothing _ = Left NotFound +resolveOrError ResolverContext{qi} Nothing _ = Left $ TableNotFound $ dumpQi qi resolveOrError ctx (Just table) field = case resolveTableFieldName table field of CoercibleField{cfIRType=""} -> Left $ ColumnNotFound (tableName table) field diff --git a/test/io/test_big_schema.py b/test/io/test_big_schema.py index 1e6ecf6c8a..1ea898eb78 100644 --- a/test/io/test_big_schema.py +++ b/test/io/test_big_schema.py @@ -65,6 +65,6 @@ def test_should_not_fail_with_stack_overflow(defaultenv): with run(env=env, wait_max_seconds=30) as postgrest: response = postgrest.session.get("/unknown-table?select=unknown-rel(*)") - assert response.status_code == 400 + assert response.status_code == 404 data = response.json() - assert data["code"] == "PGRST200" + assert data["code"] == "PGRST205" diff --git a/test/io/test_io.py b/test/io/test_io.py index e61e074cbc..5d4bbdfb67 100644 --- a/test/io/test_io.py +++ b/test/io/test_io.py @@ -973,11 +973,10 @@ def test_log_level(level, defaultenv): r'- - postgrest_test_anonymous \[.+\] "GET /unknown HTTP/1.1" 404 - "" "python-requests/.+"', output[2], ) + + assert len(output) == 5 assert "Connection" and "is available" in output[3] - assert "Connection" and "is available" in output[4] - assert "Connection" and "is used" in output[5] - assert "Connection" and "is used" in output[6] - assert len(output) == 7 + assert "Connection" and "is used" in output[4] def test_no_pool_connection_required_on_bad_http_logic(defaultenv): diff --git a/test/spec/Feature/ConcurrentSpec.hs b/test/spec/Feature/ConcurrentSpec.hs index d52d54666f..632b8ca82c 100644 --- a/test/spec/Feature/ConcurrentSpec.hs +++ b/test/spec/Feature/ConcurrentSpec.hs @@ -27,8 +27,8 @@ spec = `shouldRespondWith` [json| { "hint": null, "details":null, - "code":"42P01", - "message":"relation \"test.fakefake\" does not exist" + "code":"PGRST205", + "message":"Could not find relation 'test.fakefake' in the schema cache" } |] { matchStatus = 404 , matchHeaders = [] diff --git a/test/spec/Feature/Query/DeleteSpec.hs b/test/spec/Feature/Query/DeleteSpec.hs index 7e3c83d10e..db0d89b233 100644 --- a/test/spec/Feature/Query/DeleteSpec.hs +++ b/test/spec/Feature/Query/DeleteSpec.hs @@ -109,7 +109,15 @@ spec = context "totally unknown route" $ it "fails with 404" $ - request methodDelete "/foozle?id=eq.101" [] "" `shouldRespondWith` 404 + request methodDelete "/foozle?id=eq.101" [] "" + `shouldRespondWith` [json| + { "hint": null, "details":null, + "code":"PGRST205", + "message":"Could not find relation 'test.foozle' in the schema cache" + } |] + { matchStatus = 404 + , matchHeaders = [] + } context "table with limited privileges" $ do it "fails deleting the row when return=representation and selecting all the columns" $ diff --git a/test/spec/Feature/Query/InsertSpec.hs b/test/spec/Feature/Query/InsertSpec.hs index 0f6a1b6e7a..b57a578974 100644 --- a/test/spec/Feature/Query/InsertSpec.hs +++ b/test/spec/Feature/Query/InsertSpec.hs @@ -477,7 +477,7 @@ spec actualPgVersion = do {"id": 204, "body": "yyy"}, {"id": 205, "body": "zzz"}]|] `shouldRespondWith` - [json|{} |] + [json|{"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'test.garlic' in the schema cache"} |] { matchStatus = 404 , matchHeaders = [] } diff --git a/test/spec/Feature/Query/MultipleSchemaSpec.hs b/test/spec/Feature/Query/MultipleSchemaSpec.hs index 7b25e2ffaf..b2b75ff095 100644 --- a/test/spec/Feature/Query/MultipleSchemaSpec.hs +++ b/test/spec/Feature/Query/MultipleSchemaSpec.hs @@ -64,7 +64,13 @@ spec = } it "doesn't find another_table in schema v1" $ - request methodGet "/another_table" [("Accept-Profile", "v1")] "" `shouldRespondWith` 404 + request methodGet "/another_table" + [("Accept-Profile", "v1")] "" + `shouldRespondWith` + [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'v1.another_table' in the schema cache"} |] + { matchStatus = 404 + , matchHeaders = [] + } it "fails trying to read table from unkown schema" $ request methodGet "/parents" [("Accept-Profile", "unkown")] "" `shouldRespondWith` diff --git a/test/spec/Feature/Query/QuerySpec.hs b/test/spec/Feature/Query/QuerySpec.hs index be802fcf51..b21c824757 100644 --- a/test/spec/Feature/Query/QuerySpec.hs +++ b/test/spec/Feature/Query/QuerySpec.hs @@ -24,7 +24,12 @@ spec = do describe "Querying a nonexistent table" $ it "causes a 404" $ - get "/faketable" `shouldRespondWith` 404 + get "/faketable" + `shouldRespondWith` + [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'test.faketable' in the schema cache"} |] + { matchStatus = 404 + , matchHeaders = [] + } describe "Filtering response" $ do it "matches with equality" $ @@ -617,14 +622,12 @@ spec = do , matchHeaders = [matchContentTypeJson] } - it "cannot request a partitioned table as parent from a partition" $ + -- we only search for foreign key relationships after checking the + -- the existence of first table + it "table not found error if first table does not exist" $ get "/car_model_sales_202101?select=id,name,car_models(id,name)&order=id.asc" `shouldRespondWith` - [json| - {"hint":"Perhaps you meant 'car_model_sales' instead of 'car_model_sales_202101'.", - "details":"Searched for a foreign key relationship between 'car_model_sales_202101' and 'car_models' in the schema 'test', but no matches were found.", - "code":"PGRST200", - "message":"Could not find a relationship between 'car_model_sales_202101' and 'car_models' in the schema cache"} |] - { matchStatus = 400 + [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'test.car_model_sales_202101' in the schema cache"} |] + { matchStatus = 404 , matchHeaders = [matchContentTypeJson] } @@ -639,14 +642,10 @@ spec = do , matchHeaders = [matchContentTypeJson] } - it "cannot request partitioned tables as children from a partition" $ + it "table not found error if first table does not exist" $ get "/car_models_default?select=id,name,car_model_sales(id,name)&order=id.asc" `shouldRespondWith` - [json| - {"hint":"Perhaps you meant 'car_model_sales' instead of 'car_models_default'.", - "details":"Searched for a foreign key relationship between 'car_models_default' and 'car_model_sales' in the schema 'test', but no matches were found.", - "code":"PGRST200", - "message":"Could not find a relationship between 'car_models_default' and 'car_model_sales' in the schema cache"} |] - { matchStatus = 400 + [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'test.car_models_default' in the schema cache"} |] + { matchStatus = 404 , matchHeaders = [matchContentTypeJson] } diff --git a/test/spec/Feature/Query/UpdateSpec.hs b/test/spec/Feature/Query/UpdateSpec.hs index 3faaab4518..f8510e89db 100644 --- a/test/spec/Feature/Query/UpdateSpec.hs +++ b/test/spec/Feature/Query/UpdateSpec.hs @@ -17,7 +17,12 @@ spec = do it "indicates no table found by returning 404" $ request methodPatch "/fake" [] [json| { "real": false } |] - `shouldRespondWith` 404 + `shouldRespondWith` + [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'test.fake' in the schema cache"} |] + { matchStatus = 404 + , matchHeaders = [] + } + context "on an empty table" $ it "succeeds with status code 204" $ @@ -343,7 +348,7 @@ spec = do {"id": 204, "body": "yyy"}, {"id": 205, "body": "zzz"}]|] `shouldRespondWith` - [json|{} |] + [json| {"code":"PGRST205","details":null,"hint":null,"message":"Could not find relation 'test.garlic' in the schema cache"} |] { matchStatus = 404 , matchHeaders = [] }