From 2ba079e3eb79bcf96262fb6cc1160e6c18849fb6 Mon Sep 17 00:00:00 2001
From: Zijie He <zijiehe@chromium.org>
Date: Wed, 16 Aug 2017 18:38:58 -0700
Subject: [PATCH] Reserve last good docsetting

---
 frontend/docsettings.lua       | 17 +++++++++++-
 spec/unit/docsettings_spec.lua | 51 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/frontend/docsettings.lua b/frontend/docsettings.lua
index b5d147c25..3632aa1fe 100644
--- a/frontend/docsettings.lua
+++ b/frontend/docsettings.lua
@@ -7,6 +7,7 @@ in the so-called sidecar directory
 local DataStorage = require("datastorage")
 local dump = require("dump")
 local lfs = require("libs/libkoreader-lfs")
+local logger = require("logger")
 local purgeDir = require("ffi/util").purgeDir
 
 local DocSettings = {}
@@ -110,10 +111,14 @@ function DocSettings:open(docfile)
     local candidates = {}
     -- New sidecar file
     table.insert(candidates, buildCandidate(new.sidecar_file))
+    -- Backup file of new sidecar file
+    table.insert(candidates, buildCandidate(new.sidecar_file .. ".old"))
     -- Legacy sidecar file
     table.insert(candidates, buildCandidate(new.legacy_sidecar_file))
     -- Legacy history folder
     table.insert(candidates, buildCandidate(new.history_file))
+    -- Backup file in legacy history folder
+    table.insert(candidates, buildCandidate(new.history_file .. ".old"))
     -- Legacy kpdfview setting
     table.insert(candidates, buildCandidate(docfile..".kpdfview.lua"))
     table.sort(candidates, function(l, r)
@@ -128,7 +133,11 @@ function DocSettings:open(docfile)
     for _, k in pairs(candidates) do
         ok, stored = pcall(dofile, k[1])
         if ok then
+            logger.dbg("data is read from ", k[1])
             break
+        else
+            logger.dbg(k[1], " is invalid, remove.")
+            os.remove(k[1])
         end
     end
     if ok and stored then
@@ -172,6 +181,11 @@ function DocSettings:flush()
     local s_out = dump(self.data)
     os.setlocale('C', 'numeric')
     for _, f in pairs(serials) do
+        if lfs.attributes(f, "mode") == "file" then
+            logger.dbg("Rename ", f, " to ", f .. ".old")
+            os.rename(f, f .. ".old")
+        end
+        logger.dbg("Write to ", f)
         local f_out = io.open(f, "w")
         if f_out ~= nil then
             f_out:write("-- we can read Lua syntax here!\nreturn ")
@@ -183,7 +197,8 @@ function DocSettings:flush()
             and not G_reader_settings:readSetting(
                         "preserve_legacy_docsetting") then
                 for _, k in pairs(self.candidates) do
-                    if k[1] ~= f then
+                    if k[1] ~= f and k[1] ~= f .. ".old" then
+                        logger.dbg("Remove legacy file ", k[1])
                         os.remove(k[1])
                         -- We should not remove sidecar folder, as it may
                         -- contain Kindle history files.
diff --git a/spec/unit/docsettings_spec.lua b/spec/unit/docsettings_spec.lua
index ccec9034f..490ed55f9 100644
--- a/spec/unit/docsettings_spec.lua
+++ b/spec/unit/docsettings_spec.lua
@@ -92,4 +92,55 @@ describe("docsettings module", function()
         local name_from_history = docsettings:getNameFromHistory(history_path)
         assert.is.same(file, path_from_history .. "/" .. name_from_history)
     end)
+
+    it("should reserve last good file", function()
+        local file = "file.pdf"
+        local d = docsettings:open(file)
+        d:saveSetting("a", "a")
+        d:flush()
+        -- metadata.pdf.lua should be generated.
+        assert.Equals("file", lfs.attributes(d.sidecar_file, "mode"))
+        d:close()
+        -- metadata.pdf.lua and metadata.pdf.lua.old should be generated.
+        assert.Equals("file", lfs.attributes(d.sidecar_file, "mode"))
+        assert.Equals("file", lfs.attributes(d.sidecar_file .. ".old", "mode"))
+
+        -- write some garbage to sidecar-file.
+        local f_out = io.open(d.sidecar_file, "w")
+        f_out:write("bla bla bla")
+        f_out:close()
+
+        d = docsettings:open(file)
+        -- metadata.pdf.lua should be removed.
+        assert.are.not_equal("file", lfs.attributes(d.sidecar_file, "mode"))
+        assert.Equals("file", lfs.attributes(d.sidecar_file .. ".old", "mode"))
+        assert.Equals("a", d:readSetting("a"))
+        d:saveSetting("a", "b")
+        d:close()
+        -- metadata.pdf.lua should be generated.
+        assert.Equals("file", lfs.attributes(d.sidecar_file, "mode"))
+        assert.Equals("file", lfs.attributes(d.sidecar_file .. ".old", "mode"))
+        -- The contents in sidecar_file and sidecar_file.old are different.
+        -- a:b v.s. a:a
+
+        d = docsettings:open(file)
+        -- The content should come from sidecar_file.
+        assert.Equals("b", d:readSetting("a"))
+        -- write some garbage to sidecar-file.
+        f_out = io.open(d.sidecar_file, "w")
+        f_out:write("bla bla bla")
+        f_out:close()
+
+        -- do not flush the result, open docsettings again.
+        d = docsettings:open(file)
+        -- metadata.pdf.lua should be removed.
+        assert.are.not_equal("file", lfs.attributes(d.sidecar_file, "mode"))
+        assert.Equals("file", lfs.attributes(d.sidecar_file .. ".old", "mode"))
+        -- The content should come from sidecar_file.old.
+        assert.Equals("a", d:readSetting("a"))
+        d:close()
+        -- metadata.pdf.lua should be generated.
+        assert.Equals("file", lfs.attributes(d.sidecar_file, "mode"))
+        assert.Equals("file", lfs.attributes(d.sidecar_file .. ".old", "mode"))
+    end)
 end)