OP#10323: Never refuse a save because of where the file lies

The destination guard added alongside the s3 upload refused saving an
otconfig outside user_source/<prefix>/. Saving should always succeed
locally regardless of destination; only whether it also uploads to s3
depends on the project naming a key prefix. Drops GuardSaveDestination
and its s3 implementation.

Author: randy-seng

OTAnalytics/application/save_destination.py

@@ -2,7 +2,6 @@
 
 from OTAnalytics.application.datastore import Datastore
 from OTAnalytics.application.parser.config_parser import ConfigParser
-from OTAnalytics.application.save_destination import GuardSaveDestination
 from OTAnalytics.application.state import ConfigurationFile, CurrentKeyPrefix, FileState
 from OTAnalytics.application.upload_otconfig import UploadOtconfig
 from OTAnalytics.application.use_cases.get_current_remark import GetCurrentRemark
@@ -25,15 +24,13 @@ def __init__(
         get_current_remark: GetCurrentRemark,
         current_key_prefix: CurrentKeyPrefix,
         otconfig_upload: UploadOtconfig,
-        guard_save_destination: GuardSaveDestination,
     ) -> None:
         self._datastore = datastore
         self._config_parser = config_parser
         self._state = state
         self._get_current_remark = get_current_remark
         self._current_key_prefix = current_key_prefix
         self._otconfig_upload = otconfig_upload
-        self._guard_save_destination = guard_save_destination
 
     async def __call__(self, file: Path) -> None:
         project = self._datastore.project
@@ -56,8 +53,6 @@ async def __call__(self, file: Path) -> None:
         # The same value reaches `convert` below. Passing it to only one of the
         # two would make every project report itself as permanently unsaved.
         s3_key_prefix = self._current_key_prefix.get()
-        if s3_key_prefix is not None:
-            self._guard_save_destination(file, s3_key_prefix)
         self._config_parser.serialize(
             project=project,
             video_files=video_files,

OTAnalytics/application/use_cases/config.py

@@ -38,10 +38,6 @@
     RunConfiguration,
     RunConfigurationError,
 )
-from OTAnalytics.application.save_destination import (
-    GuardSaveDestination,
-    NoDestinationGuard,
-)
 from OTAnalytics.application.state import (
     ActionState,
     CurrentKeyPrefix,
@@ -442,7 +438,6 @@ def save_otconfig(self) -> SaveOtconfig:
             self.get_current_remark,
             self.current_key_prefix,
             self.otconfig_upload,
-            self.guard_save_destination,
         )
 
     @cached_property
@@ -454,15 +449,6 @@ def otconfig_upload(self) -> UploadOtconfig:
         """
         return NoOtconfigUpload()
 
-    @cached_property
-    def guard_save_destination(self) -> GuardSaveDestination:
-        """Refuses a save a later s3-mode load could not find again.
-
-        Overridden where s3 can be configured. This starter never sees a
-        project naming a location, so no destination needs guarding.
-        """
-        return NoDestinationGuard()
-
     @cached_property
     def current_key_prefix(self) -> CurrentKeyPrefix:
         """One holder for the whole application.

OTAnalytics/plugin_s3/save_destination.py

@@ -13,10 +13,6 @@
     RefuseAnyProjectLocation,
     ValidateProjectLocation,
 )
-from OTAnalytics.application.save_destination import (
-    GuardSaveDestination,
-    NoDestinationGuard,
-)
 from OTAnalytics.application.upload_otconfig import NoOtconfigUpload, UploadOtconfig
 from OTAnalytics.application.use_cases.ask_for_load_window import AskForLoadWindow
 from OTAnalytics.application.use_cases.provide_input_files import (
@@ -43,7 +39,6 @@
     S3TrackFileProvider,
     S3VideoFileProvider,
 )
-from OTAnalytics.plugin_s3.save_destination import RequireDestinationUnderUserSource
 from OTAnalytics.plugin_s3.store import S3Store
 from OTAnalytics.plugin_s3.upload import S3Upload
 from OTAnalytics.plugin_ui.gui_application import OtAnalyticsGuiApplicationStarter
@@ -342,12 +337,6 @@ def otconfig_upload(self) -> UploadOtconfig:
             return S3OtconfigUpload(S3Upload(self.s3_store))
         return NoOtconfigUpload()
 
