Fix for Thingiverse file downloading (#169)

* Update logging * Ignore install.sh * Handle download failures * Try all the different URLs, it's a mess * Refactor how file downloading works * Add future FIXME * Fix myminifactory downloading * Fixes * Download methods are now the same * Refactor handling of redirects
ChrisTerBeke · Dec 29, 2022 · fc2c192 · fc2c192
1 parent 645c36f
commit fc2c192
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 84 deletions.
diff --git a/.gitignore b/.gitignore
@@ -28,3 +28,6 @@ Thumbs.db
 
 # Build artifacts
 build
+
+# Local install
+install.sh
diff --git a/ThingiBrowser/ThingiBrowserService.py b/ThingiBrowser/ThingiBrowserService.py
@@ -379,18 +379,19 @@ def hideThingDetails(self) -> None:
         self._thing_details = None
         self.activeThingChanged.emit()
 
-    @pyqtSlot(int, str, name="downloadThingFile")
-    def downloadThingFile(self, file_id: int, file_name: str) -> None:
+    @pyqtSlot(str, str, name="downloadThingFile")
+    def downloadThingFile(self, download_url: str, file_name: str) -> None:
         """
         Download and load a thing file by it's ID.
         The downloaded object will be placed on the build plate.
-        :param file_id: The ID of the file.
-        :param file_name: The name of the file.
+        :param download_url: The direct download URL of the file.
+        :param file_name: The name of the file to display after loading it on the build plate.
         """
         self._is_downloading = True
         self.downloadingStateChanged.emit()
-        self._getActiveDriver().downloadThingFile(file_id, file_name,
-                                                  on_finished=lambda data: self._onDownloadFinished(data, file_name))
+        self._getActiveDriver().downloadThingFile(download_url,
+                                                  on_finished=lambda data: self._onDownloadFinished(data, file_name),
+                                                  on_failed=self._onRequestFailed)
 
     @pyqtProperty(int, notify=thingsChanged)
     def currentPage(self) -> int:
@@ -533,6 +534,8 @@ def _onRequestFailed(self, error: Optional[ApiError] = None, status_code: Option
         """
         self._is_querying = False
         self.queryingStateChanged.emit()
+        self._is_downloading = False
+        self.downloadingStateChanged.emit()
         if status_code in [401, 502]:  # Thingiverse uses 502 for certain authentication errors
             self._showAuthenticationError()
         else:

diff --git a/ThingiBrowser/api/AbstractApiClient.py b/ThingiBrowser/api/AbstractApiClient.py
@@ -132,16 +132,6 @@ def getThingFiles(self, thing_id: int, on_finished: Callable[[List[ThingFile]],
         """
         raise NotImplementedError("getThingFiles must be implemented")
 
-    @abstractmethod
-    def downloadThingFile(self, file_id: int, file_name: str, on_finished: Callable[[bytes], Any]) -> None:
-        """
-        Download a thing file by its ID.
-        :param file_id: The file ID to download.
-        :param file_name: The file's name including extension.
-        :param on_finished: Callback method to receive the async result on as bytes.
-        """
-        raise NotImplementedError("downloadThingFile must be implemented")
-
     @abstractmethod
     def getThings(self, query: str, page: int, on_finished: Callable[[List[Thing]], Any],
                   on_failed: Optional[Callable[[Optional[ApiError],Optional[int]], Any]] = None) -> None:
@@ -154,6 +144,17 @@ def getThings(self, query: str, page: int, on_finished: Callable[[List[Thing]],
         """
         raise NotImplementedError("get must be implemented")
 
+    def downloadThingFile(self, download_url: str, on_finished: Callable[[bytes], Any],
+                          on_failed: Optional[Callable[[Optional[ApiError], Optional[int]], Any]] = None) -> None:
+        """
+        Download a thing file by its ID.
+        :param file_id: The file ID to download.
+        :param file_name: The file's name including extension.
+        :param on_finished: Callback method to receive the async result on as bytes.
+        """
+        reply = self._manager.get(self._createEmptyRequest(download_url))
+        self._addCallback(reply, on_finished, on_failed, parser=ApiHelper.parseReplyAsBytes)
+
     @property
     @abstractmethod
     def _root_url(self) -> str:
@@ -164,7 +165,7 @@ def _root_url(self) -> str:
         raise NotImplementedError("_root_url must be implemented")
 
     @abstractmethod
-    def _setAuth(self, request: QNetworkRequest):
+    def _setAuth(self, request: QNetworkRequest) -> QNetworkRequest:
         """
         Get the API authentication method for this provider.
         """
@@ -179,9 +180,8 @@ def _createEmptyRequest(self, url: str, content_type: str = "application/json")
         """
         request = QNetworkRequest(QUrl(url))
         request.setHeader(QNetworkRequest.KnownHeaders.ContentTypeHeader, content_type)
-        request.setAttribute(QNetworkRequest.Attribute.RedirectPolicyAttribute, True)  # file downloads reply with a 302 first
-        self._setAuth(request)
-        return request
+        request.setAttribute(QNetworkRequest.Attribute.RedirectPolicyAttribute, QNetworkRequest.RedirectPolicy.ManualRedirectPolicy)
+        return self._setAuth(request)
 
     def _addCallback(self, reply: QNetworkReply,
                      on_finished: Callable[[Any], Any],
@@ -205,6 +205,9 @@ def parse() -> None:
                     if isinstance(response, dict):
                         error_response = ApiError(response)
                     on_failed(error_response, status_code)
+            elif status_code in [301, 302]:
+                redirect = self._manager.get(QNetworkRequest(QUrl(reply.header(QNetworkRequest.KnownHeaders.LocationHeader))))
+                self._addCallback(redirect, on_finished, on_failed, parser)
             else:
                 on_finished(response)
             reply.deleteLater()

diff --git a/ThingiBrowser/api/Analytics.py b/ThingiBrowser/api/Analytics.py
@@ -44,4 +44,4 @@ def _send(self, data: Dict[str, Any]):
         try:
             requests.post(url, headers=headers)
         except Exception as err:
-            Logger.log("w", "Could not call Analytics API: %s", err)
+            Logger.warning("Could not call Analytics API: {}".format(err))
diff --git a/ThingiBrowser/api/ApiHelper.py b/ThingiBrowser/api/ApiHelper.py
@@ -25,7 +25,7 @@ def parseReplyAsJson(cls, reply: QNetworkReply
             response = reply.readAll().data().decode()
             return status_code, json.loads(response)
         except (UnicodeDecodeError, JSONDecodeError, ValueError) as err:
-            Logger.log("e", "Could not parse the API response: %s", err)
+            Logger.error("Could not parse the API response: {}".format(err))
             return status_code, None
 
     @classmethod

diff --git a/ThingiBrowser/api/JsonObject.py b/ThingiBrowser/api/JsonObject.py
@@ -56,7 +56,7 @@ def __init__(self, _dict: Dict[str, Any]):
         self.id = None
         self.thumbnail = None
         self.name = None
-        self.url = None
+        self.download_url = None
         super().__init__(_dict)
 
 

diff --git a/ThingiBrowser/drivers/myminifactory/MyMiniFactoryApiClient.py b/ThingiBrowser/drivers/myminifactory/MyMiniFactoryApiClient.py
@@ -23,7 +23,6 @@ def __init__(self) -> None:
         self._auth_state: Optional[str] = None
         access_token = PreferencesHelper.initSetting(Settings.MYMINIFACTORY_API_TOKEN_KEY)
         if access_token and access_token != "":
-            # Get the username if we already have a token stored.
             self._getUserData()
         super().__init__()
 
@@ -53,8 +52,8 @@ def _onTokenReceived(self, state: str, token: Optional[str] = None) -> None:
     def _getUserData(self) -> None:
         url = "{}/user".format(self._root_url)
         reply = self._manager.get(self._createEmptyRequest(url))
-        self._addCallback(reply, self._onGetUserData, parser=self._parseGetUserData)
-        # TODO: handle error response
+        self._addCallback(reply, on_finished=self._onGetUserData, parser=self._parseGetUserData)
+        # FIXME: handle error response
 
     @staticmethod
     def _parseGetUserData(reply: QNetworkReply) -> Tuple[int, Optional[UserData]]:
@@ -130,26 +129,6 @@ def _parseGetCollections(reply: QNetworkReply) -> Tuple[int, Optional[List[Colle
             "description": item.get("description")
         }) for item in items]
 
-    def getThingFiles(self, thing_id: int, on_finished: Callable[[List[ThingFile]], Any],
-                      on_failed: Optional[Callable[[Optional[ApiError], Optional[int]], Any]] = None) -> None:
-        url = "{}/objects/{}".format(self._root_url, thing_id)
-        reply = self._manager.get(self._createEmptyRequest(url))
-        self._addCallback(reply, on_finished, on_failed, parser=self._parseGetThingFiles)
-
-    @staticmethod
-    def _parseGetThingFiles(reply: QNetworkReply) -> Tuple[int, Optional[List[ThingFile]]]:
-        status_code, response = ApiHelper.parseReplyAsJson(reply)
-        if not response or not isinstance(response, dict):
-            return status_code, None
-        file_id = response.get("id")
-        items = response.get("files", {}).get("items")
-        return status_code, [ThingFile({
-            "id": file_id,
-            "thumbnail": item.get("thumbnail_url"),
-            "name": item.get("filename"),
-            "url": None,
-        }) for item in items]
-
     def getThings(self, query: str, page: int, on_finished: Callable[[List[Thing]], Any],
                   on_failed: Optional[Callable[[Optional[ApiError], Optional[int]], Any]] = None) -> None:
         operator = "&" if query.find("?") > 0 else "?"
@@ -168,29 +147,39 @@ def _parseGetThings(reply: QNetworkReply) -> Tuple[int, Optional[List[Thing]]]:
             "thumbnail": item.get("images", [])[0].get("thumbnail", {}).get("url") if item.get("images") else None,
             "name": item.get("name"),
             "url": item.get("url"),
-            "description": item.get("description")
+            "description": item.get("description"),
         }) for item in items]
 
-    def downloadThingFile(self, file_id: int, file_name: str, on_finished: Callable[[bytes], Any]) -> None:
-        url = "https://www.myminifactory.com/download/{}?downloadfile={}".format(file_id, file_name)
+    def getThingFiles(self, thing_id: int, on_finished: Callable[[List[ThingFile]], Any],
+                      on_failed: Optional[Callable[[Optional[ApiError], Optional[int]], Any]] = None) -> None:
+        url = "{}/objects/{}".format(self._root_url, thing_id)
         reply = self._manager.get(self._createEmptyRequest(url))
-        self._addCallback(reply, on_finished, parser=ApiHelper.parseReplyAsBytes)
+        self._addCallback(reply, on_finished, on_failed, parser=self._parseGetThingFiles)
+
+    @staticmethod
+    def _parseGetThingFiles(reply: QNetworkReply) -> Tuple[int, Optional[List[ThingFile]]]:
+        status_code, response = ApiHelper.parseReplyAsJson(reply)
+        if not response or not isinstance(response, dict):
+            return status_code, None
+        items = response.get("files", {}).get("items")
+        return status_code, [ThingFile({
+            "id": item.get("id"),
+            "name": item.get("filename"),
+            "thumbnail": item.get("thumbnail_url"),
+            "download_url": item.get("download_url"),
+        }) for item in items]
 
     @property
     def _root_url(self):
         return "https://www.myminifactory.com/api/v2"
 
-    def _setAuth(self, request: QNetworkRequest) -> None:
+    def _setAuth(self, request: QNetworkRequest) -> QNetworkRequest:
         token = PreferencesHelper.getSettingValue(Settings.MYMINIFACTORY_API_TOKEN_KEY)
         if not token or token == "":
-            # If the user was not signed in we use a default token for the public endpoints.
-            # We'll use the 'old way' of injecting the API key in the request
-            return self._injectApiToken(request)
-        request.setRawHeader(self._strToByteArray("Authorization"), self._strToByteArray("Bearer {}".format(token)))
-
-    @staticmethod
-    def _injectApiToken(request: QNetworkRequest) -> None:
-        current_url = request.url().toString()
-        operator = "&" if current_url.find("?") > 0 else "?"
-        new_url = QUrl("{}{}key={}".format(current_url, operator, Settings.MYMINIFACTORY_API_TOKEN))
-        request.setUrl(new_url)
+            current_url = request.url().toString()
+            operator = "&" if current_url.find("?") > 0 else "?"
+            new_url = QUrl("{}{}key={}".format(current_url, operator, Settings.MYMINIFACTORY_API_TOKEN))
+            request.setUrl(new_url)
+        else:
+            request.setRawHeader(self._strToByteArray("Authorization"), self._strToByteArray("Bearer {}".format(token)))
+        return request
diff --git a/ThingiBrowser/drivers/thingiverse/ThingiverseApiClient.py b/ThingiBrowser/drivers/thingiverse/ThingiverseApiClient.py
@@ -153,26 +153,23 @@ def _parseGetThingFiles(reply: QNetworkReply) -> Tuple[int, Optional[List[ThingF
         status_code, response = ApiHelper.parseReplyAsJson(reply)
         if not response or not isinstance(response, list):
             return status_code, None
+        # FIXME: use `item.get(download_url)` to track downloads when authenticated (allows for 'download history' feature)
         return status_code, [ThingFile({
             "id": item.get("id"),
-            "thumbnail": item.get("thumbnail"),
             "name": item.get("name"),
-            "url": item.get("public_url") or item.get("url"),
+            "thumbnail": item.get("thumbnail"),
+            "download_url": item.get("direct_url"),
         }) for item in response]
 
-    def downloadThingFile(self, file_id: int, file_name: str, on_finished: Callable[[bytes], Any]) -> None:
-        url = "{}/files/{}/download".format(self._root_url, file_id)
-        reply = self._manager.get(self._createEmptyRequest(url))
-        self._addCallback(reply, on_finished, parser=ApiHelper.parseReplyAsBytes)
-
     @property
     def _root_url(self):
         return "https://api.thingiverse.com"
 
-    def _setAuth(self, request: QNetworkRequest) -> None:
+    def _setAuth(self, request: QNetworkRequest) -> QNetworkRequest:
         # FIXME: Waiting for Thingiverse app approval
         # token = PreferencesHelper.getSettingValue(Settings.THINGIVERSE_API_TOKEN_KEY)
         # if not token or token == "":
         #     # If the user was not signed in we use a default token for the public endpoints.
         #     token = Settings.THINGIVERSE_API_TOKEN
         request.setRawHeader(self._strToByteArray("Authorization"), self._strToByteArray("Bearer {}".format(Settings.THINGIVERSE_API_TOKEN)))
+        return request
diff --git a/views/ThingDetailsPage.qml b/views/ThingDetailsPage.qml
@@ -7,14 +7,9 @@ import UM 1.1 as UM
 
 ColumnLayout {
     id: detailsPage
-
-    // the active thing
     property var thing
-
-    // the files for the active thing
     property var thingFiles
 
-    // button to navigate back to the search results page
     EnhancedButton {
         text: "Back to results"
         Layout.leftMargin: 20
@@ -26,7 +21,6 @@ ColumnLayout {
         }
     }
 
-    // name
     Label {
         id: thingTitle
         text: thing && thing.name ? thing.name : ""
@@ -40,7 +34,6 @@ ColumnLayout {
         Layout.rightMargin: 20
     }
 
-    // link to web page
     Link {
         text: thing && thing.url ? thing.url : ""
         url: thing && thing.url ? thing.url : ""
@@ -49,7 +42,6 @@ ColumnLayout {
         Layout.bottomMargin: 20
     }
 
-    // description
     Label {
         id: thingDescription
         text: thing && thing.description ? thing.description : ""

diff --git a/views/ThingFilesListItem.qml b/views/ThingFilesListItem.qml
@@ -15,7 +15,6 @@ Item {
         spacing: 10
         width: parent.width
 
-        // thumbnail (forced to 75x75)
         Image {
             Layout.preferredWidth: 75
             Layout.preferredHeight: 75
@@ -25,7 +24,6 @@ Item {
             sourceSize.height: 75
         }
 
-         // file name
         Label {
             text: thingFile.name
             color: UM.Theme.getColor("text")
@@ -35,17 +33,26 @@ Item {
             Layout.fillWidth: true
         }
 
-        // download button
         EnhancedButton {
             text: "Add to build plate"
-            visible: !ThingiService.isDownloading
+            enabled: !ThingiService.isDownloading && thingFile.download_url
+            // FIXME: automatically update after signing in
+
             onClicked: {
-                ThingiService.downloadThingFile(thingFile.id, thingFile.name)
+                ThingiService.downloadThingFile(thingFile.download_url, thingFile.name)
                 Analytics.trackEvent("add_to_build_plate", "button_clicked")
             }
         }
 
-        // loading spinner
+        Label {
+            text: "Please sign in to download files"
+            visible: !thingFile.download_url
+            color: UM.Theme.getColor("text")
+            font: UM.Theme.getFont("large")
+            elide: Text.ElideRight
+            renderType: Text.NativeRendering
+        }
+
         AnimatedImage {
             visible: ThingiService.isDownloading
             source: "images/loading.gif"