Update storage class and test coverage of it

novelwriter/core/storage.py

@@ -23,6 +23,7 @@ along with this program. If not, see <https://www.gnu.org/licenses/>.
 """
 from __future__ import annotations
 
+import json
 import logging
 
 from time import time
@@ -88,13 +89,11 @@ class NWStorage:
         """Return the path used for project content. The folder must
         already exist, otherwise this property is None.
         """
-        if self._runtimePath is not None:
+        if isinstance(self._runtimePath, Path):
             contentPath = self._runtimePath / "content"
             if contentPath.is_dir():
                 return contentPath
-            else:
+        logger.error("Content path cannot be resolved")
-                logger.error("Path not found: %s", contentPath)
-                return None
         return None
 
     ##
@@ -143,7 +142,6 @@ class NWStorage:
         if self._openMode == self.MODE_INPLACE:
             # Nothing to do, so we just return
             return True
-
         return True
 
     def closeSession(self):
@@ -158,28 +156,26 @@ class NWStorage:
 
     def getXmlReader(self) -> ProjectXMLReader | None:
         """Return a properly configured ProjectXMLReader instance."""
-        if self._runtimePath is None:
+        if isinstance(self._runtimePath, Path):
-            return None
+            projFile = self._runtimePath / nwFiles.PROJ_FILE
-        projFile = self._runtimePath / nwFiles.PROJ_FILE
+            return ProjectXMLReader(projFile)
-        xmlReader = ProjectXMLReader(projFile)
+        return None
-        return xmlReader
 
     def getXmlWriter(self) -> ProjectXMLWriter | None:
         """Return a properly configured ProjectXMLWriter instance."""
-        if self._runtimePath is None:
+        if isinstance(self._runtimePath, Path):
-            return None
+            return ProjectXMLWriter(self._runtimePath)
-        xmlWriter = ProjectXMLWriter(self._runtimePath)
+        return None
-        return xmlWriter
 
     def getDocument(self, tHandle: str | None) -> NWDocument:
         """Return a document wrapper object."""
-        if self._runtimePath is not None:
+        if isinstance(self._runtimePath, Path):
             return NWDocument(self._project, tHandle)
         return NWDocument(self._project, None)
 
     def getMetaFile(self, fileName: str) -> Path | None:
         """Return the path to a file in the project meta folder."""
-        if self._runtimePath is not None:
+        if isinstance(self._runtimePath, Path):
             return self._runtimePath / "meta" / fileName
         return None
 
@@ -320,13 +316,15 @@ class NWStorage:
             # need for the remaning checks.
             return True
 
+        legacy = _LegacyStorage(self._project)
+
         # Check for legacy data folders
         for child in path.iterdir():
             if child.is_dir() and child.name.startswith("data_"):
-                self._legacyDataFolder(path, child)
+                legacy.legacyDataFolder(path, child)
 
         # Check for no longer used files, and delete them
-        self._deprecatedFiles(path)
+        legacy.deprecatedFiles(path)
 
         return True
 
@@ -334,7 +332,21 @@ class NWStorage:
     #  Legacy Project Data Handlers
     ##
 
-    def _legacyDataFolder(self, path: Path, child: Path):
+# END Class NWStorage
+
+
+class _LegacyStorage:
+    """Core: Legacy Storage Converter Utils
+
+    A class with various functions to convert old file formats and
+    file/folder layout to the current project format.
+    """
+
+    def __init__(self, project: NWProject):
+        self._project = project
+        return
+
+    def legacyDataFolder(self, path: Path, child: Path):
         """Handle the content of a legacy data folder from a version 1.0
         project.
         """
@@ -373,15 +385,20 @@ class NWStorage:
 
         return
 
-    def _deprecatedFiles(self, path: Path):
+    def deprecatedFiles(self, path: Path):
         """Handle files that are no longer used by novelWriter."""
-        sessLog = path / "meta" / "sessionStats.log"
+        self._convertOldWordList(  # Changed in 2.1 Beta 1
-        if sessLog.is_file():
+            path / "meta" / "wordlist.txt",
-            self._convertOldLogFile(sessLog, path / "meta" / nwFiles.SESS_FILE)
+            path / "meta" / nwFiles.DICT_FILE
-
+        )
-        wordList = path / "meta" / "wordlist.txt"
+        self._convertOldLogFile(  # Changed in 2.1 Beta 1
-        if wordList.is_file():
+            path / "meta" / "sessionStats.log",
-            self._convertOldWordList(wordList)
+            path / "meta" / nwFiles.SESS_FILE
+        )
+        self._convertOldOptionsFile(  # Changed in 2.1 Beta 1
+            path / "meta" / "guiOptions.json",
+            path / "meta" / nwFiles.OPTS_FILE
+        )
 
         remove = [
             path / "meta" / "tagsIndex.json",          # Renamed in 2.1 Beta 1
@@ -406,55 +423,51 @@ class NWStorage:
                 except Exception as exc:
                     logger.warning("Failed to delete: %s", item, exc_info=exc)
 
-        # Renamed in 2.1 Beta 1, but this file we want to keep
-        oldOpt = path / "meta" / "guiOptions.json"
-        newOpt = path / "meta" / nwFiles.OPTS_FILE
-        if oldOpt.is_file():
-            try:
-                oldOpt.rename(newOpt)
-                logger.info("Renamed: %s > %s", oldOpt, newOpt)
-            except Exception as exc:
-                logger.warning("Failed to rename: %s", oldOpt, exc_info=exc)
-
         return
 
-    def _convertOldWordList(self, wordList: Path) -> bool:
+    ##
+    #  Internal Functions
+    ##
+
+    def _convertOldWordList(self, wordList: Path, wordJson: Path):
         """Convert the old word list plain text file to new format."""
-        if not wordList.exists():
+        if wordJson.exists() or not wordList.exists():
-            # Nothing to convert
+            # If the new file already exists, we won't overwrite it
-            return True
+            return
 
         userDict = UserDictionary(self._project)
         try:
+            logger.info("Converting: %s", wordList)
             with open(wordList, mode="r", encoding="utf-8") as fObj:
                 for line in fObj:
                     word = line.strip()
                     if word:
                         userDict.add(word)
 
-            # Dave dictionary and clean up old file
+            # Save dictionary and clean up old file
             userDict.save()
+            assert wordJson.exists()
             wordList.unlink()
 
         except Exception:
             logger.error("Failed to convert old word list file")
             logException()
-            return False
 
-        return True
+        return
 
-    def _convertOldLogFile(self, sessLog: Path, sessJson: Path) -> bool:
+    def _convertOldLogFile(self, sessLog: Path, sessJson: Path):
         """Convert the old text log file format to the new JSON Lines
         format.
         """
         if sessJson.exists() or not sessLog.exists():
             # If the new file already exists, we won't overwrite it
-            return True
+            return
 
         try:
             data = []
             offset = 0
             session = self._project.session
+            logger.info("Converting: %s", sessLog)
             with open(sessLog, mode="r", encoding="utf-8") as fObj:
                 for record in fObj:
                     bits = record.split()
@@ -480,8 +493,40 @@ class NWStorage:
         except Exception:
             logger.error("Failed to convert old stats file")
             logException()
-            return False
 
-        return True
+        return
 
-# END Class NWStorage
+    def _convertOldOptionsFile(self, optsOld: Path, optsNew: Path):
+        """Convert the old options state file format to the format."""
+        if optsNew.exists() or not optsOld.exists():
+            # If the new file already exists, we won't overwrite it
+            return
+
+        try:
+            data = {}
+            logger.info("Converting: %s", optsOld)
+            with open(optsOld, mode="r", encoding="utf-8") as fObj:
+                data = json.load(fObj)
+
+                # Convert Outline Values
+                state = {}
+                outline = data.get("GuiOutline", {})
+                hidden = outline.get("columnHidden", {})
+                width = outline.get("columnWidth", {})
+                for key in outline.get("headerOrder", []):
+                    state[key] = [hidden.get(key, False), width.get(key, 100)]
+                data["columnState"] = state
+
+            with open(optsNew, mode="w", encoding="utf-8") as fObj:
+                json.dump({"novelWriter.guiOptions": data}, fObj, indent=2)
+
+            # If we're here, we remove the old file
+            optsOld.unlink()
+
+        except Exception:
+            logger.error("Failed to convert old options file")
+            logException()
+
+        return
+
+# END Class _LegacyStorage

novelwriter/error.py

@@ -95,7 +95,7 @@ class NWErrorMessage(QDialog):
         self.mainBox.setSpacing(16)
 
         # Pick a random window title from a set of error messages by
-        # Hex, the computer, from Discworld
+        # Hex the computer, Unseen University, Ankh-Morpork, Discworld
         self.setWindowTitle([
             "+++ Out of Cheese Error +++",
             "+++ Divide by Cucumber Error +++",

novelwriter/gui/doceditor.py

@@ -611,9 +611,9 @@ class GuiDocEditor(QTextEdit):
 
     def getText(self):
         """Get the text content of the current document. This method uses
