Calibre: Minor QoL fixes (#7528)

* CalibreMetadata: Get rid of the now useless NULL-hunt: here, this was basically looking for `rapidjson.null` to replace them with... `rapidjson.null` :?. IIRC, that's a remnant of a quirk of the previous JSON parser (possibly even the previous, *previous* JSON parser ^^).
* CalibreSearch: Update the actually relevant NULL-hunt to make it explicit: replace JSON NULLs with Lua nils, instead of relying on an implementation detail of Lua-RapidJSON, because that detail just changed data type ;).
* UIManager: Make sure tasks scheduled during the final ZMQ callback are honored. e.g., the Calibre "Disconnect" handler. This happened to mostly work purely by chance before the event loop rework.
* Calibre: Restore a proper receiveCallback handler after receiving a book, in order not to break the "Disconnect" handler's state (and, well, get a working Disconnect handler, period ^^).
* Calibre: Unbreak metadata cache when it's initialized by a search (regression since #7159).
* Calibre: Handle UTC <-> local time conversions when checking the cache's timestamp against the Calibre metadata timestamp.
* Bump base (Unbreak CRe on Android, update RapidJSON)

base

@@ -1 +1 @@
-Subproject commit ff0593e9d88908dd72d6baf5b29c8a1b175560b2
+Subproject commit 50e93546edffb91b9b0b5e89ec2bac03bc61a4dd

frontend/cache.lua

@@ -41,6 +41,9 @@ local cache_path = DataStorage:getDataDir() .. "/cache/"
 
 -- NOTE: Before 2021.04, fontlist used to squat our folder, needlessly polluting our state tracking.
 os.remove(cache_path .. "/fontinfo.dat")