-    @cached_property
-    def guard_save_destination(self) -> GuardSaveDestination:
-        if config := self.s3_config:
-            return RequireDestinationUnderUserSource(Path(config.user_source))
-        return NoDestinationGuard()
-
     @cached_property
     def reset_application(self) -> ResetApplication:
         """Resetting a project also frees the staged downloads it used.

OTAnalytics/plugin_ui/base_application.py

@@ -8,7 +8,6 @@
 from OTAnalytics.application.key_prefix import S3KeyPrefix
 from OTAnalytics.application.parser.config_parser import ConfigParser
 from OTAnalytics.application.project import Project
-from OTAnalytics.application.save_destination import UnsupportedSaveDestination
 from OTAnalytics.application.state import ConfigurationFile
 from OTAnalytics.application.use_cases.config import ConfigValidationError, SaveOtconfig
 from OTAnalytics.domain.track_repository import TrackFileRepository
@@ -31,15 +30,13 @@ async def test_correct_date(self, test_data_tmp_dir: Path) -> None:
         current_key_prefix = Mock()
         current_key_prefix.get.return_value = A_PREFIX
         otconfig_upload = AsyncMock()
-        guard_save_destination = Mock()
         use_case = SaveOtconfig(
             datastore,
             config_parser,
             file_state,
             get_current_remark,
             current_key_prefix,
             otconfig_upload,
-            guard_save_destination,
         )
 
         await use_case(output)
@@ -75,7 +72,6 @@ async def test_writes_the_same_location_it_records_as_saved(
             Mock(),
             current_key_prefix,
             AsyncMock(),
-            Mock(),
         )
 
         await use_case(test_data_tmp_dir / "test.otconfig")
@@ -103,7 +99,6 @@ async def test_uploads_when_project_names_a_key_prefix(
             Mock(),
             current_key_prefix,
             otconfig_upload,
-            Mock(),
         )
 
         await use_case(output)
@@ -128,90 +123,12 @@ async def test_does_not_upload_when_project_names_no_key_prefix(
             Mock(),
             current_key_prefix,
             otconfig_upload,
-            Mock(),
         )
 
         await use_case(test_data_tmp_dir / "test.otconfig")
 
         otconfig_upload.upload.assert_not_awaited()
 
-    async def test_guards_the_destination_when_project_names_a_key_prefix(
-        self, test_data_tmp_dir: Path
-    ) -> None:
-        track_file_repository = Mock(spec=TrackFileRepository)
-        datastore = Mock(spec=Datastore)
-        datastore._track_file_repository = track_file_repository
-        datastore.project = Project("name", start_date=datetime(2023, 1, 1))
-        config_parser = Mock(spec=ConfigParser)
-        current_key_prefix = Mock()
-        current_key_prefix.get.return_value = A_PREFIX
-        guard_save_destination = Mock()
-        output = test_data_tmp_dir / "test.otconfig"
-        use_case = SaveOtconfig(
-            datastore,
-            config_parser,
-            Mock(),
-            Mock(),
-            current_key_prefix,
-            AsyncMock(),
-            guard_save_destination,
-        )
-
-        await use_case(output)
-
-        guard_save_destination.assert_called_once_with(output, A_PREFIX)
-
-    async def test_does_not_guard_when_project_names_no_key_prefix(
-        self, test_data_tmp_dir: Path
-    ) -> None:
-        track_file_repository = Mock(spec=TrackFileRepository)
-        datastore = Mock(spec=Datastore)
-        datastore._track_file_repository = track_file_repository
-        datastore.project = Project("name", start_date=datetime(2023, 1, 1))
-        config_parser = Mock(spec=ConfigParser)
-        current_key_prefix = Mock()
-        current_key_prefix.get.return_value = None
-        guard_save_destination = Mock()
-        use_case = SaveOtconfig(
-            datastore,
-            config_parser,
-            Mock(),
-            Mock(),
-            current_key_prefix,
-            AsyncMock(),
-            guard_save_destination,
-        )
-
-        await use_case(test_data_tmp_dir / "test.otconfig")
-
-        guard_save_destination.assert_not_called()
-
-    async def test_refused_destination_is_not_serialized(
-        self, test_data_tmp_dir: Path
-    ) -> None:
-        track_file_repository = Mock(spec=TrackFileRepository)
-        datastore = Mock(spec=Datastore)
-        datastore._track_file_repository = track_file_repository
-        datastore.project = Project("name", start_date=datetime(2023, 1, 1))
-        config_parser = Mock(spec=ConfigParser)
-        current_key_prefix = Mock()
-        current_key_prefix.get.return_value = A_PREFIX
-        guard_save_destination = Mock(side_effect=UnsupportedSaveDestination())
-        use_case = SaveOtconfig(
-            datastore,
-            config_parser,
-            Mock(),
-            Mock(),
-            current_key_prefix,
-            AsyncMock(),
-            guard_save_destination,
-        )
-
-        with pytest.raises(UnsupportedSaveDestination):
-            await use_case(test_data_tmp_dir / "test.otconfig")
-
-        config_parser.serialize.assert_not_called()
-
     async def test_missing_date(self, test_data_tmp_dir: Path) -> None:
         datastore = Mock(spec=Datastore)
         datastore.project = Project("name", start_date=None)
@@ -226,7 +143,6 @@ async def test_missing_date(self, test_data_tmp_dir: Path) -> None:
             get_current_remark,
             Mock(),
             AsyncMock(),
-            Mock(),
         )
 
         with pytest.raises(ConfigValidationError) as exc_info: