NXDRIVE-2711: Show that upload is still alive for very large files (#…

…4227)

NXDRIVE-2711: Show thta upload is still alive for very large file

Co-authored-by: swetayadav1 <[email protected]>

docs/changes/5.3.3.md

@@ -12,7 +12,7 @@ Release date: `2023-xx-xx`
 
 ### Direct Transfer
 
-- [NXDRIVE-2](https://jira.nuxeo.com/browse/NXDRIVE-2):
+- [NXDRIVE-2711](https://jira.nuxeo.com/browse/NXDRIVE-2711): Show that upload is still alive for very large files
 
 ## GUI
 
@@ -83,6 +83,7 @@ Release date: `2023-xx-xx`
 - Upgraded `typing-extensions` from 4.0.1 to 4.7.1
 - Upgraded `vulture` from 2.3 to 2.9.1
 - Upgraded `wcwidth` from 0.2.5 to 0.2.6
+
 ## Technical Changes
 
--
+- Added `finalizing_status` attribute in LinkingAction class

nxdrive/client/uploader/__init__.py

@@ -416,7 +416,7 @@ def _link_blob_to_doc(
                 self._set_transfer_status(transfer, TransferStatus.ONGOING)
             raise exc
 
-    def link_blob_to_doc(  # type: ignore[return]
+    def link_blob_to_doc(
         self,
         command: str,
         transfer: Upload,
@@ -451,15 +451,19 @@ def link_blob_to_doc(  # type: ignore[return]
             kwargs["headers"] = headers
         try:
             doc_type = kwargs.get("doc_type", "")
-            if transfer.is_direct_transfer and doc_type and doc_type != "":
-                res = self._transfer_docType_file(transfer, headers, doc_type)
-            else:
-                res = self._transfer_autoType_file(command, blob, kwargs)
-
-            return res
+            return (
+                self._transfer_docType_file(transfer, headers, doc_type)
+                if transfer.is_direct_transfer and doc_type and doc_type != ""
+                else self._transfer_autoType_file(command, blob, kwargs)
+            )
         except Exception as exc:
             err = f"Error while linking blob to doc: {exc!r}"
             log.warning(err)
+            action.finalizing_status = "Error"
+            if "TCPKeepAliveHTTPSConnectionPool" not in str(exc):
+                transfer.request_uid = str(uuid4())
+                self.dao.update_upload_requestid(transfer)
+            raise exc
         finally:
             action.finish_action()
 

nxdrive/dao/engine.py

@@ -2381,6 +2381,13 @@ def update_upload(self, upload: Upload, /) -> None:
             sql = "UPDATE Uploads SET batch = ? WHERE uid = ?"
             c.execute(sql, (json.dumps(batch), upload.uid))
 
+    def update_upload_requestid(self, upload: Upload, /) -> None:
+        """In case of error during linking, update request_uid for upload"""
+        with self.lock:
+            c = self._get_write_connection().cursor()
+            sql = "UPDATE Uploads SET request_uid = ? WHERE uid = ?"
+            c.execute(sql, (upload.request_uid, upload.uid))
+
     def pause_transfer(
         self,
         nature: str,

nxdrive/data/i18n/i18n.json

@@ -106,6 +106,7 @@
     "DIRECT_TRANSFER_DETAILS": "[%1%] %2 of %3",
     "DIRECT_TRANSFER_END": "Transfer done: \"%1\"",
     "DIRECT_TRANSFER_ERROR": "Transfer error: \"%1\"",
+    "DIRECT_TRANSFER_FINALIZING_ERROR": "An error occurred during the transfer, it will resume shortly.",
     "DIRECT_TRANSFER_NO_ACCOUNT": "Cannot use the Direct Transfer feature with no account, aborting.",
     "DIRECT_TRANSFER_NOT_ALLOWED": "Direct Transfer of \"%1\" is not allowed for synced files.",
     "DIRECT_TRANSFER_NOT_ENABLED": "The Direct Transfer feature is not enabled.",

nxdrive/data/qml/TransferItem.qml

@@ -77,5 +77,13 @@ Rectangle {
                 }
             }
         }
+
+        ScaledText {
+            text: qsTr("DIRECT_TRANSFER_FINALIZING_ERROR") + tl.tr
+            color: secondaryText
+            visible: finalizing && finalizing_status
+            Layout.leftMargin: icon.width + 5
+            font.pointSize: point_size * 0.8
+        }
     }
 }

nxdrive/engine/activity.py

@@ -54,8 +54,7 @@ def get_current_action(*, thread_id: int = None) -> Optional["Action"]:
 
     @staticmethod
     def finish_action() -> None:
-        action = Action.actions.pop(current_thread_id(), None)
-        if action:
+        if action := Action.actions.pop(current_thread_id(), None):
             action.finish()
 
     def finish(self) -> None:
@@ -149,6 +148,15 @@ def progress(self, value: float, /) -> None:
 
         self.progressing.emit(self)
 
+    @property
+    def finalizing_status(self) -> str:
+        return self._finalizing_status
+
+    @finalizing_status.setter
+    def finalizing_status(self, value: str, /) -> None:
+        self._finalizing_status = value
+        self.progressing.emit(self)
+
     def get_percent(self) -> float:
         if self.size < 0 or (self.empty and not self.uploaded):
             return 0.0
@@ -257,6 +265,13 @@ def __init__(
             doc_pair=doc_pair,
         )
         self.progress = size
+        self.finalizing_status = ""
+
+    def export(self) -> Dict[str, Any]:
+        return {
+            **super().export(),
+            "finalizing_status": self.finalizing_status,
+        }
 
 
 def tooltip(doing: str):  # type: ignore

nxdrive/gui/view.py

@@ -273,6 +273,7 @@ class DirectTransferModel(QAbstractListModel):
     REMOTE_PARENT_REF = qt.UserRole + 10
     SHADOW = qt.UserRole + 11  # Tell the interface if the row should be visible or not
     DOC_PAIR = qt.UserRole + 12
+    FINALIZING_STATUS = qt.UserRole + 13
 
     def __init__(self, translate: Callable, /, *, parent: QObject = None) -> None:
         super().__init__(parent)
@@ -291,6 +292,7 @@ def __init__(self, translate: Callable, /, *, parent: QObject = None) -> None:
             self.REMOTE_PARENT_REF: b"remote_parent_ref",
             self.SHADOW: b"shadow",
             self.DOC_PAIR: b"doc_pair",
+            self.FINALIZING_STATUS: b"finalizing_status",
         }
         # Pretty print
         self.psize = partial(sizeof_fmt, suffix=self.tr("BYTE_ABBREV"))
@@ -353,6 +355,8 @@ def data(self, index: QModelIndex, role: int, /) -> Any:
             return self.psize(row["filesize"])
         if role == self.TRANSFERRED:
             return self.psize(row["filesize"] * row["progress"] / 100)
+        if role == self.FINALIZING_STATUS:
+            return row.get("finalizing_status")
         return row[self.names[role].decode()]
 
     def setData(self, index: QModelIndex, value: Any, /, *, role: int = None) -> None:
@@ -375,6 +379,9 @@ def set_progress(self, action: Dict[str, Any], /) -> None:
             self.setData(idx, action["progress"], role=self.TRANSFERRED)
             if action["action_type"] == "Linking":
                 self.setData(idx, True, role=self.FINALIZING)
+                self.setData(
+                    idx, action["finalizing_status"], role=self.FINALIZING_STATUS
+                )
 
     def add_item(self, parent: QModelIndex, n_item: Dict[str, Any], /) -> None:
         """Add an item to existing list."""

tests/unit/conftest.py

@@ -1,19 +1,23 @@
 import os
 import shutil
 import time
-from typing import Optional
+from typing import Any, Callable, Optional
 from uuid import uuid4
 
 import pytest
 
 from nxdrive.client.remote_client import Remote
+from nxdrive.constants import TransferStatus
 from nxdrive.dao.engine import EngineDAO
 from nxdrive.dao.manager import ManagerDAO
 from nxdrive.engine.engine import Engine
 from nxdrive.engine.processor import Processor
+from nxdrive.gui.view import DirectTransferModel
 from nxdrive.manager import Manager
-from nxdrive.objects import DocPair
+from nxdrive.objects import DocPair, Upload
 from nxdrive.osi import AbstractOSIntegration
+from nxdrive.qt import constants as qt
+from nxdrive.qt.imports import QObject
 from nxdrive.updater.darwin import Updater
 from nxdrive.utils import normalized_path
 
@@ -141,6 +145,13 @@ def __init__(self, tmp_path):
         super().__init__(self, final_app)
 
 
+class MockDirectTransferModel(DirectTransferModel):
+    def __init__(
+        self, translate: Callable[..., Any], /, *, parent: QObject = None
+    ) -> None:
+        super().__init__(translate, parent=parent)
+
+
 @pytest.fixture()
 def engine_dao(tmp_path):
     dao = MockEngineDAO
@@ -188,3 +199,43 @@ def processor(engine, engine_dao):
     processor.remote = Remote
     processor.dao = engine_dao
     return processor
+
+
+@pytest.fixture()
+def upload():
+    upload = Upload
+    upload.path = "/tmp"
+    upload.status = TransferStatus.ONGOING
+    upload.engine = f"{engine}"
+    upload.is_direct_edit = False
+    upload.is_direct_transfer = True
+    upload.filesize = "23.0"
+    upload.batch = {"batchID": f"{str(uuid4())}"}
+    upload.chunk_size = "345"
+    upload.remote_parent_path = "/tmp/remote_path"
+    upload.remote_parent_ref = "/tmp/remote_path_ref"
+    upload.doc_pair = "test_file"
+    upload.request_uid = str(uuid4())
+    return upload
+
+
+@pytest.fixture()
+def direct_transfer_model():
+    direct_transfer_model = MockDirectTransferModel
+    direct_transfer_model.FINALIZING_STATUS = qt.UserRole + 13
+    direct_transfer_model.items = [
+        {
+            "uid": 1,
+            "name": "a.txt",
+            "filesize": 142936511610,
+            "status": "",
+            "engine": "51a2c2dc641311ee87fb...bfc0ec09fa",
+            "progress": 100.0,
+            "doc_pair": 1,
+            "remote_parent_path": "/default-domain/User...TestFolder",
+            "remote_parent_ref": "7b7886ea-5ad9-460d-8...1607ea0081",
+            "shadow": True,
+            "finalizing": True,
+        }
+    ]
+    return direct_transfer_model

tests/unit/test_action.py

@@ -244,6 +244,9 @@ def test_finalization_action(tmp):
 
     action = LinkingAction(filepath, filepath.stat().st_size)
     assert action.type == "Linking"
+    action.finalizing_status = "Error occurred while linking"
+    details = action.export()
+    assert details["finalizing_status"] == "Error occurred while linking"
 
     Action.finish_action()
     assert action.finished

tests/unit/test_client_uploader.py

@@ -0,0 +1,50 @@
+from unittest.mock import Mock
+from uuid import uuid4
+
+import pytest
+import requests
+from nuxeo.models import FileBlob
+
+from nxdrive.client.remote_client import Remote
+from nxdrive.client.uploader import BaseUploader
+
+
+@pytest.fixture
+def baseuploader():
+    remote = Remote
+    remote.dao = Mock()
+    return BaseUploader(remote)
+
+
+def test_link_blob_to_doc(baseuploader, upload, tmp_path, monkeypatch):
+    """Test system network and server side exception handling while linking blob to document"""
+    file = tmp_path / f"{uuid4()}.txt"
+    file.write_bytes(b"content")
+
+    def mock_transfer_autoType_file(*args, **kwargs):
+        raise requests.exceptions.RequestException("Connection Error")
+
+    monkeypatch.setattr(
+        baseuploader, "_transfer_autoType_file", mock_transfer_autoType_file
+    )
+
+    # server side exceptions
+    with pytest.raises(requests.exceptions.RequestException):
+        baseuploader.link_blob_to_doc(
+            "Filemanager.Import", upload, FileBlob(str(file)), False
+        )
+
+    def mock_transfer_autoType_file(*args, **kwargs):
+        raise requests.exceptions.RequestException(
+            "TCPKeepAliveHTTPSConnectionPool: Connection Error"
+        )
+
+    monkeypatch.setattr(
+        baseuploader, "_transfer_autoType_file", mock_transfer_autoType_file
+    )
+
+    # system network disconnect
+    with pytest.raises(requests.exceptions.RequestException):
+        baseuploader.link_blob_to_doc(
+            "Filemanager.Import", upload, FileBlob(str(file)), False
+        )

tests/unit/test_engine_dao.py

@@ -1,8 +1,9 @@
 import os
 import sqlite3
 from datetime import datetime
+from multiprocessing import RLock
 from pathlib import Path
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 from uuid import uuid4
 
 from nxdrive.constants import TransferStatus
@@ -408,7 +409,7 @@ def test_migration_db_v10(engine_dao):
     """Verify Downloads after migration from v9 to v10."""
     with engine_dao("engine_migration_10.db") as dao:
         downloads = list(dao.get_downloads())
-        assert len(downloads) == 0
+        assert not downloads
 
         states = list(dao.get_states_from_partial_local(Path()))
         assert len(states) == 4
@@ -608,3 +609,21 @@ def test_migration_interface():
         assert not interface.downgrade(cursor)
         assert not interface.previous_version
         assert not interface.version
+
+
+def test_update_upload_requestid(engine_dao, upload):
+    """Test to save upload and update reuqest_uid of existing row"""
+    engine_dao.lock = RLock()
+    with engine_dao("engine_migration_18.db") as dao:
+        engine_dao.directTransferUpdated = Mock()
+        # Save New upload
+        engine_dao.save_upload(dao, upload)
+
+        assert upload.uid
+
+        previous_request_id = upload.request_uid
+        upload.request_uid = str(uuid4())
+        # Update request_uid of existing record
+        engine_dao.update_upload_requestid(dao, upload)
+
+        assert previous_request_id != upload.request_uid

tests/unit/test_utils.py

@@ -413,7 +413,7 @@ def test_request_verify_ca_bundle_file(caplog, tmp_path):
     # Save the certificate for the first time
     caplog.clear()
     cert = nxdrive.utils.requests_verify(ca_bundle, False)
-    path = "" if type(cert) == bool else cert
+    path = "" if isinstance(bool, type(cert)) else cert
     final_certificate = Path(path)
     records = [line.message for line in caplog.records]
     assert len(records) == 3
@@ -451,7 +451,7 @@ def test_request_verify_ca_bundle_file_is_str(caplog, tmp_path):
     # Save the certificate for the first time
     caplog.clear()
     cert = nxdrive.utils.requests_verify(ca_bundle, False)
-    path = "" if type(cert) == bool else cert
+    path = "" if isinstance(bool, type(cert)) else cert
     final_certificate = Path(path)
     records = [line.message for line in caplog.records]
     assert len(records) == 3
@@ -494,7 +494,7 @@ def test_request_verify_ca_bundle_file_mimic_updates(caplog, tmp_path):
     # Save the certificate for the first time
     caplog.clear()
     cert = nxdrive.utils.requests_verify(ca_bundle, False)
-    path = "" if type(cert) == bool else cert
+    path = "" if isinstance(bool, type(cert)) else cert
     final_certificate_1 = Path(path)
     records = [line.message for line in caplog.records]
     assert len(records) == 3