From ad546f9f144b604a41de045dcb2c06d727951803 Mon Sep 17 00:00:00 2001 From: Ulises Manuel <90291778+ulisesmac@users.noreply.github.com> Date: Thu, 7 Mar 2024 12:05:45 -0600 Subject: [PATCH] [#18608] Immprove collectibles fetching performance (#18921) * Add memoized versions to convert keys * Add placeholder for SVG collectibles due to errors and warnings * Add events to request collectibles per account * Update subs to list all accounts collectibles evenly * Update collectibles tab to pull new data when end is reached * Use memoized version of key transformation --- .../components/profile/collectible/view.cljs | 29 ++- .../contexts/wallet/account/tabs/view.cljs | 4 +- .../wallet/common/collectibles_tab/view.cljs | 23 +-- src/status_im/contexts/wallet/data_store.cljs | 8 +- src/status_im/contexts/wallet/events.cljs | 2 +- .../contexts/wallet/events/collectibles.cljs | 168 ++++++++++++------ .../contexts/wallet/events_test.cljs | 34 ++-- .../contexts/wallet/home/tabs/view.cljs | 5 +- .../contexts/wallet/send/events.cljs | 8 +- .../wallet/send/select_asset/view.cljs | 1 + src/status_im/subs/wallet/collectibles.cljs | 20 ++- src/utils/transforms.cljs | 4 + 12 files changed, 192 insertions(+), 114 deletions(-) diff --git a/src/quo/components/profile/collectible/view.cljs b/src/quo/components/profile/collectible/view.cljs index a96facb5b1..2075c20ff9 100644 --- a/src/quo/components/profile/collectible/view.cljs +++ b/src/quo/components/profile/collectible/view.cljs @@ -1,9 +1,9 @@ (ns quo.components.profile.collectible.view (:require + [clojure.string :as string] [quo.components.markdown.text :as text] [quo.components.profile.collectible.style :as style] - [react-native.core :as rn] - [react-native.svg :as svg])) + [react-native.core :as rn])) (defn remaining-tiles [amount] @@ -19,11 +19,28 @@ (let [svg? (and (map? resource) (:svg? resource)) image-style (style/tile-style-by-size size)] [rn/view {:style style} - (if svg? + (cond + svg? [rn/view - {:style {:border-radius (:border-radius image-style) - :overflow :hidden}} - [svg/svg-uri (assoc image-style :uri (:uri resource))]] + {:style (assoc image-style + :border-radius (:border-radius image-style) + :overflow :hidden + :justify-content :center + :align-items :center + :background-color :lightblue)} + [text/text "SVG Content"]] + + (or (string/blank? resource) (string/blank? (:uri resource))) + [rn/view + {:style (assoc image-style + :border-radius (:border-radius image-style) + :overflow :hidden + :justify-content :center + :align-items :center + :background-color :lightgray)} + [text/text "Missing image"]] + + :else ;; NOTE: using react-native-fast-image here causes a crash on devices when used inside a ;; large flatlist. The library seems to have issues with memory consumption when used with ;; large images/GIFs. diff --git a/src/status_im/contexts/wallet/account/tabs/view.cljs b/src/status_im/contexts/wallet/account/tabs/view.cljs index 194a0a9bfd..6c01bca1ff 100644 --- a/src/status_im/contexts/wallet/account/tabs/view.cljs +++ b/src/status_im/contexts/wallet/account/tabs/view.cljs @@ -17,7 +17,9 @@ (case selected-tab :assets [assets/view] :collectibles [collectibles/view - {:collectibles collectible-list + {:collectibles collectible-list + :on-end-reached #(rf/dispatch + [:wallet/request-collectibles-for-current-viewing-account]) :on-collectible-press (fn [{:keys [id]}] (rf/dispatch [:wallet/get-collectible-details id]))}] :activity [activity/view] diff --git a/src/status_im/contexts/wallet/common/collectibles_tab/view.cljs b/src/status_im/contexts/wallet/common/collectibles_tab/view.cljs index 222e5c24f1..1852d26303 100644 --- a/src/status_im/contexts/wallet/common/collectibles_tab/view.cljs +++ b/src/status_im/contexts/wallet/common/collectibles_tab/view.cljs @@ -8,7 +8,7 @@ [utils.i18n :as i18n])) (defn- view-internal - [{:keys [theme collectibles filtered? on-collectible-press]}] + [{:keys [theme collectibles filtered? on-collectible-press on-end-reached]}] (let [no-results-match-query? (and filtered? (empty? collectibles))] (cond no-results-match-query? @@ -26,14 +26,17 @@ :else [rn/flat-list - {:data collectibles - :style {:flex 1} - :content-container-style {:align-items :center} - :num-columns 2 - :render-fn (fn [{:keys [preview-url] :as collectible}] - [quo/collectible - {:images [preview-url] - :on-press #(when on-collectible-press - (on-collectible-press collectible))}])}]))) + {:data collectibles + :style {:flex 1} + :content-container-style {:align-items :center} + :window-size 11 + :num-columns 2 + :render-fn (fn [{:keys [preview-url] :as collectible}] + [quo/collectible + {:images [preview-url] + :on-press #(when on-collectible-press + (on-collectible-press collectible))}]) + :on-end-reached on-end-reached + :on-end-reached-threshold 4}]))) (def view (quo.theme/with-theme view-internal)) diff --git a/src/status_im/contexts/wallet/data_store.cljs b/src/status_im/contexts/wallet/data_store.cljs index 974c8c2d02..fd5bcd0b7c 100644 --- a/src/status_im/contexts/wallet/data_store.cljs +++ b/src/status_im/contexts/wallet/data_store.cljs @@ -1,12 +1,12 @@ (ns status-im.contexts.wallet.data-store (:require - [camel-snake-kebab.core :as csk] [camel-snake-kebab.extras :as cske] [clojure.set :as set] [clojure.string :as string] [status-im.constants :as constants] [utils.money :as money] - [utils.number :as utils.number])) + [utils.number :as utils.number] + [utils.transforms :as transforms])) (defn chain-ids-string->set [ids-string] @@ -78,7 +78,7 @@ [tokens] (-> tokens (update-keys name) - (update-vals #(cske/transform-keys csk/->kebab-case %)) + (update-vals #(cske/transform-keys transforms/->kebab-case-keyword %)) (update-vals remove-tokens-with-empty-values) (update-vals #(mapv rpc->balances-per-chain %)))) @@ -122,4 +122,4 @@ (defn parse-keypairs [keypairs] (let [renamed-data (rename-color-id-in-data keypairs)] - (cske/transform-keys csk/->kebab-case-keyword renamed-data))) + (cske/transform-keys transforms/->kebab-case-keyword renamed-data))) diff --git a/src/status_im/contexts/wallet/events.cljs b/src/status_im/contexts/wallet/events.cljs index 23fb7668e7..91cc479535 100644 --- a/src/status_im/contexts/wallet/events.cljs +++ b/src/status_im/contexts/wallet/events.cljs @@ -59,7 +59,7 @@ [:wallet :accounts] (utils.collection/index-by :address (data-store/rpc->accounts wallet-accounts))) :fx [[:dispatch [:wallet/get-wallet-token]] - [:dispatch [:wallet/request-collectibles {:start-at-index 0 :new-request? true}]] + [:dispatch [:wallet/request-collectibles-for-all-accounts {:new-request? true}]] (when new-account? [:dispatch [:wallet/navigate-to-new-account navigate-to-account]])]}))) diff --git a/src/status_im/contexts/wallet/events/collectibles.cljs b/src/status_im/contexts/wallet/events/collectibles.cljs index 5157896738..cef87516cc 100644 --- a/src/status_im/contexts/wallet/events/collectibles.cljs +++ b/src/status_im/contexts/wallet/events/collectibles.cljs @@ -1,11 +1,9 @@ (ns status-im.contexts.wallet.events.collectibles - (:require [camel-snake-kebab.core :as csk] - [camel-snake-kebab.extras :as cske] - [clojure.string :as string] + (:require [camel-snake-kebab.extras :as cske] [taoensso.timbre :as log] [utils.ethereum.chain :as chain] [utils.re-frame :as rf] - [utils.transforms :as types])) + [utils.transforms :as transforms])) (def collectible-data-types {:unique-id 0 @@ -20,33 +18,23 @@ :fetch-if-cache-old 3}) (def max-cache-age-seconds 3600) -(def collectibles-request-batch-size 1000) +(def collectibles-request-batch-size 25) -(defn displayable-collectible? - [collectible] - (let [{{:keys [image-url animation-url]} :collectible-data - {collection-image-url :image-url} :collection-data} collectible] - (or (not (string/blank? animation-url)) - (not (string/blank? image-url)) - (not (string/blank? collection-image-url))))) +(defn- move-collectibles-to-accounts + [accounts new-collectibles-per-account] + (reduce-kv (fn [acc account new-collectibles] + (update-in acc [account :collectibles] #(reduce conj (or % []) new-collectibles))) + accounts + new-collectibles-per-account)) -(defn- add-collectibles-to-accounts - [accounts collectibles] - (reduce (fn [acc {:keys [ownership] :as collectible}] - (->> ownership - (map :address) ; In ERC1155 tokens a collectible can be owned by multiple addresses. - (reduce (fn add-collectible-to-address [acc address] - (update-in acc [address :collectibles] conj collectible)) - acc))) - accounts - collectibles)) +(defn flush-collectibles + [{:keys [db]}] + (let [collectibles-per-account (get-in db [:wallet :ui :collectibles :fetched])] + {:db (-> db + (update-in [:wallet :ui :collectibles] dissoc :pending-requests :fetched) + (update-in [:wallet :accounts] move-collectibles-to-accounts collectibles-per-account))})) -(defn store-collectibles - [{:keys [db]} [collectibles]] - (let [displayable-collectibles (filter displayable-collectible? collectibles)] - {:db (update-in db [:wallet :accounts] add-collectibles-to-accounts displayable-collectibles)})) - -(rf/reg-event-fx :wallet/store-collectibles store-collectibles) +(rf/reg-event-fx :wallet/flush-collectibles-fetched flush-collectibles) (defn clear-stored-collectibles [{:keys [db]}] @@ -62,48 +50,110 @@ (rf/reg-event-fx :wallet/store-last-collectible-details store-last-collectible-details) (rf/reg-event-fx - :wallet/request-collectibles - (fn [{:keys [db]} [{:keys [start-at-index new-request?]}]] - (let [request-id 0 - collectibles-filter nil - data-type (collectible-data-types :header) - fetch-criteria {:fetch-type (fetch-type :fetch-if-not-cached) - :max-cache-age-seconds max-cache-age-seconds} - chain-ids (chain/chain-ids db) - request-params [request-id - chain-ids - (keys (get-in db [:wallet :accounts])) - collectibles-filter - start-at-index - collectibles-request-batch-size - data-type - fetch-criteria]] + :wallet/request-new-collectibles-for-account + (fn [{:keys [db]} [{:keys [request-id account amount]}]] + (let [current-collectible-idx (get-in db [:wallet :accounts account :current-collectible-idx] 0) + collectibles-filter nil + data-type (collectible-data-types :header) + fetch-criteria {:fetch-type (fetch-type :fetch-if-not-cached) + :max-cache-age-seconds max-cache-age-seconds} + chain-ids (chain/chain-ids db) + request-params [request-id + chain-ids + [account] + collectibles-filter + current-collectible-idx + amount + data-type + fetch-criteria]] {:fx [[:json-rpc/call [{:method "wallet_getOwnedCollectiblesAsync" :params request-params :on-error (fn [error] - (log/error "failed to request collectibles" - {:event :wallet/request-collectibles + (log/error "failed to request collectibles for account" + {:event :wallet/request-new-collectibles-for-account :error error - :params request-params}))}]] - (when new-request? - [:dispatch [:wallet/clear-stored-collectibles]])]}))) + :params request-params}))}]]]}))) + +(defonce collectibles-request-ids (atom 0)) + +(defn- get-unique-collectible-request-id + [amount] + (let [initial-id (deref collectibles-request-ids) + last-id (+ initial-id amount)] + (reset! collectibles-request-ids last-id) + (range initial-id last-id))) + +(rf/reg-event-fx + :wallet/request-collectibles-for-all-accounts + (fn [{:keys [db]} [{:keys [new-request?]}]] + (let [accounts (->> (get-in db [:wallet :accounts]) + (filter (fn [[_ {:keys [has-more-collectibles?]}]] + (or (nil? has-more-collectibles?) + (true? has-more-collectibles?)))) + (keys)) + num-accounts (count accounts) + collectibles-per-account (quot collectibles-request-batch-size num-accounts) + ;; We need to pass unique IDs for simultaneous requests, otherwise they'll fail + request-ids (get-unique-collectible-request-id num-accounts) + collectible-requests (map (fn [id account] + [:dispatch + [:wallet/request-new-collectibles-for-account + {:request-id id + :account account + :amount collectibles-per-account}]]) + request-ids + accounts)] + {:db (cond-> db + :always (assoc-in [:wallet :ui :collectibles :pending-requests] num-accounts) + new-request? (update-in [:wallet :accounts] update-vals #(dissoc % :collectibles))) + :fx collectible-requests}))) + +(rf/reg-event-fx + :wallet/request-collectibles-for-current-viewing-account + (fn [{:keys [db]} _] + (let [current-viewing-account (-> db :wallet :current-viewing-account-address) + [request-id] (get-unique-collectible-request-id 1)] + {:db (assoc-in db [:wallet :ui :collectibles :pending-requests] 1) + :fx [[:dispatch + [:wallet/request-new-collectibles-for-account + {:request-id request-id + :account current-viewing-account + :amount collectibles-request-batch-size}]]]}))) + +(defn- update-fetched-collectibles-progress + [db owner-address collectibles offset has-more?] + (-> db + (assoc-in [:wallet :ui :collectibles :fetched owner-address] collectibles) + (assoc-in [:wallet :accounts owner-address :current-collectible-idx] + (+ offset (count collectibles))) + (assoc-in [:wallet :accounts owner-address :has-more-collectibles?] has-more?))) (rf/reg-event-fx :wallet/owned-collectibles-filtering-done - (fn [_ [{:keys [message]}]] - (let [{:keys [has-more offset - collectibles]} (cske/transform-keys csk/->kebab-case-keyword (types/json->clj message)) - start-at-index (+ offset (count collectibles))] - {:fx [[:dispatch [:wallet/store-collectibles collectibles]] - (when has-more - [:dispatch [:wallet/request-collectibles {:start-at-index start-at-index}]])]}))) + (fn [{:keys [db]} [{:keys [message]}]] + (let [{:keys [offset ownershipStatus collectibles + hasMore]} (transforms/json->clj message) + collectibles (cske/transform-keys transforms/->kebab-case-keyword collectibles) + pending-requests (dec (get-in db [:wallet :ui :collectibles :pending-requests])) + owner-address (some->> ownershipStatus + first + key + name)] + {:db (cond-> db + :always (assoc-in [:wallet :ui :collectibles :pending-requests] pending-requests) + owner-address (update-fetched-collectibles-progress owner-address + collectibles + offset + hasMore)) + :fx [(when (zero? pending-requests) + [:dispatch [:wallet/flush-collectibles-fetched]])]}))) (rf/reg-event-fx :wallet/get-collectible-details (fn [_ [collectible-id]] (let [request-id 0 - collectible-id-converted (cske/transform-keys csk/->PascalCaseKeyword collectible-id) + collectible-id-converted (cske/transform-keys transforms/->PascalCaseKeyword collectible-id) data-type (collectible-data-types :details) request-params [request-id [collectible-id-converted] data-type]] {:fx [[:json-rpc/call @@ -118,8 +168,8 @@ (rf/reg-event-fx :wallet/get-collectible-details-done (fn [_ [{:keys [message]}]] - (let [response (cske/transform-keys csk/->kebab-case-keyword - (types/json->clj message)) + (let [response (cske/transform-keys transforms/->kebab-case-keyword + (transforms/json->clj message)) {:keys [collectibles]} response collectible (first collectibles)] (if collectible diff --git a/src/status_im/contexts/wallet/events_test.cljs b/src/status_im/contexts/wallet/events_test.cljs index 96a6afbd11..99ddc7a30a 100644 --- a/src/status_im/contexts/wallet/events_test.cljs +++ b/src/status_im/contexts/wallet/events_test.cljs @@ -56,24 +56,8 @@ (is (match? (:db effects) expected-db)))) (deftest store-collectibles - (testing "(displayable-collectible?) helper function" - (let [expected-results [[true - {:collectible-data {:image-url "https://..." :animation-url "https://..."}}] - [true {:collectible-data {:image-url "" :animation-url "https://..."}}] - [true {:collectible-data {:image-url nil :animation-url "https://..."}}] - [true {:collectible-data {:image-url "https://..." :animation-url ""}}] - [true {:collectible-data {:image-url "https://..." :animation-url nil}}] - [false {:collectible-data {:image-url "" :animation-url nil}}] - [false {:collectible-data {:image-url nil :animation-url nil}}] - [false {:collectible-data {:image-url nil :animation-url ""}}] - [false {:collectible-data {:image-url "" :animation-url ""}}]]] - (doseq [[result collection] expected-results] - (is (match? result (collectibles/displayable-collectible? collection)))))) - - (testing "save-collectibles-request-details" - (let [db {:wallet {:accounts {"0x1" {} - "0x3" {}}}} - collectible-1 {:collectible-data {:image-url "https://..." :animation-url "https://..."} + (testing "flush-collectibles" + (let [collectible-1 {:collectible-data {:image-url "https://..." :animation-url "https://..."} :ownership [{:address "0x1" :balance "1"}]} collectible-2 {:collectible-data {:image-url "" :animation-url "https://..."} @@ -82,12 +66,18 @@ collectible-3 {:collectible-data {:image-url "" :animation-url nil} :ownership [{:address "0x2" :balance "1"}]} - collectibles [collectible-1 collectible-2 collectible-3] - expected-db {:wallet {:accounts {"0x1" {:collectibles (list collectible-2 collectible-1)} + db {:wallet {:ui {:collectibles {:pending-requests 0 + :fetched {"0x1" [collectible-1 + collectible-2] + "0x2" [collectible-3]}}} + :accounts {"0x1" {} + "0x3" {}}}} + expected-db {:wallet {:ui {:collectibles {}} + :accounts {"0x1" {:collectibles (list collectible-1 collectible-2)} "0x2" {:collectibles (list collectible-3)} "0x3" {}}}} - effects (collectibles/store-collectibles {:db db} [collectibles]) - result-db (:db effects)] + result-db (:db (collectibles/flush-collectibles {:db db}))] + (is (match? result-db expected-db))))) (deftest clear-stored-collectibles diff --git a/src/status_im/contexts/wallet/home/tabs/view.cljs b/src/status_im/contexts/wallet/home/tabs/view.cljs index e7c9428609..845d0b653d 100644 --- a/src/status_im/contexts/wallet/home/tabs/view.cljs +++ b/src/status_im/contexts/wallet/home/tabs/view.cljs @@ -9,12 +9,15 @@ (defn view [{:keys [selected-tab]}] - (let [collectible-list (rf/sub [:wallet/all-collectibles])] + (let [collectible-list (rf/sub [:wallet/all-collectibles-list]) + request-collectibles #(rf/dispatch + [:wallet/request-collectibles-for-all-accounts {}])] [rn/view {:style style/container} (case selected-tab :assets [assets/view] :collectibles [collectibles/view {:collectibles collectible-list + :on-end-reached request-collectibles :on-collectible-press (fn [{:keys [id]}] (rf/dispatch [:wallet/get-collectible-details id]))}] [activity/view])])) diff --git a/src/status_im/contexts/wallet/send/events.cljs b/src/status_im/contexts/wallet/send/events.cljs index 9222404e90..e814b8f506 100644 --- a/src/status_im/contexts/wallet/send/events.cljs +++ b/src/status_im/contexts/wallet/send/events.cljs @@ -1,6 +1,5 @@ (ns status-im.contexts.wallet.send.events (:require - [camel-snake-kebab.core :as csk] [camel-snake-kebab.extras :as cske] [clojure.string :as string] [native-module.core :as native-module] @@ -11,7 +10,8 @@ [utils.address :as address] [utils.money :as money] [utils.number] - [utils.re-frame :as rf])) + [utils.re-frame :as rf] + [utils.transforms :as transforms])) (rf/reg-event-fx :wallet/clean-send-data (fn [{:keys [db]}] @@ -24,7 +24,7 @@ (rf/reg-event-fx :wallet/suggested-routes-success (fn [{:keys [db]} [suggested-routes timestamp]] (when (= (get-in db [:wallet :ui :send :suggested-routes-call-timestamp]) timestamp) - (let [suggested-routes-data (cske/transform-keys csk/->kebab-case suggested-routes) + (let [suggested-routes-data (cske/transform-keys transforms/->kebab-case-keyword suggested-routes) chosen-route (:best suggested-routes-data)] {:db (-> db (assoc-in [:wallet :ui :send :suggested-routes] suggested-routes-data) @@ -90,7 +90,7 @@ (rf/reg-event-fx :wallet/clean-selected-token (fn [{:keys [db]}] - {:db (update-in db [:wallet :ui :send] dissoc :token :type)})) + {:db (assoc-in db [:wallet :ui :send :token] nil)})) (rf/reg-event-fx :wallet/clean-selected-collectible (fn [{:keys [db]}] diff --git a/src/status_im/contexts/wallet/send/select_asset/view.cljs b/src/status_im/contexts/wallet/send/select_asset/view.cljs index 991a64d44e..e28233e2e3 100644 --- a/src/status_im/contexts/wallet/send/select_asset/view.cljs +++ b/src/status_im/contexts/wallet/send/select_asset/view.cljs @@ -33,6 +33,7 @@ [collectibles-tab/view {:collectibles collectibles :filtered? search-performed? + :on-end-reached #(rf/dispatch [:wallet/request-collectibles-for-current-viewing-account]) :on-collectible-press #(rf/dispatch [:wallet/send-select-collectible {:collectible % :stack-id :wallet-select-asset}])}])) diff --git a/src/status_im/subs/wallet/collectibles.cljs b/src/status_im/subs/wallet/collectibles.cljs index 5a9bb95976..4016bc2280 100644 --- a/src/status_im/subs/wallet/collectibles.cljs +++ b/src/status_im/subs/wallet/collectibles.cljs @@ -45,13 +45,21 @@ (-> current-account :collectibles add-collectibles-preview-url))) (re-frame/reg-sub - :wallet/all-collectibles + :wallet/all-collectibles-list :<- [:wallet] - (fn [wallet] - (->> wallet - :accounts - (mapcat (comp :collectibles val)) - (add-collectibles-preview-url)))) + (fn [{:keys [accounts]}] + (let [max-collectibles (->> accounts + (map (comp count :collectibles val)) + (apply max)) + all-collectibles (map (fn [[_address {:keys [collectibles]}]] + (let [amount-to-add (- max-collectibles (count collectibles)) + empty-collectibles (repeat amount-to-add nil)] + (reduce conj collectibles empty-collectibles))) + accounts)] + (->> all-collectibles + (apply interleave) + (remove nil?) + (add-collectibles-preview-url))))) (re-frame/reg-sub :wallet/current-viewing-account-collectibles-filtered diff --git a/src/utils/transforms.cljs b/src/utils/transforms.cljs index ace1700468..b143e2edae 100644 --- a/src/utils/transforms.cljs +++ b/src/utils/transforms.cljs @@ -1,6 +1,7 @@ (ns utils.transforms (:refer-clojure :exclude [js->clj]) (:require + [camel-snake-kebab.core :as csk] [cljs-bean.core :as clj-bean] [oops.core :as oops] [reagent.impl.template :as reagent.template] @@ -20,6 +21,9 @@ (try (js->clj (.parse js/JSON json)) (catch js/Error _ (when (string? json) json))))) +(def ->kebab-case-keyword (memoize csk/->kebab-case-keyword)) +(def ->PascalCaseKeyword (memoize csk/->PascalCaseKeyword)) + (defn json->js [json] (when-not (= json "undefined")