STAR-322: Using QNetworkReply.finished signal instead of QNetworkAccessManager.finished

cura/NetworkClient.py

@@ -1,9 +1,9 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # Cura is released under the terms of the LGPLv3 or higher.
 from time import time
-from typing import Optional, Dict, Callable, List, Union, Tuple
+from typing import Optional, Dict, Callable, List, Union
 
-from PyQt5.QtCore import QUrl
+from PyQt5.QtCore import QUrl, QObject
 from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkReply, QHttpMultiPart, QNetworkRequest, QHttpPart, \
     QAuthenticator
 
@@ -13,9 +13,10 @@ 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:
+class NetworkClient(QObject):
     
     def __init__(self) -> None:
+        super().__init__()
 
         # Network manager instance to use for this client.
         self._manager = None  # type: Optional[QNetworkAccessManager]
@@ -29,11 +30,6 @@ class NetworkClient:
         application = Application.getInstance()
         self._user_agent = "%s/%s " % (application.getApplicationName(), application.getVersion())
 
-        # Uses to store callback methods for finished network requests.
-        # This allows us to register network calls with a callback directly instead of having to dissect the reply.
-        # The key is created out of a tuple (operation, url)
-        self._on_finished_callbacks = {}  # type: Dict[Tuple[int, str], Callable[[QNetworkReply], None]]
-
         # QHttpMultiPart objects need to be kept alive and not garbage collected during the
         # HTTP which uses them. We hold references to these QHttpMultiPart objects here.
         self._kept_alive_multiparts = {}  # type: Dict[QNetworkReply, QHttpMultiPart]
@@ -43,7 +39,6 @@ class NetworkClient:
         if self._manager:
             return
         self._manager = QNetworkAccessManager()
-        self._manager.finished.connect(self._handleOnFinished)
         self._last_manager_create_time = time()
         self._manager.authenticationRequired.connect(self._onAuthenticationRequired)
 
@@ -51,7 +46,6 @@ class NetworkClient:
     def stop(self) -> None:
         if not self._manager:
             return
-        self._manager.finished.disconnect(self._handleOnFinished)
         self._manager.authenticationRequired.disconnect(self._onAuthenticationRequired)
         self._manager = None
 
@@ -69,32 +63,6 @@ class NetworkClient:
         self._last_request_time = time()
         return request
 
-    ##  Executes the correct callback method when a network request finishes.
-    def _handleOnFinished(self, reply: QNetworkReply) -> None:
-
-        Logger.log("i", "On finished %s %s", reply.attribute(QNetworkRequest.HttpStatusCodeAttribute), reply.url().toString())
-
-        # Due to garbage collection, we need to cache certain bits of post operations.
-        # As we don't want to keep them around forever, delete them if we get a reply.
-        if reply.operation() == QNetworkAccessManager.PostOperation:
-            self._clearCachedMultiPart(reply)
-
-        # No status code means it never even reached remote.
-        if reply.attribute(QNetworkRequest.HttpStatusCodeAttribute) is None:
-            return
-
-        # Not used by this class itself, but children might need it for better network handling.
-        # An example of this is the _update method in the NetworkedPrinterOutputDevice.
-        self._last_response_time = time()
-        
-        # Find the right callback and execute it.
-        # It always takes the full reply as single parameter.
-        callback_key = reply.operation(), reply.url().toString()
-        if callback_key in self._on_finished_callbacks:
-            self._on_finished_callbacks[callback_key](reply)
-        else:
-            Logger.log("w", "Received reply to URL %s but no callbacks are registered", reply.url())
-
     ##  Removes all cached Multi-Part items.
     def _clearCachedMultiPart(self, reply: QNetworkReply) -> None:
         if reply in self._kept_alive_multiparts:
@@ -105,12 +73,6 @@ class NetworkClient:
     def _onAuthenticationRequired(reply: QNetworkReply, authenticator: QAuthenticator) -> None:
         Logger.log("w", "Request to {} required authentication but was not given".format(reply.url().toString()))
 
-    ##  Register a method to be executed when the associated network request finishes.
-    def _registerOnFinishedCallback(self, reply: QNetworkReply,
-                                    on_finished: Optional[Callable[[QNetworkReply], None]]) -> None:
-        if on_finished is not None:
-            self._on_finished_callbacks[reply.operation(), reply.url().toString()] = on_finished
-
     ##  Add a part to a Multi-Part form.
     @staticmethod
     def _createFormPart(content_header: str, data: bytes, content_type: Optional[str] = None) -> QHttpPart:
@@ -144,13 +106,10 @@ class NetworkClient:
 
         body = data if isinstance(data, bytes) else data.encode()  # type: bytes
         reply = self._manager.put(request, body)
-        self._registerOnFinishedCallback(reply, on_finished)
+        callback = self._createCallback(reply, on_finished)
-
+        reply.finished.connect(callback)
         if on_progress is not None:
