Merge pull request #2063 from Hzj-jie/master

Remove legacy document settings after flushing

frontend/docsettings.lua

@@ -63,29 +63,33 @@ function DocSettings:open(docfile)
             -- can handle two files with only different suffixes.
             new.sidecar_file = sidecar.."/metadata."..
                                docfile:match(".*%.(.*)")..".lua"
-            new.legacy_sidecar_file = sidecar.."/"..
-                                      docfile:match(".*%/(.*)")..".lua"
+            if docfile:find("/") then
+                new.legacy_sidecar_file = sidecar.."/"..
+                                          docfile:match(".*%/(.*)")..".lua"
+            else
+                new.legacy_sidecar_file = sidecar.."/"..docfile..".lua"
+            end
         end
 
-        local candidates = {}
+        new.candidates = {}
         -- New sidecar file
-        table.insert(candidates, buildCandidate(new.sidecar_file))
+        table.insert(new.candidates, buildCandidate(new.sidecar_file))
         -- Legacy sidecar file
-        table.insert(candidates, buildCandidate(new.legacy_sidecar_file))
+        table.insert(new.candidates, buildCandidate(new.legacy_sidecar_file))
         -- Legacy history folder
-        table.insert(candidates, buildCandidate(new.history_file))
+        table.insert(new.candidates, buildCandidate(new.history_file))
         -- Legacy kpdfview setting
-        table.insert(candidates, buildCandidate(docfile..".kpdfview.lua"))
-        table.sort(candidates, function(l, r)
-                                   if l == nil then
-                                       return false
-                                   elseif r == nil then
-                                       return true
-                                   else
-                                       return l[2] > r[2]
-                                   end
-                               end)
-        for _, k in pairs(candidates) do
+        table.insert(new.candidates, buildCandidate(docfile..".kpdfview.lua"))
+        table.sort(new.candidates, function(l, r)
+                                       if l == nil then
+                                           return false
+                                       elseif r == nil then
+                                           return true
+                                       else
+                                           return l[2] > r[2]
+                                       end
+                                   end)
+        for _, k in pairs(new.candidates) do
             ok, stored = pcall(dofile, k[1])
             if ok then
                 break
@@ -131,6 +135,19 @@ function DocSettings:flush()
             f_out:write(s_out)
             f_out:write("\n")
             f_out:close()
+
+            if self.candidates ~= nil
+            and not G_reader_settings:readSetting(
+                        "preserve_legacy_docsetting") then
+                for _, k in pairs(self.candidates) do
+                    if k[1] ~= f then
+                        os.remove(k[1])
+                        -- We should not remove sidecar folder, as it may
+                        -- contain Kindle history files.
+                    end
+                end
+            end
+
             break
         end
     end

spec/unit/docsettings_spec.lua

@@ -1,12 +1,76 @@
 describe("docsettings module", function()
-    local docsettings
+    local docsettings, lfs
+
     setup(function()
         require("commonrequire")
         docsettings = require("docsettings")
+        lfs = require("libs/libkoreader-lfs")
     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"))
         assert.Equals("baz.sdr", docsettings:getSidecarDir("baz.pdf"))
     end)
+
+    it("should read legacy history file", function()
+        local file = "file.pdf"
+        local d = docsettings:open(file)
+        d:saveSetting("a", "b")
+        d:saveSetting("c", "d")
+        d:close()
+        -- Now the sidecar file should be written.
+
+        local legacy_files = {
+            d.history_file,
+            d.sidecar .. "/file.pdf.lua",
+            "file.pdf.kpdfview.lua",
+        }
+
+        for _, f in pairs(legacy_files) do
+            assert.False(os.rename(d.sidecar_file, f) == nil)
+            d = docsettings:open(file)
+            assert.True(os.remove(d.sidecar_file) == nil)
+            -- Legacy history files should not be removed before flush has been
+            -- called.
+            assert.Equals(lfs.attributes(f, "mode"), "file")
+            assert.Equals(d:readSetting("a"), "b")
+            assert.Equals(d:readSetting("c"), "d")
+            assert.Equals(d:readSetting("e"), nil)
+            d:close()
+            -- legacy history files should be removed as sidecar_file is
+            -- preferred.
+            assert.True(os.remove(f) == nil)
+        end
+
+        assert.False(os.remove(d.sidecar_file) == nil)
+        d:purge()
+    end)
+
+    it("should respect newest history file", function()
+        local file = "file.pdf"
+        local d = docsettings:open(file)
+
+        local legacy_files = {
+            d.history_file,
+            d.sidecar .. "/file.pdf.lua",
+            "file.pdf.kpdfview.lua",
+        }
+
+        -- docsettings:flush will remove legacy files.
+        for i, v in pairs(legacy_files) do
+            d:saveSetting("a", i)
+            d:flush()
+            assert.False(os.rename(d.sidecar_file, v.."1") == nil)
+        end
+
+        d:close()
+        for _, v in pairs(legacy_files) do
+            assert.False(os.rename(v.."1", v) == nil)
+        end
+
+        d = docsettings:open(file)
+        assert.Equals(d:readSetting("a"), #legacy_files)
+        d:close()
+        d:purge()
+    end)
 end)