-        QTextDocument->toRawText instead of toPlainText(). The former preserves
+        QTextDocument->toRawText instead of toPlainText. The former preserves
         non-breaking spaces, the latter does not. We still want to get rid of
-        page and line separators though.
+        paragraph and line separators though.
         See: https://doc.qt.io/qt-5/qtextdocument.html#toPlainText
         """
         theText = self.document().toRawText()

tests/test_core/test_core_storage.py

@@ -19,22 +19,24 @@ You should have received a copy of the GNU General Public License
 along with this program. If not, see <https://www.gnu.org/licenses/>.
 """
 
-from zipfile import ZipFile
+import json
 import pytest
 
+from pathlib import Path
+from zipfile import ZipFile
+
 from tools import C, buildTestProject, writeFile
 from mocked import causeOSError
 
 from novelwriter import CONFIG
 from novelwriter.constants import nwFiles
 from novelwriter.core.project import NWProject
-from novelwriter.core.storage import NWStorage
+from novelwriter.core.storage import NWStorage, _LegacyStorage
 from novelwriter.core.projectxml import ProjectXMLReader, ProjectXMLWriter
 
 
 class MockProject:
     """Test class for projects."""
-
     pass
 
 
@@ -112,7 +114,7 @@ def testCoreStorage_LockFile(monkeypatch, fncPath):
     """Test the project lock file."""
     monkeypatch.setattr("novelwriter.core.storage.time", lambda: 1000.0)
 
