From 81bd72f4d7147886c2fcb046e83346a0f5d229ba Mon Sep 17 00:00:00 2001
From: Tom Harding <i.am.tom.harding@gmail.com>
Date: Tue, 8 Oct 2024 14:31:37 +0200
Subject: [PATCH] Report a path for `UnknownModelCollection` during model
 resolution (#1216)

### What

Like #1192, this PR adds a context to another error raised during the
`models` resolution step. Specifically, `UnknownModelCollection`.

### How

These PRs are extremely simple, it's just that there are going to be
dozens of them. When an error refers to a thing, you wrap that thing in
`Spanned`, and then try to extract the `path` at the error site. Most of
the time, it's pretty straightforward.

---------

Co-authored-by: Daniel Chambers <daniel@hasura.io>
V3_GIT_ORIGIN_REV_ID: d3ea71d3d9a42cbf804add54f5f6e75ab68c6765
---
 v3/changelog.md                               |  3 +++
 .../src/stages/models/source.rs               | 20 ++++++++++++-------
 v3/crates/open-dds/metadata.jsonschema        | 14 +++----------
 v3/crates/open-dds/src/models.rs              |  2 +-
 4 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/v3/changelog.md b/v3/changelog.md
index 13a04dfe28c6a..c4c1465fbb189 100644
--- a/v3/changelog.md
+++ b/v3/changelog.md
@@ -4,6 +4,9 @@
 
 ### Added
 
+- Added contexts to more MBS errors: when a model refers to a collection that
+  doesn't exist, the path to the offending reference will be reported.
+
 ### Fixed
 
 - Fix local `docker-compose.yaml` file so that running `docker compose up`
diff --git a/v3/crates/metadata-resolve/src/stages/models/source.rs b/v3/crates/metadata-resolve/src/stages/models/source.rs
index d2f3b3d6c3a1d..4550e6f4f9359 100644
--- a/v3/crates/metadata-resolve/src/stages/models/source.rs
+++ b/v3/crates/metadata-resolve/src/stages/models/source.rs
@@ -69,10 +69,16 @@ pub(crate) fn resolve_model_source(
         .schema
         .collections
         .get(model_source.collection.as_str())
-        .ok_or_else(|| ModelsError::UnknownModelCollection {
-            model_name: model.name.clone(),
-            data_connector: qualified_data_connector_name.clone(),
-            collection: model_source.collection.clone(),
+        .ok_or_else(|| crate::WithContext::Contextualised {
+            error: ModelsError::UnknownModelCollection {
+                model_name: model.name.clone(),
+                data_connector: qualified_data_connector_name.clone(),
+                collection: model_source.collection.value.clone(),
+            },
+            context: error_context::Context(vec![error_context::Step {
+                message: "Collection name given here".to_string(),
+                path: model_source.collection.path.clone(),
+            }]),
         })?;
     let source_collection_type =
         DataConnectorObjectType::from(source_collection.collection_type.as_str());
@@ -103,7 +109,7 @@ pub(crate) fn resolve_model_source(
     .map_err(|err| ModelsError::ModelCollectionArgumentMappingError {
         data_connector_name: qualified_data_connector_name.clone(),
         model_name: model.name.clone(),
-        collection_name: model_source.collection.clone(),
+        collection_name: model_source.collection.value.clone(),
         error: err,
     })?;
 
@@ -112,7 +118,7 @@ pub(crate) fn resolve_model_source(
         .map(|issue| ModelsIssue::FunctionArgumentMappingIssue {
             data_connector_name: qualified_data_connector_name.clone(),
             model_name: model.name.clone(),
-            collection_name: model_source.collection.clone(),
+            collection_name: model_source.collection.value.clone(),
             issue,
         })
         .collect();
@@ -147,7 +153,7 @@ pub(crate) fn resolve_model_source(
         )
         .map(Arc::new)
         .map_err(ModelsError::from)?,
-        collection: model_source.collection.clone(),
+        collection: model_source.collection.value.clone(),
         collection_type: source_collection_type,
         type_mappings,
         argument_mappings,
diff --git a/v3/crates/open-dds/metadata.jsonschema b/v3/crates/open-dds/metadata.jsonschema
index f349505b6dc04..355d2dbec5219 100644
--- a/v3/crates/open-dds/metadata.jsonschema
+++ b/v3/crates/open-dds/metadata.jsonschema
@@ -722,12 +722,6 @@
       },
       "additionalProperties": false
     },
-    "CollectionName": {
-      "$id": "https://hasura.io/jsonschemas/metadata/CollectionName",
-      "title": "CollectionName",
-      "description": "The name of a collection in a data connector.",
-      "type": "string"
-    },
     "ColumnFieldMapping": {
       "$id": "https://hasura.io/jsonschemas/metadata/ColumnFieldMapping",
       "title": "ColumnFieldMapping",
@@ -2738,12 +2732,10 @@
           "pattern": "^[_a-zA-Z][_a-zA-Z0-9]*$"
         },
         "collection": {
+          "$id": "https://hasura.io/jsonschemas/metadata/CollectionName",
+          "title": "CollectionName",
           "description": "The collection in the data connector that backs this model.",
-          "allOf": [
-            {
-              "$ref": "#/definitions/CollectionName"
-            }
-          ]
+          "type": "string"
         },
         "argumentMapping": {
           "description": "Mapping from model argument names to data connector collection argument names.",
diff --git a/v3/crates/open-dds/src/models.rs b/v3/crates/open-dds/src/models.rs
index afb7d0cdd68e9..fce1e45daf566 100644
--- a/v3/crates/open-dds/src/models.rs
+++ b/v3/crates/open-dds/src/models.rs
@@ -235,7 +235,7 @@ pub struct ModelSource {
     pub data_connector_name: Spanned<DataConnectorName>,
 
     /// The collection in the data connector that backs this model.
-    pub collection: CollectionName,
+    pub collection: Spanned<CollectionName>,
 
     /// Mapping from model argument names to data connector collection argument names.
     #[opendd(default)]