-            # TODO: Do we need to disconnect() as well?
             reply.uploadProgress.connect(on_progress)
-            reply.finished.connect(lambda r: Logger.log("i", "On finished %s %s", url, r))
-            reply.error.connect(lambda r: Logger.log("i", "On error %s %s", url, r))
 
     ## Sends a delete request to the given path.
     #  url: The path after the API prefix.
@@ -158,7 +117,8 @@ class NetworkClient:
     def delete(self, url: str, on_finished: Optional[Callable[[QNetworkReply], None]]) -> None:
         request = self._createEmptyRequest(url)
         reply = self._manager.deleteResource(request)
-        self._registerOnFinishedCallback(reply, on_finished)
+        callback = self._createCallback(reply, on_finished)
+        reply.finished.connect(callback)
 
     ## Sends a get request to the given path.
     #  \param url: The path after the API prefix.
@@ -166,7 +126,8 @@ class NetworkClient:
     def get(self, url: str, on_finished: Optional[Callable[[QNetworkReply], None]]) -> None:
         request = self._createEmptyRequest(url)
         reply = self._manager.get(request)
-        self._registerOnFinishedCallback(reply, on_finished)
+        callback = self._createCallback(reply, on_finished)
+        reply.finished.connect(callback)
 
     ## Sends a post request to the given path.
     #  \param url: The path after the API prefix.
@@ -180,9 +141,10 @@ class NetworkClient:
 
         body = data if isinstance(data, bytes) else data.encode()  # type: bytes
         reply = self._manager.post(request, body)
+        callback = self._createCallback(reply, on_finished)
+        reply.finished.connect(callback)
         if on_progress is not None:
             reply.uploadProgress.connect(on_progress)
-        self._registerOnFinishedCallback(reply, on_finished)
 
     ##  Does a POST request with form data to the given URL.
     def postForm(self, url: str, header_data: str, body_data: bytes,
@@ -205,10 +167,19 @@ class NetworkClient:
 
         reply = self._manager.post(request, multi_post_part)
 
+        def callback():
+            on_finished(reply)
+            self._clearCachedMultiPart(reply)
+
+        reply.finished.connect(callback)
+
         self._kept_alive_multiparts[reply] = multi_post_part
 
         if on_progress is not None:
             reply.uploadProgress.connect(on_progress)
             
-        self._registerOnFinishedCallback(reply, on_finished)
         return reply
+
+    @staticmethod
+    def _createCallback(reply: QNetworkReply, on_finished: Optional[Callable[[QNetworkReply], None]] = None):
+        return lambda: on_finished(reply)

plugins/UM3NetworkPrinting/src/Cloud/CloudApiClient.py

@@ -4,11 +4,11 @@ import json
 from json import JSONDecodeError
 from typing import Callable, List, Type, TypeVar, Union, Optional, Tuple, Dict, Any
 
-from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply
+from PyQt5.QtCore import QObject, QUrl
+from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply, QNetworkAccessManager
 
 from UM.Logger import Logger
 from cura.API import Account
-from cura.NetworkClient import NetworkClient
 from .ResumableUpload import ResumableUpload
 from ..Models import BaseModel
 from .Models.CloudClusterResponse import CloudClusterResponse
@@ -21,7 +21,7 @@ from .Models.CloudPrintJobResponse import CloudPrintJobResponse
 
 ## The cloud API client is responsible for handling the requests and responses from the cloud.
 #  Each method should only handle models instead of exposing Any HTTP details.
-class CloudApiClient(NetworkClient):
+class CloudApiClient:
 
     # The cloud URL to use for this remote cluster.
     # TODO: Make sure that this URL goes to the live api before release
@@ -34,6 +34,7 @@ class CloudApiClient(NetworkClient):
     #  \param on_error: The callback to be called whenever we receive errors from the server.
     def __init__(self, account: Account, on_error: Callable[[List[CloudErrorObject]], None]) -> None:
         super().__init__()
+        self._manager = QNetworkAccessManager()
         self._account = account
         self._on_error = on_error
 
@@ -46,14 +47,18 @@ class CloudApiClient(NetworkClient):
     #  \param on_finished: The function to be called after the result is parsed.
     def getClusters(self, on_finished: Callable[[List[CloudClusterResponse]], Any]) -> None:
         url = "{}/clusters".format(self.CLUSTER_API_ROOT)
-        self.get(url, on_finished=self._wrapCallback(on_finished, CloudClusterResponse))
+        reply = self._manager.get(self._createEmptyRequest(url))
+        callback = self._wrapCallback(reply, on_finished, CloudClusterResponse)
+        reply.finished.connect(callback)
 
     ## Retrieves the status of the given cluster.
     #  \param cluster_id: The ID of the cluster.
     #  \param on_finished: The function to be called after the result is parsed.
     def getClusterStatus(self, cluster_id: str, on_finished: Callable[[CloudClusterStatus], Any]) -> None:
         url = "{}/clusters/{}/status".format(self.CLUSTER_API_ROOT, cluster_id)
-        self.get(url, on_finished=self._wrapCallback(on_finished, CloudClusterStatus))
+        reply = self._manager.get(self._createEmptyRequest(url))
+        callback = self._wrapCallback(reply, on_finished, CloudClusterStatus)
+        reply.finished.connect(callback)
 
     ## Requests the cloud to register the upload of a print job mesh.
     #  \param request: The request object.
@@ -62,7 +67,9 @@ class CloudApiClient(NetworkClient):
                       ) -> None:
         url = "{}/jobs/upload".format(self.CURA_API_ROOT)
         body = json.dumps({"data": request.toDict()})