+-- Ditto for Calibre
+os.remove(cache_path .. "/calibre-libraries.lua")
+os.remove(cache_path .. "/calibre-books.dat")
 
 --[[
 -- return a snapshot of disk cached items for subsequent check

frontend/ui/message/streammessagequeue.lua

@@ -57,11 +57,11 @@ end
 
 function StreamMessageQueue:waitEvent()
     local data = ""
-    -- Successive zframes may be tens or hundreds in some cases
-    -- if they are concatenated in a single loop it may run up memory of the
-    -- machine. And it did happened when receiving file data from Calibre server.
-    -- Here we receive only receive 10 packages at most in one waitEvent loop, and
-    -- call receiveCallback immediately.
+    -- Successive zframes may come in batches of tens or hundreds in some cases.
+    -- If they are concatenated in a single loop, it may consume a significant amount
+    -- of memory. And it's fairly easy to trigger when receiving file data from Calibre.
+    -- So, throttle reception to 10 packages at most in one waitEvent loop,
+    -- after which we immediately call receiveCallback.
     local wait_packages = 10
     while czmq.zpoller_wait(self.poller, 0) ~= nil and wait_packages > 0 do
         local id_frame = czmq.zframe_recv(self.socket)

frontend/ui/uimanager.lua

@@ -1580,9 +1580,10 @@ function UIManager:handleInput()
         self:_repaint()
     until not self._task_queue_dirty
 
-    -- run ZMQs if any
-    self:processZMQs()
-
+    -- NOTE: Compute deadline *before* processing ZMQs, in order to be able to catch tasks scheduled *during*
+    --       the final ZMQ callback.
+    --       This ensures that we get to honor a single ZMQ_TIMEOUT *after* the final ZMQ callback,
+    --       which gives us a chance for another iteration, meaning going through _checkTasks to catch said scheduled tasks.
     -- Figure out how long to wait.
     -- Ultimately, that'll be the earliest of INPUT_TIMEOUT, ZMQ_TIMEOUT or the next earliest scheduled task.
     local deadline
@@ -1606,6 +1607,9 @@ function UIManager:handleInput()
         deadline = wait_until
     end
 
+    -- Run ZMQs if any
+    self:processZMQs()
+
     -- If allowed, entering standby (from which we can wake by input) must trigger in response to event
     -- this function emits (plugin), or within waitEvent() right after (hardware).
     -- Anywhere else breaks preventStandby/allowStandby invariants used by background jobs while UI is left running.

plugins/calibre.koplugin/metadata.lua

@@ -27,9 +27,20 @@ local used_metadata = {
     "series_index"
 }
 
-local function slim(book)
+-- The search metadata cache requires an even smaller subset
+local search_used_metadata = {
+    "lpath",
+    "size",
+    "title",
+    "authors",
+    "tags",
+    "series",
+    "series_index"
+}
+
+local function slim(book, is_search)
     local slim_book = {}
-    for _, k in ipairs(used_metadata) do
+    for _, k in ipairs(is_search and search_used_metadata or used_metadata) do
         if k == "series" or k == "series_index" then
             slim_book[k] = book[k] or rapidjson.null
         elseif k == "tags" then
@@ -125,16 +136,8 @@ end
 
 -- saves books' metadata to JSON file
 function CalibreMetadata:saveBookList()
-    -- replace bad table values with null
     local file = self.metadata
     local books = self.books
-    for index, book in ipairs(books) do
-        for key, item in pairs(book) do
-            if type(item) == "function" then
-                books[index][key] = rapidjson.null
-            end
-        end
-    end
     rapidjson.dump(rapidjson.array(books), file, { pretty = true })
 end
 
@@ -173,13 +176,7 @@ end
 
 -- gets the book metadata at the given index
 function CalibreMetadata:getBookMetadata(index)
-    local book = self.books[index]
-    for key, value in pairs(book) do
-        if type(value) == "function" then
-            book[key] = rapidjson.null
-        end
-    end
-    return book
+    return self.books[index]
 end
 
 -- removes deleted books from table
@@ -200,10 +197,16 @@ function CalibreMetadata:prune()
 end
 
 -- removes unused metadata from books
-function CalibreMetadata:cleanUnused()
+function CalibreMetadata:cleanUnused(is_search)
     for index, book in ipairs(self.books) do
-        self.books[index] = slim(book)
+        self.books[index] = slim(book, is_search)
     end
+
+    -- We don't want to stomp on the library's actual JSON db for metadata searches.
+    if is_search then
+        return
+    end
+
     self:saveBookList()
 end
 
@@ -256,6 +259,7 @@ function CalibreMetadata:init(dir, is_search)
 
     local msg
     if is_search then
+        self:cleanUnused(is_search)
         msg = string.format("(search) in %.3f milliseconds: %d books",
             (TimeVal:now() - start):tomsecs(), #self.books)
     else

plugins/calibre.koplugin/search.lua

@@ -18,7 +18,9 @@ local Screen = require("device").screen
 local Size = require("ui/size")
 local TimeVal = require("ui/timeval")
 local UIManager = require("ui/uimanager")
+local lfs = require("libs/libkoreader-lfs")
 local logger = require("logger")
+local rapidjson = require("rapidjson")
 local util = require("util")
 local _ = require("gettext")
 local T = require("ffi/util").template
@@ -84,7 +86,7 @@ end
 local function getBooksBySeries(t, series)
     local result = {}
     for _, book in ipairs(t) do
-        if book.series and type(book.series) ~= "function" then
+        if book.series and book.series ~= rapidjson.null then
             if book.series == series then
                 table.insert(result, book)
             end
@@ -112,7 +114,7 @@ end
 local function searchBySeries(t, query, case_insensitive)
     local freq = {}
     for _, book in ipairs(t) do
-        if book.series and type(book.series) ~= "function" then
+        if book.series and book.series ~= rapidjson.null then
             if match(book.series, query, case_insensitive) then
                 freq[book.series] = (freq[book.series] or 0) + 1
             end
@@ -147,7 +149,7 @@ local function getBookInfo(book)
         tags = _("Tags:") .. " " .. tags
     end
     local series
-    if book.series and type(book.series) ~= "function" then
+    if book.series and book.series ~= rapidjson.null then
         series = _("Series:") .. " " .. book.series
     end
     return string.format("%s\n%s\n%s%s%s", title, authors,
@@ -168,12 +170,12 @@ local CalibreSearch = InputContainer:new{
         "find_by_path",
     },
 
+    cache_dir = DataStorage:getDataDir() .. "/cache/calibre",
     cache_libs = Persist:new{
-        path = DataStorage:getDataDir() .. "/cache/calibre-libraries.lua",
+        path = DataStorage:getDataDir() .. "/cache/calibre/libraries.lua",
     },
-
     cache_books = Persist:new{
-        path = DataStorage:getDataDir() .. "/cache/calibre-books.dat",
+        path = DataStorage:getDataDir() .. "/cache/calibre/books.dat",
         codec = "bitser",
     },
 }
@@ -494,6 +496,7 @@ function CalibreSearch:prompt(message)
                 paths = paths .. "\n" .. _("SD card") .. ": " .. sd_paths
             end
 
+            lfs.mkdir(self.cache_dir)
             self.cache_libs:save(self.libraries)
             self:invalidateCache()
             self.books = self:getMetadata()
@@ -561,11 +564,27 @@ function CalibreSearch:getMetadata()
     -- try to load metadata from cache
     if self.cache_metadata then
         local function cacheIsNewer(timestamp)
-            local file_timestamp = self.cache_books:timestamp()
-            if not timestamp or not file_timestamp then return false end
+            local cache_timestamp = self.cache_books:timestamp()
+            -- stat returns a true Epoch (UTC)
+            if not timestamp or not cache_timestamp then return false end
             local Y, M, D, h, m, s = timestamp:match("(%d+)-(%d+)-(%d+)T(%d+):(%d+):(%d+)")
-            local date = os.time({year = Y, month = M, day = D, hour = h, min = m, sec = s})
-            return file_timestamp > date
+            -- calibre also stores this in UTC (c.f., calibre.utils.date.isoformat)...
+            -- But os.time uses mktime, which converts it to *local* time...
+            -- Meaning we'll have to jump through a lot of stupid hoops to make the two agree...
+            local meta_timestamp = os.time({year = Y, month = M, day = D, hour = h, min = m, sec = s})
+            -- To that end, compute the local timezone's offset to UTC via strftime's %z token...
+            local tz = os.date("%z") -- +hhmm or -hhmm
+            -- We deal with a time_t, so, convert that to seconds...
+            local tz_sign, tz_hours, tz_minutes = tz:match("([+-])(%d%d)(%d%d)")
+            local utc_diff = (tonumber(tz_hours) * 60 * 60) + (tonumber(tz_minutes) * 60)
+            if tz_sign == "-" then
+                utc_diff = -utc_diff
+            end
+            meta_timestamp = meta_timestamp + utc_diff
+            logger.dbg("CalibreSearch:getMetadata: Cache timestamp   :", cache_timestamp, os.date("!%FT%T.000000+00:00", cache_timestamp), os.date("(%F %T %z)", cache_timestamp))
+            logger.dbg("CalibreSearch:getMetadata: Metadata timestamp:", meta_timestamp, timestamp, os.date("(%F %T %z)", meta_timestamp))
+
+            return cache_timestamp > meta_timestamp
         end
 
         local cache, err = self.cache_books:load()
@@ -594,7 +613,7 @@ function CalibreSearch:getMetadata()
         local serialized_table = {}
         local function removeNull(t)
             for _, key in ipairs({"series", "series_index"}) do
-                if type(t[key]) == "function" then
+                if t[key] == rapidjson.null then
                     t[key] = nil
                 end
             end
@@ -603,7 +622,11 @@ function CalibreSearch:getMetadata()
         for index, book in ipairs(books) do
             table.insert(serialized_table, index, removeNull(book))
         end
-        self.cache_books:save(serialized_table)
+        lfs.mkdir(self.cache_dir)
+        local ok, err = self.cache_books:save(serialized_table)
+        if not ok then
+            logger.info("Failed to serialize calibre metadata cache:", err)
+        end
     end
     logger.info(string.format(template, #books, "calibre", (TimeVal:now() - start):tomsecs()))
     return books

plugins/calibre.koplugin/wireless.lua

@@ -127,41 +127,49 @@ function CalibreWireless:checkCalibreServer(host, port)
     return false
 end
 
+-- Standard JSON/control opcodes receive callback
+function CalibreWireless:JSONReceiveCallback(host, port)
+    -- NOTE: Closure trickery because we need a reference to *this* self *inside* the callback,
+    --       which will be called as a function from another object (namely, StreamMessageQueue).
+    local this = self
+    return function(data)
+        this:onReceiveJSON(data)
+        if not this.connect_message then
+            this.password_check_callback = function()
+                local msg
+                if this.invalid_password then
+                    msg = _("Invalid password")
+                    this.invalid_password = nil
+                    this:disconnect()
+                elseif this.disconnected_by_server then
+                    msg = _("Disconnected by calibre")
+                    this.disconnected_by_server = nil
+                else
+                    msg = T(_("Connected to calibre server at %1"),
+                        BD.ltr(T("%1:%2", this.calibre_socket.host, this.calibre_socket.port)))
+                end
+                UIManager:show(InfoMessage:new{
+                    text = msg,
+                    timeout = 2,
+                })
+            end
+            this.connect_message = true
+            UIManager:scheduleIn(1, this.password_check_callback)
+            if this.failed_connect_callback then
+                -- Don't disconnect if we connect in 10 seconds
+                UIManager:unschedule(this.failed_connect_callback)
+            end
+        end
+    end
+end
+
 function CalibreWireless:initCalibreMQ(host, port)
     local StreamMessageQueue = require("ui/message/streammessagequeue")
     if self.calibre_socket == nil then
         self.calibre_socket = StreamMessageQueue:new{
             host = host,
             port = port,
-            receiveCallback = function(data)
-                self:onReceiveJSON(data)
-                if not self.connect_message then
-                    self.password_check_callback = function()
-                        local msg
-                        if self.invalid_password then
-                            msg = _("Invalid password")
-                            self.invalid_password = nil
-                            self:disconnect()
-                        elseif self.disconnected_by_server then
-                            msg = _("Disconnected by calibre")
-                            self.disconnected_by_server = nil
-                        else
-                            msg = T(_("Connected to calibre server at %1"),
-                                BD.ltr(T("%1:%2", host, port)))
-                        end
-                        UIManager:show(InfoMessage:new{
-                            text = msg,
-                            timeout = 2,
-                        })
-                    end
-                    self.connect_message = true
-                    UIManager:scheduleIn(1, self.password_check_callback)
-                    if self.failed_connect_callback then
-                        --don't disconnect if we connect in 10 seconds
-                        UIManager:unschedule(self.failed_connect_callback)
-                    end
-                end
-            end,
+            receiveCallback = self:JSONReceiveCallback(),
         }
         self.calibre_socket:start()
         self.calibre_messagequeue = UIManager:insertZMQ(self.calibre_socket)
@@ -524,7 +532,7 @@ function CalibreWireless:sendBook(arg)
     else
         local msg = T(_("Can't receive file %1/%2: %3\nNo space left on device"),
             arg.thisBook + 1, arg.totalBooks, BD.filepath(filename))
-        if self:isCalibreAtLeast(4,18,0) then
+        if self:isCalibreAtLeast(4, 18, 0) then
             -- report the error back to calibre
             self:sendJsonData('ERROR', {message = msg})
             return
@@ -561,11 +569,9 @@ function CalibreWireless:sendBook(arg)
                 CalibreMetadata:saveBookList()
                 updateDir(inbox_dir)
             end
-            -- switch to JSON data receiving mode
-            calibre_socket.receiveCallback = function(json_data)
-                calibre_device:onReceiveJSON(json_data)
-            end
-            -- if calibre sends multiple files there may be left JSON data
+            -- switch back to JSON data receiving mode
+            calibre_socket.receiveCallback = calibre_device:JSONReceiveCallback()
+            -- if calibre sends multiple files there may be leftover JSON data
             calibre_device.buffer = data:sub(#to_write_data + 1) or ""
             --logger.info("device buffer", calibre_device.buffer)
             if calibre_device.buffer ~= "" then
@@ -671,12 +677,12 @@ function CalibreWireless:sendToCalibre(arg)
     file:close()
 end
 
-function CalibreWireless:isCalibreAtLeast(x,y,z)
+function CalibreWireless:isCalibreAtLeast(x, y, z)
     local v = self.calibre.version
-    local function semanticVersion(a,b,c)
+    local function semanticVersion(a, b, c)
         return ((a * 100000) + (b * 1000)) + c
     end
-    return semanticVersion(v[1],v[2],v[3]) >= semanticVersion(x,y,z)
+    return semanticVersion(v[1], v[2], v[3]) >= semanticVersion(x, y, z)
 end
 
 return CalibreWireless