From d6845d94b1486632096f64f42a42a6fa02c5340c Mon Sep 17 00:00:00 2001 From: Hzj_jie Date: Wed, 19 Jul 2017 05:56:17 -0700 Subject: [PATCH] ReadHistory: support reload (#3003) Allows ReadHistory to reload according to the modification timestamp of history.lua. --- frontend/readhistory.lua | 50 ++++++++++++--- spec/unit/readhistory_spec.lua | 111 +++++++++++++++++++++++++++++---- 2 files changed, 141 insertions(+), 20 deletions(-) diff --git a/frontend/readhistory.lua b/frontend/readhistory.lua index ababe5ba1..0e66c234b 100644 --- a/frontend/readhistory.lua +++ b/frontend/readhistory.lua @@ -1,14 +1,15 @@ -local lfs = require("libs/libkoreader-lfs") local DataStorage = require("datastorage") local DocSettings = require("docsettings") -local realpath = require("ffi/util").realpath -local joinPath = require("ffi/util").joinPath local dump = require("dump") +local joinPath = require("ffi/util").joinPath +local lfs = require("libs/libkoreader-lfs") +local realpath = require("ffi/util").realpath local history_file = joinPath(DataStorage:getDataDir(), "history.lua") local ReadHistory = { hist = {}, + last_read_time = 0, } local function buildEntry(input_time, input_file) @@ -40,6 +41,7 @@ local function timeFirstOrdering(l, r) end function ReadHistory:_indexing(start) + assert(self ~= nil) -- TODO(Hzj_jie): Use binary search to find an item when deleting it. for i = start, #self.hist, 1 do self.hist[i].index = i @@ -47,7 +49,9 @@ function ReadHistory:_indexing(start) end function ReadHistory:_sort() - local autoremove_deleted_items_from_history = G_reader_settings:readSetting("autoremove_deleted_items_from_history") or false + assert(self ~= nil) + local autoremove_deleted_items_from_history = + not G_reader_settings:nilOrFalse("autoremove_deleted_items_from_history") if autoremove_deleted_items_from_history then self:clearMissing() end @@ -70,6 +74,7 @@ end -- Reduces total count in hist list to a reasonable number by removing last -- several items. function ReadHistory:_reduce() + assert(self ~= nil) while #self.hist > 500 do table.remove(self.hist, #self.hist) end @@ -77,6 +82,7 @@ end -- Flushes current history table into file. function ReadHistory:_flush() + assert(self ~= nil) local content = {} for k, v in pairs(self.hist) do content[k] = { @@ -89,18 +95,28 @@ function ReadHistory:_flush() f:close() end --- Reads history table from file +--- Reads history table from file. +-- @treturn boolean true if the history_file has been updated and reloaded. function ReadHistory:_read() + assert(self ~= nil) + local history_file_modification_time = lfs.attributes(history_file, "modification") + if history_file_modification_time == nil + or history_file_modification_time <= self.last_read_time then + return false + end + self.last_read_time = history_file_modification_time local ok, data = pcall(dofile, history_file) if ok and data then for k, v in pairs(data) do table.insert(self.hist, buildEntry(v.time, v.file)) end end + return true end -- Reads history from legacy history folder function ReadHistory:_readLegacyHistory() + assert(self ~= nil) local history_dir = DataStorage:getHistoryDir() for f in lfs.dir(history_dir) do local path = joinPath(history_dir, f) @@ -120,13 +136,12 @@ function ReadHistory:_readLegacyHistory() end function ReadHistory:_init() - self:_read() - self:_readLegacyHistory() - self:_sort() - self:_reduce() + assert(self ~= nil) + self:reload() end function ReadHistory:clearMissing() + assert(self ~= nil) for i = #self.hist, 1, -1 do if self.hist[i].file == nil or lfs.attributes(self.hist[i].file, "mode") ~= "file" then table.remove(self.hist, i) @@ -135,6 +150,7 @@ function ReadHistory:clearMissing() end function ReadHistory:removeItemByPath(path) + assert(self ~= nil) for i = #self.hist, 1, -1 do if self.hist[i].file == path then self:removeItem(self.hist[i]) @@ -144,6 +160,7 @@ function ReadHistory:removeItemByPath(path) end function ReadHistory:removeItem(item) + assert(self ~= nil) table.remove(self.hist, item.index) os.remove(DocSettings:getHistoryPath(item.file)) self:_indexing(item.index) @@ -151,6 +168,7 @@ function ReadHistory:removeItem(item) end function ReadHistory:addItem(file) + assert(self ~= nil) if file ~= nil and lfs.attributes(file, "mode") == "file" then table.insert(self.hist, 1, buildEntry(os.time(), file)) -- TODO(zijiehe): We do not need to sort if we can use binary insert and @@ -161,6 +179,20 @@ function ReadHistory:addItem(file) end end +--- Reloads history from history_file. +-- @treturn boolean true if history_file has been updated and reload happened. +function ReadHistory:reload() + assert(self ~= nil) + if self:_read() then + self:_readLegacyHistory() + self:_sort() + self:_reduce() + return true + end + + return false +end + ReadHistory:_init() return ReadHistory diff --git a/spec/unit/readhistory_spec.lua b/spec/unit/readhistory_spec.lua index 11474e2db..11a704b30 100644 --- a/spec/unit/readhistory_spec.lua +++ b/spec/unit/readhistory_spec.lua @@ -1,51 +1,60 @@ describe("ReadHistory module", function() local DocSettings local DataStorage + local joinPath local mkdir local realpath - local joinPath + local reload local usleep - local history_file local function file(name) return joinPath(DataStorage:getDataDir(), name) end + local function test_data_dir() + return joinPath(DataStorage:getDataDir(), "testdata") + end + local function test_file(name) - return joinPath(joinPath(DataStorage:getDataDir(), "testdata"), name) + return joinPath(test_data_dir(), name) end local function legacy_history_file(name) return DocSettings:getHistoryPath(realpath(test_file(name))) end - local function reload() - package.loaded["readhistory"] = nil - return require("readhistory") - end - local function rm(file) os.remove(file) end + local function mv(source, target) + os.rename(source, target) + end + local function touch(file) local f = io.open(file, "w") f:close() end - local function assert_item_is(h, i, name) + local function assert_item_is(h, i, name, fileRemoved) assert.is.same(h.hist[i].text, name) assert.is.same(h.hist[i].index, i) - assert.is.same(h.hist[i].file, realpath(test_file(name))) + assert.is.same(h.hist[i].file, joinPath(realpath(test_data_dir()), name)) + if fileRemoved then + assert.is_nil(realpath(test_file(name))) + else + assert.is.same(h.hist[i].file, realpath(test_file(name))) + end end setup(function() require("commonrequire") DocSettings = require("docsettings") DataStorage = require("datastorage") + joinPath = require("ffi/util").joinPath mkdir = require("libs/libkoreader-lfs").mkdir realpath = require("ffi/util").realpath - joinPath = require("ffi/util").joinPath + reload = function() return package.reload("readhistory") end usleep = require("ffi/util").usleep mkdir(joinPath(DataStorage:getDataDir(), "testdata")) @@ -277,4 +286,84 @@ describe("ReadHistory module", function() rm(to_file(i)) end end) + + it("should reload the history file if it updated", function() + -- Prepare a history.lua file with two items a and b. + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + mv(file("history.lua"), file("history.backup")) + + h = reload() + assert.is.same(#h.hist, 0) + mv(file("history.backup"), file("history.lua")) + h:reload() + + assert.is.same(#h.hist, 2) + assert_item_is(h, 1, "a") + assert_item_is(h, 2, "b") + + rm(test_file("a")) + rm(test_file("b")) + end) + + local function testAutoRemoveDeletedItems() + -- Prepare a history.lua file with two items a and b. + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + + rm(test_file("a")) + + h:reload() + assert.is.same(#h.hist, 1) + assert_item_is(h, 1, "b") + + rm(test_file("b")) + end + + local function testDoNotAutoRemoveDeletedItems() + -- Prepare a history.lua file with two items a and b. + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + + rm(test_file("a")) + + h:reload() + assert.is.same(#h.hist, 2) + assert_item_is(h, 1, "a", true) + assert_item_is(h, 2, "b") + + rm(test_file("b")) + end + + it("should automatically remove deleted items from history if setting has been set", + function() + G_reader_settings:saveSetting("autoremove_deleted_items_from_history", "true") + testAutoRemoveDeletedItems() + G_reader_settings:delSetting("autoremove_deleted_items_from_history") + end) + + it("should not automatically remove deleted items from history if setting has not been set", + function() + G_reader_settings:delSetting("autoremove_deleted_items_from_history") + testDoNotAutoRemoveDeletedItems() + end) + + it("should not automatically remove deleted items from history if setting has been set to false", + function() + G_reader_settings:saveSetting("autoremove_deleted_items_from_history", "false") + testDoNotAutoRemoveDeletedItems() + G_reader_settings:delSetting("autoremove_deleted_items_from_history") + end) end)