-        self.put(url, body, on_finished=self._wrapCallback(on_finished, CloudPrintJobResponse))
+        reply = self._manager.put(self._createEmptyRequest(url), body.encode())
+        callback = self._wrapCallback(reply, on_finished, CloudPrintJobResponse)
+        reply.finished.connect(callback)
 
     ## Requests the cloud to register the upload of a print job mesh.
     #  \param upload_response: The object received after requesting an upload with `self.requestUpload`.
@@ -72,7 +79,7 @@ class CloudApiClient(NetworkClient):
     #  \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],
                    on_progress: Callable[[int], Any], on_error: Callable[[], Any]):
-        ResumableUpload(upload_response.upload_url, upload_response.content_type, mesh, on_finished,
+        ResumableUpload(self._manager, upload_response.upload_url, upload_response.content_type, mesh, on_finished,
                         on_progress, on_error).start()
 
     # Requests a cluster to print the given print job.
@@ -81,13 +88,17 @@ class CloudApiClient(NetworkClient):
     #  \param on_finished: The function to be called after the result is parsed.
     def requestPrint(self, cluster_id: str, job_id: str, on_finished: Callable[[CloudPrintResponse], Any]) -> None:
         url = "{}/clusters/{}/print/{}".format(self.CLUSTER_API_ROOT, cluster_id, job_id)
-        self.post(url, data = "", on_finished=self._wrapCallback(on_finished, CloudPrintResponse))
+        reply = self._manager.post(self._createEmptyRequest(url), b"")
+        callback = self._wrapCallback(reply, on_finished, CloudPrintResponse)
+        reply.finished.connect(callback)
 
     ##  We override _createEmptyRequest in order to add the user credentials.
     #   \param url: The URL to request
     #   \param content_type: The type of the body contents.
     def _createEmptyRequest(self, path: str, content_type: Optional[str] = "application/json") -> QNetworkRequest:
-        request = super()._createEmptyRequest(path, content_type)
+        request = QNetworkRequest(QUrl(path))
+        if content_type:
+            request.setHeader(QNetworkRequest.ContentTypeHeader, content_type)
         if self._account.isLoggedIn:
             request.setRawHeader(b"Authorization", "Bearer {}".format(self._account.accessToken).encode())
         Logger.log("i", "Created request for URL %s. Logged in = %s", path, self._account.isLoggedIn)
@@ -132,10 +143,11 @@ class CloudApiClient(NetworkClient):
     #  \param model: The type of the model to convert the response to. It may either be a single record or a list.
     #  \return: A function that can be passed to the
     def _wrapCallback(self,
+                      reply: QNetworkReply,
                       on_finished: Callable[[Union[Model, List[Model]]], Any],
                       model: Type[Model],
                       ) -> Callable[[QNetworkReply], None]:
-        def parse(reply: QNetworkReply) -> None:
+        def parse() -> None:
             status_code, response = self._parseReply(reply)
             return self._parseModels(response, on_finished, model)
         return parse

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDevice.py

@@ -172,8 +172,6 @@ class CloudOutputDevice(NetworkedPrinterOutputDevice):
         if self._last_response_time and time() - self._last_response_time < self.CHECK_CLUSTER_INTERVAL:
             return  # avoid calling the cloud too often
 
-        Logger.log("i", "Requesting update for %s after %s", self._device_id,
-                   self._last_response_time and time() - self._last_response_time)
         if self._account.isLoggedIn:
             self.setAuthenticationState(AuthState.Authenticated)
             self._api.getClusterStatus(self._device_id, self._onStatusCallFinished)

plugins/UM3NetworkPrinting/src/Cloud/CloudOutputDeviceManager.py

@@ -125,13 +125,14 @@ class CloudOutputDeviceManager:
     ## Handles an API error received from the cloud.
     #  \param errors: The errors received
     def _onApiError(self, errors: List[CloudErrorObject]) -> None:
-        message = ". ".join(e.title for e in errors)  # TODO: translate errors
+        text = ". ".join(e.title for e in errors)  # TODO: translate errors
-        Message(
+        message = Message(
-            text = message,
+            text = text,
             title = self.I18N_CATALOG.i18nc("@info:title", "Error"),
             lifetime = 10,
             dismissable = True
-        ).show()
+        )
+        message.show()
 
     def start(self):
         if self._running:
@@ -141,7 +142,6 @@ class CloudOutputDeviceManager:
         # When switching machines we check if we have to activate a remote cluster.
         application.globalContainerStackChanged.connect(self._connectToActiveMachine)
         self._update_timer.timeout.connect(self._getRemoteClusters)
-        self._api.start()
         self._onLoginStateChanged(is_logged_in = self._account.isLoggedIn)
 
     def stop(self):
@@ -152,5 +152,4 @@ class CloudOutputDeviceManager:
         # When switching machines we check if we have to activate a remote cluster.
         application.globalContainerStackChanged.disconnect(self._connectToActiveMachine)
         self._update_timer.timeout.disconnect(self._getRemoteClusters)
-        self._api.stop()
         self._onLoginStateChanged(is_logged_in = False)

plugins/UM3NetworkPrinting/src/Cloud/ResumableUpload.py

@@ -1,14 +1,14 @@
 # Copyright (c) 2018 Ultimaker B.V.
 # !/usr/bin/env python
 # -*- coding: utf-8 -*-
-from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply
+from PyQt5.QtCore import QUrl
+from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply, QNetworkAccessManager
 from typing import Optional, Callable, Any, Tuple
 
 from UM.Logger import Logger
-from cura.NetworkClient import NetworkClient
 
 
-class ResumableUpload(NetworkClient):
+class ResumableUpload:
     MAX_RETRIES = 10
     BYTES_PER_REQUEST = 256 * 1024
     RETRY_HTTP_CODES = {500, 502, 503, 504}
@@ -18,9 +18,9 @@ class ResumableUpload(NetworkClient):
     #  \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, url: str, content_type: str, data: bytes,
+    def __init__(self, manager: QNetworkAccessManager, url: str, content_type: str, data: bytes,
                  on_finished: Callable[[], Any], on_progress: Callable[[int], Any], on_error: Callable[[], Any]):
-        super().__init__()
+        self._manager = manager
         self._url = url
         self._content_type = content_type
         self._data = data
@@ -32,12 +32,14 @@ class ResumableUpload(NetworkClient):
         self._sent_bytes = 0
         self._retries = 0
         self._finished = False
+        self._reply = None  # type: Optional[QNetworkReply]
 
-    ##  We override _createEmptyRequest in order to add the user credentials.
+    ##  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 _createEmptyRequest(self, path: str, content_type: Optional[str] = "application/json") -> QNetworkRequest:
+    def _createRequest(self) -> QNetworkRequest:
-        request = super()._createEmptyRequest(path, content_type = self._content_type)
+        request = QNetworkRequest(QUrl(self._url))
+        request.setHeader(QNetworkRequest.ContentTypeHeader, self._content_type)
         
         first_byte, last_byte = self._chunkRange()
         content_range = "bytes {}-{}/{}".format(first_byte, last_byte - 1, len(self._data))
@@ -51,7 +53,6 @@ class ResumableUpload(NetworkClient):
         return self._sent_bytes, last_byte
 
     def start(self) -> None:
-        super().start()
         if self._finished:
             self._sent_bytes = 0
             self._retries = 0
@@ -59,7 +60,6 @@ class ResumableUpload(NetworkClient):
         self._uploadChunk()
 
     def stop(self):
-        super().stop()
         Logger.log("i", "Stopped uploading")
         self._finished = True
 
@@ -68,47 +68,43 @@ class ResumableUpload(NetworkClient):
             raise ValueError("The upload is already finished")
 
         first_byte, last_byte = self._chunkRange()
-        # self.put(self._url, data = self._data[first_byte:last_byte], content_type = self._content_type,
+        request = self._createRequest()
-        #          on_finished = self.finishedCallback, on_progress = self._progressCallback)
-        request = self._createEmptyRequest(self._url, content_type=self._content_type)
 
-        reply = self._manager.put(request, self._data[first_byte:last_byte])
+        self._reply = self._manager.put(request, self._data[first_byte:last_byte])
-        reply.finished.connect(lambda: self._finishedCallback(reply))
+        self._reply.finished.connect(self._finishedCallback)
-        reply.uploadProgress.connect(self._progressCallback)
+        self._reply.uploadProgress.connect(self._progressCallback)
-        reply.error.connect(self._errorCallback)
+        self._reply.error.connect(self._errorCallback)
-        if reply.isFinished():
-            self._finishedCallback(reply)
 
     def _progressCallback(self, bytes_sent: int, bytes_total: int) -> None:
         Logger.log("i", "Progress callback %s / %s", bytes_sent, bytes_total)
         if bytes_total:
             self._on_progress(int((self._sent_bytes + bytes_sent) / len(self._data) * 100))
 
-    def _errorCallback(self, reply: QNetworkReply) -> None:
+    def _errorCallback(self) -> None:
-        body = bytes(reply.readAll()).decode()
+        body = bytes(self._reply.readAll()).decode()
         Logger.log("e", "Received error while uploading: %s", body)
         self.stop()
         self._on_error()
 
-    def _finishedCallback(self, reply: QNetworkReply) -> None:
+    def _finishedCallback(self) -> None:
         Logger.log("i", "Finished callback %s %s",
-                   reply.attribute(QNetworkRequest.HttpStatusCodeAttribute), reply.url().toString())
+                   self._reply.attribute(QNetworkRequest.HttpStatusCodeAttribute), self._reply.url().toString())
 
