From 1c5077bb93d6a573eee3ac10a8fa69c8f7c443c0 Mon Sep 17 00:00:00 2001 From: torkus <48790775+torkus@users.noreply.github.com> Date: Sun, 23 Apr 2023 13:08:12 +0930 Subject: [PATCH] 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 | 3 ++ TODO.md | 49 ++++++++++++++++++++++++-------- src/strongbox/catalogue.clj | 3 +- src/strongbox/cli.clj | 54 +++++++++++++++++++----------------- src/strongbox/core.clj | 7 +++-- test/strongbox/cli_test.clj | 34 +++++++++++++++++------ test/strongbox/core_test.clj | 7 +++-- 7 files changed, 104 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7c5335b..e2407206 100644 --- a/CHANGELOG.md +++ b/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" diff --git a/TODO.md b/TODO.md index 7529a24f..27ecc0f7 100644 --- a/TODO.md +++ b/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 diff --git a/src/strongbox/catalogue.clj b/src/strongbox/catalogue.clj index 70028cfa..31520aff 100644 --- a/src/strongbox/catalogue.clj +++ b/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 diff --git a/src/strongbox/cli.clj b/src/strongbox/cli.clj index 97af7e6c..bf2067b0 100644 --- a/src/strongbox/cli.clj +++ b/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) diff --git a/src/strongbox/core.clj b/src/strongbox/core.clj index 7ea1748d..a33fe009 100644 --- a/src/strongbox/core.clj +++ b/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`." diff --git a/test/strongbox/cli_test.clj b/test/strongbox/cli_test.clj index 40e6f100..46b972cf 100644 --- a/test/strongbox/cli_test.clj +++ b/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)))))))) ;; diff --git a/test/strongbox/core_test.clj b/test/strongbox/core_test.clj index cf987235..410af24c 100644 --- a/test/strongbox/core_test.clj +++ b/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")))))