STAR-322: Avoiding lambdas and direct callbacks to avoid gc

cura/NetworkClient.py

@@ -3,7 +3,7 @@
 from time import time
 from typing import Optional, Dict, Callable, List, Union
 
-from PyQt5.QtCore import QUrl, QObject
+from PyQt5.QtCore import QUrl
 from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkReply, QHttpMultiPart, QNetworkRequest, QHttpPart, \
     QAuthenticator
 
@@ -13,7 +13,7 @@ from UM.Logger import Logger
 
 ##  Abstraction of QNetworkAccessManager for easier networking in Cura.
 #   This was originally part of NetworkedPrinterOutputDevice but was moved out for re-use in other classes.
-class NetworkClient(QObject):
+class NetworkClient:
     
     def __init__(self) -> None:
         super().__init__()

plugins/UM3NetworkPrinting/src/Cloud/CloudApiClient.py

@@ -4,12 +4,12 @@ import json
 from json import JSONDecodeError
 from typing import Callable, List, Type, TypeVar, Union, Optional, Tuple, Dict, Any
 
-from PyQt5.QtCore import QObject, QUrl
+from PyQt5.QtCore import QUrl
 from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply, QNetworkAccessManager
 
 from UM.Logger import Logger
 from cura.API import Account
-from .ResumableUpload import ResumableUpload
+from .MeshUploader import MeshUploader
 from ..Models import BaseModel
 from .Models.CloudClusterResponse import CloudClusterResponse
 from .Models.CloudErrorObject import CloudErrorObject
@@ -37,6 +37,7 @@ class CloudApiClient:
         self._manager = QNetworkAccessManager()
         self._account = account
         self._on_error = on_error
+        self._upload = None  # type: Optional[MeshUploader]
 
     ## Gets the account used for the API.
     @property
@@ -77,10 +78,10 @@ class CloudApiClient:
     #  \param on_finished: The function to be called after the result is parsed. It receives the print job ID.
     #  \param on_progress: A function to be called during upload progress. It receives a percentage (0-100).
     #  \param on_error: A function to be called if the upload fails. It receives a dict with the error.