-        status_code = reply.attribute(QNetworkRequest.HttpStatusCodeAttribute)
+        status_code = self._reply.attribute(QNetworkRequest.HttpStatusCodeAttribute)
 
         if self._retries < self.MAX_RETRIES and status_code in self.RETRY_HTTP_CODES:
             self._retries += 1
-            Logger.log("i", "Retrying %s/%s request %s", self._retries, self.MAX_RETRIES, reply.url().toString())
+            Logger.log("i", "Retrying %s/%s request %s", self._retries, self.MAX_RETRIES, self._reply.url().toString())
             self._uploadChunk()
             return
 
         if status_code > 308:
-            self._errorCallback(reply)
+            self._errorCallback()
             return
 
-        body = bytes(reply.readAll()).decode()
+        body = bytes(self._reply.readAll()).decode()
         Logger.log("w", "status_code: %s, Headers: %s, body: %s", status_code,
-                   [bytes(header).decode() for header in reply.rawHeaderList()], body)
+                   [bytes(header).decode() for header in self._reply.rawHeaderList()], body)
 
         first_byte, last_byte = self._chunkRange()
         self._sent_bytes += last_byte - first_byte

plugins/UM3NetworkPrinting/tests/Cloud/NetworkManagerMock.py

@@ -4,12 +4,27 @@ import json
 from typing import Dict, Tuple, Union, Optional
 from unittest.mock import MagicMock
 