-    storage = NWStorage(MockProject())
+    storage = NWStorage(MockProject())  # type: ignore
     assert storage.isOpen() is False
 
     # Project not open, so cannot read/write lock file
@@ -169,10 +171,46 @@ def testCoreStorage_LockFile(monkeypatch, fncPath):
 # END Test testCoreStorage_LockFile
 
 
+@pytest.mark.core
+def testCoreStorage_ZipIt(monkeypatch, mockGUI, fncPath, tstPaths, mockRnd):
+    """Test making a zip archive of a project."""
+    zipFile = tstPaths.tmpDir / "project.zip"
+
+    theProject = NWProject(mockGUI)
+    storage = theProject.storage
+    assert storage.zipIt(zipFile) is False
+
+    # Make a project
+    mockRnd.reset()
+    buildTestProject(theProject, fncPath)
+
+    # Fail to create archive
+    with monkeypatch.context() as mp:
+        mp.setattr("novelwriter.core.storage.ZipFile.write", causeOSError)
+        assert storage.zipIt(zipFile) is False
+
+    # Create archive
+    assert storage.zipIt(zipFile) is True
+
+    # Check content
+    with ZipFile(zipFile, mode="r") as archive:
+        names = archive.namelist()
+        assert nwFiles.PROJ_FILE in names
+        assert f"meta/{nwFiles.OPTS_FILE}" in names
+        assert f"meta/{nwFiles.INDEX_FILE}" in names
+        assert f"content/{C.hTitlePage}.nwd" in names
+        assert f"content/{C.hChapterDoc}.nwd" in names
+        assert f"content/{C.hSceneDoc}.nwd" in names
+
+    theProject.closeProject()
+
+# END Test testCoreStorage_ZipIt
+
+
 @pytest.mark.core
 def testCoreStorage_PrepareStorage(monkeypatch, fncPath):
     """Test the project path preparation functions."""
-    storage = NWStorage(MockProject())
+    storage = NWStorage(MockProject())  # type: ignore
     assert storage.isOpen() is False
 
     # No path set
@@ -208,9 +246,18 @@ def testCoreStorage_PrepareStorage(monkeypatch, fncPath):
     storage._runtimePath = fncPath
     assert storage._prepareStorage(checkLegacy=False, newProject=True) is False
 
-    # Legacy Data Folder
+# END Test testCoreStorage_PrepareStorage
-    # ==================
+
+
+@pytest.mark.core
+def testCoreStorage_LegacyDataFolder(monkeypatch, fncPath):
+    """Test project file format 1.0 folder structure conversion."""
+    project = MockProject()
+    storage = NWStorage(project)  # type: ignore
+    assert storage.isOpen() is False
     storage._runtimePath = fncPath
