user-catalogue, improvements now that github has it's own catalogue (#…

…397)

* cli, refresh-user-catalogue will only attempt an exhanustive lookup if addon not found in full catalogue.
  full catalogue includes the github catalogue these days.
* updated CHANGELOG

CHANGELOG.md

@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+* refreshing the user-catalogue now checks imported/starred addons against the full catalogue before checking online.
+    - if it fails to find addon in catalogue, it will fall back to checking online like before.
+    - the user-catalogue is ostensibly for addons without a catalogue (github, gitlab) but is now also for 'starred' addons. Now that we have a github catalogue, attempting to refresh addons from the catalogue is much faster.
 * menu labels for the installed addons table columns have been tweaked (see `View` -> `Columns`)
     - "installed" is now "installed version"
     - "available" is now "available version"

TODO.md

@@ -28,23 +28,51 @@ see CHANGELOG.md for a more formal list of changes by release
     - done, via favouriting, but! it's not available on the installed addon pane page
     - done
 
-## todo
-
-* user catalogue, what is happening now that regular, non-github, addons can be favourited?
+* user catalogue, what is happening now that regular non-github addons can be favourited?
     - do they need to have their details refreshed?
+        - yes, they are copies from whatever catalogue they came from.
     - also, we have a github catalogue now, I bet the majority of these updates can be pulled directly from catalogues.
-        - it looks like refreshing the user catalogue is actually finding the addon in the catalogue, expanding it and then installing it
-            - I don't think it should be installed!
-                - at least, finding and installing should be separate steps
-                    - for example, import-addon should find-addon then install-addon
-                    - and refresh-user-catalogue-item should be find-addon and then update-catalogue
-                        - and doesn't the catalogue it's looking in already contain the user-catalogue?
+        - import-addon will
+            - calls find-addon 
+                - for github, gitlab and curseforge addons
+                    - this involves calls to various apis and checks and things
+                - otherwise, looks up the addon in the catalogue
+                - calls expand-summary
+                - skips the optional dry-run installation
+            - calls expand-summary 
+                - (again)
+            - calls add-addon to add addon to user-catalogue ..
+            - writes user-catalogue to disk
+            - forces an update of the :db by setting it to nil
+
+        - refresh-user-catalogue
+            - calls refresh-user-catalogue-item on each item:
+                - calls find-addon
+                    - for github, gitlab and curseforge addons
+                        - this involves calls to various apis and checks and things
+                    - otherwise, looks up the addon in the catalogue
+                    - calls expand-summary
+                    - skips the optional dry-run installation
+                - calls add-addon to add addon to user-catalogue ..
+            - writes user-catalogue to disk
+
+        - we want to:
+            - check the catalogue for the addon first
+                - I think find-addon should be skipped as we already have the addon as an addon-summary with a url, source, source-id, etc.
+            - if not found in catalogue ... ? 
+                - import it with find-addon?
+    - done
+
+## todo
 
 * user catalogue, schedule refreshes
     - ensure the user catalogue doesn't get too stale and perform an update in the background if it looks like it's very old
         - update README
     - perhaps a preference?
 
+* display github requests remaining
+    - ...
+
 * user catalogue, refreshing may guarantee exceeding github limit.
     - if we know this, add a warning? refuse?
 
@@ -60,9 +88,6 @@ see CHANGELOG.md for a more formal list of changes by release
     - ...?
     - a button on the opposite of the log popup button but similar that will show some overall stats
 
-* display github requests remaining
-    - ...
-
 * gui, raw data, 'key' column too small for 'supported-game-tracks'
 
 * update screenshots

src/strongbox/catalogue.clj

