From 9e0d9e9692a425e26d4efb16fb8a4bb733a20055 Mon Sep 17 00:00:00 2001 From: Ben Soares Date: Thu, 21 Feb 2019 17:59:25 +0000 Subject: [PATCH] JAL-3141 Sensible refactoring for confirming deleting 'newer' old backup files --- resources/lang/Messages.properties | 10 +- resources/lang/Messages_es.properties | 10 +- src/jalview/io/BackupFiles.java | 415 ++++++++++++++++++--------------- 3 files changed, 243 insertions(+), 192 deletions(-) diff --git a/resources/lang/Messages.properties b/resources/lang/Messages.properties index b5f8d67..d751266 100644 --- a/resources/lang/Messages.properties +++ b/resources/lang/Messages.properties @@ -1398,7 +1398,13 @@ label.previously_saved_scheme = Previously saved scheme label.no_backup_files = NO BACKUP FILES label.include_backup_files = Include backup files label.cancel_changes = Cancel changes -label.warning_confirm_change_reverse = Warning!\nIf you change the increment/decrement of the backup filename number, without changing the suffix or digits,\nthis may cause loss of backup files created with the previous backup filename scheme.\nAre you sure you wish to do this? +label.warning_confirm_change_reverse = Warning!\nIf you change the increment/decrement of the backup filename number, without changing the suffix or number of digits,\nthis may cause loss of backup files created with the previous backup filename scheme.\nAre you sure you wish to do this? label.change_increment_decrement = Change increment/decrement? label.was_previous = was {0} -label.newerdelete_line = Backup file\n''{0}''\t(modified {1})\nis to be deleted and replaced by apparently older file\n''{2}''\t(modified {3}).\nConfirm deletion of ''{0}''? +label.newerdelete_replacement_line = 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}). +label.confirm_deletion_or_rename = Confirm deletion of ''{0}'' or rename to ''{1}''? +label.newerdelete_line = 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}). +label.confirm_deletion = Confirm deletion of ''{0}''? +label.delete = Delete +label.rename = Rename +label.keep = Keep \ No newline at end of file diff --git a/resources/lang/Messages_es.properties b/resources/lang/Messages_es.properties index c32f6f0..ba50577 100644 --- a/resources/lang/Messages_es.properties +++ b/resources/lang/Messages_es.properties @@ -1399,7 +1399,13 @@ label.previously_saved_scheme = Esquema previamente guardado label.no_backup_files = NO ARCHIVOS DE RESPALDOS label.include_backup_files = Incluir archivos de respaldos label.cancel_changes = Cancelar cambios -label.warning_confirm_change_reverse = ¡Advertencia!\nSi cambia el incremento/decremento del número de archivos de respaldos, sin cambiar el sufijo o los dígitos,\nesto puede causar la pérdida de los archivos de respaldos creados con el esquema anterior de nombre de archivo de respaldos.\n¿Está seguro de que desea hacer esto? +label.warning_confirm_change_reverse = ¡Advertencia!\nSi cambia el incremento/decremento del número de archivos de respaldos, sin cambiar el sufijo o número de dígitos,\nesto puede causar la pérdida de los archivos de respaldos creados con el esquema anterior de nombre de archivo de respaldos.\n¿Está seguro de que desea hacer esto? label.change_increment_decrement = ¿Cambiar de incremento/decremento? label.was_previous = era {0} -label.newerdelete_line = El archivo de respaldo\n''{0}''\t(modificado {1})\ndebe eliminarse y reemplazarse por un archivo aparentemente más antiguo\n''{2}''\t(modificado {3}).\nConfirmar eliminar ''{0}''? +label.newerdelete_replacement_line = El archivo de respaldo\n''{0}''\t(modificado {2}, tamaño {4})\nserá borrado y reemplazarse por un archivo aparentemente más antiguo\n''{1}''\t(modificado {3}, tamaño {5}). +label.confirm_delete_or_rename = Confirmar borrar ''{0}'', o cambiar el nombre a ''{1}''? +label.newerdelete_line = El archivo de respaldo\n''{0}''\t(modificado {2}, tamaño {4})\nserá borrado pero es mas nuevo que el archivo de respaldo restante más antiguo\n''{1}''\t(modified {3}, size {5}). +label.confirm_delete = Confirmar eliminar ''{0}''? +label.delete = Borrar +label.rename = Cambiar +label.keep = Mantener \ No newline at end of file diff --git a/src/jalview/io/BackupFiles.java b/src/jalview/io/BackupFiles.java index 2939cd9..bd23a2d 100644 --- a/src/jalview/io/BackupFiles.java +++ b/src/jalview/io/BackupFiles.java @@ -86,10 +86,7 @@ public class BackupFiles private boolean tempFileWriteSuccess; // array of files to be deleted, with extra information - private ArrayList deleteFiles = new ArrayList<>(); - - // next backup filename - private File nextBackupFile; + private ArrayList deleteFiles = new ArrayList<>(); // date formatting for modification times private static final SimpleDateFormat sdf = new SimpleDateFormat( @@ -127,7 +124,8 @@ public class BackupFiles { String tempfilename = file.getName(); File tempdir = file.getParentFile(); - temp = File.createTempFile(tempfilename, TEMP_FILE_EXT, tempdir); + temp = File.createTempFile(tempfilename, TEMP_FILE_EXT + "_newfile", + tempdir); } else { @@ -251,9 +249,6 @@ public class BackupFiles // Create/move backups up one deleteFiles.clear(); - //File[] oldFilesToDelete = null; - //File[] newerOldFilesToDelete = null; // put files with newer modification - // timestamps in here to warn the user! // find existing backup files BackupFilenameFilter bff = new BackupFilenameFilter(basename, suffix, @@ -277,9 +272,6 @@ public class BackupFiles { // backup style numbering - File lastfile = null; - File lastfiletobedeleted = null; - String lastfiletobedeletedoriginalname = null; int tempMax = noMax ? -1 : max; // noMax == true means no limits @@ -297,6 +289,8 @@ public class BackupFiles } } + File previousFile = null; + File fileToBeDeleted = null; for (int n = tempMax; n > 0; n--) { String backupfilename = dir + File.separatorChar @@ -307,74 +301,83 @@ public class BackupFiles if (!backupfile_n.exists()) { // no "oldest" file to delete - lastfile = backupfile_n; - lastfiletobedeleted = null; + previousFile = backupfile_n; + fileToBeDeleted = null; continue; } - // check the modification time of the previous file if it's going to - // be deleted - if (lastfiletobedeleted != null) + // check the modification time of this (backupfile_n) and the previous + // file (fileToBeDeleted) if the previous file is going to be deleted + if (fileToBeDeleted != null) { - long oldLMT = lastfiletobedeleted.lastModified(); - long newLMT = backupfile_n.lastModified(); - if (oldLMT > newLMT) + File replacementFile = backupfile_n; + long fileToBeDeletedLMT = fileToBeDeleted.lastModified(); + long replacementFileLMT = replacementFile.lastModified(); + + try { - String oldLMTString = sdf - .format(lastfiletobedeleted.lastModified()); - String newLMTString = sdf.format(backupfile_n.lastModified()); - System.out.println("WARNING! I am set to delete backupfile " - + lastfiletobedeleted.getName() + " (was '" - + lastfiletobedeletedoriginalname + "')" - + " has modification time " - + oldLMTString - + " which is newer than its replacement " - + backupfile_n.getName() + " with modification time " - + newLMTString); - - addDeleteFile(lastfiletobedeleted, backupfile_n, true, true, - " (" + MessageManager.formatMessage( - "label.was_previous", new String[] - { lastfile.getName() }) + ")"); + File oldestTempFile = nextTempFile(fileToBeDeleted.getName(), + dirFile); + + if (fileToBeDeletedLMT > replacementFileLMT) + { + String fileToBeDeletedLMTString = sdf + .format(fileToBeDeletedLMT); + 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 its replacement " + + replacementFile.getName() + + " with modification time " + + replacementFileLMTString); + + boolean delete = confirmNewerDeleteFile(fileToBeDeleted, + replacementFile, true); + + if (delete) + { + // User has confirmed delete -- no need to add it to the list + fileToBeDeleted.delete(); + } + else + { + fileToBeDeleted.renameTo(oldestTempFile); + } + } + else + { + fileToBeDeleted.renameTo(oldestTempFile); + addDeleteFile(oldestTempFile); + } + + } catch (Exception e) + { + System.out.println( + "Error occurred, probably making new temp file for '" + + fileToBeDeleted.getName() + "'"); + e.printStackTrace(); } // reset - lastfiletobedeleted = null; - lastfiletobedeletedoriginalname = null; + fileToBeDeleted = null; } if (!noMax && n == tempMax && backupfile_n.exists()) { - // move the largest (max) rolled file to a temp file and add to the - // delete list - try - { - File temp = File.createTempFile(backupfilename, TEMP_FILE_EXT, - dirFile); - backupfile_n.renameTo(temp); - - String message = "(" + MessageManager - .formatMessage("label.was_previous", new String[] - { backupfile_n.getName() }) + ")"; - addDeleteFile(temp, backupfile_n, true, false, message); - - lastfiletobedeleted = temp; - } catch (IOException e) - { - System.out.println( - "IOException when creating temporary file for backupfilename"); - } + fileToBeDeleted = backupfile_n; } else { - // Just In Case - if (lastfile != null) + if (previousFile != null) { - ret = ret && backupfile_n.renameTo(lastfile); + ret = ret && backupfile_n.renameTo(previousFile); } } - lastfile = backupfile_n; + previousFile = backupfile_n; } // index to use for the latest backup @@ -393,15 +396,66 @@ public class BackupFiles // need to delete some files to keep number of backups to designated // max int numToDelete = bfTreeMap.size() - max + 1; + // the "replacement" file is the latest backup file being kept (it's + // not replacing though) + File replacementFile = numToDelete < backupFiles.length + ? backupFiles[numToDelete] + : null; for (int i = 0; i < numToDelete; i++) { - addDeleteFile(backupFiles[i], null, true, false, null); + // check the deletion files for modification time of the last + // backupfile being saved + File fileToBeDeleted = backupFiles[i]; + boolean delete = true; + + boolean newer = false; + if (replacementFile != null) + { + long fileToBeDeletedLMT = fileToBeDeleted.lastModified(); + long replacementFileLMT = replacementFile != null + ? replacementFile.lastModified() + : Long.MAX_VALUE; + if (fileToBeDeletedLMT > replacementFileLMT) + { + String fileToBeDeletedLMTString = sdf + .format(fileToBeDeletedLMT); + 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); + + delete = confirmNewerDeleteFile(fileToBeDeleted, + replacementFile, false); + if (delete) + { + // User has confirmed delete -- no need to add it to the list + fileToBeDeleted.delete(); + delete = false; + } + else + { + // keeping file, nothing to do! + } + } + } + if (delete) + { + addDeleteFile(fileToBeDeleted); + } + } } nextIndexNum = bfTreeMap.lastKey() + 1; - } } @@ -409,8 +463,7 @@ public class BackupFiles String latestBackupFilename = dir + File.separatorChar + BackupFilenameParts.getBackupFilename(nextIndexNum, basename, suffix, digits); - nextBackupFile = new File(latestBackupFilename); - ret |= file.renameTo(nextBackupFile); + ret |= file.renameTo(new File(latestBackupFilename)); if (tidyUp) { @@ -420,90 +473,120 @@ public class BackupFiles return ret; } + private static File nextTempFile(String filename, File dirFile) + throws IOException + { + File temp = null; + COUNT: for (int i = 1; i < 1000; i++) + { + File trythis = new File(dirFile, + filename + '~' + Integer.toString(i)); + if (!trythis.exists()) + { + temp = trythis; + break COUNT; + } + + } + if (temp == null) + { + temp = File.createTempFile(filename, TEMP_FILE_EXT, dirFile); + } + return temp; + } + private void tidyUpFiles() { deleteOldFiles(); } - private void deleteOldFiles() + private static boolean confirmNewerDeleteFile(File fileToBeDeleted, + File replacementFile, boolean replace) { - if (deleteFiles != null && !deleteFiles.isEmpty()) - { - boolean confirm = confirmDelete; - // delete old backup/version files + StringBuilder messageSB = new StringBuilder(); + + File ftbd = fileToBeDeleted; + String ftbdLMT = sdf.format(ftbd.lastModified()); + String ftbdSize = Long.toString(ftbd.length()); - // check for newer files - boolean newerDelete = hasNewerDeleteFile(); - StringBuilder newerDeleteSB = null; - if (newerDelete) + File rf = replacementFile; + String rfLMT = sdf.format(rf.lastModified()); + String rfSize = Long.toString(rf.length()); + + int confirmButton = JvOptionPane.NO_OPTION; + if (replace) + { + File saveFile = null; + try { - newerDeleteSB = new StringBuilder(); - for (int i = 0; i < deleteFiles.size(); i++) - { - DeleteFile df = deleteFiles.get(i); - if (df.newer && df.delete) - { - String oldName = df.oldFile.getName(); - String oldLMT = sdf.format(df.oldFile.lastModified()); - String newLMT = sdf.format(df.newFile.lastModified()); - if (newerDeleteSB.length() > 0) - { - newerDeleteSB.append("\n"); - } - newerDeleteSB.append( - MessageManager.formatMessage("label.newerdelete_line", - new String[] - { oldName, oldLMT, df.newFile.getName(), - newLMT }) - ); - if (df.info != null - && df.info.length() > 0) - { - newerDeleteSB.append(" "); - newerDeleteSB.append(df.info); - } - confirm = true; - } - } + saveFile = nextTempFile(ftbd.getName(), ftbd.getParentFile()); + } catch (Exception e) + { + System.out.println( + "Error when confirming to keep backup file newer than other backup files."); + e.printStackTrace(); } + messageSB.append(MessageManager.formatMessage( + "label.newerdelete_replacement_line", new String[] + { ftbd.getName(), rf.getName(), ftbdLMT, rfLMT, ftbdSize, + rfSize })); + messageSB.append("\n\n"); + messageSB.append(MessageManager.formatMessage( + "label.confirm_deletion_or_rename", new String[] + { ftbd.getName(), saveFile.getName() })); + String[] options = new String[] { + MessageManager.getString("label.delete"), + MessageManager.getString("label.rename") }; + + confirmButton = JvOptionPane.showOptionDialog(Desktop.desktop, + messageSB.toString(), + MessageManager.getString("label.backupfiles_confirm_delete"), + JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE, + null, options, options[0]); + } + else + { + messageSB.append(MessageManager + .formatMessage("label.newerdelete_line", new String[] + { ftbd.getName(), rf.getName(), ftbdLMT, rfLMT, ftbdSize, + rfSize })); + messageSB.append("\n\n"); + messageSB.append(MessageManager + .formatMessage("label.confirm_deletion", new String[] + { ftbd.getName() })); + String[] options = new String[] { + MessageManager.getString("label.delete"), + MessageManager.getString("label.keep") }; + + confirmButton = JvOptionPane.showOptionDialog(Desktop.desktop, + messageSB.toString(), + MessageManager.getString("label.backupfiles_confirm_delete"), + JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE, + null, options, options[0]); + } + + // return should be TRUE if file is to be deleted (added to the deleteFiles + // list) + return (confirmButton == JvOptionPane.YES_OPTION); + } + + private void deleteOldFiles() + { + if (deleteFiles != null && !deleteFiles.isEmpty()) + { boolean doDelete = false; - StringBuilder deleteSB = null; + StringBuilder messageSB = null; if (confirmDelete && deleteFiles.size() > 0) { - deleteSB = new StringBuilder(); - deleteSB.append(MessageManager + messageSB = new StringBuilder(); + messageSB.append(MessageManager .getString("label.backupfiles_confirm_delete_old_files")); for (int i = 0; i < deleteFiles.size(); i++) { - DeleteFile df = deleteFiles.get(i); - if (!df.delete) - { - break; - } - deleteSB.append("\n"); - deleteSB.append(df.oldFile.getName()); - if (df.info != null - && df.info.length() > 0) - { - deleteSB.append("\n"); - deleteSB.append(df.info); - } - } - confirm = true; - } - - if (confirm) - { - StringBuilder messageSB = new StringBuilder(); - if (deleteSB != null && deleteSB.length() > 0) - { - messageSB.append(deleteSB); - } - if (newerDeleteSB != null && newerDeleteSB.length() > 0) - { + File df = deleteFiles.get(i); messageSB.append("\n"); - messageSB.append(newerDeleteSB); + messageSB.append(df.getName()); } int confirmButton = JvOptionPane.showConfirmDialog(Desktop.desktop, @@ -523,10 +606,9 @@ public class BackupFiles { for (int i = 0; i < deleteFiles.size(); i++) { - File fileToDelete = deleteFiles.get(i).oldFile; + File fileToDelete = deleteFiles.get(i); fileToDelete.delete(); - // System.out.println("DELETING '" + fileToDelete.getName() + - // "'"); + System.out.println("DELETING '" + fileToDelete.getName() + "'"); } } @@ -653,69 +735,26 @@ public class BackupFiles return bfTreeMap; } - private boolean addDeleteFile(File oldFile, File newFile, boolean delete, - boolean newer, String info) + /* + private boolean addDeleteFile(File fileToBeDeleted, File originalFile, + boolean delete, boolean newer) + { + return addDeleteFile(fileToBeDeleted, originalFile, null, delete, newer); + } + */ + private boolean addDeleteFile(File fileToBeDeleted) { boolean ret = false; - int pos = deleteFiles.indexOf(oldFile); + int pos = deleteFiles.indexOf(fileToBeDeleted); if (pos > -1) { - DeleteFile df = deleteFiles.get(pos); - if (newFile != null) - { - df.newFile = newFile; - } - df.delete |= delete; - df.newer |= newer; - df.info += ';' + info; - ret = true; + return true; } else { - deleteFiles - .add(new DeleteFile(oldFile, newFile, delete, newer, info)); + deleteFiles.add(fileToBeDeleted); } return ret; } - private boolean hasNewerDeleteFile() - { - for (int i = 0; i < deleteFiles.size(); i++) - { - DeleteFile df = deleteFiles.get(i); - if (df.newer) - { - return true; - } - } - return false; - } -} - -class DeleteFile -{ - protected File oldFile; - - protected File newFile; - - protected boolean delete; - - protected boolean newer; - - protected String info; - - protected DeleteFile(File oldFile, File newFile, boolean delete, - boolean newer, String info) - { - this.oldFile = oldFile; - this.newFile = newFile; - this.delete = delete; - this.newer = newer; - this.info = info; - } - - public boolean equals(File file) - { - return this.oldFile.equals(file); - } } -- 1.7.10.2