+    assert storage._prepareStorage() is True
+    legacy = _LegacyStorage(project)  # type: ignore
 
     data = []
     files = []
@@ -235,7 +282,7 @@ def testCoreStorage_PrepareStorage(monkeypatch, fncPath):
 
     # Process folders
     for i in range(9):
-        storage._legacyDataFolder(fncPath, data[i])
+        legacy.legacyDataFolder(fncPath, data[i])
 
     # Files form 0 to 8 should now be in content
     for c in "012345678":
@@ -250,14 +297,14 @@ def testCoreStorage_PrepareStorage(monkeypatch, fncPath):
     assert data[8].exists()
 
     # So does folder X, which is invalid
-    storage._legacyDataFolder(fncPath, data[16])
+    legacy.legacyDataFolder(fncPath, data[16])
     assert data[16].exists()
 
     # Fail cleanup of folder 9
     with monkeypatch.context() as mp:
         mp.setattr("pathlib.Path.rename", causeOSError)
         mp.setattr("pathlib.Path.unlink", causeOSError)
-        storage._legacyDataFolder(fncPath, data[9])
+        legacy.legacyDataFolder(fncPath, data[9])
         assert data[9].exists()
         assert not (fncPath / "content" / "9000000000009.nwd").exists()
 
@@ -266,10 +313,24 @@ def testCoreStorage_PrepareStorage(monkeypatch, fncPath):
     for c in "0123456789abcdef":
         assert (fncPath / "content" / f"{c}00000000000{c}.nwd").exists()
 
