From 7461e396dd0a5f94261b0b11f6f154221bcc8db4 Mon Sep 17 00:00:00 2001
From: Hzj_jie <Hzj_jie@hotmail.com>
Date: Thu, 12 Oct 2017 10:52:01 -0700
Subject: [PATCH] [chore] Ignore empty files and tables in DocSettings (#3348)

---
 frontend/docsettings.lua       | 20 ++++++-----
 spec/unit/docsettings_spec.lua | 63 ++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/frontend/docsettings.lua b/frontend/docsettings.lua
index 3632aa1fe..f219fe075 100644
--- a/frontend/docsettings.lua
+++ b/frontend/docsettings.lua
@@ -15,6 +15,7 @@ local DocSettings = {}
 local HISTORY_DIR = DataStorage:getHistoryDir()
 
 local function buildCandidate(file_path)
+    -- Ignore empty files.
     if lfs.attributes(file_path, "mode") == "file" then
         return { file_path, lfs.attributes(file_path, "modification") }
     else
@@ -92,7 +93,6 @@ end
 function DocSettings:open(docfile)
     -- TODO(zijiehe): Remove history_path, use only sidecar.
     local new = {}
-    local ok, stored
     new.history_file = self:getHistoryPath(docfile)
 
     local sidecar = self:getSidecarDir(docfile)
@@ -130,15 +130,19 @@ function DocSettings:open(docfile)
                                    return l[2] > r[2]
                                end
                            end)
+    local ok, stored
     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])
+        -- Ignore empty files
+        if lfs.attributes(k[1], "size") > 0 then
+            ok, stored = pcall(dofile, k[1])
+            -- Ignore the empty table.
+            if ok and next(stored) ~= nil then
+                logger.dbg("data is read from ", k[1])
+                break
+            end
         end
+        logger.dbg(k[1], " is invalid, remove.")
+        os.remove(k[1])
     end
     if ok and stored then
         new.data = stored
diff --git a/spec/unit/docsettings_spec.lua b/spec/unit/docsettings_spec.lua
index 490ed55f9..732be2229 100644
--- a/spec/unit/docsettings_spec.lua
+++ b/spec/unit/docsettings_spec.lua
@@ -143,4 +143,67 @@ describe("docsettings module", function()
         assert.Equals("file", lfs.attributes(d.sidecar_file, "mode"))
         assert.Equals("file", lfs.attributes(d.sidecar_file .. ".old", "mode"))
     end)
+
+    describe("ignore empty sidecar file", function()
+        it("should ignore empty 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"))
+
+            -- reset the sidecar_file to an empty file.
+            local f_out = io.open(d.sidecar_file, "w")
+            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
+        end)
+
+        it("should ignore empty table", 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"))
+
+            -- reset the sidecar_file to an empty file.
+            local f_out = io.open(d.sidecar_file, "w")
+            f_out:write("{                               }                 ")
+            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
+        end)
+    end)
 end)