-from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkReply, QNetworkRequest
+from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkRequest
 
 from UM.Logger import Logger
 from UM.Signal import Signal
 
 
+class FakeSignal:
+    def __init__(self):
+        self._callbacks = []
+
+    def connect(self, callback):
+        self._callbacks.append(callback)
+
+    def disconnect(self, callback):
+        self._callbacks.remove(callback)
+
+    def emit(self, *args, **kwargs):
+        for callback in self._callbacks:
+            callback(*args, **kwargs)
+
+
 ## This class can be used to mock the QNetworkManager class and test the code using it.
 #  After patching the QNetworkManager class, requests are prepared before they can be executed.
 #  Any requests not prepared beforehand will cause KeyErrors.
@@ -27,7 +42,7 @@ class NetworkManagerMock:
     ## Initializes the network manager mock.
     def __init__(self) -> None:
         # a dict with the prepared replies, using the format {(http_method, url): reply}
-        self.replies = {}  # type: Dict[Tuple[str, str], QNetworkReply]
+        self.replies = {}  # type: Dict[Tuple[str, str], MagicMock]
         self.request_bodies = {}  # type: Dict[Tuple[str, str], bytes]
 
         # signals used in the network manager.
@@ -64,6 +79,8 @@ class NetworkManagerMock:
         reply_mock.url().toString.return_value = url
         reply_mock.operation.return_value = self._OPERATIONS[method]
         reply_mock.attribute.return_value = status_code
+        reply_mock.finished = FakeSignal()
+        reply_mock.isFinished.return_value = False
         reply_mock.readAll.return_value = response if isinstance(response, bytes) else json.dumps(response).encode()
         self.replies[method, url] = reply_mock
         Logger.log("i", "Prepared mock {}-response to {} {}", status_code, method, url)
@@ -78,6 +95,8 @@ class NetworkManagerMock:
     def flushReplies(self) -> None:
         for key, reply in self.replies.items():
             Logger.log("i", "Flushing reply to {} {}", *key)
+            reply.isFinished.return_value = True
+            reply.finished.emit()
             self.finished.emit(reply)
         self.reset()
 

plugins/UM3NetworkPrinting/tests/Cloud/TestCloudApiClient.py

@@ -8,6 +8,8 @@ from unittest.mock import patch, MagicMock
 
 from cura.CuraApplication import CuraApplication
 from src.Cloud.CloudApiClient import CloudApiClient
