fix(koreader): open the dashboard offline without waiting for wi-fi

Open the cached dashboard at once from the menu, gesture and Dashboard
button, then ask for a connection once through KOReader's Wi-Fi setting
and refresh in place. With no cache, show the unavailable state and Retry.

Fixes #1741

koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua

@@ -307,9 +307,7 @@ function BookOrbitCatalog:init()
     self.paths = self.stack
     UIManager:nextTick(function()
         self:hydrateOnDeviceMaps()
-        if self:shouldRefreshDashboardOnOpen() then
+        self:refreshDashboardOnOpen()
-            self:loadDashboardRoot(true, { invisible = true })
-        end
         -- One-off cache migration, queued behind the refresh so its directory
         -- walk cannot delay the first dashboard request.
         UIManager:scheduleIn(1.0, function()
@@ -475,10 +473,15 @@ function BookOrbitCatalog:markStackDirty()
     end
 end
 
+-- Going home shows the cached dashboard at once and refreshes it behind the
+-- redraw, like opening the catalog does, so it never waits on a connection.
 function BookOrbitCatalog:goDashboard()
     self.stack = {}
-    self:updateReturnPath()
+    local items, context = self:initialDashboardContext()
-    self:loadDashboardRoot(true)
+    self:switchTo(context.title or self.title, items, context, false)
+    UIManager:nextTick(function()
+        self:refreshDashboardOnOpen()
+    end)
 end
 
 function BookOrbitCatalog:sectionSubtitle(page, q, has_next)
@@ -2421,11 +2424,9 @@ function BookOrbitCatalog:onReturn()
         if self.current_context.kind == "dashboard" then
             self:scheduleThumbnailDownloads(self.dashboardBooks(self.current_context.dashboard))
         end
-        if self:shouldRefreshDashboardOnOpen() then
+        UIManager:nextTick(function()
-            UIManager:nextTick(function()
+            self:refreshDashboardOnOpen()
-                self:loadDashboardRoot(true, { invisible = true })
+        end)
-            end)
-        end
     end
     self:updateLeftIcon()
     if dirty then

koreader-plugin/bookorbit.koplugin/bookorbit_catalog_dashboard.lua

@@ -178,22 +178,22 @@ function CatalogDashboard:cachedDashboardContext(cached, opts)
 end
 
 -- What the catalog widget opens on, built without touching the network: the
--- cached dashboard when there is one, otherwise a placeholder the pending
+-- cached dashboard when there is one, otherwise a placeholder the refresh
--- refresh replaces in place.
+-- replaces in place. Offline the placeholder says the dashboard could not be
+-- loaded, because a connection may never come.
 function CatalogDashboard:initialDashboardContext()
     local cached = self:dashboardCache()
     if cached then
         return self:cachedDashboardContext(cached)
     end
-    if self:shouldRefreshDashboardOnOpen() then
+    if NetworkMgr:isConnected() then
         return self:dashboardContext(nil, { stale = true, loading = true })
     end
     return self:dashboardContext(nil, { stale = true, unavailable = true })
 end
 
--- An offline-tolerant open (auto-open, offline browse) never asks for a
+-- An offline-tolerant catalog never asks for a connection on its own. An
--- connection on its own; only an explicit browse goes through the connected
+-- explicit open is not offline-tolerant until its first refresh has asked.
--- gate.
 function CatalogDashboard:shouldRefreshDashboardOnOpen()
     if self.prefer_cached_dashboard and not NetworkMgr:isConnected() then
         return false
@@ -201,6 +201,17 @@ function CatalogDashboard:shouldRefreshDashboardOnOpen()
     return true
 end
 
+-- The refresh behind a dashboard that has just been shown. An explicit open
+-- asks for a connection once, through the user's Wi-Fi policy, while the
+-- cached dashboard is already on screen. Returning to the dashboard later
+-- keeps the cache while offline rather than asking again.
+function CatalogDashboard:refreshDashboardOnOpen()
+    if self:shouldRefreshDashboardOnOpen() then
+        self:loadDashboardRoot(true, { invisible = true })
+    end
+    self.prefer_cached_dashboard = true
+end
+
 function CatalogDashboard:dashboardRoot(opts)
     local function requestIsCurrent()
         return not opts or not opts.is_current or opts.is_current()
@@ -325,6 +336,11 @@ function CatalogDashboard:loadDashboardRoot(replace, opts)
             and self.current_context == expected_context
     end
     self:runConnected(function()
+        -- The screen said the dashboard could not be loaded while this waited
+        -- for a connection, so the fetch shows that it is now loading.
+        if expected_context and expected_context.unavailable then
+            request_opts.invisible = nil
+        end
         local items, context = self:dashboardRoot(request_opts)
         if self.catalog_closed or request_generation ~= self.dashboard_request_generation
                 or self.current_context ~= expected_context then

koreader-plugin/bookorbit.koplugin/main.lua

@@ -1113,19 +1113,16 @@ function BookOrbit:openCatalogBrowser(prefer_cached_dashboard)
     UIManager:show(self.catalog_browser)
 end
 
-function BookOrbit:browseCatalog(allow_offline)
+-- Opening never waits for a connection: the catalog shows the cached dashboard
+-- at once and asks for one itself when it refreshes. An offline-tolerant open
+-- (the auto-open) does not ask at all.
+function BookOrbit:browseCatalog(prefer_cached_dashboard)
     if not self:isLoggedIn() then
         promptLogin()
         return
     end
 
-    if allow_offline then
+    self:openCatalogBrowser(prefer_cached_dashboard == true)
-        self:openCatalogBrowser(true)
-    else
-        NetworkMgr:runWhenConnected(function()
-            self:openCatalogBrowser(false)
-        end)
-    end
 end
 
 -- Manual sync triggers

koreader-plugin/spec/bookorbit_catalog_dashboard_startup_test.lua

@@ -192,4 +192,97 @@ assertEqual(last_fetch_opts.invisible, true, "automatic dashboard refresh is non
 assertEqual(switches, 0, "stale dashboard response cannot replace a navigated view")
 assertEqual(catalog.current_context, navigated_context, "navigation context remains current")
 
+-- An explicit open while offline shows the cache at once and asks for a
+-- connection a single time, through the user's Wi-Fi policy. Returning to the
+-- dashboard while still offline keeps the cache rather than asking again.
+local function openedCatalog(opts)
+    local opened = newCatalog(opts)
+    local _, initial = opened:initialDashboardContext()
+    opened.current_context = initial
+    opened.connection_requests = {}
+    function opened:runConnected(fn)
+        table.insert(self.connection_requests, fn)
+    end
+    function opened:switchTo(_, _, next_context)
+        self.current_context = next_context
+    end
+    return opened
+end
+
+connected = false
+catalog = openedCatalog{ cache = cached_dashboard }
+assertEqual(catalog.current_context.dashboard, cached_dashboard, "an offline explicit open shows the cache at once")
+catalog:refreshDashboardOnOpen()
+assertEqual(#catalog.connection_requests, 1, "an offline explicit open asks for a connection")
+catalog:refreshDashboardOnOpen()
+assertEqual(#catalog.connection_requests, 1, "returning to the dashboard offline does not ask again")
+
+-- The connection arrives: the cached dashboard is refreshed in place, quietly.
+connected = true
+requests = 0
+catalog.connection_requests[1]()
+assertEqual(requests, 1, "the refresh runs once the connection arrives")
+assertEqual(last_fetch_opts.invisible, true, "a refresh behind the cached dashboard stays non-modal")
+assertEqual(catalog.current_context.stale, false, "the cached dashboard is replaced by fresh data")
+catalog:refreshDashboardOnOpen()
+assertEqual(#catalog.connection_requests, 2, "returning to the dashboard online still refreshes")
+
+-- With no cache an offline explicit open says the dashboard could not be
+-- loaded rather than promising a load that may never come, and still asks.
+connected = false
+catalog = openedCatalog{}
+assertEqual(catalog.current_context.unavailable, true, "an offline open with no cache reports unavailable")
+assertEqual(catalog.current_context.loading, false, "an offline open with no cache does not claim to be loading")
+catalog:refreshDashboardOnOpen()
+assertEqual(#catalog.connection_requests, 1, "an offline open with no cache still asks for a connection")
+connected = true
+catalog.connection_requests[1]()
+assertEqual(last_fetch_opts.invisible, nil, "once connected, the fetch shows that the dashboard is loading")
+assertEqual(catalog.current_context.unavailable, false, "the unavailable placeholder is replaced")
+
+-- The auto-open never asks for a connection, offline or on later returns.
+connected = false
+catalog = openedCatalog{ prefer_cached_dashboard = true, cache = cached_dashboard }
+catalog:refreshDashboardOnOpen()
+catalog:refreshDashboardOnOpen()
+assertEqual(#catalog.connection_requests, 0, "an offline auto-open never asks for a connection")
+connected = true
+
+-- Going home from deep in the catalog (the Dashboard button, hold Return)
+-- switches to the cached dashboard at once instead of waiting on a connection.
+-- bookorbit_catalog.lua cannot be loaded outside KOReader, so the method is
+-- lifted out and run against the real dashboard methods.
+local catalog_source = assert(io.open("koreader-plugin/bookorbit.koplugin/bookorbit_catalog.lua", "rb"))
+local go_dashboard_source = assert(catalog_source:read("*a"):match("\n(function BookOrbitCatalog:goDashboard%(.-\nend)\n"),
+    "goDashboard is defined in bookorbit_catalog.lua")
+catalog_source:close()
+local next_ticks = {}
+local go_dashboard_env = {
+    BookOrbitCatalog = {},
+    UIManager = {
+        nextTick = function(_, fn)
+            table.insert(next_ticks, fn)
+        end,
+    },
+}
+local go_dashboard_chunk = assert(loadstring(go_dashboard_source, "goDashboard"))
+setfenv(go_dashboard_chunk, go_dashboard_env)
+go_dashboard_chunk()
+
+connected = false
+catalog = openedCatalog{ cache = cached_dashboard }
+catalog:refreshDashboardOnOpen()
+local detail_context = { kind = "detail" }
+catalog.current_context = detail_context
+catalog.stack = { { context = catalog.current_context } }
+catalog.goDashboard = go_dashboard_env.BookOrbitCatalog.goDashboard
+catalog.updateReturnPath = function() end
+catalog:goDashboard()
+assertEqual(catalog.current_context.kind, "dashboard", "going home offline shows the dashboard at once")
+assertEqual(catalog.current_context.dashboard, cached_dashboard, "going home offline shows the cached dashboard")
+assertEqual(#catalog.stack, 0, "going home clears the navigation stack")
+assertEqual(#next_ticks, 1, "the refresh is queued behind the redraw")
+next_ticks[1]()
+assertEqual(#catalog.connection_requests, 1, "going home offline does not ask for a connection again")
+
 print("bookorbit_catalog_dashboard_startup_test.lua: ok")

koreader-plugin/spec/bookorbit_dashboard_offline_open_test.lua

@@ -0,0 +1,132 @@
+-- Issue 1741: opening the dashboard must never wait for a connection. Every
+-- entry point opens the catalog at once; only the auto-open is offline-tolerant
+-- from the start, and asking for a connection is left to the catalog's refresh.
+
+package.loaded["gettext"] = function(text)
+    return text
+end
+package.loaded["ffi/util"] = {
+    template = function(text)
+        return text
+    end,
+}
+package.loaded["ui/uimanager"] = {}
+package.loaded["device"] = { model = "test-device" }
+package.loaded["ui/widget/infomessage"] = { new = function(_, opts) return opts end }
+package.loaded["ui/widget/inputdialog"] = {}
+package.loaded["ui/widget/multiinputdialog"] = {}
+package.loaded["ui/widget/notification"] = {}
+package.loaded["ffi/sha2"] = { md5 = function(value) return value end }
+package.loaded["util"] = {
+    trim = function(value)
+        return tostring(value or ""):match("^%s*(.-)%s*$")
+    end,
+}
+package.loaded["bookorbit_api"] = {}
+package.loaded["bookorbit_highlight_diagnostics"] = {}
+package.loaded["bookorbit_sweep"] = {}
+
+local waits = 0
+local function waitForConnection()
+    waits = waits + 1
+    return true
+end
+local NetworkMgr = {
+    isConnected = function()
+        return false
+    end,
+    runWhenConnected = waitForConnection,
+    willRerunWhenConnected = waitForConnection,
+    beforeWifiAction = waitForConnection,
+}
+package.loaded["ui/network/manager"] = NetworkMgr
+
+package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path
+
+local MainMenu = require("bookorbit_main_menu")
+
+local function assertEqual(actual, expected, label)
+    if actual ~= expected then
+        error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual)))
+    end
+end
+
+local function read(path)
+    local file = assert(io.open(path, "rb"))
+    local content = file:read("*a")
+    file:close()
+    return content
+end
+
+-- The entry points live in main.lua, which cannot be loaded whole outside
+-- KOReader, so each one is lifted out and run against stubs.
+local main = read("koreader-plugin/bookorbit.koplugin/main.lua")
+local prompted_login = 0
+local env = {
+    BookOrbit = {},
+    NetworkMgr = NetworkMgr,
+    promptLogin = function()
+        prompted_login = prompted_login + 1
+    end,
+}
+for _, name in ipairs({ "isLoggedIn", "browseCatalog", "onBookOrbitOpenDashboard", "maybeAutoOpenCatalog" }) do
+    local source = assert(main:match("\n(function BookOrbit:" .. name .. "%(.-\nend)\n"), name .. " is defined in main.lua")
+    local chunk = assert(loadstring(source, name))
+    setfenv(chunk, env)
+    chunk()
+end
+
+local opened = {}
+local plugin = {
+    SYNC_STRATEGY = { PROMPT = 1, SILENT = 2, DISABLE = 3 },
+    settings = { server_url = "http://bookorbit.test/api/v1", username = "reader", userkey = "key" },
+}
+MainMenu.install(plugin)
+for name, fn in pairs(env.BookOrbit) do
+    plugin[name] = fn
+end
+plugin.settingsMenu = function()
+    return {}
+end
+plugin.openCatalogBrowser = function(_, prefer_cached_dashboard)
+    table.insert(opened, prefer_cached_dashboard)
+end
+
+local function openDashboardMenuItem()
+    local menu_items = {}
+    plugin:addToMainMenu(menu_items)
+    for _, item in ipairs(menu_items.bookorbit.sub_item_table) do
+        if item.id == "open_dashboard" then return item end
+    end
+    error("the BookOrbit menu has an Open dashboard item")
+end
+
+-- Tools > BookOrbit > Open dashboard while offline.
+openDashboardMenuItem().callback()
+assertEqual(#opened, 1, "the menu opens the dashboard while offline")
+assertEqual(opened[1], false, "the menu open is explicit, so its refresh may ask for a connection")
+assertEqual(waits, 0, "the menu does not wait for a connection before opening")
+
+-- The gesture and dispatcher action.
+opened = {}
+plugin:onBookOrbitOpenDashboard()
+assertEqual(#opened, 1, "the dispatcher action opens the dashboard while offline")
+assertEqual(opened[1], false, "the dispatcher action is an explicit open")
+assertEqual(waits, 0, "the dispatcher action does not wait for a connection")
+
+-- The auto-open stays offline-tolerant: it never asks for a connection.
+opened = {}
+plugin.ui = { file_chooser = {} }
+plugin:maybeAutoOpenCatalog("filemanager")
+assertEqual(#opened, 1, "the auto-open opens the dashboard")
+assertEqual(opened[1], true, "the auto-open prefers the cached dashboard")
+assertEqual(waits, 0, "the auto-open does not wait for a connection")
+
+-- Logged out, nothing opens and the user is told to log in.
+opened = {}
+plugin.settings = {}
+plugin:browseCatalog()
+assertEqual(#opened, 0, "a logged-out open does not open the catalog")
+assertEqual(prompted_login, 1, "a logged-out open asks the user to log in")
+
+print("bookorbit_dashboard_offline_open_test.lua: ok")