Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up source deletion #1386

Merged
merged 3 commits into from
Jan 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 78 additions & 16 deletions securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ def setup(self, controller: Controller) -> None:
Pass through the controller object to this widget.
"""
self.controller = controller
self.controller.source_deletion_successful.connect(self._on_source_deletion_successful)
self.source_list.setup(controller)

def show_sources(self, sources: List[Source]) -> None:
Expand Down Expand Up @@ -683,6 +684,33 @@ def refresh_source_conversations(self) -> None:
except sqlalchemy.exc.InvalidRequestError as e:
logger.debug("Error refreshing source conversations: %s", e)

def _hide_source(self, source_uuid: str) -> None:
"""
Hide the source and corresponding conversation.
"""
self._hide_conversation(source_uuid)
self.source_list.hide_source(source_uuid)

visible_item_exists = False
for i in range(self.source_list.count()):
if not self.source_list.isRowHidden(i):
visible_item_exists = True
break

if visible_item_exists:
self.empty_conversation_view.show_no_source_selected_message()
self.empty_conversation_view.show()
else:
self.empty_conversation_view.show_no_sources_message()
self.empty_conversation_view.show()

def _hide_conversation(self, source_uuid: str) -> None:
try:
conversation_wrapper = self.source_conversations[source_uuid]
conversation_wrapper.hide()
except KeyError:
logger.debug("No SourceConversationWrapper for {} to hide".format(source_uuid))

def delete_conversation(self, source_uuid: str) -> None:
"""
When we delete a source, we should delete its SourceConversationWrapper,
Expand All @@ -709,6 +737,16 @@ def set_conversation(self, widget: QWidget) -> None:
self.view_layout.addWidget(widget)
widget.show()

@pyqtSlot(str, datetime)
def _on_source_deletion_successful(self, source_uuid: str, timestamp: datetime) -> None:
"""
Hide the source until a sync removes it from the GUI to ensure that a stale sync does not
re-add a source. This method will no longer be necessary once Journalist API v2 is
implemented (see https://github.com/freedomofpress/securedrop/issues/5104) and we can know
explicitly when a source is new.
"""
self._hide_source(source_uuid)


class EmptyConversationView(QWidget):

Expand Down Expand Up @@ -877,21 +915,8 @@ def update(self, sources: List[Source]) -> List[str]:
continue

# Delete widgets for sources not in the supplied sourcelist
deleted_uuids = []
sources_to_delete = [
self.source_items[uuid] for uuid in self.source_items if uuid not in sources_to_update
]
for source_item in sources_to_delete:
if source_item.isSelected():
self.setCurrentItem(None)

source_widget = self.itemWidget(source_item)
self.takeItem(self.row(source_item))
if source_widget.source_uuid in self.source_items:
del self.source_items[source_widget.source_uuid]

deleted_uuids.append(source_widget.source_uuid)
source_widget.deleteLater()
sources_to_delete = [uuid for uuid in self.source_items if uuid not in sources_to_update]
deleted_uuids = self.delete_sources(sources_to_delete)

# Update the remaining widgets
for i in range(self.count()):
Expand Down Expand Up @@ -921,6 +946,37 @@ def update(self, sources: List[Source]) -> List[str]:
# conversation widgets
return deleted_uuids

def hide_source(self, uuid: str) -> None:
try:
source_item = self.source_items[uuid]
if source_item.isSelected():
self.setCurrentItem(None)

source_item.setHidden(True)
except KeyError:
logger.debug("No SourceListWidgetItem for {} to hide".format(uuid))

def delete_sources(self, source_uuids: List[str]) -> List[str]:
deleted_uuids = []

for uuid in source_uuids:
try:
source_item = self.source_items[uuid]
if source_item.isSelected():
self.setCurrentItem(None)

source_widget = self.itemWidget(source_item)
self.takeItem(self.row(source_item))
if source_widget.source_uuid in self.source_items:
del self.source_items[source_widget.source_uuid]

deleted_uuids.append(source_widget.source_uuid)
source_widget.deleteLater()
except KeyError:
continue

return deleted_uuids

