diff --git a/src/library/PostgREST/Response/OpenAPI.hs b/src/library/PostgREST/Response/OpenAPI.hs index 9276b7d50..13065e075 100644 --- a/src/library/PostgREST/Response/OpenAPI.hs +++ b/src/library/PostgREST/Response/OpenAPI.hs @@ -30,8 +30,8 @@ import PostgREST.Network (escapeHostName) import PostgREST.Query.OpenApi (TableAccess (..), TablesAccess) import PostgREST.SchemaCache (SchemaCache (..)) import PostgREST.SchemaCache.Identifiers (FieldName, QualifiedIdentifier (..)) -import PostgREST.SchemaCache.Relationship (Cardinality (..), Relationship (..), - RelationshipsMap) +import PostgREST.SchemaCache.Relationship (Cardinality (..), Junction (..), + Relationship (..), RelationshipsMap) import PostgREST.SchemaCache.Routine (FuncVolatility (..), Routine (..), RoutineParam (..)) import PostgREST.SchemaCache.Table (Column (..), Table (..), TablesMap, @@ -110,13 +110,35 @@ parseDefault colType colDefault = makeTableDef :: RelationshipsMap -> (Table, TableAccess) -> (Text, Schema) makeTableDef rels (t, access) = (tn, (mempty :: Schema) - & description .~ tableDescription t + & description .~ tblDescription & type_ ?~ SwaggerObject & properties .~ fromList (makeProperty t rels <$> cols) & required .~ fmap colName (filter (not . colNullable) cols)) where tn = tableName t cols = accessibleCols t (taSelectCols access) + tblDescription = case m2mMarkers t rels of + [] -> tableDescription t + ms -> Just $ maybe "" (`T.append` "\n\n") (tableDescription t) <> T.intercalate "\n" ms + +-- | Emits markers for the many-to-many relationships of a table, so that clients +-- can render these relations. The marker includes the target table(embedding key), +-- the junction table and the junction columns referencing source and target. +m2mMarkers :: Table -> RelationshipsMap -> [Text] +m2mMarkers tbl rels = mapMaybe m2mMarker searchedRels + where + searchedRels = fromMaybe mempty $ HM.lookup (QualifiedIdentifier (tableSchema tbl) (tableName tbl), tableSchema tbl) rels + m2mMarker Relationship{relForeignTable, relCardinality=M2M junction} = + Just $ T.intercalate "" + [ "" + ] + m2mMarker _ = Nothing + junctionSourceCol junction = maybe mempty snd (headMay $ junColsSource junction) + junctionTargetCol junction = maybe mempty snd (headMay $ junColsTarget junction) accessibleCols :: Table -> [FieldName] -> [Column] accessibleCols t cols = filter ((`elem` cols) . colName) (tableColumnsList t) @@ -144,11 +166,18 @@ makeProperty tbl rels col = (colName col, Inline s) (\(a, b) -> T.intercalate "" ["This is a Foreign Key to `", a, ".", b, "`."]) <$> fTblCol pk :: Bool pk = colName col `elem` tablePKCols tbl + uniqueNotes :: [Text] + uniqueNotes = mapMaybe uniqueNote (filter (colName col `elem`) (tableUniqueCols tbl)) + where + uniqueNote cols + | length cols == 1 = Just "This is a Unique column." + | otherwise = Just $ "This is part of a composite unique constraint." n = catMaybes [ Just "Note:" , if pk then Just "This is a Primary Key." else Nothing - , fk ] + <> uniqueNotes + <> catMaybes [fk] d = if length n > 1 then Just $ T.append (maybe "" (`T.append` "\n\n") $ colDescription col) (T.intercalate "\n" n) diff --git a/src/library/PostgREST/SchemaCache.hs b/src/library/PostgREST/SchemaCache.hs index 00ecefe0c..80f9a9906 100644 --- a/src/library/PostgREST/SchemaCache.hs +++ b/src/library/PostgREST/SchemaCache.hs @@ -232,6 +232,7 @@ decodeTables = <*> column HD.bool <*> column HD.bool <*> arrayColumn HD.text + <*> arrayColumn (HD.listArray (HD.nonNullable HD.text)) <*> parseCols (compositeArrayColumn (Column <$> compositeField HD.text @@ -676,6 +677,25 @@ tablesSqlQuery pgVer = AND NOT pg_is_other_temp_schema(r.relnamespace) AND NOT a.attisdropped GROUP BY r.oid + ), + tbl_unique_cols AS ( + SELECT + r.oid AS relid, + array_agg(cols ORDER BY c.oid) AS unique_cols + FROM pg_class r + JOIN pg_constraint c + ON r.oid = c.conrelid + JOIN LATERAL ( + SELECT array_agg(a.attname::text ORDER BY k.ord) AS cols + FROM unnest(c.conkey) WITH ORDINALITY AS k(attnum, ord) + JOIN pg_attribute a ON a.attrelid = r.oid AND a.attnum = k.attnum + ) col_info ON TRUE + WHERE + c.contype = 'u' + AND r.relkind IN ('r', 'p') + AND r.relnamespace NOT IN ('pg_catalog'::regnamespace, 'information_schema'::regnamespace) + AND NOT pg_is_other_temp_schema(r.relnamespace) + GROUP BY r.oid ) SELECT n.nspname AS table_schema, @@ -709,11 +729,13 @@ tablesSqlQuery pgVer = ) ) AS deletable, coalesce(tpks.pk_cols, '{}') as pk_cols, + coalesce(tunq.unique_cols, '{}') as unique_cols, coalesce(cols_agg.columns, '{}') as columns FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace LEFT JOIN pg_description d on d.objoid = c.oid and d.objsubid = 0 and d.classoid = 'pg_class'::regclass LEFT JOIN tbl_pk_cols tpks ON c.oid = tpks.relid + LEFT JOIN tbl_unique_cols tunq ON c.oid = tunq.relid LEFT JOIN columns_agg cols_agg ON c.oid = cols_agg.relid WHERE c.relkind IN ('v','r','m','f','p') AND c.relnamespace NOT IN ('pg_catalog'::regnamespace, 'information_schema'::regnamespace) diff --git a/src/library/PostgREST/SchemaCache/Table.hs b/src/library/PostgREST/SchemaCache/Table.hs index d728d79f1..2b4f82936 100644 --- a/src/library/PostgREST/SchemaCache/Table.hs +++ b/src/library/PostgREST/SchemaCache/Table.hs @@ -29,6 +29,10 @@ data Table = Table , tableUpdatable :: Bool , tableDeletable :: Bool , tablePKCols :: [FieldName] + -- ^ Each element is the position-ordered column list of a unique + -- constraint. A single-column unique constraint is represented by a + -- single-element list. + , tableUniqueCols :: [[FieldName]] , tableColumns :: ColumnMap } deriving (Show, Generic, JSON.ToJSON) diff --git a/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbTables].yaml b/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbTables].yaml index dca22f9e0..195d33d26 100644 --- a/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbTables].yaml +++ b/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbTables].yaml @@ -8,6 +8,7 @@ tableName: authors_only tablePKCols: [] tableSchema: public + tableUniqueCols: [] tableUpdatable: true - - qiName: cats @@ -39,6 +40,7 @@ tablePKCols: - id tableSchema: public + tableUniqueCols: [] tableUpdatable: true - - qiName: items_w_isolation_level @@ -69,6 +71,7 @@ tableName: items_w_isolation_level tablePKCols: [] tableSchema: public + tableUniqueCols: [] tableUpdatable: true - - qiName: directors @@ -100,6 +103,7 @@ tablePKCols: - id tableSchema: public + tableUniqueCols: [] tableUpdatable: true - - qiName: projects @@ -112,6 +116,7 @@ tableName: projects tablePKCols: [] tableSchema: public + tableUniqueCols: [] tableUpdatable: true - - qiName: infinite_recursion @@ -124,6 +129,7 @@ tableName: infinite_recursion tablePKCols: [] tableSchema: public + tableUniqueCols: [] tableUpdatable: false - - qiName: awards @@ -182,6 +188,7 @@ tablePKCols: - id tableSchema: public + tableUniqueCols: [] tableUpdatable: true - - qiName: films @@ -222,6 +229,7 @@ tablePKCols: - id tableSchema: public + tableUniqueCols: [] tableUpdatable: true - - qiName: items @@ -243,4 +251,5 @@ tableName: items tablePKCols: [] tableSchema: public + tableUniqueCols: [] tableUpdatable: true diff --git a/test/io/test_connection.py b/test/io/test_connection.py index 872eb52b7..36caf0485 100644 --- a/test/io/test_connection.py +++ b/test/io/test_connection.py @@ -239,7 +239,7 @@ def test_pool_acquisition_timeout(level, defaultenv, metapostgrest): assert data["message"] == "Timed out acquiring connection from connection pool." # ensure the message appears on the logs as well - output = sorted(postgrest.read_stdout(nlines=10)) + output = sorted(drain_stdout(postgrest)) if level == "crit": assert len(output) == 0 diff --git a/test/io/test_log.py b/test/io/test_log.py index 6ef89e492..d582d0b41 100644 --- a/test/io/test_log.py +++ b/test/io/test_log.py @@ -44,7 +44,7 @@ def test_log_level(level, defaultenv): response = postgrest.session.get("/") assert response.status_code == 200 - output = postgrest.read_stdout(nlines=9) + output = drain_stdout(postgrest) if level == "crit": assert len(output) == 0 @@ -82,7 +82,7 @@ def test_log_level(level, defaultenv): r'- - postgrest_test_anonymous \[.+\] "GET / HTTP/1.1" 200 \d+ "" "python-requests/.+"', ], ) - assert len(output) == 9 + assert len(output) > 3 assert any("Connection" and "is available" in line for line in output) assert any("Connection" and "is used" in line for line in output) @@ -403,7 +403,7 @@ def test_db_error_logging_to_stderr(level, defaultenv, metapostgrest): assert response.status_code == 500 # ensure the message appears on the logs - output = postgrest.read_stdout(nlines=8) + output = drain_stdout(postgrest) if level == "crit": assert len(output) == 0 diff --git a/test/spec/Feature/OpenApi/OpenApiSpec.hs b/test/spec/Feature/OpenApi/OpenApiSpec.hs index 694a30e96..d9057e024 100644 --- a/test/spec/Feature/OpenApi/OpenApiSpec.hs +++ b/test/spec/Feature/OpenApi/OpenApiSpec.hs @@ -285,10 +285,58 @@ spec withConfig = withConfig baseCfg $ describe "OpenAPI" $ do { "format": "int32", "type": "integer", - "description": "Note:\nThis is a Foreign Key to `second.id`." + "description": "Note:\nThis is a Unique column.\nThis is a Foreign Key to `second.id`." } |] + it "includes a unique description for a column with a unique constraint" $ do + r <- simpleBody <$> get "/" + + let uniqueKey = r ^? key "definitions" . key "single_unique" . key "properties" . key "unique_key" + + liftIO $ + uniqueKey `shouldBe` Just + [aesonQQ| + { + "format": "int32", + "type": "integer", + "description": "Note:\nThis is a Unique column." + } + |] + + it "includes the column list of a composite unique constraint" $ do + r <- simpleBody <$> get "/" + + let compoundKey1 = r ^? key "definitions" . key "compound_unique" . key "properties" . key "key1" + compoundKey2 = r ^? key "definitions" . key "compound_unique" . key "properties" . key "key2" + + liftIO $ do + compoundKey1 `shouldBe` Just + [aesonQQ| + { + "format": "int32", + "type": "integer", + "description": "Note:\nThis is part of a composite unique constraint." + } + |] + compoundKey2 `shouldBe` Just + [aesonQQ| + { + "format": "int32", + "type": "integer", + "description": "Note:\nThis is part of a composite unique constraint." + } + |] + + it "includes m2m relationship markers in the table description" $ do + r <- simpleBody <$> get "/" + + let beingDescription = r ^? key "definitions" . key "being" . key "description" + + liftIO $ + beingDescription `shouldBe` Just + [aesonQQ|""|] + describe "Foreign table" $ it "includes foreign table properties" $ do