+from src.Cloud.Models.CloudClusterResponse import CloudClusterResponse
+from src.Cloud.Models.CloudClusterStatus import CloudClusterStatus
 from src.Cloud.Models.CloudPrintJobResponse import CloudPrintJobResponse
 from src.Cloud.Models.CloudPrintJobUploadRequest import CloudPrintJobUploadRequest
 from src.Cloud.Models.CloudErrorObject import CloudErrorObject
@@ -15,7 +17,6 @@ from tests.Cloud.Fixtures import readFixture, parseFixture
 from .NetworkManagerMock import NetworkManagerMock
 
 
-@patch("cura.NetworkClient.QNetworkAccessManager")
 class TestCloudApiClient(TestCase):
     def _errorHandler(self, errors: List[CloudErrorObject]):
         raise Exception("Received unexpected error: {}".format(errors))
@@ -27,15 +28,14 @@ class TestCloudApiClient(TestCase):
 
         self.app = CuraApplication.getInstance()
         self.network = NetworkManagerMock()
+        with patch("src.Cloud.CloudApiClient.QNetworkAccessManager", return_value = self.network):
             self.api = CloudApiClient(self.account, self._errorHandler)
 
-    def test_GetClusters(self, network_mock):
+    def test_getClusters(self):
-        network_mock.return_value = self.network
-
         result = []
 
-        with open("{}/Fixtures/getClusters.json".format(os.path.dirname(__file__)), "rb") as f:
+        response = readFixture("getClusters")
-            response = f.read()
+        data = parseFixture("getClusters")["data"]
 
         self.network.prepareReply("GET", "https://api-staging.ultimaker.com/connect/v1/clusters", 200, response)
         # the callback is a function that adds the result of the call to getClusters to the result list
@@ -43,32 +43,26 @@ class TestCloudApiClient(TestCase):
 
         self.network.flushReplies()
 
-        self.assertEqual(2, len(result))
+        self.assertEqual([CloudClusterResponse(**data[0]), CloudClusterResponse(**data[1])], result)
-
-    def test_getClusterStatus(self, network_mock):
-        network_mock.return_value = self.network
 
+    def test_getClusterStatus(self):
         result = []
 
-        with open("{}/Fixtures/getClusterStatusResponse.json".format(os.path.dirname(__file__)), "rb") as f:
+        response = readFixture("getClusterStatusResponse")
-            response = f.read()
+        data = parseFixture("getClusterStatusResponse")["data"]
 
         self.network.prepareReply("GET",
                                   "https://api-staging.ultimaker.com/connect/v1/clusters/R0YcLJwar1ugh0ikEZsZs8NWKV6vJP_LdYsXgXqAcaNC/status",
                                   200, response
                                   )
-        self.api.getClusterStatus("R0YcLJwar1ugh0ikEZsZs8NWKV6vJP_LdYsXgXqAcaNC", lambda status: result.append(status))
+        self.api.getClusterStatus("R0YcLJwar1ugh0ikEZsZs8NWKV6vJP_LdYsXgXqAcaNC", lambda s: result.append(s))
 
         self.network.flushReplies()
 
-        self.assertEqual(len(result), 1)
+        self.assertEqual([CloudClusterStatus(**data)], result)
-        status = result[0]
 
-        self.assertEqual(len(status.printers), 2)
+    def test_requestUpload(self):
-        self.assertEqual(len(status.print_jobs), 1)
         
-    def test_requestUpload(self, network_mock):
-        network_mock.return_value = self.network
         results = []
 
         response = readFixture("putJobUploadResponse")
@@ -78,11 +72,11 @@ class TestCloudApiClient(TestCase):
         self.api.requestUpload(request, lambda r: results.append(r))
         self.network.flushReplies()
 
-        self.assertEqual(results[0].content_type, "text/plain")
+        self.assertEqual(["text/plain"], [r.content_type for r in results])
-        self.assertEqual(results[0].status, "uploading")
+        self.assertEqual(["uploading"], [r.status for r in results])
+
+    def test_uploadMesh(self):
         
-    def test_uploadMesh(self, network_mock):
-        network_mock.return_value = self.network
         results = []
         progress = MagicMock()
 
@@ -101,8 +95,8 @@ class TestCloudApiClient(TestCase):
 
         self.assertEqual(["sent"], results)
 
-    def test_requestPrint(self, network_mock):
+    def test_requestPrint(self):
-        network_mock.return_value = self.network
+        
         results = []
 
         response = readFixture("postJobPrintResponse")
@@ -120,7 +114,6 @@ class TestCloudApiClient(TestCase):
 
         self.network.flushReplies()
 
-        self.assertEqual(len(results), 1)
+        self.assertEqual([job_id], [r.job_id for r in results])
-        self.assertEqual(results[0].job_id, job_id)
+        self.assertEqual([cluster_job_id], [r.cluster_job_id for r in results])
-        self.assertEqual(results[0].cluster_job_id, cluster_job_id)
+        self.assertEqual(["queued"], [r.status for r in results])
-        self.assertEqual(results[0].status, "queued")

