From ced671bc6916c929a8d8224a7106e84abfe26184 Mon Sep 17 00:00:00 2001 From: Zijie He Date: Mon, 12 Sep 2016 20:02:48 -0700 Subject: [PATCH] Add readhistory_spec / docsettings_spec --- frontend/docsettings.lua | 7 +- frontend/readhistory.lua | 35 +++-- spec/unit/docsettings_spec.lua | 12 +- spec/unit/readhistory_spec.lua | 280 +++++++++++++++++++++++++++++++++ 4 files changed, 318 insertions(+), 16 deletions(-) create mode 100644 spec/unit/readhistory_spec.lua diff --git a/frontend/docsettings.lua b/frontend/docsettings.lua index 9f7a7ecd6..8ecfeb486 100644 --- a/frontend/docsettings.lua +++ b/frontend/docsettings.lua @@ -36,12 +36,11 @@ end function DocSettings:getNameFromHistory(hist_name) if hist_name == nil or hist_name == '' then return nil end - hist_name = string.match(hist_name, "%b[]") - if hist_name == nil or hist_name == '' then return nil end + local s = string.match(hist_name, "%b[]") + if s == nil or s == '' then return nil end -- at first, search for path length - local s = string.len(hist_name) -- and return the rest of string without 4 last characters (".lua") - return string.sub(hist_name, s+2, -5) + return string.sub(hist_name, string.len(s)+2, -5) end function DocSettings:open(docfile) diff --git a/frontend/readhistory.lua b/frontend/readhistory.lua index 43bcedbb4..baba54bd4 100644 --- a/frontend/readhistory.lua +++ b/frontend/readhistory.lua @@ -1,6 +1,7 @@ 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") @@ -14,7 +15,7 @@ local function buildEntry(input_time, input_file) return { time = input_time, text = input_file:gsub(".*/", ""), - file = input_file, + file = realpath(input_file), callback = function() local ReaderUI = require("apps/reader/readerui") ReaderUI:showReader(input_file) @@ -22,6 +23,22 @@ local function buildEntry(input_time, input_file) } end +local function fileFirstOrdering(l, r) + if l.file == r.file then + return l.time > r.time + else + return l.file < r.file + end +end + +local function timeFirstOrdering(l, r) + if l.time == r.time then + return l.file < r.file + else + return l.time > r.time + end +end + function ReadHistory:_indexing(start) -- TODO(Hzj_jie): Use binary search to find an item when deleting it. for i = start, #self.hist, 1 do @@ -31,22 +48,18 @@ end function ReadHistory:_sort() for i = #self.hist, 1, -1 do - if lfs.attributes(self.hist[i].file, "mode") ~= "file" then + if self.hist[i].file == nil or lfs.attributes(self.hist[i].file, "mode") ~= "file" then table.remove(self.hist, i) end end - table.sort(self.hist, function(l, r) return l.file < r.file end) + table.sort(self.hist, fileFirstOrdering) -- TODO(zijiehe): Use binary insert instead of a loop to deduplicate. for i = #self.hist, 2, -1 do if self.hist[i].file == self.hist[i - 1].file then - if self.hist[i].time < self.hist[i - 1].time then - table.remove(self.hist, i) - else - table.remove(self.hist,i - 1) - end + table.remove(self.hist, i) end end - table.sort(self.hist, function(v1, v2) return v1.time > v2.time end) + table.sort(self.hist, timeFirstOrdering) self:_indexing(1) end @@ -75,7 +88,7 @@ end -- Reads history table from file function ReadHistory:_read() local ok, data = pcall(dofile, history_file) - if ok then + if ok and data then for k, v in pairs(data) do table.insert(self.hist, buildEntry(v.time, v.file)) end @@ -94,7 +107,7 @@ function ReadHistory:_readLegacyHistory() if file ~= nil and file ~= "" then table.insert( self.hist, - buildEntry(lfs.attributes(path, "modification"), + buildEntry(lfs.attributes(joinPath(history_dir, f), "modification"), joinPath(path, file))) end end diff --git a/spec/unit/docsettings_spec.lua b/spec/unit/docsettings_spec.lua index 6677bf731..a887ee375 100644 --- a/spec/unit/docsettings_spec.lua +++ b/spec/unit/docsettings_spec.lua @@ -1,11 +1,13 @@ describe("docsettings module", function() - local docsettings, lfs + local docsettings, lfs, util setup(function() require("commonrequire") docsettings = require("docsettings") lfs = require("libs/libkoreader-lfs") + util = require("ffi/util") end) + it("should generate sidecar directory path", function() assert.Equals("../../foo.sdr", docsettings:getSidecarDir("../../foo.pdf")) assert.Equals("/foo/bar.sdr", docsettings:getSidecarDir("/foo/bar.pdf")) @@ -73,4 +75,12 @@ describe("docsettings module", function() d:close() d:purge() end) + + it("should build correct legacy history path", function() + local file = "/a/b/c--d/c.txt" + local history_path = util.basename(docsettings:getHistoryPath(file)) + local path_from_history = docsettings:getPathFromHistory(history_path) + local name_from_history = docsettings:getNameFromHistory(history_path) + assert.is.same(file, path_from_history .. "/" .. name_from_history) + end) end) diff --git a/spec/unit/readhistory_spec.lua b/spec/unit/readhistory_spec.lua new file mode 100644 index 000000000..16942ecf8 --- /dev/null +++ b/spec/unit/readhistory_spec.lua @@ -0,0 +1,280 @@ +describe("ReadHistory module", function() + local DocSettings + local DataStorage + local mkdir + local realpath + local joinPath + local usleep + local history_file + + local function file(name) + return joinPath(DataStorage:getDataDir(), name) + end + + local function test_file(name) + return joinPath(joinPath(DataStorage:getDataDir(), "testdata"), 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 touch(file) + local f = io.open(file, "w") + f:close() + end + + local function assert_item_is(h, i, name) + 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))) + end + + setup(function() + require("commonrequire") + DocSettings = require("docsettings") + DataStorage = require("datastorage") + mkdir = require("libs/libkoreader-lfs").mkdir + realpath = require("ffi/util").realpath + joinPath = require("ffi/util").joinPath + usleep = require("ffi/util").usleep + + mkdir(joinPath(DataStorage:getDataDir(), "testdata")) + end) + + it("should read empty history.lua", function() + rm(file("history.lua")) + local h = reload() + assert.is.same(#h.hist, 0) + touch(file("history.lua")) + h = reload() + assert.is.same(#h.hist, 0) + end) + + it("should read non-empty history.lua", function() + rm(file("history.lua")) + local h = reload() + touch(test_file("a")) + h:addItem(test_file("a")) + h = reload() + assert.is.same(#h.hist, 1) + assert_item_is(h, 1, "a") + rm(test_file("a")) + end) + + it("should order legacy and history.lua", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + local h = reload() + h:addItem(test_file("a")) + usleep(1000000) + touch(legacy_history_file("b")) + h = reload() + assert.is.same(#h.hist, 2) + assert_item_is(h, 1, "b") + assert_item_is(h, 2, "a") + rm(legacy_history_file("b")) + rm(test_file("a")) + rm(test_file("b")) + end) + + it("should read legacy history folder", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + touch(test_file("c")) + touch(test_file("d")) + touch(test_file("e")) + touch(test_file("f")) + local h = reload() + h:addItem(test_file("f")) + usleep(1000000) + touch(legacy_history_file("c")) + usleep(1000000) + touch(legacy_history_file("b")) + usleep(1000000) + h:addItem(test_file("d")) + usleep(1000000) + touch(legacy_history_file("a")) + usleep(1000000) + h:addItem(test_file("e")) + h = reload() + assert.is.same(#h.hist, 6) + assert_item_is(h, 1, "e") + assert_item_is(h, 2, "a") + assert_item_is(h, 3, "d") + assert_item_is(h, 4, "b") + assert_item_is(h, 5, "c") + assert_item_is(h, 6, "f") + + rm(legacy_history_file("c")) + rm(legacy_history_file("b")) + rm(legacy_history_file("a")) + rm(test_file("a")) + rm(test_file("b")) + rm(test_file("c")) + rm(test_file("d")) + rm(test_file("e")) + rm(test_file("f")) + end) + + it("should add item", function() + rm(file("history.lua")) + touch(test_file("a")) + local h = reload() + h:addItem(test_file("a")) + assert.is.same(#h.hist, 1) + assert_item_is(h, 1, "a") + rm(test_file("a")) + end) + + it("should be able to remove the first item", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + touch(test_file("c")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + h:addItem(test_file("c")) + h:removeItem(h.hist[1]) + assert_item_is(h, 1, "b") + assert_item_is(h, 2, "c") + h:removeItem(h.hist[1]) + assert_item_is(h, 1, "c") + rm(test_file("a")) + rm(test_file("b")) + rm(test_file("c")) + end) + + it("should be able to remove an item in the middle", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + touch(test_file("c")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + h:addItem(test_file("c")) + h:removeItem(h.hist[2]) + assert_item_is(h, 1, "a") + assert_item_is(h, 2, "c") + rm(test_file("a")) + rm(test_file("b")) + rm(test_file("c")) + end) + + it("should be able to remove the last item", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + touch(test_file("c")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + h:addItem(test_file("c")) + h:removeItem(h.hist[3]) + assert_item_is(h, 1, "a") + assert_item_is(h, 2, "b") + h:removeItem(h.hist[2]) + assert_item_is(h, 1, "a") + rm(test_file("a")) + rm(test_file("b")) + rm(test_file("c")) + end) + + it("should be able to remove two items", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + touch(test_file("c")) + touch(test_file("d")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + h:addItem(test_file("c")) + h:addItem(test_file("d")) + h:removeItem(h.hist[3]) -- remove c + h:removeItem(h.hist[2]) -- remove b + assert_item_is(h, 1, "a") + assert_item_is(h, 2, "d") + rm(test_file("a")) + rm(test_file("b")) + rm(test_file("c")) + rm(test_file("d")) + end) + + it("should be able to remove three items", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + touch(test_file("c")) + touch(test_file("d")) + touch(test_file("e")) + local h = reload() + h:addItem(test_file("a")) + h:addItem(test_file("b")) + h:addItem(test_file("c")) + h:addItem(test_file("d")) + h:addItem(test_file("e")) + h:removeItem(h.hist[2]) -- remove b + h:removeItem(h.hist[2]) -- remove c + h:removeItem(h.hist[3]) -- remove e + assert_item_is(h, 1, "a") + assert_item_is(h, 2, "d") + rm(test_file("a")) + rm(test_file("b")) + rm(test_file("c")) + rm(test_file("d")) + rm(test_file("e")) + end) + + it("should remove duplicate entry", function() + rm(file("history.lua")) + touch(test_file("a")) + touch(test_file("b")) + local h = reload() + h:addItem(test_file("b")) + h:addItem(test_file("b")) + touch(legacy_history_file("a")) + h:addItem(test_file("a")) -- ensure a is before b + h = reload() + assert.is.same(#h.hist, 2) + assert_item_is(h, 1, "a") + assert_item_is(h, 2, "b") + + rm(legacy_history_file("a")) + rm(test_file("a")) + rm(test_file("b")) + end) + + it("should reduce the total count", function() + local function to_file(i) + return test_file(string.format("%04d", i)) + end + rm(file("history.lua")) + local h = reload() + for i = 1000, 1, -1 do + touch(to_file(i)) + h:addItem(to_file(i)) + end + + for i = 1, 500 do -- at most 500 items are stored + assert_item_is(h, i, string.format("%04d", i)) + end + + for i = 1, 1000 do + rm(to_file(i)) + end + end) +end)