-    # Deprecated Files
+# END Test testCoreStorage_LegacyDataFolder
-    # ================
+
+
+@pytest.mark.core
+def testCoreStorage_DeprecatedFiles(monkeypatch, fncPath):
+    """Test cleanup of deprecated files."""
+    project = MockProject()
+    storage = NWStorage(project)  # type: ignore
+    assert storage.isOpen() is False
+    storage._runtimePath = fncPath
+    assert storage._prepareStorage() is True
+    legacy = _LegacyStorage(project)  # type: ignore
+
+    # Files/Folders to be Deleted or Renamed
+    # ======================================
 
     remove = [
+        fncPath / "meta" / "tagsIndex.json",
         fncPath / "meta" / "mainOptions.json",
         fncPath / "meta" / "exportOptions.json",
         fncPath / "meta" / "outlineOptions.json",
@@ -286,48 +347,130 @@ def testCoreStorage_PrepareStorage(monkeypatch, fncPath):
 
     with monkeypatch.context() as mp:
         mp.setattr("pathlib.Path.unlink", causeOSError)
-        storage._deprecatedFiles(fncPath)
+        legacy.deprecatedFiles(fncPath)
         for depFile in remove:
             assert depFile.exists()
 
-    storage._deprecatedFiles(fncPath)
+    legacy.deprecatedFiles(fncPath)
     for depFile in remove:
         assert not depFile.exists()
 
-# END Test testCoreStorage_PrepareStorage
+# END Test testCoreStorage_DeprecatedFiles
 
 
 @pytest.mark.core
-def testCoreStorage_ZipIt(monkeypatch, mockGUI, fncPath, tstPaths, mockRnd):
+def testCoreStorage_OldFormatConvert(monkeypatch, mockGUI, fncPath):
-    """Test making a zip archive of a project."""
+    """Test cleanup of deprecated files that needs to be converted."""
-    zipFile = tstPaths.tmpDir / "project.zip"
+    project = NWProject(mockGUI)
+    buildTestProject(project, fncPath)
+    legacy = _LegacyStorage(project)
 
-    theProject = NWProject(mockGUI)
+    # The build project functions saves the project, so we must delete
-    storage = theProject.storage
+    # the old gui options file
-    assert storage.zipIt(zipFile) is False
+    (fncPath / "meta" / nwFiles.OPTS_FILE).unlink()
 
-    # Make a project
+    # Word List
-    mockRnd.reset()
+    wordListOld: Path = fncPath / "meta" / "wordlist.txt"
-    buildTestProject(theProject, fncPath)
+    wordListNew: Path = fncPath / "meta" / nwFiles.DICT_FILE
 
-    # Fail to create archive
+    wordListOld.write_text((
+        "word_a\n"
+        "word_b\n"
+        "word_c\n"
+    ), encoding="utf-8")
+
+    assert wordListOld.exists() is True
+    assert wordListNew.exists() is False
+
+    # Log File
+    sessLogOld: Path = fncPath / "meta" / "sessionStats.log"
+    sessLogNew: Path = fncPath / "meta" / nwFiles.SESS_FILE
+
+    sessLogOld.write_text((
+        "# Offset 150\n"
+        "# Start Time         End Time                Novel     Notes    Idle\n"
+        "2021-02-02 02:02:02  2021-02-02 03:03:03       200      200       10\n"
+        "2021-03-03 03:03:03  2021-03-03 04:04:04       300      300       20\n"
+    ), encoding="utf-8")
+
+    assert sessLogOld.exists() is True
+    assert sessLogNew.exists() is False
+
+    # Options File
+    optionsOld: Path = fncPath / "meta" / "guiOptions.json"
+    optionsNew: Path = fncPath / "meta" / nwFiles.OPTS_FILE
+
+    optionsOld.write_text(json.dumps({
+        "GuiProjectSettings": {
+            "winWidth": 570,
+            "winHeight": 375,
+        },
+        "GuiOutline": {
+            "headerOrder": ["TITLE", "LEVEL", "LABEL", "LINE"],
+            "columnWidth": {"TITLE": 325, "LEVEL": 40, "LABEL": 267, "LINE": 40},
+            "columnHidden": {"TITLE": False, "LEVEL": True, "LABEL": False, "LINE": True},
+        },
+    }, indent=2), encoding="utf-8")
+
+    assert optionsOld.exists() is True
+    assert optionsNew.exists() is False
+
+    # Check Failure
     with monkeypatch.context() as mp:
-        mp.setattr("novelwriter.core.storage.ZipFile.write", causeOSError)
+        mp.setattr("builtins.open", causeOSError)
-        assert storage.zipIt(zipFile) is False
+        legacy.deprecatedFiles(fncPath)
+        assert wordListOld.exists() is True
+        assert wordListNew.exists() is False
+        assert sessLogOld.exists() is True
+        assert sessLogNew.exists() is False
+        assert optionsOld.exists() is True
+        assert optionsNew.exists() is False
 
-    # Create archive
+    # Check Success
-    assert storage.zipIt(zipFile) is True
+    legacy.deprecatedFiles(fncPath)
+    assert wordListOld.exists() is False
+    assert wordListNew.exists() is True
+    assert sessLogOld.exists() is False
+    assert sessLogNew.exists() is True
+    assert optionsOld.exists() is False
+    assert optionsNew.exists() is True
 
-    # Check content
+    # Check Word List
-    with ZipFile(zipFile, mode="r") as archive:
+    data = json.loads(wordListNew.read_text(encoding="utf-8"))
-        names = archive.namelist()
+    assert "word_a" in data["novelWriter.userDict"]
-        assert nwFiles.PROJ_FILE in names
+    assert "word_b" in data["novelWriter.userDict"]
-        assert f"meta/{nwFiles.OPTS_FILE}" in names
+    assert "word_c" in data["novelWriter.userDict"]
-        assert f"meta/{nwFiles.INDEX_FILE}" in names
-        assert f"content/{C.hTitlePage}.nwd" in names
-        assert f"content/{C.hChapterDoc}.nwd" in names
-        assert f"content/{C.hSceneDoc}.nwd" in names
 
-    theProject.closeProject()
+    # Check Session Log
+    data = list(project.session.iterRecords())
+    assert data[0] == {"type": "initial", "offset": 150}
+    assert data[1] == {
+        "type": "record",
+        "start": "2021-02-02 02:02:02",
+        "end": "2021-02-02 03:03:03",
+        "novel": 200,
+        "notes": 200,
+        "idle": 10,
+    }
+    assert data[2] == {
+        "type": "record",
+        "start": "2021-03-03 03:03:03",
+        "end": "2021-03-03 04:04:04",
+        "novel": 300,
+        "notes": 300,
+        "idle": 20,
+    }
 
-# END Test testCoreStorage_ZipIt
+    # Check Options File
+    data = json.loads(optionsNew.read_text(encoding="utf-8"))
+    assert data["novelWriter.guiOptions"]["GuiProjectSettings"] == {
+        "winWidth": 570, "winHeight": 375
+    }
+    assert data["novelWriter.guiOptions"]["columnState"] == {
+        "TITLE": [False, 325],
+        "LEVEL": [True, 40],
+        "LABEL": [False, 267],
+        "LINE": [True, 40]
+    }
+
+# END Test testCoreStorage_OldFormatConvert