plugins/UM3NetworkPrinting/tests/Cloud/TestCloudOutputDevice.py

@@ -13,7 +13,6 @@ from tests.Cloud.Fixtures import readFixture, parseFixture
 from .NetworkManagerMock import NetworkManagerMock
 
 
-@patch("cura.NetworkClient.QNetworkAccessManager")
 class TestCloudOutputDevice(TestCase):
     CLUSTER_ID = "RIZ6cZbWA_Ua7RZVJhrdVfVpf0z-MqaSHQE4v8aRTtYq"
     JOB_ID = "ABCDefGHIjKlMNOpQrSTUvYxWZ0-1234567890abcDE="
@@ -30,7 +29,9 @@ class TestCloudOutputDevice(TestCase):
         self.network = NetworkManagerMock()
         self.account = MagicMock(isLoggedIn=True, accessToken="TestAccessToken")
         self.onError = MagicMock()
-        self.device = CloudOutputDevice(CloudApiClient(self.account, self.onError), self.CLUSTER_ID, self.HOST_NAME)
+        with patch("src.Cloud.CloudApiClient.QNetworkAccessManager", return_value = self.network):
+            self._api = CloudApiClient(self.account, self.onError)
+        self.device = CloudOutputDevice(self._api, self.CLUSTER_ID, self.HOST_NAME)
         self.cluster_status = parseFixture("getClusterStatusResponse")
         self.network.prepareReply("GET", self.STATUS_URL, 200, readFixture("getClusterStatusResponse"))
 
@@ -38,8 +39,7 @@ class TestCloudOutputDevice(TestCase):
         super().tearDown()
         self.network.flushReplies()
 
-    def test_status(self, network_mock):
+    def test_status(self):
-        network_mock.return_value = self.network
         self.device._update()
         self.network.flushReplies()
 
@@ -69,32 +69,34 @@ class TestCloudOutputDevice(TestCase):
         self.assertEqual({job["name"] for job in self.cluster_status["data"]["print_jobs"]},
                          {job.name for job in self.device.printJobs})
 
-    def test_remove_print_job(self, network_mock):
+    def test_remove_print_job(self):
-        network_mock.return_value = self.network
         self.device._update()
         self.network.flushReplies()
         self.assertEqual(1, len(self.device.printJobs))
 
         self.cluster_status["data"]["print_jobs"].clear()
         self.network.prepareReply("GET", self.STATUS_URL, 200, self.cluster_status)
+
+        self.device._last_response_time = None
         self.device._update()
         self.network.flushReplies()
         self.assertEqual([], self.device.printJobs)
 
-    def test_remove_printers(self, network_mock):
+    def test_remove_printers(self):
-        network_mock.return_value = self.network
         self.device._update()
         self.network.flushReplies()
         self.assertEqual(2, len(self.device.printers))
 
         self.cluster_status["data"]["printers"].clear()
         self.network.prepareReply("GET", self.STATUS_URL, 200, self.cluster_status)
+
+        self.device._last_response_time = None
         self.device._update()
         self.network.flushReplies()
         self.assertEqual([], self.device.printers)
 
     @patch("cura.CuraApplication.CuraApplication.getGlobalContainerStack")
-    def test_print_to_cloud(self, global_container_stack_mock, network_mock):
+    def test_print_to_cloud(self, global_container_stack_mock):
         active_machine_mock = global_container_stack_mock.return_value
         active_machine_mock.getMetaDataEntry.side_effect = {"file_formats": "application/gzip"}.get
 
@@ -104,7 +106,6 @@ class TestCloudOutputDevice(TestCase):
         self.network.prepareReply("PUT", request_upload_response["data"]["upload_url"], 201, b"{}")
         self.network.prepareReply("POST", self.PRINT_URL, 200, request_print_response)
 