-    def uploadMesh(self, upload_response: CloudPrintJobResponse, mesh: bytes, on_finished: Callable[[], Any],
+    def uploadMesh(self, print_job: CloudPrintJobResponse, mesh: bytes, on_finished: Callable[[], Any],
                    on_progress: Callable[[int], Any], on_error: Callable[[], Any]):
-        ResumableUpload(self._manager, upload_response.upload_url, upload_response.content_type, mesh, on_finished,
+        self._upload = MeshUploader(self._manager, print_job, mesh, on_finished, on_progress, on_error)
-                        on_progress, on_error).start()
+        self._upload.start()
 
     # Requests a cluster to print the given print job.
     #  \param cluster_id: The ID of the cluster.

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py

@@ -8,11 +8,13 @@ from typing import Dict, List, Optional, Set
 from PyQt5.QtCore import QObject, QUrl, pyqtProperty, pyqtSignal, pyqtSlot
 
 from UM import i18nCatalog
+from UM.Backend.Backend import BackendState
 from UM.FileHandler.FileHandler import FileHandler
 from UM.Logger import Logger
 from UM.Message import Message
 from UM.Qt.Duration import Duration, DurationFormat
 from UM.Scene.SceneNode import SceneNode
+from cura.CuraApplication import CuraApplication
 from cura.PrinterOutput.NetworkedPrinterOutputDevice import AuthState, NetworkedPrinterOutputDevice
 from cura.PrinterOutput.PrinterOutputModel import PrinterOutputModel
 from plugins.UM3NetworkPrinting.src.Cloud.CloudOutputController import CloudOutputController
@@ -92,6 +94,8 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         self._device_id = device_id
         self._account = api_client.account
 
+        CuraApplication.getInstance().getBackend().backendStateChange.connect(self._onBackendStateChange)
+
         # We use the Cura Connect monitor tab to get most functionality right away.
         self._monitor_view_qml_path = os.path.join(os.path.dirname(os.path.abspath(__file__)),
                                                    "../../resources/qml/MonitorStage.qml")
@@ -116,6 +120,17 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         # A set of the user's job IDs that have finished
         self._finished_jobs = set()  # type: Set[str]
 
+        # Reference to the uploaded print job
+        self._mesh = None  # type: Optional[bytes]
+        self._uploaded_print_job = None  # type: Optional[CloudPrintJobResponse]
+
+    def disconnect(self) -> None:
+        CuraApplication.getInstance().getBackend().backendStateChange.disconnect(self._onBackendStateChange)
+
+    def _onBackendStateChange(self, _: BackendState) -> None:
+        self._mesh = None
+        self._uploaded_print_job = None
+
     ## Gets the host name of this device
     @property
     def host_name(self) -> str:
@@ -146,7 +161,16 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
 
         # Show an error message if we're already sending a job.
         if self._progress.visible:
-            self._onUploadError(T.BLOCKED_UPLOADING)
+            Message(
+                text = T.BLOCKED_UPLOADING,
+                title = T.ERROR,
+                lifetime = 10,
+            ).show()
+            return
+
+        if self._uploaded_print_job:
+            # the mesh didn't change, let's not upload it again
+            self._api.requestPrint(self._device_id, self._uploaded_print_job.job_id, self._onPrintRequested)
             return
 
         # Indicate we have started sending a job.
@@ -157,14 +181,15 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
             Logger.log("e", "Missing file or mesh writer!")
             return self._onUploadError(T.COULD_NOT_EXPORT)
 
-        mesh_bytes = mesh_format.getBytes(nodes)
+        mesh = mesh_format.getBytes(nodes)
 
+        self._mesh = mesh
         request = CloudPrintJobUploadRequest(
             job_name = file_name,
-            file_size = len(mesh_bytes),
+            file_size = len(mesh),
             content_type = mesh_format.mime_type,
         )
-        self._api.requestUpload(request, lambda response: self._onPrintJobCreated(mesh_bytes, response))
+        self._api.requestUpload(request, self._onPrintJobCreated)
 
     ##  Called when the network data should be updated.
     def _update(self) -> None:
@@ -281,21 +306,22 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
     ## Uploads the mesh when the print job was registered with the cloud API.
     #  \param mesh: The bytes to upload.
     #  \param job_response: The response received from the cloud API.
-    def _onPrintJobCreated(self, mesh: bytes, job_response: CloudPrintJobResponse) -> None:
+    def _onPrintJobCreated(self, job_response: CloudPrintJobResponse) -> None:
         self._progress.show()
-        self._api.uploadMesh(job_response, mesh, lambda: self._onPrintJobUploaded(job_response.job_id),
+        self._uploaded_print_job = job_response
-                             self._progress.update, self._onUploadError)
+        self._api.uploadMesh(job_response, self._mesh, self._onPrintJobUploaded, self._progress.update,
+                             self._onUploadError)
 
     ## Requests the print to be sent to the printer when we finished uploading the mesh.
-    #  \param job_id: The ID of the job.
+    def _onPrintJobUploaded(self) -> None:
-    def _onPrintJobUploaded(self, job_id: str) -> None:
         self._progress.update(100)
-        self._api.requestPrint(self._device_id, job_id, self._onPrintRequested)
+        self._api.requestPrint(self._device_id, self._uploaded_print_job.job_id, self._onPrintRequested)
 
     ## Displays the given message if uploading the mesh has failed
     #  \param message: The message to display.
     def _onUploadError(self, message = None) -> None:
         self._progress.hide()
+        self._uploaded_print_job = None
         Message(
             text = message or T.UPLOAD_ERROR,
             title = T.ERROR,

plugins/UM3NetworkPrinting/src/Cloud/MeshUploader.py

@@ -6,9 +6,10 @@ from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply, QNetworkAccessManage
 from typing import Optional, Callable, Any, Tuple
 
 from UM.Logger import Logger
+from src.Cloud.Models.CloudPrintJobResponse import CloudPrintJobResponse
 
 
-class ResumableUpload:
+class MeshUploader:
     MAX_RETRIES = 10
     BYTES_PER_REQUEST = 256 * 1024
     RETRY_HTTP_CODES = {500, 502, 503, 504}
@@ -18,11 +19,10 @@ class ResumableUpload:
     #  \param content_length: The total content length of the file, in bytes.
     #  \param http_method: The HTTP method to be used, e.g. "POST" or "PUT".
     #  \param timeout: The timeout for each chunk upload. Important: If None, no timeout is applied at all.
-    def __init__(self, manager: QNetworkAccessManager, url: str, content_type: str, data: bytes,
+    def __init__(self, manager: QNetworkAccessManager, print_job: CloudPrintJobResponse, data: bytes,
                  on_finished: Callable[[], Any], on_progress: Callable[[int], Any], on_error: Callable[[], Any]):
         self._manager = manager
-        self._url = url
+        self._print_job = print_job
-        self._content_type = content_type
         self._data = data
 
         self._on_finished = on_finished
@@ -34,17 +34,21 @@ class ResumableUpload:
         self._finished = False
         self._reply = None  # type: Optional[QNetworkReply]
 
+    @property
+    def printJob(self):
+        return self._print_job
+
     ##  We override _createRequest in order to add the user credentials.
     #   \param url: The URL to request
     #   \param content_type: The type of the body contents.
     def _createRequest(self) -> QNetworkRequest:
-        request = QNetworkRequest(QUrl(self._url))
+        request = QNetworkRequest(QUrl(self._print_job.upload_url))
-        request.setHeader(QNetworkRequest.ContentTypeHeader, self._content_type)
+        request.setHeader(QNetworkRequest.ContentTypeHeader, self._print_job.content_type)
         
         first_byte, last_byte = self._chunkRange()
         content_range = "bytes {}-{}/{}".format(first_byte, last_byte - 1, len(self._data))
         request.setRawHeader(b"Content-Range", content_range.encode())
-        Logger.log("i", "Uploading %s to %s", content_range, self._url)
+        Logger.log("i", "Uploading %s to %s", content_range, self._print_job.upload_url)
 
         return request
 

plugins/UM3NetworkPrinting/tests/Cloud/TestCloudOutputDevice.py

@@ -5,6 +5,7 @@ from unittest import TestCase
 from unittest.mock import patch, MagicMock
 
 from UM.Scene.SceneNode import SceneNode
+from UM.Signal import Signal
 from cura.CuraApplication import CuraApplication
 from cura.PrinterOutput.PrinterOutputModel import PrinterOutputModel
 from src.Cloud.CloudApiClient import CloudApiClient
@@ -26,6 +27,9 @@ class TestCloudOutputDevice(TestCase):
     def setUp(self):
         super().setUp()
         self.app = CuraApplication.getInstance()
+        self.backend = MagicMock(backendStateChange = Signal())
+        self.app.setBackend(self.backend)
+
         self.network = NetworkManagerMock()
         self.account = MagicMock(isLoggedIn=True, accessToken="TestAccessToken")
         self.onError = MagicMock()