JAL-3667 Fixed single backupfile error deleting the backupfile. Changed console outpu...

diff --git a/src/jalview/io/BackupFiles.java b/src/jalview/io/BackupFiles.java
index aaf5e10..d18dc35 100644 (file)
--- a/src/jalview/io/BackupFiles.java
+++ b/src/jalview/io/BackupFiles.java
@@ -139,11 +139,11 @@ public class BackupFiles
       }
     } catch (IOException e)
     {
-      System.out.println(
+      Cache.log.error(
               "Could not create temp file to save into (IOException)");
     } catch (Exception e)
     {
-      System.out.println("Exception ctreating temp file for saving");
+      Cache.log.error("Exception ctreating temp file for saving");
     }
     this.setTempFile(temp);
   }
@@ -197,7 +197,7 @@ public class BackupFiles
       path = this.getTempFile().getCanonicalPath();
     } catch (IOException e)
     {
-      System.out.println(
+      Cache.log.error(
               "IOException when getting Canonical Path of temp file '"
                       + this.getTempFile().getName() + "'");
     }
@@ -235,6 +235,11 @@ public class BackupFiles
             || suffix.length() == 0)
     {
       // nothing to do
+      Cache.log.debug("BACKUPFILES rollBackupFiles nothing to do." + ", "
+              + "filename: " + (file != null ? file.getName() : "null")
+              + ", " + "file exists: " + file.exists() + ", " + "enabled: "
+              + enabled + ", " + "max: " + max + ", " + "suffix: '" + suffix
+              + "'");
       return true;
     }
 
@@ -244,9 +249,10 @@ public class BackupFiles
     {
       dirFile = file.getParentFile();
       dir = dirFile.getCanonicalPath();
+      Cache.log.debug("BACKUPFILES: dir: " + dir);
     } catch (Exception e)
     {
-      System.out.println(
+      Cache.log.error(
               "Could not get canonical path for file '" + file + "'");
       return false;
     }
@@ -264,6 +270,8 @@ public class BackupFiles
     File[] backupFiles = dirFile.listFiles(bff);
     int nextIndexNum = 0;
 
+    Cache.log
+            .debug("BACKUPFILES backupFiles.length: " + backupFiles.length);
     if (backupFiles.length == 0)
     {
       // No other backup files. Just need to move existing file to backupfile_1
@@ -332,7 +340,7 @@ public class BackupFiles
                         .format(fileToBeDeletedLMT);
                 String replacementFileLMTString = sdf
                         .format(replacementFileLMT);
-                System.out.println("WARNING! I am set to delete backupfile "
+                Cache.log.warn("WARNING! I am set to delete backupfile "
                         + fileToBeDeleted.getName()
                         + " has modification time "
                         + fileToBeDeletedLMTString
@@ -362,10 +370,10 @@ public class BackupFiles
 
             } catch (Exception e)
             {
-              System.out.println(
+              Cache.log.error(
                       "Error occurred, probably making new temp file for '"
                               + fileToBeDeleted.getName() + "'");
-              e.printStackTrace();
+              Cache.log.error(e.getStackTrace());
             }
 
             // reset
@@ -390,19 +398,37 @@ public class BackupFiles
         // index to use for the latest backup
         nextIndexNum = 1;
       }
-      else
+      else // not reverse numbering
       {
         // version style numbering (with earliest file deletion if max files
         // reached)
 
         bfTreeMap.values().toArray(backupFiles);
+        StringBuilder bfsb = new StringBuilder();
+        for (int i = 0; i < backupFiles.length; i++)
+        {
+          if (bfsb.length() > 0)
+          {
+            bfsb.append(", ");
+          }
+          bfsb.append(backupFiles[i].getName());
+        }
+        Cache.log.debug("BACKUPFILES backupFiles: " + bfsb.toString());
 
         // noMax == true means keep all backup files
         if ((!noMax) && bfTreeMap.size() >= max)
         {
+          Cache.log.debug("BACKUPFILES noMax: " + noMax + ", " + "max: "
+                  + max + ", " + "bfTreeMap.size(): " + bfTreeMap.size());
           // need to delete some files to keep number of backups to designated
-          // max
-          int numToDelete = bfTreeMap.size() - max + 1;
+          // max.
+          // Note that if the suffix is not numbered then do not delete any
+          // backup files later or we'll delete the new backup file (there can
+          // be only one).
+          int numToDelete = suffix.indexOf(NUM_PLACEHOLDER) > -1
+                  ? bfTreeMap.size() - max + 1
+                  : 0;
+          Cache.log.debug("BACKUPFILES numToDelete: " + numToDelete);
           // the "replacement" file is the latest backup file being kept (it's
           // not replacing though)
           File replacementFile = numToDelete < backupFiles.length
@@ -415,6 +441,9 @@ public class BackupFiles
             File fileToBeDeleted = backupFiles[i];
             boolean delete = true;
 
+            Cache.log.debug(
+                    "BACKUPFILES fileToBeDeleted: " + fileToBeDeleted);
+
             boolean newer = false;
             if (replacementFile != null)
             {
@@ -429,15 +458,14 @@ public class BackupFiles
                 String replacementFileLMTString = sdf
                         .format(replacementFileLMT);
 
-                System.out
-                        .println("WARNING! I am set to delete backupfile '"
-                                + fileToBeDeleted.getName()
-                                + "' has modification time "
-                                + fileToBeDeletedLMTString
-                                + " which is newer than the oldest backupfile being kept '"
-                                + replacementFile.getName()
-                                + "' with modification time "
-                                + replacementFileLMTString);
+                Cache.log.warn("WARNING! I am set to delete backupfile '"
+                        + fileToBeDeleted.getName()
+                        + "' has modification time "
+                        + fileToBeDeletedLMTString
+                        + " which is newer than the oldest backupfile being kept '"
+                        + replacementFile.getName()
+                        + "' with modification time "
+                        + replacementFileLMTString);
 
                 delete = confirmNewerDeleteFile(fileToBeDeleted,
                         replacementFile, false);
@@ -445,17 +473,23 @@ public class BackupFiles
                 {
                   // User has confirmed delete -- no need to add it to the list
                   fileToBeDeleted.delete();
+                  Cache.log.debug("BACKUPFILES deleting fileToBeDeleted: "
+                          + fileToBeDeleted);
                   delete = false;
                 }
                 else
                 {
                   // keeping file, nothing to do!
+                  Cache.log.debug("BACKUPFILES keeping fileToBeDeleted: "
+                          + fileToBeDeleted);
                 }
               }
             }
             if (delete)
             {
               addDeleteFile(fileToBeDeleted);
+              Cache.log.debug("BACKUPFILES addDeleteFile(fileToBeDelted): "
+                      + fileToBeDeleted);
             }
 
           }
@@ -470,10 +504,15 @@ public class BackupFiles
     String latestBackupFilename = dir + File.separatorChar
             + BackupFilenameParts.getBackupFilename(nextIndexNum, basename,
                     suffix, digits);
+    Cache.log.debug("BACKUPFILES Moving old file [" + file
+            + "] to latestBackupFilename [" + latestBackupFilename + "]");
     ret |= moveFileToFile(file, new File(latestBackupFilename));
+    Cache.log.debug("BACKUPFILES moving " + latestBackupFilename + " to "
+            + file + " was " + (ret ? "" : "NOT ") + "successful");
 
     if (tidyUp)
     {
+      Cache.log.debug("BACKUPFILES tidying up files");
       tidyUpFiles();
     }
 
@@ -529,7 +568,7 @@ public class BackupFiles
         saveFile = nextTempFile(ftbd.getName(), ftbd.getParentFile());
       } catch (Exception e)
       {
-        System.out.println(
+        Cache.log.error(
                 "Error when confirming to keep backup file newer than other backup files.");
         e.printStackTrace();
       }
@@ -537,10 +576,14 @@ public class BackupFiles
               "label.newerdelete_replacement_line", new String[]
               { ftbd.getName(), rf.getName(), ftbdLMT, rfLMT, ftbdSize,
                   rfSize }));
+      // "Backup file\n''{0}''\t(modified {2}, size {4})\nis to be deleted and
+      // replaced by apparently older file \n''{1}''\t(modified {3}, size
+      // {5}).""
       messageSB.append("\n\n");
       messageSB.append(MessageManager.formatMessage(
               "label.confirm_deletion_or_rename", new String[]
               { ftbd.getName(), saveFile.getName() }));
+      // "Confirm deletion of ''{0}'' or rename to ''{1}''?"
       String[] options = new String[] {
           MessageManager.getString("label.delete"),
           MessageManager.getString("label.rename") };
@@ -548,6 +591,7 @@ public class BackupFiles
       confirmButton = JvOptionPane.showOptionDialog(Desktop.desktop,
               messageSB.toString(),
               MessageManager.getString("label.backupfiles_confirm_delete"),
+              // "Confirm delete"
               JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE,
               null, options, options[0]);
     }
@@ -557,10 +601,14 @@ public class BackupFiles
               .formatMessage("label.newerdelete_line", new String[]
               { ftbd.getName(), rf.getName(), ftbdLMT, rfLMT, ftbdSize,
                   rfSize }));
+      // "Backup file\n''{0}''\t(modified {2}, size {4})\nis to be deleted but
+      // is newer than the oldest remaining backup file \n''{1}''\t(modified
+      // {3}, size {5})."
       messageSB.append("\n\n");
       messageSB.append(MessageManager
               .formatMessage("label.confirm_deletion", new String[]
               { ftbd.getName() }));
+      // "Confirm deletion of ''{0}''?"
       String[] options = new String[] {
           MessageManager.getString("label.delete"),
           MessageManager.getString("label.keep") };
@@ -568,6 +616,7 @@ public class BackupFiles
       confirmButton = JvOptionPane.showOptionDialog(Desktop.desktop,
               messageSB.toString(),
               MessageManager.getString("label.backupfiles_confirm_delete"),
+              // "Confirm delete"
               JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE,
               null, options, options[0]);
     }
@@ -587,6 +636,8 @@ public class BackupFiles
         messageSB = new StringBuilder();
         messageSB.append(MessageManager
                 .getString("label.backupfiles_confirm_delete_old_files"));
+        // "Delete the following older backup files? (see the Backups tab in
+        // Preferences for more options)"
         for (int i = 0; i < deleteFiles.size(); i++)
         {
           File df = deleteFiles.get(i);
@@ -597,12 +648,14 @@ public class BackupFiles
                   new String[]
                   { sdf.format(df.lastModified()),
                       Long.toString(df.length()) }));
+          // "(modified {0}, size {1})"
         }
 
         int confirmButton = JvOptionPane.showConfirmDialog(Desktop.desktop,
                 messageSB.toString(),
                 MessageManager
                         .getString("label.backupfiles_confirm_delete"),
+                // "Confirm delete"
                 JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE);
 
         doDelete = (confirmButton == JvOptionPane.YES_OPTION);
@@ -617,8 +670,10 @@ public class BackupFiles
         for (int i = 0; i < deleteFiles.size(); i++)
         {
           File fileToDelete = deleteFiles.get(i);
+          Cache.log.debug(
+                  "BACKUPFILES deleting fileToDelete:" + fileToDelete);
           fileToDelete.delete();
-          System.out.println("DELETING '" + fileToDelete.getName() + "'");
+          Cache.log.warn("deleting '" + fileToDelete.getName() + "'");
         }
       }
 
@@ -653,7 +708,7 @@ public class BackupFiles
     boolean rename = false;
     if (write)
     {
-      roll = this.rollBackupFiles(false);
+      roll = this.rollBackupFiles(false); // tidyUpFiles at the end
       rename = this.renameTempFile();
     }
 
@@ -666,9 +721,11 @@ public class BackupFiles
     boolean okay = roll && rename;
     if (!okay)
     {
+      boolean yesno = false;
       StringBuilder messageSB = new StringBuilder();
       messageSB.append(MessageManager.getString(
               "label.backupfiles_confirm_save_file_backupfiles_roll_wrong"));
+      // "Something possibly went wrong with the backups of this file."
       if (rename)
       {
         if (messageSB.length() > 0)
@@ -677,6 +734,7 @@ public class BackupFiles
         }
         messageSB.append(MessageManager.getString(
                 "label.backupfiles_confirm_save_new_saved_file_ok"));
+        // "The new saved file seems okay."
       }
       else
       {
@@ -686,12 +744,14 @@ public class BackupFiles
         }
         messageSB.append(MessageManager.getString(
                 "label.backupfiles_confirm_save_new_saved_file_not_ok"));
+        // "The new saved file might not be okay."
       }
 
       int confirmButton = JvOptionPane.showConfirmDialog(Desktop.desktop,
               messageSB.toString(),
               MessageManager
                       .getString("label.backupfiles_confirm_save_file"),
+              // "Confirm save file"
               JvOptionPane.OK_OPTION, JvOptionPane.WARNING_MESSAGE);
       okay = confirmButton == JvOptionPane.OK_OPTION;
     }
@@ -716,7 +776,7 @@ public class BackupFiles
       dirFile = file.getParentFile();
     } catch (Exception e)
     {
-      System.out.println(
+      Cache.log.error(
               "Could not get canonical path for file '" + file + "'");
       return new TreeMap<>();
     }
@@ -774,12 +834,20 @@ public class BackupFiles
     Path newPath = Paths.get(newFile.getAbsolutePath());
     try
     {
+      // delete destination file - not usually necessary but Just In Case...
+      newFile.delete();
       Files.move(oldPath, newPath, StandardCopyOption.REPLACE_EXISTING);
       ret = true;
     } catch (IOException e)
     {
       Cache.log.warn("Could not move file '" + oldPath.toString() + "' to '"
               + newPath.toString() + "'");
+      Cache.log.error(e.getStackTrace());
+      ret = false;
+    } catch (Exception e)
+    {
+      Cache.log.error(e.getMessage());
+      Cache.log.error(e.getStackTrace());
       ret = false;
     }
     return ret;