Skip to content

Commit

Permalink
WIP: Device should not depend on controller.
Browse files Browse the repository at this point in the history
  • Loading branch information
rocodes committed Jan 25, 2024
1 parent a9d8e94 commit 07c54c6
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 128 deletions.
3 changes: 3 additions & 0 deletions client/securedrop_client/export_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,6 @@ class ExportStatus(Enum):
CALLED_PROCESS_ERROR = "CALLED_PROCESS_ERROR"
ERROR_USB_CONFIGURATION = "ERROR_USB_CONFIGURATION"
UNEXPECTED_RETURN_STATUS = "UNEXPECTED_RETURN_STATUS"

# Client-side error only
ERROR_MISSING_FILES = "ERROR_MISSING_FILES" # All files meant for export are missing
8 changes: 2 additions & 6 deletions client/securedrop_client/gui/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,8 @@ def _on_triggered(self) -> None:
# out of scope, any pending file removal will be performed
# by the operating system.
with open(file_path, "r") as f:
export = ExportDevice(self._controller, str[file_path])
dialog = PrintConversationTranscriptDialog(
export, TRANSCRIPT_FILENAME, str(file_path)
)
export = ExportDevice([file_path])
dialog = PrintConversationTranscriptDialog(export, TRANSCRIPT_FILENAME, str(file_path))
dialog.exec()


Expand Down Expand Up @@ -238,7 +236,6 @@ def _on_triggered(self) -> None:
# out of scope, any pending file removal will be performed
# by the operating system.
with open(file_path, "r") as f:

export_device = ExportDevice([file_path])
dialog = ExportConversationTranscriptDialog(
export_device, TRANSCRIPT_FILENAME, str(file_path)
Expand Down Expand Up @@ -328,7 +325,6 @@ def _prepare_to_export(self) -> None:
# out of scope, any pending file removal will be performed
# by the operating system.
with ExitStack() as stack:

export_device = ExportDevice(file_locations)
files = [
stack.enter_context(open(file_location, "r")) for file_location in file_locations
Expand Down
30 changes: 22 additions & 8 deletions client/securedrop_client/gui/conversation/export/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ class Device(QObject):
Files are archived in a specified format, (see `export` README).
A list of valid filepaths must be supplied. Caller is
responsible for querying database and ensuring files are present.
A list of valid filepaths must be supplied.
"""

_METADATA_FN = "metadata.json"
Expand Down Expand Up @@ -61,7 +60,7 @@ class Device(QObject):
def __init__(self, filepaths: [str]) -> None:
super().__init__()

self._filepaths = filepaths
self._filepaths_list = filepaths

def run_printer_preflight_checks(self) -> None:
"""
Expand Down Expand Up @@ -113,7 +112,7 @@ def export_file_to_usb_drive(self, file_uuid: str, passphrase: str) -> None:
is_downloaded is set to False.
"""

self._send_file_to_usb_device([self._filepaths], passphrase)
self._send_file_to_usb_device(self._filepaths_list, passphrase)

def print_transcript(self, file_location: str) -> None:
"""
Expand All @@ -127,7 +126,7 @@ def print_file(self, file_uuid: str) -> None:
so that is_downloaded is set to False.
"""

self._print([self._filepaths])
self._print(self._filepaths_list)

def _run_qrexec_export(self, archive_path: str) -> ExportStatus:
"""
Expand Down Expand Up @@ -199,10 +198,25 @@ def _create_archive(
# When more than one file is added to the archive,
# extra care must be taken to prevent name collisions.
is_one_of_multiple_files = len(filepaths) > 1
missing_count = 0
for filepath in filepaths:
self._add_file_to_archive(
archive, filepath, prevent_name_collisions=is_one_of_multiple_files
)
if not (os.path.exists(filepath)):
missing_count += 1
logger.debug(
f"'{filepath}' does not exist, and will not be included in archive"
)
# Controller checks files and keeps a reference open during export,
# so this shouldn't be reachable
logger.warning("File not found at specified filepath, skipping")
else:
self._add_file_to_archive(
archive, filepath, prevent_name_collisions=is_one_of_multiple_files
)
if missing_count == len(filepaths):
# Context manager will delete archive even if an exception occurs
# since the archive is in a TemporaryDirectory
logger.error("Files were moved or missing")
raise ExportError(ExportStatus.ERROR_MISSING_FILES)

return archive_path

Expand Down
15 changes: 11 additions & 4 deletions client/securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2255,8 +2255,6 @@ def __init__(

self.controller = controller

self._export_device = conversation.ExportDevice(controller)

self.file = self.controller.get_file(file_uuid)
self.uuid = file_uuid
self.index = index
Expand Down Expand Up @@ -2455,11 +2453,16 @@ def _on_export_clicked(self) -> None:
"""
Called when the export button is clicked.
"""
file_location = self.file.location(self.controller.data_dir)

if not self.controller.downloaded_file_exists(self.file):
logger.debug("Clicked export but file not downloaded")
return

export_device = conversation.ExportDevice([file_location])

self.export_dialog = conversation.ExportFileDialog(
self._export_device, self.uuid, self.file.filename
export_device, self.uuid, self.file.filename
)
self.export_dialog.show()

Expand All @@ -2469,9 +2472,13 @@ def _on_print_clicked(self) -> None:
Called when the print button is clicked.
"""
if not self.controller.downloaded_file_exists(self.file):
logger.debug("Clicked print but file not downloaded")
return

dialog = conversation.PrintFileDialog(self._export_device, self.uuid, self.file.filename)
filepath = self.file.location(self.controller.data_dir)
export_device = conversation.ExportDevice([filepath])

dialog = conversation.PrintFileDialog(export_device, self.uuid, self.file.filename)
dialog.exec()

def _on_left_click(self) -> None:
Expand Down
16 changes: 8 additions & 8 deletions client/tests/functional/test_export_file_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,12 @@ def check_for_export_dialog():


@pytest.mark.vcr()
def test_export_file_dialog_locked(
functional_test_logged_in_context, qtbot, mocker, mock_export_service
):
def test_export_file_dialog_locked(functional_test_logged_in_context, qtbot, mocker):
"""
Download a file, export it, and verify that the export is complete by checking that the label of
the export dialog's continue button is "DONE".
"""
export_dialog = _setup_export(
functional_test_logged_in_context, qtbot, mocker, mock_export_service
)
export_dialog = _setup_export(functional_test_logged_in_context, qtbot, mocker)

assert export_dialog.passphrase_form.isHidden() is True

Expand All @@ -100,14 +96,18 @@ def check_password_form():

@pytest.mark.vcr()
def test_export_file_dialog_device_already_unlocked(
functional_test_logged_in_context, qtbot, mocker, mock_export_service_unlocked_device
functional_test_logged_in_context,
qtbot,
mocker,
):
"""
Download a file, export it, and verify that the export is complete by checking that the label of
the export dialog's continue button is "DONE".
"""
export_dialog = _setup_export(
functional_test_logged_in_context, qtbot, mocker, mock_export_service_unlocked_device
functional_test_logged_in_context,
qtbot,
mocker,
)

def check_skip_password_prompt_for_unlocked_device():
Expand Down
Loading

0 comments on commit 07c54c6

Please sign in to comment.