diff --git a/src/MCPClient/lib/clientScripts/policy_check.py b/src/MCPClient/lib/clientScripts/policy_check.py index be6da962fe..dfe2e4891d 100755 --- a/src/MCPClient/lib/clientScripts/policy_check.py +++ b/src/MCPClient/lib/clientScripts/policy_check.py @@ -164,7 +164,7 @@ def _we_check_this_type_of_file(self) -> bool: else: return True - def _file_is_derivative(self, for_access=False) -> bool: + def _file_is_derivative(self, for_access: bool = False) -> bool: """Return ``True`` if the target file is a derivative; ``False`` otherwise. """ @@ -466,20 +466,6 @@ def sip_policy_checks_dir(self) -> Optional[str]: return self._sip_policy_checks_dir -def _get_shared_path(argv): - try: - return argv[4] - except IndexError: - return None - - -def _get_file_type(argv): - try: - return argv[5] - except IndexError: - return "original" - - def call(jobs: List[Job]) -> None: with transaction.atomic(): for job in jobs: @@ -487,8 +473,8 @@ def call(jobs: List[Job]) -> None: file_path = job.args[1] file_uuid = job.args[2] sip_uuid = job.args[3] - shared_path = _get_shared_path(job.args) - file_type = _get_file_type(job.args) + shared_path = job.args[4] + file_type = job.args[5] try: job.set_status( diff --git a/tests/MCPClient/test_policy_check.py b/tests/MCPClient/test_policy_check.py index 320c5c0a46..7bf4a77bb1 100644 --- a/tests/MCPClient/test_policy_check.py +++ b/tests/MCPClient/test_policy_check.py @@ -1,6 +1,7 @@ import json import pathlib import uuid +from typing import Any from unittest import mock import policy_check @@ -78,9 +79,27 @@ def access_file_format_version( ) +STDOUT = json.dumps( + {"eventOutcomeInformation": "pass", "eventOutcomeDetailNote": "a note"} +) + + +@pytest.fixture +def EXECUTE_OR_RUN() -> Any: + with mock.patch( + "policy_check.executeOrRun", + return_value=( + 0, + STDOUT, + "", + ), + ): + yield + + @pytest.mark.django_db @mock.patch("policy_check.executeOrRun") -def test_policy_check_if_rules_exist( +def test_policy_check_succeeds_if_rules_exist( execute_or_run: mock.Mock, sip_file: models.File, sip: models.SIP, @@ -138,8 +157,15 @@ def test_policy_check_if_rules_exist( @pytest.mark.django_db -@mock.patch("policy_check.executeOrRun") -def test_policy_checker_check_if_no_rules_exist( +@mock.patch( + "policy_check.executeOrRun", + return_value=( + 1, + STDOUT, + "", + ), +) +def test_policy_checker_warns_if_rules_do_not_exist( execute_or_run: mock.Mock, sip_file: models.File, sip: models.SIP, @@ -160,13 +186,6 @@ def test_policy_checker_check_if_no_rules_exist( JobContext=mock.MagicMock(), spec=Job, ) - expected_stdout = json.dumps( - { - "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"', - } - ) - execute_or_run.return_value = (0, expected_stdout, "") policy_check.call([job]) @@ -178,8 +197,15 @@ def test_policy_checker_check_if_no_rules_exist( @pytest.mark.django_db -@mock.patch("policy_check.executeOrRun") -def test_policy_checker_check_if_file_does_not_exist( +@mock.patch( + "policy_check.executeOrRun", + return_value=( + 1, + STDOUT, + "", + ), +) +def test_policy_checker_warns_if_file_does_not_exist( execute_or_run: mock.Mock, sip_file: models.File, sip: models.SIP, @@ -200,13 +226,6 @@ def test_policy_checker_check_if_file_does_not_exist( JobContext=mock.MagicMock(), spec=Job, ) - expected_stdout = json.dumps( - { - "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"', - } - ) - execute_or_run.return_value = (0, expected_stdout, "") policy_check.call([job]) @@ -218,8 +237,15 @@ def test_policy_checker_check_if_file_does_not_exist( @pytest.mark.django_db -@mock.patch("policy_check.executeOrRun") -def test_policy_checker_execute_rule_command_returns_failed( +@mock.patch( + "policy_check.executeOrRun", + return_value=( + 1, + STDOUT, + "", + ), +) +def test_policy_checker_fails_if_execute_rule_command_fails( execute_or_run: mock.Mock, sip_file: models.File, sip: models.SIP, @@ -241,13 +267,6 @@ def test_policy_checker_execute_rule_command_returns_failed( JobContext=mock.MagicMock(), spec=Job, ) - expected_stdout = json.dumps( - { - "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"', - } - ) - execute_or_run.return_value = (1, expected_stdout, "") policy_check.call([job]) @@ -262,8 +281,15 @@ def test_policy_checker_execute_rule_command_returns_failed( @pytest.mark.django_db -@mock.patch("policy_check.executeOrRun") -def test_policy_checker_execute_rule_command_returns_failed_if_event_outcome_information_other_than_pass( +@mock.patch( + "policy_check.executeOrRun", + return_value=( + 1, + STDOUT, + "", + ), +) +def test_policy_checker_fails_if_event_outcome_information_in_output_is_not_pass( execute_or_run: mock.Mock, sip_file: models.File, sip: models.SIP, @@ -285,13 +311,6 @@ def test_policy_checker_execute_rule_command_returns_failed_if_event_outcome_inf JobContext=mock.MagicMock(), spec=Job, ) - expected_stdout = json.dumps( - { - "eventOutcomeInformation": "", - "eventOutcomeDetailNote": "", - } - ) - execute_or_run.return_value = (0, expected_stdout, "") policy_check.call([job]) @@ -306,11 +325,11 @@ def test_policy_check_verifies_file_type_is_preservation( preservation_file: models.File, sip: models.SIP, fprule_policy_check: fprmodels.FPRule, - preservation_file_format_version, - format, - format_version, - shared_directory_path, -): + preservation_file_format_version: models.FileFormatVersion, + format: fprmodels.Format, + format_version: fprmodels.FormatVersion, + shared_directory_path: str, +) -> None: job = mock.Mock( args=[ "policy_check", @@ -326,7 +345,7 @@ def test_policy_check_verifies_file_type_is_preservation( expected_stdout = json.dumps( { "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"', + "eventOutcomeDetailNote": "a note", } ) execute_or_run.return_value = (0, expected_stdout, "") @@ -346,9 +365,8 @@ def test_policy_check_verifies_file_type_is_preservation( @pytest.mark.django_db -@mock.patch("policy_check.executeOrRun") def test_policy_check_fails_if_file_is_not_preservation_derivative( - execute_or_run: mock.Mock, + EXECUTE_OR_RUN: mock.Mock, preservation_file: models.File, sip: models.SIP, fprule_policy_check: fprmodels.FPRule, @@ -369,13 +387,6 @@ def test_policy_check_fails_if_file_is_not_preservation_derivative( JobContext=mock.MagicMock(), spec=Job, ) - expected_stdout = json.dumps( - { - "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"', - } - ) - execute_or_run.return_value = (0, expected_stdout, "") policy_check.call([job]) @@ -415,7 +426,7 @@ def test_policy_check_verifies_file_type_is_access( expected_stdout = json.dumps( { "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": f'format="{format.description}"; version="{format_version.version}"; result="Well-Formed and valid"', + "eventOutcomeDetailNote": "a note", } ) execute_or_run.return_value = (0, expected_stdout, "") @@ -479,9 +490,8 @@ def test_policy_check_fails_if_file_is_not_access_derivative( @pytest.mark.django_db -@mock.patch("policy_check.executeOrRun") def test_policy_checker_saves_policy_check_result_into_logs_directory( - execute_or_run: mock.Mock, + EXECUTE_OR_RUN: mock.Mock, sip_file: models.File, sip: models.SIP, fprule_policy_check: fprmodels.FPRule, @@ -504,16 +514,6 @@ def test_policy_checker_saves_policy_check_result_into_logs_directory( JobContext=mock.MagicMock(), spec=Job, ) - expected_stdout = json.dumps( - { - "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": "MediaConch policy check result against policy file MP3 has duration: All policy checks passed: Does the audio duration exist?; MP3 has duration", - "policy": '\n\n Rudimentary test to check for an MP3 having a duration value.\n mp3\n', - "policyFileName": "MP3 has duration", - "stdout": "foobar", - } - ) - execute_or_run.return_value = (0, expected_stdout, "") policy_check.call([job]) @@ -521,9 +521,8 @@ def test_policy_checker_saves_policy_check_result_into_logs_directory( @pytest.mark.django_db -@mock.patch("policy_check.executeOrRun") def test_policy_checker_saves_policy_check_result_into_submission_documentation_directory( - execute_or_run: mock.Mock, + EXECUTE_OR_RUN: mock.Mock, sip_file: models.File, sip: models.SIP, fprule_policy_check: fprmodels.FPRule, @@ -531,7 +530,7 @@ def test_policy_checker_saves_policy_check_result_into_submission_documentation_ format: fprmodels.Format, format_version: fprmodels.FormatVersion, shared_directory_path: pathlib.Path, -): +) -> None: submission_documentation_directory = ( shared_directory_path / "metadata" / "submissionDocumentation" ) @@ -548,16 +547,6 @@ def test_policy_checker_saves_policy_check_result_into_submission_documentation_ JobContext=mock.MagicMock(), spec=Job, ) - expected_stdout = json.dumps( - { - "eventOutcomeInformation": "pass", - "eventOutcomeDetailNote": "MediaConch policy check result against policy file MP3 has duration: All policy checks passed: Does the audio duration exist?; MP3 has duration", - "policy": '\n\n Rudimentary test to check for an MP3 having a duration value.\n mp3\n', - "policyFileName": "MP3 has duration", - "stdout": "foobar", - } - ) - execute_or_run.return_value = (0, expected_stdout, "") policy_check.call([job]) @@ -565,16 +554,8 @@ def test_policy_checker_saves_policy_check_result_into_submission_documentation_ @pytest.mark.django_db -@mock.patch( - "policy_check.executeOrRun", - return_value=( - 0, - json.dumps({"eventOutcomeInformation": "pass"}), - "", - ), -) def test_policy_checker_checks_manually_normalized_access_derivative_file( - execute_or_run: mock.Mock, + EXECUTE_OR_RUN: mock.Mock, transfer: models.Transfer, sip_file: models.File, sip: models.SIP,