-        network_mock.return_value = self.network
         file_handler = MagicMock()
         file_handler.getSupportedFileTypesWrite.return_value = [{
             "extension": "gcode.gz",

plugins/UM3NetworkPrinting/tests/Cloud/TestCloudOutputDeviceManager.py

@@ -10,7 +10,6 @@ from tests.Cloud.Fixtures import parseFixture, readFixture
 from .NetworkManagerMock import NetworkManagerMock
 
 
-@patch("cura.NetworkClient.QNetworkAccessManager")
 class TestCloudOutputDeviceManager(TestCase):
     URL = "https://api-staging.ultimaker.com/connect/v1/clusters"
 
@@ -18,6 +17,7 @@ class TestCloudOutputDeviceManager(TestCase):
         super().setUp()
         self.app = CuraApplication.getInstance()
         self.network = NetworkManagerMock()
+        with patch("src.Cloud.CloudApiClient.QNetworkAccessManager", return_value = self.network):
             self.manager = CloudOutputDeviceManager()
         self.clusters_response = parseFixture("getClusters")
         self.network.prepareReply("GET", self.URL, 200, readFixture("getClusters"))
@@ -25,6 +25,7 @@ class TestCloudOutputDeviceManager(TestCase):
     def tearDown(self):
         try:
             self._beforeTearDown()
+            self.manager.stop()
         finally:
             super().tearDown()
 
@@ -47,17 +48,17 @@ class TestCloudOutputDeviceManager(TestCase):
             device_manager.removeOutputDevice(device["cluster_id"])
 
     ## Runs the initial request to retrieve the clusters.
-    def _loadData(self, network_mock):
+    def _loadData(self):
-        network_mock.return_value = self.network
+        self.manager.start()
-        self.manager._account.loginStateChanged.emit(True)
+        self.manager._onLoginStateChanged(is_logged_in = True)
-        self.manager._update_timer.timeout.emit()
+        self.network.flushReplies()
 
-    def test_device_is_created(self, network_mock):
+    def test_device_is_created(self):
         # just create the cluster, it is checked at tearDown
-        self._loadData(network_mock)
+        self._loadData()
 
-    def test_device_is_updated(self, network_mock):
+    def test_device_is_updated(self):
-        self._loadData(network_mock)
+        self._loadData()
 
         # update the cluster from member variable, which is checked at tearDown
         self.clusters_response["data"][0]["host_name"] = "New host name"
@@ -65,8 +66,8 @@ class TestCloudOutputDeviceManager(TestCase):
 
         self.manager._update_timer.timeout.emit()
 
-    def test_device_is_removed(self, network_mock):
+    def test_device_is_removed(self):
-        self._loadData(network_mock)
+        self._loadData()
 
         # delete the cluster from member variable, which is checked at tearDown
         del self.clusters_response["data"][1]
@@ -75,41 +76,39 @@ class TestCloudOutputDeviceManager(TestCase):
         self.manager._update_timer.timeout.emit()
 
     @patch("cura.CuraApplication.CuraApplication.getGlobalContainerStack")
-    def test_device_connects_by_cluster_id(self, global_container_stack_mock, network_mock):
+    def test_device_connects_by_cluster_id(self, global_container_stack_mock):
         active_machine_mock = global_container_stack_mock.return_value
         cluster1, cluster2 = self.clusters_response["data"]
         cluster_id = cluster1["cluster_id"]
         active_machine_mock.getMetaDataEntry.side_effect = {"um_cloud_cluster_id": cluster_id}.get
 
-        self._loadData(network_mock)
+        self._loadData()
-        self.network.flushReplies()
 
         self.assertTrue(self.app.getOutputDeviceManager().getOutputDevice(cluster1["cluster_id"]).isConnected())
         self.assertFalse(self.app.getOutputDeviceManager().getOutputDevice(cluster2["cluster_id"]).isConnected())
         self.assertEquals([], active_machine_mock.setMetaDataEntry.mock_calls)
 
     @patch("cura.CuraApplication.CuraApplication.getGlobalContainerStack")
-    def test_device_connects_by_network_key(self, global_container_stack_mock, network_mock):
+    def test_device_connects_by_network_key(self, global_container_stack_mock):
         active_machine_mock = global_container_stack_mock.return_value
 
         cluster1, cluster2 = self.clusters_response["data"]
         network_key = cluster2["host_name"] + ".ultimaker.local"
         active_machine_mock.getMetaDataEntry.side_effect = {"um_network_key": network_key}.get
 
-        self._loadData(network_mock)
+        self._loadData()
-        self.network.flushReplies()
 
         self.assertFalse(self.app.getOutputDeviceManager().getOutputDevice(cluster1["cluster_id"]).isConnected())
         self.assertTrue(self.app.getOutputDeviceManager().getOutputDevice(cluster2["cluster_id"]).isConnected())
 
         active_machine_mock.setMetaDataEntry.assert_called_with("um_cloud_cluster_id", cluster2["cluster_id"])
 
-    @patch("UM.Message.Message.show")
+    @patch("src.Cloud.CloudOutputDeviceManager.Message")
-    def test_api_error(self, message_mock, network_mock):
+    def test_api_error(self, message_mock):
         self.clusters_response = {
             "errors": [{"id": "notFound", "title": "Not found!", "http_status": "404", "code": "notFound"}]
         }
         self.network.prepareReply("GET", self.URL, 200, self.clusters_response)
-        self._loadData(network_mock)
+        self._loadData()
-        self.network.flushReplies()
+        message_mock.assert_called_once_with(text='Not found!', title='Error', lifetime=10, dismissable=True)
-        message_mock.assert_called_once_with()
+        message_mock.return_value.show.assert_called_once_with()