Fix collection schemas not having a unique constraint for the _id c…

…olumn (#32)
hasura · Apr 9, 2024 · 1b96bb8 · 1b96bb8
1 parent 3d151cf
commit 1b96bb8
Show file tree

Hide file tree

Showing 13 changed files with 47 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ This changelog documents the changes between release versions.
 - Fix bug in v2 to v3 conversion of query responses containing nested objects ([PR #27](https://github.com/hasura/ndc-mongodb/pull/27))
 - Fixed bug where use of aggregate functions in queries would fail ([#26](https://github.com/hasura/ndc-mongodb/pull/26))
 - Rename Any type to ExtendedJSON to make its representation clearer ([#30](https://github.com/hasura/ndc-mongodb/pull/30))
+- The collection primary key `_id` property now has a unique constraint generated in the NDC schema for it ([#32](https://github.com/hasura/ndc-mongodb/pull/32))
 
 ## [0.0.3] - 2024-03-28
 - Use separate schema files for each collection ([PR #14](https://github.com/hasura/ndc-mongodb/pull/14))

diff --git a/crates/mongodb-connector/src/schema.rs b/crates/mongodb-connector/src/schema.rs
@@ -1,4 +1,5 @@
 use lazy_static::lazy_static;
+use mongodb_support::BsonScalarType;
 use std::collections::BTreeMap;
 
 use configuration::{native_procedure::NativeProcedure, schema, Configuration};
@@ -15,8 +16,8 @@ pub async fn get_schema(
     config: &Configuration,
 ) -> Result<models::SchemaResponse, connector::SchemaError> {
     let schema = &config.schema;
-    let collections = schema.collections.iter().map(map_collection).collect();
     let object_types = config.object_types().map(map_object_type).collect();
+    let collections = schema.collections.iter().map(|(collection_name, collection)| map_collection(&object_types, collection_name, collection)).collect();
 
     let procedures = config
         .native_procedures
@@ -86,14 +87,36 @@ fn map_type(t: &schema::Type) -> models::Type {
     map_normalized_type(&t.clone().normalize_type())
 }
 
-fn map_collection((name, collection): (&String, &schema::Collection)) -> models::CollectionInfo {
+fn get_primary_key_uniqueness_constraint(object_types: &BTreeMap<String, models::ObjectType>, name: &str, collection: &schema::Collection) -> Option<(String, models::UniquenessConstraint)> {
+    // Check to make sure our collection's object type contains the _id objectid field
+    // If it doesn't (should never happen, all collections need an _id column), don't generate the constraint
+    let object_type = object_types.get(&collection.r#type)?;
+    let id_field = object_type.fields.get("_id")?;
+    match &id_field.r#type {
+        models::Type::Named { name } => {
+            if *name == BsonScalarType::ObjectId.graphql_name() { Some(()) } else { None }
+        },
+        models::Type::Nullable { .. } => None,
+        models::Type::Array { .. } => None,
+        models::Type::Predicate { .. } => None,
+    }?;
+    let uniqueness_constraint = models::UniquenessConstraint {
+        unique_columns: vec!["_id".into()]
+    };
+    let constraint_name = format!("{}_id", name);
+    Some((constraint_name, uniqueness_constraint))
+}
+
+fn map_collection(object_types: &BTreeMap<String, models::ObjectType>, name: &str, collection: &schema::Collection) -> models::CollectionInfo {
+    let pk_constraint = get_primary_key_uniqueness_constraint(object_types, name, collection);
+
     models::CollectionInfo {
-        name: name.clone(),
+        name: name.to_owned(),
         collection_type: collection.r#type.clone(),
         description: collection.description.clone(),
         arguments: Default::default(),
         foreign_keys: Default::default(),
-        uniqueness_constraints: Default::default(),
+        uniqueness_constraints: BTreeMap::from_iter(pk_constraint),
     }
 }
 

diff --git a/fixtures/connector/chinook/schema/Album.json b/fixtures/connector/chinook/schema/Album.json
@@ -25,13 +25,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection Album"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/Artist.json b/fixtures/connector/chinook/schema/Artist.json
@@ -22,13 +22,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection Artist"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/Customer.json b/fixtures/connector/chinook/schema/Customer.json
@@ -93,13 +93,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+             "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection Customer"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/Employee.json b/fixtures/connector/chinook/schema/Employee.json
@@ -109,13 +109,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+             "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection Employee"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/Genre.json b/fixtures/connector/chinook/schema/Genre.json
@@ -22,13 +22,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection Genre"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/Invoice.json b/fixtures/connector/chinook/schema/Invoice.json
@@ -65,9 +65,7 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },

diff --git a/fixtures/connector/chinook/schema/InvoiceLine.json b/fixtures/connector/chinook/schema/InvoiceLine.json
@@ -35,9 +35,7 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },

diff --git a/fixtures/connector/chinook/schema/MediaType.json b/fixtures/connector/chinook/schema/MediaType.json
@@ -22,13 +22,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection MediaType"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/Playlist.json b/fixtures/connector/chinook/schema/Playlist.json
@@ -22,13 +22,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection Playlist"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/PlaylistTrack.json b/fixtures/connector/chinook/schema/PlaylistTrack.json
@@ -20,13 +20,11 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },
       "description": "Object type for collection PlaylistTrack"
     }
   }
-}
+}
diff --git a/fixtures/connector/chinook/schema/Track.json b/fixtures/connector/chinook/schema/Track.json
@@ -63,9 +63,7 @@
         },
         "_id": {
           "type": {
-            "nullable": {
-              "scalar": "objectId"
-            }
+            "scalar": "objectId"
           }
         }
       },