|
12 | 12 | from unittest.mock import patch |
13 | 13 |
|
14 | 14 | from flask import current_app |
| 15 | +from sqlalchemy import event, inspect as sa_inspect |
15 | 16 | from cornflow.app import create_app |
16 | 17 | from cornflow.tests import base_test_execution |
17 | 18 |
|
|
30 | 31 | VIEWER_ROLE, |
31 | 32 | ) |
32 | 33 | from cornflow.tests.const import ( |
| 34 | + DAG_URL, |
33 | 35 | EXECUTION_FILES_CLEANUP_URL, |
34 | 36 | EXECUTION_FILES_URL, |
35 | 37 | EXECUTION_PATH, |
| 38 | + EXECUTION_SOLUTION_PATH, |
36 | 39 | EXECUTION_URL_NORUN, |
37 | 40 | INSTANCE_PATH, |
38 | 41 | INSTANCE_URL, |
@@ -492,3 +495,162 @@ def test_cleanup_is_service_only(self): |
492 | 495 | self.assertEqual("0 files were deleted.", response.json["message"]) |
493 | 496 |
|
494 | 497 | # endregion |
| 498 | + |
| 499 | + |
| 500 | +class TestExecutionListDataLoading(CustomTestCase): |
| 501 | + """ |
| 502 | + Tests that the execution list endpoint does not load heavy columns from the |
| 503 | + database and returns only the expected basic fields. |
| 504 | + """ |
| 505 | + |
| 506 | + def setUp(self): |
| 507 | + super().setUp() |
| 508 | + # Load instance fixture and create a parent instance |
| 509 | + with open(INSTANCE_PATH) as f: |
| 510 | + instance_payload = json.load(f) |
| 511 | + self.instance_id = self.create_new_row( |
| 512 | + INSTANCE_URL, InstanceModel, instance_payload |
| 513 | + ) |
| 514 | + |
| 515 | + # Load execution fixture and create the execution (without triggering a run) |
| 516 | + with open(EXECUTION_PATH) as f: |
| 517 | + execution_payload = json.load(f) |
| 518 | + execution_payload["instance_id"] = self.instance_id |
| 519 | + self.execution_id = self.create_new_row( |
| 520 | + EXECUTION_URL_NORUN, ExecutionModel, execution_payload |
| 521 | + ) |
| 522 | + |
| 523 | + # Use a service user to push solution data into the execution via the DAG |
| 524 | + # endpoint so that the `data` column is non-NULL in the database. |
| 525 | + service_token = self.create_service_user() |
| 526 | + with open(EXECUTION_SOLUTION_PATH) as f: |
| 527 | + solution_data = json.load(f) |
| 528 | + self.update_row( |
| 529 | + url=DAG_URL + self.execution_id + "/", |
| 530 | + change={"data": solution_data}, |
| 531 | + payload_to_check={}, |
| 532 | + check_payload=False, |
| 533 | + token=service_token, |
| 534 | + ) |
| 535 | + |
| 536 | + def _capture_queries_for_get_all_objects(self): |
| 537 | + captured_queries = [] |
| 538 | + |
| 539 | + def _listener(conn, cursor, statement, parameters, context, executemany): |
| 540 | + captured_queries.append(statement) |
| 541 | + |
| 542 | + engine = db.engine |
| 543 | + event.listen(engine, "before_cursor_execute", _listener) |
| 544 | + try: |
| 545 | + executions = ExecutionModel.get_all_objects(user=self.user) |
| 546 | + finally: |
| 547 | + event.remove(engine, "before_cursor_execute", _listener) |
| 548 | + |
| 549 | + return executions, captured_queries |
| 550 | + |
| 551 | + def test_data_is_deferred_in_list_via_sqlalchemy_inspect(self): |
| 552 | + executions = ExecutionModel.get_all_objects(user=self.user) |
| 553 | + |
| 554 | + self.assertGreater( |
| 555 | + len(executions), |
| 556 | + 0, |
| 557 | + "Expected at least one execution to be returned by get_all_objects", |
| 558 | + ) |
| 559 | + |
| 560 | + for execution in executions: |
| 561 | + state = sa_inspect(execution) |
| 562 | + self.assertIn( |
| 563 | + "data", |
| 564 | + state.unloaded, |
| 565 | + "'data' should be deferred (not loaded eagerly) in the list query.", |
| 566 | + ) |
| 567 | + |
| 568 | + def test_data_is_not_in_select_via_sql_interception(self): |
| 569 | + executions, captured_queries = self._capture_queries_for_get_all_objects() |
| 570 | + |
| 571 | + self.assertGreater( |
| 572 | + len(executions), |
| 573 | + 0, |
| 574 | + "Expected at least one execution to be returned by get_all_objects", |
| 575 | + ) |
| 576 | + |
| 577 | + self.assertTrue( |
| 578 | + len(captured_queries) > 0, |
| 579 | + "No SQL queries were captured; the event listener may not have fired.", |
| 580 | + ) |
| 581 | + |
| 582 | + data_in_query = any( |
| 583 | + '"data"' in q or " data," in q.lower() or " data " in q.lower() |
| 584 | + for q in captured_queries |
| 585 | + ) |
| 586 | + self.assertFalse( |
| 587 | + data_in_query, |
| 588 | + "The SELECT generated by get_all_objects must not include the 'data' column. " |
| 589 | + "Captured queries: " + str(captured_queries), |
| 590 | + ) |
| 591 | + |
| 592 | + def test_list_endpoint_does_not_return_indicators(self): |
| 593 | + from cornflow.tests.const import EXECUTION_URL |
| 594 | + |
| 595 | + response = self.client.get( |
| 596 | + EXECUTION_URL, |
| 597 | + follow_redirects=True, |
| 598 | + headers=self.get_header_with_auth(self.token), |
| 599 | + ) |
| 600 | + |
| 601 | + self.assertEqual( |
| 602 | + 200, |
| 603 | + response.status_code, |
| 604 | + f"GET /execution/ returned unexpected status {response.status_code}", |
| 605 | + ) |
| 606 | + |
| 607 | + items = response.json |
| 608 | + self.assertIsInstance(items, list) |
| 609 | + self.assertGreater(len(items), 0, "Expected at least one execution in the list") |
| 610 | + |
| 611 | + for item in items: |
| 612 | + self.assertNotIn( |
| 613 | + "indicators", |
| 614 | + item, |
| 615 | + "'indicators' must not appear in execution list items.", |
| 616 | + ) |
| 617 | + |
| 618 | + def test_list_endpoint_returns_basic_fields(self): |
| 619 | + from cornflow.tests.const import EXECUTION_URL |
| 620 | + |
| 621 | + response = self.client.get( |
| 622 | + EXECUTION_URL, |
| 623 | + follow_redirects=True, |
| 624 | + headers=self.get_header_with_auth(self.token), |
| 625 | + ) |
| 626 | + |
| 627 | + self.assertEqual(200, response.status_code) |
| 628 | + |
| 629 | + items = response.json |
| 630 | + self.assertIsInstance(items, list) |
| 631 | + self.assertGreater(len(items), 0, "Expected at least one execution in the list") |
| 632 | + |
| 633 | + required_fields = [ |
| 634 | + "id", |
| 635 | + "name", |
| 636 | + "description", |
| 637 | + "created_at", |
| 638 | + "updated_at", |
| 639 | + "user_id", |
| 640 | + "username", |
| 641 | + "data_hash", |
| 642 | + "state", |
| 643 | + "message", |
| 644 | + "config", |
| 645 | + "instance_id", |
| 646 | + "schema", |
| 647 | + "log", |
| 648 | + ] |
| 649 | + |
| 650 | + for item in items: |
| 651 | + for field in required_fields: |
| 652 | + self.assertIn( |
| 653 | + field, |
| 654 | + item, |
| 655 | + f"Required field '{field}' is missing from the execution list response.", |
| 656 | + ) |
0 commit comments