@@ -84,7 +84,8 @@
 ;;
 
 (defn-spec toc2summary (s/nilable :addon/summary)
-  "accepts toc or toc+nfo data and emits a version of the data that validates as an `:addon/summary`"
+  "accepts toc or toc+nfo data and emits a version of the data that validates as an `:addon/summary`.
+  used as a last ditch effort to match installed addons against the catalogue by guessing a bunch of parameters."
   [toc (s/or :just-toc :addon/toc, :mixed :addon/toc+nfo)]
   (when-not (:ignore? toc)
     (let [sink nil

src/strongbox/cli.clj

@@ -706,33 +706,38 @@
 
 (defn-spec refresh-user-catalogue-item nil?
   "refresh the details of an individual `addon` in the user catalogue, optionally writing the updated catalogue to file."
-  ([addon :addon/summary]
-   (refresh-user-catalogue-item addon true))
-  ([addon :addon/summary, write? boolean?]
-   (logging/with-addon addon
-     (info "refreshing details")
-     (try
-       (let [attempt-dry-run? false
-             refreshed-addon (find-addon (:url addon) attempt-dry-run?)]
-         (if refreshed-addon
-           (do (core/add-user-addon! refreshed-addon)
-               (when write?
-                 (core/write-user-catalogue!))
-               (info "... done!"))
-           (warn "failed to refresh catalogue entry")))
-       (catch Exception e
-         (error (format "an unexpected error happened while updating the details for '%s' in the user-catalogue: %s"
-                        (:name addon) (.getMessage e))))))))
+  [addon :addon/summary, db :addon/summary-list]
+  (logging/with-addon addon
+    (info "refreshing user-catalogue addon:" (:name addon))
+    (try
+      (let [{:keys [source source-id url]} addon
+            refreshed-addon (core/db-addon-by-source-and-source-id db source source-id)
+            attempt-dry-run? false
+            refreshed-addon (or refreshed-addon
+                                (find-addon url attempt-dry-run?))]
+        (if-not refreshed-addon
+          (warn (format "failed to refresh details of addon in user-catalogue: couldn't find addon '%s' in catalogue or online"
+                        (:name addon)))
+          (core/add-user-addon! refreshed-addon)))
+
+      (catch Exception e
+        (error (format "an unexpected error happened while updating the details for '%s' in the user-catalogue: %s"
+                       (:name addon) (.getMessage e)))))))
 
 (defn-spec refresh-user-catalogue nil?
   "refresh the details of all addons in the user catalogue, writing the updated catalogue to file once."
   []
   (binding [http/*cache* (core/cache)]
-    (info (format "refreshing \"%s\", this may take a minute ..." (-> (core/paths :user-catalogue-file) fs/base-name)))
-    (let [write? false]
+    (let [path (fs/base-name (core/paths :user-catalogue-file)) ;; "user-catalogue.json"
+          ;; we can't assume the full-catalogue is available.
+          _ (core/download-catalogue (core/get-catalogue-location :full))
+          db (catalogue/read-catalogue (core/catalogue-local-path :full))
+          full-catalogue (or (:addon-summary-list db) [])]
+      (info (format "refreshing \"%s\", this may take a minute ..." path))
       (doseq [user-addon (core/get-state :user-catalogue :addon-summary-list)]
-        (refresh-user-catalogue-item user-addon write?)))
-    (core/write-user-catalogue!))
+        (refresh-user-catalogue-item user-addon full-catalogue))
+      (core/write-user-catalogue!)
+      (info (format "\"%s\" has been refreshed" path))))
   nil)
 
 ;;
@@ -824,11 +829,10 @@
   (logging/with-addon addon
     (if-not (s/valid? :addon/source-map addon)
       (error "failed to star addon, it is missing some basic information ('source' and 'source-id').")
-      (let [catalogue-addon (first
-                             (core/db-addon-by-source-and-source-id
-                              (core/get-state :db) (:source addon) (:source-id addon)))]
+      (let [catalogue-addon (core/db-addon-by-source-and-source-id
+                             (core/get-state :db) (:source addon) (:source-id addon))]
         (if-not (s/valid? :addon/summary catalogue-addon)
-          (error "failed to star addon, it isn't matched to the catalogue yet.")
+          (error "failed to star addon, it is not matched to the catalogue.")
           (add-summary-to-user-catalogue catalogue-addon)))))
   nil)
 

src/strongbox/core.clj

@@ -838,12 +838,13 @@
       (or (-find-first-in-db db installed-addon match-on-list)
           installed-addon))))
 
-(defn-spec db-addon-by-source-and-source-id :addon/summary-list
-  "returns a list of addon summaries from `db` whose source and source-id exactly match the given `source` and `source-id`."
+(defn-spec db-addon-by-source-and-source-id (s/nilable :addon/summary)
+  "returns the first addon summary from `db` whose source and source-id exactly match the given `source` and `source-id`.
+  there should only ever be one or zero such addons."
   [db :addon/summary-list, source :addon/source, source-id :addon/source-id]
   (let [xf (filter #(and (= source (:source %))
                          (= source-id (:source-id %))))]
-    (into [] xf db)))
+    (first (into [] xf db))))
 
 (defn-spec db-addon-by-source-and-name :addon/summary-list
   "returns a list of addon summaries from `db` whose source and name exactly match the given `source` and `name`."

test/strongbox/cli_test.clj

@@ -634,16 +634,20 @@
 
 ;; todo: no import-addon-gitlab ?
 
-(deftest refresh-user-catalogue
+(deftest refresh-user-catalogue--not-in-catalogue
   (testing "the user catalogue can be 'refreshed', pulling in updated information from github and the current catalogue"
     (with-running-app+opts {:ui nil}
       (let [;; user-catalogue with a bunch of addons across all hosts that the user has added.
             user-catalogue-fixture (fixture-path "user-catalogue--populated.json")
 
-            ;; default app catalogue, contains newer versions of the addon summaries in the user-catalogue.
-            ;; this is because the catalogue is updated periodically and the user-catalogue is not.
+            ;; default app catalogue. this is what the user should have selected before and after refresh,
+            ;; despite using the 'full' catalogue for the refresh.
             short-catalogue (slurp (fixture-path "user-catalogue--short-catalogue.json"))
 
+            ;; full app catalogue, contains newer versions of the addon summaries in the user-catalogue.
+            ;; this is because regular catalogues are updated periodically and the user-catalogue is not.
+            full-catalogue short-catalogue
+
             tukui-fixture (slurp (fixture-path "user-catalogue--tukui.json"))
             tukui-classic-fixture (slurp (fixture-path "user-catalogue--tukui-classic.json"))
             tukui-classic-tbc-fixture (slurp (fixture-path "user-catalogue--tukui-classic-tbc.json"))
@@ -659,7 +663,10 @@
             gitlab-blob-fixture (slurp (fixture-path "user-catalogue--gitlab-blob.json"))
             gitlab-releases-fixture (slurp (fixture-path "user-catalogue--gitlab-releases.json"))
 
-            fake-routes {"https://raw.githubusercontent.com/ogri-la/strongbox-catalogue/master/short-catalogue.json"
+            fake-routes {"https://raw.githubusercontent.com/ogri-la/strongbox-catalogue/master/full-catalogue.json"
+                         {:get (fn [req] {:status 200 :body full-catalogue})}
+
+                         "https://raw.githubusercontent.com/ogri-la/strongbox-catalogue/master/short-catalogue.json"
                          {:get (fn [req] {:status 200 :body short-catalogue})}
 
                          "https://www.tukui.org/api.php?addons"
@@ -713,6 +720,9 @@
           ;; sanity check, ensure user-catalogue loaded
           (is (= expected-num (count (core/get-state :db))))
 
+          ;; ensure default short-catalogue is being used
+          (is (= :short (:name (core/current-catalogue))))
+
           ;; we need to load the short-catalogue using newer versions of what is in the user-catalogue
           ;; the user-catalogue is then matched against db, the newer summary returned and written to the user catalogue
 
@@ -726,30 +736,36 @@
                                             (assoc :datestamp today))]
             (cli/refresh-user-catalogue)
             ;; ensure new user-catalogue matches expectations
-            (is (= expected-user-catalogue (core/get-user-catalogue)))))))))
+            (is (= expected-user-catalogue (core/get-user-catalogue)))
+
+            ;; ensure the selected catalogue hasn't changed despite downloading the full catalogue
+            (is (= :short (:name (core/current-catalogue))))))))))
 
 (deftest refresh-user-catalogue-item
   (testing "individual addons can be refreshed, writing the changes to disk afterwards."
     (let [user-catalogue (catalogue/new-catalogue [helper/addon-summary])
           new-addon (merge helper/addon-summary {:updated-date "2022-02-02T02:02:02"})
-          expected (assoc user-catalogue :addon-summary-list [new-addon])]
+          expected (assoc user-catalogue :addon-summary-list [new-addon])
+          db []]
       (with-running-app
         (swap! core/state assoc :user-catalogue user-catalogue)
         (core/write-user-catalogue!)
         (with-redefs [cli/find-addon (fn [& args] new-addon)]
-          (cli/refresh-user-catalogue-item helper/addon-summary))
+          (cli/refresh-user-catalogue-item helper/addon-summary db))
         (is (= expected (core/get-state :user-catalogue)))))))
 
 (deftest refresh-user-catalogue-item--no-catalogue
   (testing "looking for an addon that doesn't exist in the catalogue isn't a total failure"
     (with-running-app
-      (is (nil? (cli/refresh-user-catalogue-item helper/addon-summary))))))
+      (let [db []]
+        (is (nil? (cli/refresh-user-catalogue-item helper/addon-summary db)))))))
 
 (deftest refresh-user-catalogue-item--unhandled-exception
   (testing "unhandled exceptions while refreshing a user-catalogue item isn't a total failure"
     (with-running-app
       (with-redefs [cli/find-addon (fn [& args] (throw (Exception. "catastrophe!")))]
-        (is (nil? (cli/refresh-user-catalogue-item helper/addon-summary)))))))
+        (let [db []]
+          (is (nil? (cli/refresh-user-catalogue-item helper/addon-summary db))))))))
 
 ;;
 

test/strongbox/core_test.clj

@@ -1869,7 +1869,8 @@
       (is (= expected (core/db-match-installed-addon-list-with-catalogue db installed-addon-list))))))
 
 (deftest db-addon-by-source-and-source-id
-  (let [db [helper/addon-summary]
+  (let [expected helper/addon-summary
+        db [helper/addon-summary]
         {:keys [source source-id]} helper/addon-summary]
-    (is (= db (core/db-addon-by-source-and-source-id db source source-id)))
-    (is (= [] (core/db-addon-by-source-and-source-id db "wowinterface" "foo")))))
+    (is (= expected (core/db-addon-by-source-and-source-id db source source-id)))
+    (is (nil? (core/db-addon-by-source-and-source-id db "wowinterface" "foo")))))