Simplify the conversion of very old project data (#1083)

novelwriter/core/project.py

@@ -438,7 +438,7 @@ class NWProject():
         legacyList = []  # Cleanup is done later
         for projItem in os.listdir(self.projPath):
             logger.verbose("Project contains: %s", projItem)
-            if projItem.startswith("data_"):
+            if projItem.startswith("data_") and len(projItem) == 6:
                 legacyList.append(projItem)
 
         # Project Lock
@@ -500,7 +500,7 @@ class NWProject():
         # Check File Type
         # ===============
 
-        if not nwxRoot == "novelWriterXML":
+        if nwxRoot != "novelWriterXML":
             self.mainGui.makeAlert(self.tr(
                 "Project file does not appear to be a novelWriterXML file."
             ), nwAlert.ERROR)
@@ -642,11 +642,14 @@ class NWProject():
 
         # Sort out old file locations
         if legacyList:
-            errList = []
+            try:
-            for projItem in legacyList:
+                for projItem in legacyList:
-                errList = self._legacyDataFolder(projItem, errList)
+                    self._legacyDataFolder(projItem)
-            if errList:
+            except Exception:
-                self.mainGui.makeAlert(errList, nwAlert.ERROR)
+                self.mainGui.makeAlert(self.tr(
+                    "There was an error while converting project version 1.0. "
+                    "Some data may not have been preserved."
+                ), nwAlert.ERROR)
 
         # Clean up no longer used files
         self._deprecatedFiles()
@@ -1558,82 +1561,37 @@ class NWProject():
     #  Legacy Data Structure Handlers
     ##
 
-    def _legacyDataFolder(self, theFolder, errList):
+    def _legacyDataFolder(self, dataDir):
         """Clean up legacy data folders.
         """
-        theData = os.path.join(self.projPath, theFolder)
+        dataPath = os.path.join(self.projPath, dataDir)
-        if not os.path.isdir(theData):
+        if not os.path.isdir(dataPath):
-            errList.append(self.tr("Not a folder: {0}").format(theData))
+            return False
-            return errList
 
-        logger.info("Old data folder %s found", theFolder)
+        logger.info("Old data folder found: %s", dataDir)
 
         # Move Documents to Content
-        for dataItem in os.listdir(theData):
+        for dataItem in os.listdir(dataPath):
-            theFile = os.path.join(theData, dataItem)
+            dataFile = os.path.join(dataPath, dataItem)
-            if not os.path.isfile(theFile):
+            if not os.path.isfile(dataFile):
-                theErr = self._moveUnknownItem(theData, dataItem)
-                if theErr:
-                    errList.append(theErr)
                 continue
 
             if len(dataItem) == 21 and dataItem.endswith("_main.nwd"):
-                tHandle = theFolder[-1]+dataItem[:12]
+                tHandle = dataDir[-1] + dataItem[:12]
-                newPath = os.path.join(self.projContent, tHandle+".nwd")
+                newPath = os.path.join(self.projContent, f"{tHandle}.nwd")
-                try:
+                os.rename(dataFile, newPath)
-                    os.rename(theFile, newPath)
+                logger.info("Moved file: %s", dataFile)
-                    logger.info("Moved file: %s", theFile)
-                    logger.info("New location: %s", newPath)
-                except Exception:
-                    errList.append(self.tr("Could not move: {0}").format(theFile))
-                    logger.error("Could not move: %s", theFile)
-                    logException()
 
             elif len(dataItem) == 21 and dataItem.endswith("_main.bak"):
-                try:
+                os.unlink(dataFile)
-                    os.unlink(theFile)
+                logger.info("Deleted file: %s", dataFile)
-                    logger.info("Deleted file: %s", theFile)
-                except Exception:
-                    errList.append(self.tr("Could not delete: {0}").format(theFile))
-                    logger.error("Could not delete: %s", theFile)
-                    logException()
-
-            else:
-                theErr = self._moveUnknownItem(theData, dataItem)
-                if theErr:
-                    errList.append(theErr)
 
         # Remove Data Folder
-        try:
+        if not os.listdir(dataPath):
-            os.rmdir(theData)
+            os.rmdir(dataPath)
-            logger.info("Deleted folder: %s", theFolder)
+            logger.info("Deleted folder: %s", dataDir)
-        except Exception:
-            errList.append(self.tr("Could not delete: {0}").format(theFolder))
-            logger.error("Could not delete: %s", theFolder)
-            logException()
 
-        return errList
+        return True
-
-    def _moveUnknownItem(self, theDir, theItem):
-        """Move an item that doesn't belong in the project folder to
-        a junk folder.
-        """
-        theJunk = os.path.join(self.projPath, "junk")
-        if not self._checkFolder(theJunk):
-            return self.tr("Could not make folder: {0}").format(theJunk)
-
-        theSrc = os.path.join(theDir, theItem)
-        theDst = os.path.join(theJunk, theItem)
-
-        try:
-            os.rename(theSrc, theDst)
-            logger.info("Moved to junk: %s", theSrc)
-        except Exception:
-            logger.error("Could not move item %s to junk", theSrc)
-            logException()
-            return self.tr("Could not move item {0} to {1}.").format(theSrc, theJunk)
-
-        return ""
 
     def _deprecatedFiles(self):
         """Delete files that are no longer used by novelWriter.

tests/test_core/test_core_project.py

@@ -463,12 +463,15 @@ def testCoreProject_Open(monkeypatch, nwMinimal, mockGUI):
     os.rename(oName, rName)
 
     # Add some legacy stuff that cannot be removed
-    writeFile(os.path.join(nwMinimal, "junk"), "stuff")
+    with monkeypatch.context() as mp:
-    os.mkdir(os.path.join(nwMinimal, "data_0"))
+        mp.setattr(theProject, "_legacyDataFolder", causeOSError)
-    writeFile(os.path.join(nwMinimal, "data_0", "junk"), "stuff")
+        os.mkdir(os.path.join(nwMinimal, "data_0"))
-    mockGUI.clear()
+        writeFile(os.path.join(nwMinimal, "data_0", "123456789abc_main.nwd"), "stuff")
-    assert theProject.openProject(nwMinimal) is True
+        writeFile(os.path.join(nwMinimal, "data_0", "123456789abc_main.bak"), "stuff")
-    assert "data_0" in mockGUI.lastAlert
+        mockGUI.clear()
+        assert theProject.openProject(nwMinimal) is True
+        assert "version 1.0" in mockGUI.lastAlert
+
     assert theProject.closeProject()
 
 # END Test testCoreProject_Open
@@ -1141,14 +1144,6 @@ def testCoreProject_OldFormat(mockGUI, nwOldProj):
         os.path.join(nwOldProj, "meta",  "sessionLogOptions.json"),
     ]
 
-    # Add some files that shouldn't be there
-    deleteFiles.append(os.path.join(nwOldProj, "data_f", "whatnow.nwd"))
-    deleteFiles.append(os.path.join(nwOldProj, "data_f", "whatnow.txt"))
-
-    # Add some folders that shouldn't be there
-    os.mkdir(os.path.join(nwOldProj, "stuff"))
-    os.mkdir(os.path.join(nwOldProj, "data_1", "stuff"))
-
     # Create mock files
     os.mkdir(os.path.join(nwOldProj, "cache"))
     for aFile in deleteFiles:
@@ -1162,7 +1157,6 @@ def testCoreProject_OldFormat(mockGUI, nwOldProj):
     for aFile in deleteFiles:
         assert not os.path.isfile(aFile)
 
-    assert not os.path.isdir(os.path.join(nwOldProj, "data_1", "stuff"))
     assert not os.path.isdir(os.path.join(nwOldProj, "data_1"))
     assert not os.path.isdir(os.path.join(nwOldProj, "data_7"))
     assert not os.path.isdir(os.path.join(nwOldProj, "data_8"))
@@ -1170,12 +1164,6 @@ def testCoreProject_OldFormat(mockGUI, nwOldProj):
     assert not os.path.isdir(os.path.join(nwOldProj, "data_a"))
     assert not os.path.isdir(os.path.join(nwOldProj, "data_f"))
 
-    # Check stuff that has been moved
-    assert os.path.isdir(os.path.join(nwOldProj, "junk"))
-    assert os.path.isdir(os.path.join(nwOldProj, "junk", "stuff"))
-    assert os.path.isfile(os.path.join(nwOldProj, "junk", "whatnow.nwd"))
-    assert os.path.isfile(os.path.join(nwOldProj, "junk", "whatnow.txt"))
-
     # Check that files we want to keep are in the right place
     assert os.path.isdir(os.path.join(nwOldProj, "cache"))
     assert os.path.isdir(os.path.join(nwOldProj, "content"))
@@ -1217,7 +1205,7 @@ def testCoreProject_LegacyData(monkeypatch, mockGUI, fncDir):
 
     with monkeypatch.context() as mp:
         mp.setattr("os.unlink", causeOSError)
-        assert not theProject._deprecatedFiles()
+        assert theProject._deprecatedFiles() is False
 
     assert theProject._deprecatedFiles()
     assert not os.path.isfile(tstFile)
@@ -1226,63 +1214,36 @@ def testCoreProject_LegacyData(monkeypatch, mockGUI, fncDir):
     tstFile = os.path.join(fncDir, "data_0")
     writeFile(tstFile, "stuff")
     assert os.path.isfile(tstFile)
-
+    assert theProject._legacyDataFolder(tstFile) is False
-    errList = []
-    errList = theProject._legacyDataFolder(tstFile, errList)
-    assert len(errList) > 0
-
-    # Move folder in data folder, shouldn't be there
-    tstData = os.path.join(fncDir, "data_1")
-    errItem = os.path.join(fncDir, "data_1", "stuff")
-    os.mkdir(tstData)
-    os.mkdir(errItem)
-    assert os.path.isdir(tstData)
-    assert os.path.isdir(errItem)
-
-    # This causes a failure to create the 'junk' folder
-    with monkeypatch.context() as mp:
-        mp.setattr("os.mkdir", causeOSError)
-        errList = []
-        errList = theProject._legacyDataFolder(tstData, errList)
-        assert len(errList) > 0
-
-    # This causes a failure to move 'stuff' to 'junk'
-    with monkeypatch.context() as mp:
-        mp.setattr("os.rename", causeOSError)
-        errList = []
-        errList = theProject._legacyDataFolder(tstData, errList)
-        assert len(errList) > 0
-
-    # This should be successful
-    errList = []
-    errList = theProject._legacyDataFolder(tstData, errList)
-    assert len(errList) == 0
-    assert os.path.isdir(os.path.join(fncDir, "junk", "stuff"))
 
     # Check renaming/deleting of old document files
-    tstData = os.path.join(fncDir, "data_2")
+    tstData2 = os.path.join(fncDir, "data_2")
-    tstDoc1m = os.path.join(tstData, "000000000001_main.nwd")
+    tstData3 = os.path.join(fncDir, "data_3")
-    tstDoc1b = os.path.join(tstData, "000000000001_main.bak")
+    tstDoc1m = os.path.join(tstData2, "000000000001_main.nwd")
-    tstDoc2m = os.path.join(tstData, "000000000002_main.nwd")
+    tstDoc1b = os.path.join(tstData2, "000000000001_main.bak")
-    tstDoc2b = os.path.join(tstData, "000000000002_main.bak")
+    tstDoc2m = os.path.join(tstData2, "000000000002_main.nwd")
-    tstDoc3m = os.path.join(tstData, "tooshort003_main.nwd")
+    tstDoc2b = os.path.join(tstData2, "000000000002_main.bak")
-    tstDoc3b = os.path.join(tstData, "tooshort003_main.bak")
+    tstDoc3m = os.path.join(tstData3, "tooshort003_main.nwd")
+    tstDoc3b = os.path.join(tstData3, "tooshort003_main.bak")
+    tstDir4a = os.path.join(tstData3, "stuff")
 
-    os.mkdir(tstData)
+    os.mkdir(tstData2)
+    os.mkdir(tstData3)
     writeFile(tstDoc1m, "stuff")
     writeFile(tstDoc1b, "stuff")
     writeFile(tstDoc2m, "stuff")
     writeFile(tstDoc2b, "stuff")
     writeFile(tstDoc3m, "stuff")
     writeFile(tstDoc3b, "stuff")
+    os.mkdir(tstDir4a)
 
     # Make the above fail
     with monkeypatch.context() as mp:
         mp.setattr("os.rename", causeOSError)
         mp.setattr("os.unlink", causeOSError)
-        errList = []
+        with pytest.raises(OSError):
-        errList = theProject._legacyDataFolder(tstData, errList)
+            theProject._legacyDataFolder(tstData2)
-        assert len(errList) > 0
+            theProject._legacyDataFolder(tstData3)
         assert os.path.isfile(tstDoc1m)
         assert os.path.isfile(tstDoc1b)
         assert os.path.isfile(tstDoc2m)
@@ -1291,15 +1252,16 @@ def testCoreProject_LegacyData(monkeypatch, mockGUI, fncDir):
         assert os.path.isfile(tstDoc3b)
 
     # And succeed ...
-    errList = []
+    assert theProject._legacyDataFolder(tstData2) is True
-    errList = theProject._legacyDataFolder(tstData, errList)
+    assert theProject._legacyDataFolder(tstData3) is True
-    assert len(errList) == 0
 
-    assert not os.path.isdir(tstData)
+    assert not os.path.isdir(tstData2)
+    assert os.path.isdir(tstData3)
     assert os.path.isfile(os.path.join(fncDir, "content", "2000000000001.nwd"))
     assert os.path.isfile(os.path.join(fncDir, "content", "2000000000002.nwd"))
-    assert os.path.isfile(os.path.join(fncDir, "junk", "tooshort003_main.nwd"))
+    assert os.path.isfile(os.path.join(fncDir, tstData3, "tooshort003_main.nwd"))
-    assert os.path.isfile(os.path.join(fncDir, "junk", "tooshort003_main.bak"))
+    assert os.path.isfile(os.path.join(fncDir, tstData3, "tooshort003_main.bak"))
+    assert os.path.isdir(tstDir4a)
 
 # END Test testCoreProject_LegacyData