From 082da78f4c61424eedbd1c26eb1c1f84513ca099 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Mon, 19 Apr 2021 18:41:49 -0500 Subject: [PATCH] fix: disregard internal junction when embedding Added test cases for a lone internal junction and an internal junction exposed homonym --- src/PostgREST/DbStructure.hs | 20 +++++- test/Feature/EmbedDisambiguationSpec.hs | 22 ++++++ test/fixtures/data.sql | 20 ++++++ test/fixtures/privileges.sql | 11 +++ test/fixtures/schema.sql | 92 +++++++++++++++++++++++++ 5 files changed, 164 insertions(+), 1 deletion(-) diff --git a/src/PostgREST/DbStructure.hs b/src/PostgREST/DbStructure.hs index 4a6f7b56a..d2abbbe0e 100644 --- a/src/PostgREST/DbStructure.hs +++ b/src/PostgREST/DbStructure.hs @@ -101,7 +101,7 @@ getDbStructure schemas extraSearchPath pgVer prepared = do cols' = addForeignKeys rels cols keys' = addViewPrimaryKeys srcCols keys - return DbStructure { + return $ removeInternal schemas $ DbStructure { dbTables = tabs , dbColumns = cols' , dbRelationships = rels @@ -110,6 +110,24 @@ getDbStructure schemas extraSearchPath pgVer prepared = do , pgVersion = pgVer } +-- | Remove db objects that belong to an internal schema(not exposed through the API) from the DbStructure. +removeInternal :: [Schema] -> DbStructure -> DbStructure +removeInternal schemas dbStruct = + DbStructure { + dbTables = filter (\x -> tableSchema x `elem` schemas) $ dbTables dbStruct + , dbColumns = filter (\x -> tableSchema (colTable x) `elem` schemas) (dbColumns dbStruct) + , dbRelationships = filter (\x -> tableSchema (relTable x) `elem` schemas && + tableSchema (relForeignTable x) `elem` schemas && + not (hasInternalJunction x)) $ dbRelationships dbStruct + , dbPrimaryKeys = filter (\x -> tableSchema (pkTable x) `elem` schemas) $ dbPrimaryKeys dbStruct + , dbProcs = dbProcs dbStruct -- procs are only obtained from the exposed schemas, no need to filter them. + , pgVersion = pgVersion dbStruct + } + where + hasInternalJunction rel = case relCardinality rel of + M2M Junction{junTable} -> tableSchema junTable `notElem` schemas + _ -> False + decodeTables :: HD.Result [Table] decodeTables = HD.rowList tblRow diff --git a/test/Feature/EmbedDisambiguationSpec.hs b/test/Feature/EmbedDisambiguationSpec.hs index f2c763502..859d1a326 100644 --- a/test/Feature/EmbedDisambiguationSpec.hs +++ b/test/Feature/EmbedDisambiguationSpec.hs @@ -431,3 +431,25 @@ spec = [json|[{"id":1,"users":[{"id":1},{"id":3}]},{"id":2,"users":[{"id":1}]},{"id":3,"users":[{"id":1}]},{"id":4,"users":[{"id":1}]},{"id":5,"users":[{"id":2},{"id":3}]},{"id":6,"users":[{"id":2}]},{"id":7,"users":[{"id":2}]},{"id":8,"users":[]}]|] { matchHeaders = [matchContentTypeJson] } + context "m2m embed when there's a junction in an internal schema" $ do + -- https://github.com/PostgREST/postgrest/issues/1736 + it "works with no ambiguity when there's an exposed view of the junction" $ do + get "/screens?select=labels(name)" `shouldRespondWith` + [json|[{"labels":[{"name":"fruit"}]}, {"labels":[{"name":"vehicles"}]}, {"labels":[{"name":"vehicles"}, {"name":"fruit"}]}]|] + { matchHeaders = [matchContentTypeJson] } + get "/actors?select=*,films(*)" `shouldRespondWith` + [json|[ {"id":1,"name":"john","films":[{"id":12,"title":"douze commandements"}]}, + {"id":2,"name":"mary","films":[{"id":2001,"title":"odyssée de l'espace"}]}]|] + { matchHeaders = [matchContentTypeJson] } + it "doesn't work if the junction is only internal" $ + get "/end_1?select=end_2(*)" `shouldRespondWith` + [json|{"message":"Could not find foreign keys between these entities. No relationship found between end_1 and end_2"}|] + { matchStatus = 400 + , matchHeaders = [matchContentTypeJson] } + it "shouldn't try to embed if the private junction has an exposed homonym" $ + -- ensures the "invalid reference to FROM-clause entry for table "rollen" error doesn't happen. + -- Ref: https://github.com/PostgREST/postgrest/issues/1587#issuecomment-734995669 + get "/schauspieler?select=filme(*)" `shouldRespondWith` + [json|{"message":"Could not find foreign keys between these entities. No relationship found between schauspieler and filme"}|] + { matchStatus = 400 + , matchHeaders = [matchContentTypeJson] } diff --git a/test/fixtures/data.sql b/test/fixtures/data.sql index 8762b39d8..cfe48122d 100644 --- a/test/fixtures/data.sql +++ b/test/fixtures/data.sql @@ -643,3 +643,23 @@ INSERT INTO v2.another_table VALUES(5, 'value 5'), (6, 'value 6'); TRUNCATE TABLE private.stuff CASCADE; INSERT INTO private.stuff (id, name) VALUES (1, 'stuff 1'); + +TRUNCATE TABLE private.screens CASCADE; +INSERT INTO private.screens(name) VALUES ('banana'), ('helicopter'), ('formula 1 banana'); + +INSERT INTO private.labels(name) VALUES ('vehicles'), ('fruit'); + +INSERT INTO private.label_screen(label_id, screen_id) VALUES + ((SELECT id FROM labels WHERE name='vehicles'), (SELECT id FROM screens WHERE name='helicopter')), + ((SELECT id FROM labels WHERE name='vehicles'), (SELECT id FROM screens WHERE name='formula 1 banana')), + ((SELECT id FROM labels WHERE name='fruit'), (SELECT id FROM screens WHERE name='banana')), + ((SELECT id FROM labels WHERE name='fruit'), (SELECT id FROM screens WHERE name='formula 1 banana')); + +TRUNCATE TABLE private.actors CASCADE; +INSERT INTO private.actors (id, name) VALUES (1,'john'), (2,'mary'); + +TRUNCATE TABLE private.films CASCADE; +INSERT INTO private.films (id, title) VALUES (12,'douze commandements'), (2001,'odyssée de l''espace'); + +TRUNCATE TABLE private.personnages CASCADE; +INSERT INTO private.personnages (film_id, role_id, character) VALUES (12,1,'méchant'), (2001,2,'astronaute'); diff --git a/test/fixtures/privileges.sql b/test/fixtures/privileges.sql index 4628d6f17..c9c40e94a 100644 --- a/test/fixtures/privileges.sql +++ b/test/fixtures/privileges.sql @@ -138,6 +138,17 @@ GRANT ALL ON TABLE , v2.another_table , v1.children , v2.children + , screens + , labels + , label_screen + , actors + , films + , personnages + , end_1 + , end_2 + , schauspieler + , filme + , rollen TO postgrest_test_anonymous; GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous; diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 4931a654f..19b7e0770 100644 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -1976,3 +1976,95 @@ create or replace function test.reload_pgrst_config() returns void as $_$ begin perform pg_notify('pgrst', 'reload config'); end $_$ language plpgsql ; + +create table private.screens ( + id serial primary key, + name text not null default 'new screen' +); + +create table private.labels ( + id serial primary key, + name text not null default 'new label' +); + +create table private.label_screen ( + label_id int not null references private.labels(id) on update cascade on delete cascade, + screen_id int not null references private.screens(id) on update cascade on delete cascade, + constraint label_screen_pkey primary key (label_id, screen_id) +); + +create view test.labels as +select * from private.labels; + +create view test.screens as +select * from private.screens; + +create view test.label_screen as +select * from private.label_screen; + +create table private.actors ( + id int, + name text, + constraint actors_id primary key (id) +); + +create table private.films ( + id integer, + title text, + constraint films_id primary key (id) +); + +create table private.personnages ( + film_id int not null, + role_id int not null, + character text not null, + constraint personnages_film_id_role_id primary key (film_id, role_id), + constraint personnages_film_id_fkey foreign key (film_id) references private.films(id) not deferrable, + constraint personnages_role_id_fkey foreign key (role_id) references private.actors(id) not deferrable +); + +create view test.actors as +select * from private.actors; + +create view test.films as +select * from private.films; + +create view test.personnages as +select * from private.personnages; + +create table test.end_1( + id int primary key, + name text +); + +create table test.end_2( + id int primary key, + name text +); + +create table private.junction( + end_1_id int not null references test.end_1(id) on update cascade on delete cascade, + end_2_id int not null references test.end_2(id) on update cascade on delete cascade, + primary key (end_1_id, end_2_id) +); + +create table test.schauspieler ( + id int primary key, + name text +); + +create table test.filme ( + id int primary key, + titel text +); + +create table test.rollen (); + +create table private.rollen ( + film_id int not null, + rolle_id int not null, + charakter text not null, + primary key (film_id, rolle_id), + foreign key (film_id) references test.filme(id), + foreign key (rolle_id) references test.schauspieler(id) +);