From da52ee0d1de5027a78baef23a093913b7447e2b3 Mon Sep 17 00:00:00 2001 From: Helen Lin Date: Thu, 12 Jun 2025 11:51:24 -0700 Subject: [PATCH 1/3] feat: do not require _id for managed collection --- src/aind_data_access_api/document_db.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/aind_data_access_api/document_db.py b/src/aind_data_access_api/document_db.py index 8691eca..13700a1 100644 --- a/src/aind_data_access_api/document_db.py +++ b/src/aind_data_access_api/document_db.py @@ -429,7 +429,10 @@ def aggregate_docdb_records(self, pipeline: List[dict]) -> List[dict]: def insert_one_docdb_record(self, record: dict) -> Response: """Insert one new record""" - if record.get("_id") is None: + is_managed_collection = ( + self.database == "metadata_index" and self.collection == "assets" + ) + if record.get("_id") is None and not is_managed_collection: raise ValueError("Record does not have an _id field.") response = self._insert_one_record( json.loads(json.dumps(record, default=str)), From c1842d908d84608aa2f167e63c1220a20d826c77 Mon Sep 17 00:00:00 2001 From: Helen Lin Date: Thu, 12 Jun 2025 12:04:29 -0700 Subject: [PATCH 2/3] test: update unit tests --- tests/test_document_db.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test_document_db.py b/tests/test_document_db.py index 6a11ea2..0218507 100644 --- a/tests/test_document_db.py +++ b/tests/test_document_db.py @@ -579,6 +579,29 @@ def test_insert_one_docdb_record(self, mock_insert: MagicMock): json.loads(json.dumps(record, default=str)), ) + @patch("aind_data_access_api.document_db.Client._insert_one_record") + def test_insert_one_docdb_record_managed(self, mock_insert: MagicMock): + """Tests inserting one docdb record when the target collection is + managed.""" + client = MetadataDbClient( + host=self.example_client_args["host"], + database="metadata_index", + collection="assets", + ) + mock_insert.return_value = {"message": "success"} + # _id is not required for managed collections + record = { + "name": "modal_00000_2000-10-10_10-10-10", + "created": datetime(2000, 10, 10, 10, 10, 10), + "location": "some_url", + "subject": {"subject_id": "00000", "sex": "Female"}, + } + response = client.insert_one_docdb_record(record) + self.assertEqual({"message": "success"}, response) + mock_insert.assert_called_once_with( + json.loads(json.dumps(record, default=str)), + ) + @patch("aind_data_access_api.document_db.Client._insert_one_record") def test_insert_one_docdb_record_invalid(self, mock_insert: MagicMock): """Tests inserting one docdb record if record is invalid""" From d0cce6d7039a6e5cff03478931fb7ea8b983685e Mon Sep 17 00:00:00 2001 From: Helen Lin Date: Fri, 13 Jun 2025 09:37:33 -0700 Subject: [PATCH 3/3] feat: remove _id check for inserts --- src/aind_data_access_api/document_db.py | 5 --- tests/test_document_db.py | 41 ------------------------- 2 files changed, 46 deletions(-) diff --git a/src/aind_data_access_api/document_db.py b/src/aind_data_access_api/document_db.py index 13700a1..d6b68ed 100644 --- a/src/aind_data_access_api/document_db.py +++ b/src/aind_data_access_api/document_db.py @@ -429,11 +429,6 @@ def aggregate_docdb_records(self, pipeline: List[dict]) -> List[dict]: def insert_one_docdb_record(self, record: dict) -> Response: """Insert one new record""" - is_managed_collection = ( - self.database == "metadata_index" and self.collection == "assets" - ) - if record.get("_id") is None and not is_managed_collection: - raise ValueError("Record does not have an _id field.") response = self._insert_one_record( json.loads(json.dumps(record, default=str)), ) diff --git a/tests/test_document_db.py b/tests/test_document_db.py index 0218507..d4be5a7 100644 --- a/tests/test_document_db.py +++ b/tests/test_document_db.py @@ -579,47 +579,6 @@ def test_insert_one_docdb_record(self, mock_insert: MagicMock): json.loads(json.dumps(record, default=str)), ) - @patch("aind_data_access_api.document_db.Client._insert_one_record") - def test_insert_one_docdb_record_managed(self, mock_insert: MagicMock): - """Tests inserting one docdb record when the target collection is - managed.""" - client = MetadataDbClient( - host=self.example_client_args["host"], - database="metadata_index", - collection="assets", - ) - mock_insert.return_value = {"message": "success"} - # _id is not required for managed collections - record = { - "name": "modal_00000_2000-10-10_10-10-10", - "created": datetime(2000, 10, 10, 10, 10, 10), - "location": "some_url", - "subject": {"subject_id": "00000", "sex": "Female"}, - } - response = client.insert_one_docdb_record(record) - self.assertEqual({"message": "success"}, response) - mock_insert.assert_called_once_with( - json.loads(json.dumps(record, default=str)), - ) - - @patch("aind_data_access_api.document_db.Client._insert_one_record") - def test_insert_one_docdb_record_invalid(self, mock_insert: MagicMock): - """Tests inserting one docdb record if record is invalid""" - client = MetadataDbClient(**self.example_client_args) - record_no__id = { - "id": "abc-123", - "name": "modal_00000_2000-10-10_10-10-10", - "created": datetime(2000, 10, 10, 10, 10, 10), - "location": "some_url", - "subject": {"subject_id": "00000", "sex": "Female"}, - } - with self.assertRaises(ValueError) as e: - client.insert_one_docdb_record(record_no__id) - self.assertEqual( - "Record does not have an _id field.", str(e.exception) - ) - mock_insert.assert_not_called() - @patch("aind_data_access_api.document_db.Client._upsert_one_record") def test_upsert_one_docdb_record(self, mock_upsert: MagicMock): """Tests upserting one docdb record"""