def initial_update(self, sources: List[Source]) -> None:
"""
Initialise the list with the passed in list of sources.
Expand Down Expand Up @@ -1181,11 +1237,12 @@ def __init__(
self.controller = controller
self.controller.sync_started.connect(self._on_sync_started)
self.controller.conversation_deleted.connect(self._on_conversation_deleted)
controller.conversation_deletion_successful.connect(
self.controller.conversation_deletion_successful.connect(
self._on_conversation_deletion_successful
)
self.controller.conversation_deletion_failed.connect(self._on_conversation_deletion_failed)
self.controller.source_deleted.connect(self._on_source_deleted)
self.controller.source_deletion_successful.connect(self._on_source_deletion_successful)
self.controller.source_deletion_failed.connect(self._on_source_deletion_failed)
self.controller.authentication_state.connect(self._on_authentication_changed)
source_selected_signal.connect(self._on_source_selected)
Expand Down Expand Up @@ -1435,6 +1492,11 @@ def _on_source_deleted(self, source_uuid: str) -> None:
if self.source_uuid == source_uuid:
self.start_account_deletion()

@pyqtSlot(str, datetime)
def _on_source_deletion_successful(self, source_uuid: str, timestamp: datetime) -> None:
if self.source_uuid == source_uuid:
self.deletion_scheduled_timestamp = timestamp

@pyqtSlot(str)
def _on_source_deletion_failed(self, source_uuid: str) -> None:
if self.source_uuid == source_uuid:
Expand Down
16 changes: 13 additions & 3 deletions securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,15 @@ class Controller(QObject):
"""
This signal indicates that a deletion attempt was successful at the server.

Emits:
str: the source UUID
datetime: the timestamp for when the deletion succeeded
"""
source_deletion_successful = pyqtSignal(str, datetime)

"""
This signal indicates that a deletion attempt was successful at the server.

Emits:
str: the source UUID
datetime: the timestamp for when the deletion succeeded
Expand Down Expand Up @@ -1016,7 +1025,7 @@ def on_file_download_failure(self, exception: Exception) -> None:
def on_delete_conversation_success(self, uuid: str) -> None:
"""
If the source collection has been successfully scheduled for deletion on the server, emit a
signal and sync.
signal.
"""
logger.info("Conversation %s successfully deleted at server", uuid)
self.conversation_deletion_successful.emit(uuid, datetime.utcnow())
Expand All @@ -1030,10 +1039,11 @@ def on_delete_conversation_failure(self, e: Exception) -> None:

def on_delete_source_success(self, source_uuid: str) -> None:
"""
Rely on sync to delete the source locally so we know for sure it was deleted
If the source has been successfully scheduled for deletion on the server, emit a
signal.
"""
logger.info("Source %s successfully deleted at server", source_uuid)
self.api_sync.sync()
self.source_deletion_successful.emit(source_uuid, datetime.utcnow())

def on_delete_source_failure(self, e: Exception) -> None:
if isinstance(e, DeleteSourceJobException):
Expand Down
14 changes: 2 additions & 12 deletions securedrop_client/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,12 @@ def __init__(

self.sync_thread.started.connect(self.api_sync_bg_task.sync)

self.timer = QTimer()
self.timer.setInterval(self.TIME_BETWEEN_SYNCS_MS)
self.timer.timeout.connect(self.sync)

def start(self, api_client: API) -> None:
"""
Start metadata syncs.
"""
self.api_client = api_client

self.timer.start()

if not self.sync_thread.isRunning():
logger.debug("Starting sync thread")
self.api_sync_bg_task.api_client = self.api_client
Expand All @@ -74,18 +68,14 @@ def on_sync_success(self) -> None:
Start another sync on success.
"""
self.sync_success.emit()
QTimer.singleShot(self.TIME_BETWEEN_SYNCS_MS, self.api_sync_bg_task.sync)

def on_sync_failure(self, result: Exception) -> None:
"""
Only start another sync on failure if the reason is a timeout request.
"""
self.sync_failure.emit(result)

def sync(self) -> None:
"""
Start an immediate sync.
"""
QTimer.singleShot(1, self.api_sync_bg_task.sync)
QTimer.singleShot(self.TIME_BETWEEN_SYNCS_MS, self.api_sync_bg_task.sync)


class ApiSyncBackgroundTask(QObject):
Expand Down
Loading