feat: Improve error message for ambiguous embedding

- Adds a relevant hint that includes unambiguous embedding suggestions.
- Joins origin and target into one single embedding key

CHANGELOG.md

@@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - #1938, Allow escaping inside double quotes with a backslash, e.g. `?col=in.("Double\"Quote")`, `?col=in.("Back\\slash")` - @steve-chavez
 - #1075, Allow filtering top-level resource based on embedded resources filters. This is enabled by adding `!inner` to the embedded resource, e.g. `/projects?select=*,clients!inner(*)&clients.id=eq.12`- @steve-chavez, @Iced-Sun
 - #1988, Allow specifying `unknown` for the `is` operator - @steve-chavez
+- #2031, Improve error message for ambiguous embedding and add a relevant hint that includes unambiguous embedding suggestions - @laurenceisla
 
 ### Fixed
 

src/PostgREST/Error.hs

@@ -97,8 +97,8 @@ instance JSON.ToJSON ApiRequestError where
     "hint"    .= ("If a new foreign key between these entities was created in the database, try reloading the schema cache." :: Text),
     "message" .= ("Could not find a relationship between " <> parent <> " and " <> child <> " in the schema cache" :: Text)]
   toJSON (AmbiguousRelBetween parent child rels) = JSON.object [
-    "hint"    .= ("By following the 'details' key, disambiguate the request by changing the url to /origin?select=relationship(*) or /origin?select=target!relationship(*)" :: Text),
-    "message" .= ("More than one relationship was found for " <> parent <> " and " <> child :: Text),
+    "hint"    .= ("Try changing '" <> child <> "' to one of the following: " <> relHint rels <> ". Find the desired relationship in the 'details' key." :: Text),
+    "message" .= ("Could not embed because more than one relationship was found for '" <> parent <> "' and '" <> child <> "'" :: Text),
     "details" .= (compressedRel <$> rels) ]
   toJSON (AmbiguousRpc procs)  = JSON.object [
     "hint"    .= ("Try renaming the parameters or the function itself in the database so function overloading can be resolved" :: Text),
@@ -129,23 +129,31 @@ compressedRel Relationship{..} =
     fmtTbl Table{..} = tableSchema <> "." <> tableName
     fmtEls els = "[" <> T.intercalate ", " els <> "]"
   in
-  JSON.object $ [
-    "origin"      .= fmtTbl relTable
-  , "target"      .= fmtTbl relForeignTable
-  ] ++
-  case relCardinality of
-    M2M Junction{..} -> [
-        "cardinality" .= ("m2m" :: Text)
-      , "relationship" .= (fmtTbl junTable <> fmtEls [junConstraint1] <> fmtEls [junConstraint2])
-      ]
-    M2O cons -> [
-        "cardinality" .= ("m2o" :: Text)
-      , "relationship" .= (cons <> fmtEls (colName <$> relColumns) <> fmtEls (colName <$> relForeignColumns))
-      ]
-    O2M cons -> [
-        "cardinality" .= ("o2m" :: Text)
-      , "relationship" .= (cons <> fmtEls (colName <$> relColumns) <> fmtEls (colName <$> relForeignColumns))
-      ]
+  JSON.object $
+    ("embedding" .= (tableName relTable <> " with " <> tableName relForeignTable :: Text))
+    : case relCardinality of
+        M2M Junction{..} -> [
+            "cardinality" .= ("many-to-many" :: Text)
+          , "relationship" .= (fmtTbl junTable <> fmtEls [junConstraint1] <> fmtEls [junConstraint2])
+          ]
+        M2O cons -> [
+            "cardinality" .= ("many-to-one" :: Text)
+          , "relationship" .= (cons <> fmtEls (colName <$> relColumns) <> fmtEls (colName <$> relForeignColumns))
+          ]
+        O2M cons -> [
+            "cardinality" .= ("one-to-many" :: Text)
+          , "relationship" .= (cons <> fmtEls (colName <$> relColumns) <> fmtEls (colName <$> relForeignColumns))
+          ]
+
+relHint :: [Relationship] -> Text
+relHint rels = T.intercalate ", " (hintList <$> rels)
+  where
+    hintList Relationship{..} =
+      let buildHint rel = "'" <> tableName relForeignTable <> "!" <> rel <> "'" in
+      case relCardinality of
+        M2M Junction{..} -> buildHint (tableName junTable)
+        M2O cons         -> buildHint cons
+        O2M cons         -> buildHint cons
 
 data PgError = PgError Authenticated SQL.UsageError
 type Authenticated = Bool

test/Feature/EmbedDisambiguationSpec.hs

@@ -19,20 +19,18 @@ spec =
             {
               "details": [
                 {
-                    "cardinality": "m2o",
+                    "cardinality": "many-to-one",
                     "relationship": "message_sender_fkey[sender][id]",
-                    "origin": "test.message",
-                    "target": "test.person"
+                    "embedding": "message with person"
                 },
                 {
-                    "cardinality": "m2o",
+                    "cardinality": "many-to-one",
                     "relationship": "message_sender_fkey[sender][id]",
-                    "origin": "test.message",
-                    "target": "test.person_detail"
+                    "embedding": "message with person_detail"
                 }
               ],
-              "hint": "By following the 'details' key, disambiguate the request by changing the url to /origin?select=relationship(*) or /origin?select=target!relationship(*)",
-              "message": "More than one relationship was found for message and sender"
+              "hint": "Try changing 'sender' to one of the following: 'person!message_sender_fkey', 'person_detail!message_sender_fkey'. Find the desired relationship in the 'details' key.",
+              "message": "Could not embed because more than one relationship was found for 'message' and 'sender'"
             }
           |]
           { matchStatus  = 300
@@ -45,26 +43,23 @@ spec =
             {
               "details": [
                 {
-                  "cardinality": "m2o",
+                  "cardinality": "many-to-one",
                   "relationship": "main_project[main_project_id][big_project_id]",
-                  "origin": "test.sites",
-                  "target": "test.big_projects"
+                  "embedding": "sites with big_projects"
                 },
                 {
-                  "cardinality": "m2m",
+                  "cardinality": "many-to-many",
                   "relationship": "test.jobs[jobs_site_id_fkey][jobs_big_project_id_fkey]",
-                  "origin": "test.sites",
-                  "target": "test.big_projects"
+                  "embedding": "sites with big_projects"
                 },
                 {
-                  "cardinality": "m2m",
+                  "cardinality": "many-to-many",
                   "relationship": "test.main_jobs[jobs_site_id_fkey][jobs_big_project_id_fkey]",
-                  "origin": "test.sites",
-                  "target": "test.big_projects"
+                  "embedding": "sites with big_projects"
                 }
               ],
-              "hint": "By following the 'details' key, disambiguate the request by changing the url to /origin?select=relationship(*) or /origin?select=target!relationship(*)",
-              "message": "More than one relationship was found for sites and big_projects"
+              "hint": "Try changing 'big_projects' to one of the following: 'big_projects!main_project', 'big_projects!jobs', 'big_projects!main_jobs'. Find the desired relationship in the 'details' key.",
+              "message": "Could not embed because more than one relationship was found for 'sites' and 'big_projects'"
             }
           |]
           { matchStatus  = 300
@@ -77,20 +72,18 @@ spec =
             {
               "details": [
                 {
-                    "cardinality": "m2o",
+                    "cardinality": "many-to-one",
                     "relationship": "agents_department_id_fkey[department_id][id]",
-                    "origin": "test.agents",
-                    "target": "test.departments"
+                    "embedding": "agents with departments"
                 },
                 {
-                    "cardinality": "o2m",
+                    "cardinality": "one-to-many",
                     "relationship": "departments_head_id_fkey[id][head_id]",
-                    "origin": "test.agents",
-                    "target": "test.departments"
+                    "embedding": "agents with departments"
                 }
               ],
-              "hint": "By following the 'details' key, disambiguate the request by changing the url to /origin?select=relationship(*) or /origin?select=target!relationship(*)",
-              "message": "More than one relationship was found for agents and departments"
+              "hint": "Try changing 'departments' to one of the following: 'departments!agents_department_id_fkey', 'departments!departments_head_id_fkey'. Find the desired relationship in the 'details' key.",
+              "message": "Could not embed because more than one relationship was found for 'agents' and 'departments'"
             }
            |]
           { matchStatus  = 300
@@ -106,32 +99,28 @@ spec =
             {
               "details": [
                 {
-                  "cardinality": "m2m",
+                  "cardinality": "many-to-many",
                   "relationship": "test.whatev_jobs[whatev_jobs_site_id_1_fkey][whatev_jobs_project_id_1_fkey]",
-                  "origin": "test.whatev_sites",
-                  "target": "test.whatev_projects"
+                  "embedding": "whatev_sites with whatev_projects"
                 },
                 {
-                  "cardinality": "m2m",
+                  "cardinality": "many-to-many",
                   "relationship": "test.whatev_jobs[whatev_jobs_site_id_1_fkey][whatev_jobs_project_id_2_fkey]",
-                  "origin": "test.whatev_sites",
-                  "target": "test.whatev_projects"
+                  "embedding": "whatev_sites with whatev_projects"
                 },
                 {
-                  "cardinality": "m2m",
+                  "cardinality": "many-to-many",
                   "relationship": "test.whatev_jobs[whatev_jobs_site_id_2_fkey][whatev_jobs_project_id_1_fkey]",
-                  "origin": "test.whatev_sites",
-                  "target": "test.whatev_projects"
+                  "embedding": "whatev_sites with whatev_projects"
                 },
                 {
-                  "cardinality": "m2m",
+                  "cardinality": "many-to-many",
                   "relationship": "test.whatev_jobs[whatev_jobs_site_id_2_fkey][whatev_jobs_project_id_2_fkey]",
-                  "origin": "test.whatev_sites",
-                  "target": "test.whatev_projects"
+                  "embedding": "whatev_sites with whatev_projects"
                 }
               ],
-              "hint": "By following the 'details' key, disambiguate the request by changing the url to /origin?select=relationship(*) or /origin?select=target!relationship(*)",
-              "message": "More than one relationship was found for whatev_sites and whatev_projects"
+              "hint": "Try changing 'whatev_projects' to one of the following: 'whatev_projects!whatev_jobs', 'whatev_projects!whatev_jobs', 'whatev_projects!whatev_jobs', 'whatev_projects!whatev_jobs'. Find the desired relationship in the 'details' key.",
+              "message": "Could not embed because more than one relationship was found for 'whatev_sites' and 'whatev_projects'"
             }
           |]
           { matchStatus  = 300