From 6f9b31b54b75f0a3627f1ae10e1fe23c13f0f2bc Mon Sep 17 00:00:00 2001 From: Mike Storey Date: Sat, 12 Jul 2025 14:06:50 -0400 Subject: [PATCH 1/2] feat: Make migration and test_data endpoints consistent, simplify DELETE to return event, update stepci tests for File object lists --- configurator/routes/migration_routes.py | 23 ++------------ configurator/routes/test_data_routes.py | 16 +++------- tests/routes/test_migration_routes.py | 40 ++++++++++++++++++------- tests/routes/test_test_data_routes.py | 11 +++++-- tests/stepci/migrations.yaml | 2 +- 5 files changed, 45 insertions(+), 47 deletions(-) diff --git a/configurator/routes/migration_routes.py b/configurator/routes/migration_routes.py index a6362d4..04176bc 100644 --- a/configurator/routes/migration_routes.py +++ b/configurator/routes/migration_routes.py @@ -1,7 +1,7 @@ from flask import Blueprint, request, jsonify from configurator.utils.config import Config from configurator.utils.configurator_exception import ConfiguratorEvent, ConfiguratorException -from configurator.utils.file_io import FileIO +from configurator.utils.file_io import FileIO, File from configurator.utils.route_decorators import event_route import logging import os @@ -17,11 +17,7 @@ def create_migration_routes(): @event_route("MIG-01", "GET_MIGRATIONS", "listing migrations") def get_migrations(): files = FileIO.get_documents(config.MIGRATIONS_FOLDER) - filenames = [file.file_name for file in files] - return jsonify(filenames) - - - + return jsonify([file.to_dict() for file in files]) # GET /api/migrations// - Get a migration file @migration_routes.route('//', methods=['GET']) @@ -45,20 +41,7 @@ def put_migration(file_name): @migration_routes.route('//', methods=['DELETE']) @event_route("MIG-06", "DELETE_MIGRATION", "deleting migration") def delete_migration(file_name): - event = ConfiguratorEvent(event_id="MIG-06", event_type="DELETE_MIGRATION") - try: - delete_event = FileIO.delete_document(config.MIGRATIONS_FOLDER, file_name) - if delete_event.status == "SUCCESS": - event.record_success() - else: - event.append_events([delete_event]) - event.record_failure("error deleting migration") - except ConfiguratorException as e: - event.append_events([e.event]) - event.record_failure("error deleting migration") - except Exception as e: - event.record_failure("unexpected error deleting migration", {"error": str(e)}) - return jsonify(event.to_dict()) + return jsonify(FileIO.delete_document(config.MIGRATIONS_FOLDER, file_name).to_dict()) logger.info("Migration Flask Routes Registered") return migration_routes \ No newline at end of file diff --git a/configurator/routes/test_data_routes.py b/configurator/routes/test_data_routes.py index 29a35a7..f6d8295 100644 --- a/configurator/routes/test_data_routes.py +++ b/configurator/routes/test_data_routes.py @@ -1,7 +1,7 @@ from flask import Blueprint, request, jsonify, abort, Response from configurator.utils.config import Config from configurator.utils.configurator_exception import ConfiguratorEvent, ConfiguratorException -from configurator.utils.file_io import FileIO +from configurator.utils.file_io import FileIO, File from configurator.utils.route_decorators import event_route import json import logging @@ -33,10 +33,7 @@ def create_test_data_routes(): def get_data_files(): files = FileIO.get_documents(config.TEST_DATA_FOLDER) # Only include .json files - return jsonify([ - {**{('file_name' if k == 'name' else k): v for k, v in file.to_dict().items()}} - for file in files if file.file_name.endswith('.json') - ]) + return jsonify([file.to_dict() for file in files if file.file_name.endswith('.json')]) @@ -69,19 +66,14 @@ def update_test_data(file_name): if not file_name.endswith('.json'): abort(400, description='Test data files must be .json') file = FileIO.put_document(config.TEST_DATA_FOLDER, file_name, request.json) - d = file.to_dict() - d['file_name'] = d.pop('name', file_name) - return jsonify(d) + return jsonify(file.to_dict()) @test_data_routes.route('//', methods=['DELETE']) @event_route("TST-04", "DELETE_TEST_DATA", "deleting test data") def delete_test_data(file_name): if not file_name.endswith('.json'): abort(404) - file = FileIO.delete_document(config.TEST_DATA_FOLDER, file_name) - d = file.to_dict() - d['file_name'] = d.pop('name', file_name) - return jsonify(d) + return jsonify(FileIO.delete_document(config.TEST_DATA_FOLDER, file_name).to_dict()) logger.info("test_data Flask Routes Registered") return test_data_routes \ No newline at end of file diff --git a/tests/routes/test_migration_routes.py b/tests/routes/test_migration_routes.py index 38e4aca..47d33d7 100644 --- a/tests/routes/test_migration_routes.py +++ b/tests/routes/test_migration_routes.py @@ -38,9 +38,16 @@ def test_list_migrations(self): resp = self.app.get("/api/migrations/") self.assertEqual(resp.status_code, 200) data = resp.get_json() - # For successful responses, expect data directly, not wrapped in event envelope - self.assertIn("mig1.json", data) - self.assertIn("mig2.json", data) + # For successful responses, expect File objects with metadata + self.assertIsInstance(data, list) + self.assertTrue(any("mig1.json" in file.get("file_name", "") for file in data)) + self.assertTrue(any("mig2.json" in file.get("file_name", "") for file in data)) + # Check that each item has the expected File object structure + for file in data: + self.assertIn("file_name", file) + self.assertIn("created_at", file) + self.assertIn("updated_at", file) + self.assertIn("size", file) def test_get_migration(self): resp = self.app.get("/api/migrations/mig1.json/") @@ -80,7 +87,7 @@ def test_delete_migration(self): resp = self.app.delete("/api/migrations/mig1.json/") self.assertEqual(resp.status_code, 200) data = resp.get_json() - # For successful responses, expect ConfiguratorEvent with SUCCESS status + # For successful responses, expect ConfiguratorEvent object self.assertIn("id", data) self.assertIn("type", data) self.assertIn("status", data) @@ -89,7 +96,7 @@ def test_delete_migration(self): def test_delete_migration_not_found(self): resp = self.app.delete("/api/migrations/doesnotexist.json/") - self.assertEqual(resp.status_code, 200) # Now returns 200 with failure event + self.assertEqual(resp.status_code, 200) # Should return event with FAILURE status data = resp.get_json() self.assertIn("id", data) self.assertIn("type", data) @@ -111,9 +118,9 @@ def test_list_migrations_success(self, mock_file_io): """Test successful GET /api/migrations/.""" # Arrange mock_file1 = Mock() - mock_file1.file_name = "migration1.json" + mock_file1.to_dict.return_value = {"file_name": "migration1.json", "size": 100, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"} mock_file2 = Mock() - mock_file2.file_name = "migration2.json" + mock_file2.to_dict.return_value = {"file_name": "migration2.json", "size": 200, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"} mock_files = [mock_file1, mock_file2] with patch('configurator.routes.migration_routes.FileIO') as mock_file_io: @@ -125,7 +132,7 @@ def test_list_migrations_success(self, mock_file_io): # Assert self.assertEqual(response.status_code, 200) response_data = response.json - self.assertEqual(response_data, ["migration1.json", "migration2.json"]) + self.assertEqual(response_data, [{"file_name": "migration1.json", "size": 100, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"}, {"file_name": "migration2.json", "size": 200, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"}]) @patch('configurator.routes.migration_routes.FileIO') def test_list_migrations_general_exception(self, mock_file_io): @@ -222,6 +229,7 @@ def test_delete_migration_success(self, mock_file_io, mock_exists): mock_exists.return_value = True mock_event = Mock() mock_event.status = "SUCCESS" + mock_event.to_dict.return_value = {"id": "MIG-06", "type": "DELETE_MIGRATION", "status": "SUCCESS", "data": "test_migration.json deleted"} mock_file_io.delete_document.return_value = mock_event # Act @@ -240,13 +248,16 @@ def test_delete_migration_success(self, mock_file_io, mock_exists): def test_delete_migration_general_exception(self, mock_file_io): """Test DELETE /api/migrations/ when FileIO raises a general exception.""" # Arrange - mock_file_io.delete_document.side_effect = Exception("Unexpected error") + mock_event = Mock() + mock_event.status = "FAILURE" + mock_event.to_dict.return_value = {"id": "MIG-06", "type": "DELETE_MIGRATION", "status": "FAILURE", "data": "Unexpected error"} + mock_file_io.delete_document.return_value = mock_event # Act response = self.client.delete('/api/migrations/test_migration.json/') # Assert - self.assertEqual(response.status_code, 200) # Now returns 200 with failure event + self.assertEqual(response.status_code, 200) # Should return event with FAILURE status response_data = response.json self.assertIn("id", response_data) self.assertIn("type", response_data) @@ -269,8 +280,10 @@ def test_get_migrations_success(self): # Arrange mock_file1 = Mock() mock_file1.file_name = "migration1.json" + mock_file1.to_dict.return_value = {"file_name": "migration1.json", "size": 100, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"} mock_file2 = Mock() mock_file2.file_name = "migration2.json" + mock_file2.to_dict.return_value = {"file_name": "migration2.json", "size": 200, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"} mock_files = [mock_file1, mock_file2] with patch('configurator.routes.migration_routes.FileIO') as mock_file_io: @@ -282,7 +295,12 @@ def test_get_migrations_success(self): # Assert self.assertEqual(response.status_code, 200) response_data = response.json - self.assertEqual(response_data, ["migration1.json", "migration2.json"]) + self.assertIsInstance(response_data, list) + self.assertEqual(len(response_data), 2) + self.assertIn("file_name", response_data[0]) + self.assertIn("file_name", response_data[1]) + self.assertEqual(response_data[0]["file_name"], "migration1.json") + self.assertEqual(response_data[1]["file_name"], "migration2.json") if __name__ == "__main__": unittest.main() \ No newline at end of file diff --git a/tests/routes/test_test_data_routes.py b/tests/routes/test_test_data_routes.py index 9ba87b0..c66d9f1 100644 --- a/tests/routes/test_test_data_routes.py +++ b/tests/routes/test_test_data_routes.py @@ -91,7 +91,7 @@ def test_put_data_file_success(self, mock_file_io): # Arrange test_data = {"data": "test content"} mock_file = Mock() - mock_file.to_dict.return_value = {"name": "test_file.json", "size": 100, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"} + mock_file.to_dict.return_value = {"file_name": "test_file.json", "size": 100, "created_at": "2023-01-01T00:00:00", "updated_at": "2023-01-01T00:00:00"} mock_file_io.put_document.return_value = mock_file # Act @@ -126,7 +126,8 @@ def test_delete_data_file_success(self, mock_file_io): """Test successful DELETE /api/test_data/.""" # Arrange mock_event = Mock() - mock_event.to_dict.return_value = {"deleted": True, "file_name": "test_file.json"} + mock_event.status = "SUCCESS" + mock_event.to_dict.return_value = {"id": "TST-04", "type": "DELETE_TEST_DATA", "status": "SUCCESS", "data": "test_file.json deleted"} mock_file_io.delete_document.return_value = mock_event # Act @@ -135,7 +136,11 @@ def test_delete_data_file_success(self, mock_file_io): # Assert self.assertEqual(response.status_code, 200) response_data = response.json - self.assertEqual(response_data, {"deleted": True, "file_name": "test_file.json"}) + self.assertIn("id", response_data) + self.assertIn("type", response_data) + self.assertIn("status", response_data) + self.assertIn("data", response_data) + self.assertEqual(response_data["status"], "SUCCESS") @patch('configurator.routes.test_data_routes.FileIO') def test_delete_data_file_general_exception(self, mock_file_io): diff --git a/tests/stepci/migrations.yaml b/tests/stepci/migrations.yaml index cb86432..93a987e 100644 --- a/tests/stepci/migrations.yaml +++ b/tests/stepci/migrations.yaml @@ -98,7 +98,7 @@ tests: check: status: /200/ jsonpath: - $: /.*test_migration\.json.*/ + $[*].file_name: /.*test_migration\.json.*/ # Cleanup - DELETE the test migration - name: Delete Test Migration (Cleanup) From e54b4f7dc28e8b82d5658b3b8763dcb475201c95 Mon Sep 17 00:00:00 2001 From: Mike Storey Date: Sat, 12 Jul 2025 14:09:30 -0400 Subject: [PATCH 2/2] Openapi updates --- docs/openapi.yaml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 6cda38f..8af121e 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -620,13 +620,11 @@ paths: - Migrations responses: '200': - description: List of migration file names + description: List of migration files content: application/json: schema: - type: array - items: - type: string + $ref: '#/components/schemas/files' '500': description: Processing error content: @@ -716,7 +714,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/events' + $ref: '#/components/schemas/file' '500': description: Processing error content: