From ae1c1f209b535294ab95f3850dc95a87a402f991 Mon Sep 17 00:00:00 2001 From: Ben Soares Date: Wed, 24 Jun 2020 23:29:40 +0100 Subject: [PATCH] JAL-3667 Fixed single backupfile error deleting the backupfile. Changed console output to Cache.log.... --- src/jalview/io/BackupFiles.java | 114 +++++++++++++++++++++++++++++++-------- 1 file changed, 91 insertions(+), 23 deletions(-) diff --git a/src/jalview/io/BackupFiles.java b/src/jalview/io/BackupFiles.java index aaf5e10..d18dc35 100644 --- 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; -- 1.7.10.2