From 0b266ea40bfb194f843840cd474b8b3a27e0df84 Mon Sep 17 00:00:00 2001 From: Ben Soares Date: Fri, 15 Feb 2019 19:38:34 +0000 Subject: [PATCH] JAL-3141 re-order deletion of old files and check lastModified times with their replacements --- resources/lang/Messages.properties | 8 +- resources/lang/Messages_es.properties | 8 +- src/jalview/io/BackupFiles.java | 316 +++++++++++++++++++++++++++------ src/jalview/jbgui/GPreferences.java | 39 ++-- 4 files changed, 290 insertions(+), 81 deletions(-) diff --git a/resources/lang/Messages.properties b/resources/lang/Messages.properties index ac6cdc4..dc044d4 100644 --- a/resources/lang/Messages.properties +++ b/resources/lang/Messages.properties @@ -1363,7 +1363,9 @@ label.free_text_search = Free Text Search label.backupfiles_confirm_delete = Confirm delete label.backupfiles_confirm_delete_old_files = Delete the following older backup files? (see the Backups tab in Preferences for more options) label.backupfiles_confirm_save_file = Confirm save file -label.backupfiles_confirm_save_file_backupfiles_roll_wrong = Something possibly went wrong with the backups of this file, write the new file anyway? +label.backupfiles_confirm_save_file_backupfiles_roll_wrong = Something possibly went wrong with the backups of this file. +label.backupfiles_confirm_save_new_saved_file_ok = The new saved file seems okay. +label.backupfiles_confirm_save_new_saved_file_not_ok = The new saved file might not be okay. label.backups = Backups label.backup = Backup label.backup_files = Backup Files @@ -1397,4 +1399,6 @@ 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.change_increment_decrement = Change increment/decrement? \ No newline at end of file +label.change_increment_decrement = Change increment/decrement? +label.was_previous = was {0} +label.newerdelete_line = Backup file\n'{0}'\nwith modification time\n\t{1}\nis to be deleted and replaced by apparently older file\n'{2}'\nwith modification time\n\t{3}.\nConfirm deletion of '{0}'? \ No newline at end of file diff --git a/resources/lang/Messages_es.properties b/resources/lang/Messages_es.properties index 49b23ea..747f7d2 100644 --- a/resources/lang/Messages_es.properties +++ b/resources/lang/Messages_es.properties @@ -1364,7 +1364,9 @@ label.free_text_search = B label.backupfiles_confirm_delete = Confirmar borrar label.backupfiles_confirm_delete_old_files = ¿Borrar los siguientes archivos? (ver la pestaña 'Copias' de la ventana de Preferencias para más opciones) label.backupfiles_confirm_save_file = Confirmar guardar archivo -label.backupfiles_confirm_save_file_backupfiles_roll_wrong = Posiblemente algo está mal con los archivos de copia de seguridad. ¿Guardar el nuevo archivo? +label.backupfiles_confirm_save_file_backupfiles_roll_wrong = Posiblemente algo está mal con los archivos de respaldos. +label.backupfiles_confirm_save_new_saved_file_ok = El nuevo archivo guardado parece estar bien. +label.backupfiles_confirm_save_new_saved_file_not_ok = El nuevo archivo guardado podría no estar bien. label.backups = Respaldos label.backup = Respaldo label.backup_files = Archivos de respaldos @@ -1398,4 +1400,6 @@ 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.change_increment_decrement = ¿Cambiar de incremento/decremento? \ No newline at end of file +label.change_increment_decrement = ¿Cambiar de incremento/decremento? +label.was_previous = era {0} +label.newerdelete_line = Backup file\n'{0}'\nwith modification time\n\t{1}\nis to be deleted and replaced by apparently older file\n'{2}'\nwith modification time\n\t{3}.\nConfirm deletion of '{0}'? \ No newline at end of file diff --git a/src/jalview/io/BackupFiles.java b/src/jalview/io/BackupFiles.java index 695349c..5617cf7 100644 --- a/src/jalview/io/BackupFiles.java +++ b/src/jalview/io/BackupFiles.java @@ -7,7 +7,8 @@ import jalview.util.MessageManager; import java.io.File; import java.io.IOException; -import java.util.Arrays; +import java.text.SimpleDateFormat; +import java.util.ArrayList; import java.util.HashMap; import java.util.Map; import java.util.TreeMap; @@ -84,6 +85,16 @@ public class BackupFiles // flag set to see if file save to temp file was successful private boolean tempFileWriteSuccess; + // array of files to be deleted, with extra information + private ArrayList deleteFiles = new ArrayList<>(); + + // next backup filename + private File nextBackupFile; + + // date formatting for modification times + private static final SimpleDateFormat sdf = new SimpleDateFormat( + "yyyy-MM-dd HH:mm:ss"); + public BackupFiles(String filename) { this(new File(filename)); @@ -207,7 +218,11 @@ public class BackupFiles // roll the backupfiles public boolean rollBackupFiles() { + return this.rollBackupFiles(true); + } + public boolean rollBackupFiles(boolean tidyUp) + { // file doesn't yet exist or backups are not enabled or template is null or // empty if ((!file.exists()) || (!enabled) || max < 0 || suffix == null @@ -235,14 +250,16 @@ public class BackupFiles boolean ret = true; // Create/move backups up one - File[] oldFilesToDelete = null; + 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, digits); File[] backupFiles = dirFile.listFiles(bff); int nextIndexNum = 0; - String confirmDeleteExtraInfo = null; if (backupFiles.length == 0) { @@ -253,12 +270,17 @@ public class BackupFiles { TreeMap bfTreeMap = sortBackupFilesAsTreeMap( backupFiles, basename); + // bfTreeMap now a sorted list of , + // mappings if (reverseOrder) { // backup style numbering File lastfile = null; + File lastfiletobedeleted = null; + String lastfiletobedeletedoriginalname = null; + int tempMax = noMax ? -1 : max; // noMax == true means no limits // look for first "gap" in backupFiles @@ -274,11 +296,9 @@ public class BackupFiles tempMax = i; } } - - // for (int m = 0; m < tempMax; m++) + for (int n = tempMax; n > 0; n--) { - // int n = tempMax - m; String backupfilename = dir + File.separatorChar + BackupFilenameParts.getBackupFilename(n, basename, suffix, digits); @@ -286,12 +306,44 @@ public class BackupFiles if (!backupfile_n.exists()) { + // no "oldest" file to delete lastfile = backupfile_n; + lastfiletobedeleted = null; continue; } - // if (m == 0 && backupfile_n.exists()) - if ((!noMax) && n == tempMax && backupfile_n.exists()) + // check the modification time of the previous file if it's going to + // be deleted + if (lastfiletobedeleted != null) + { + long oldLMT = lastfiletobedeleted.lastModified(); + long newLMT = backupfile_n.lastModified(); + if (oldLMT > newLMT) + { + 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[] + { backupfile_n.getName() }) + ")"); + } + + // reset + lastfiletobedeleted = null; + lastfiletobedeletedoriginalname = null; + } + + if (!noMax && n == tempMax && backupfile_n.exists()) { // move the largest (max) rolled file to a temp file and add to the // delete list @@ -301,9 +353,12 @@ public class BackupFiles dirFile); backupfile_n.renameTo(temp); - oldFilesToDelete = new File[] { temp }; - confirmDeleteExtraInfo = "(was " + backupfile_n.getName() - + ")"; + 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( @@ -338,8 +393,10 @@ public class BackupFiles // need to delete some files to keep number of backups to designated // max int numToDelete = bfTreeMap.size() - max + 1; - oldFilesToDelete = Arrays.copyOfRange(backupFiles, 0, - numToDelete); + for (int i = 0; i < numToDelete; i++) + { + addDeleteFile(backupFiles[i], null, true, false, null); + } } @@ -348,60 +405,125 @@ public class BackupFiles } } - deleteOldFiles(oldFilesToDelete, confirmDeleteExtraInfo); - // Let's make the new backup file!! yay, got there at last! String latestBackupFilename = dir + File.separatorChar + BackupFilenameParts.getBackupFilename(nextIndexNum, basename, suffix, digits); - File latestBackupFile = new File(latestBackupFilename); - ret = ret && file.renameTo(latestBackupFile); + nextBackupFile = new File(latestBackupFilename); + ret |= file.renameTo(nextBackupFile); + + if (tidyUp) + { + tidyUpFiles(); + } return ret; } - private void deleteOldFiles(File[] oldFilesToDelete, - String confirmDeleteExtraInfo) + private void tidyUpFiles() + { + deleteOldFiles(); + } + + private void deleteOldFiles() { - if (oldFilesToDelete != null && oldFilesToDelete.length > 0) + if (deleteFiles != null && !deleteFiles.isEmpty()) { + boolean confirm = confirmDelete; // delete old backup/version files - boolean delete = false; - if (confirmDelete) + // check for newer files + boolean newerDelete = hasNewerDeleteFile(); + StringBuilder newerDeleteSB = null; + if (newerDelete) { - StringBuilder confirmMessage = new StringBuilder(); - confirmMessage.append(MessageManager + 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; + } + } + } + + boolean doDelete = false; + StringBuilder deleteSB = null; + if (confirmDelete && deleteFiles.size() > 0) + { + deleteSB = new StringBuilder(); + deleteSB.append(MessageManager .getString("label.backupfiles_confirm_delete_old_files")); - for (File f : oldFilesToDelete) + 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) { - confirmMessage.append("\n"); - confirmMessage.append(f.getName()); + messageSB.append(deleteSB); } - if (confirmDeleteExtraInfo != null - && confirmDeleteExtraInfo.length() > 0) + if (newerDeleteSB != null && newerDeleteSB.length() > 0) { - confirmMessage.append("\n"); - confirmMessage.append(confirmDeleteExtraInfo); + messageSB.append("\n"); + messageSB.append(newerDeleteSB); } - int confirm = JvOptionPane.showConfirmDialog(Desktop.desktop, - confirmMessage.toString(), + + int confirmButton = JvOptionPane.showConfirmDialog(Desktop.desktop, + messageSB.toString(), MessageManager .getString("label.backupfiles_confirm_delete"), JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE); - delete = (confirm == JvOptionPane.YES_OPTION); + doDelete = (confirmButton == JvOptionPane.YES_OPTION); } else { - delete = true; + doDelete = true; } - if (delete) + if (doDelete) { - for (int i = 0; i < oldFilesToDelete.length; i++) + for (int i = 0; i < deleteFiles.size(); i++) { - File fileToDelete = oldFilesToDelete[i]; + File fileToDelete = deleteFiles.get(i).oldFile; fileToDelete.delete(); // System.out.println("DELETING '" + fileToDelete.getName() + // "'"); @@ -409,6 +531,8 @@ public class BackupFiles } } + + deleteFiles.clear(); } private TreeMap sortBackupFilesAsTreeMap( @@ -435,13 +559,11 @@ public class BackupFiles boolean write = this.getWriteSuccess(); boolean roll = false; + boolean rename = false; if (write) { - roll = this.rollBackupFiles(); - } - else - { - return false; + roll = this.rollBackupFiles(false); + rename = this.renameTempFile(); } /* @@ -450,25 +572,40 @@ public class BackupFiles * trying to roll the backup files, and most likely the filename needed will already * be vacant so renaming the temp file is nearly always correct! */ - if (!roll) + boolean okay = roll && rename; + if (!okay) { - int confirm = JvOptionPane.showConfirmDialog(Desktop.desktop, - MessageManager.getString( - "label.backupfiles_confirm_save_file_backupfiles_roll_wrong"), - MessageManager - .getString("label.backupfiles_confirm_save_file"), - JvOptionPane.YES_NO_OPTION, JvOptionPane.WARNING_MESSAGE); - - if (confirm == JvOptionPane.YES_OPTION) + StringBuilder messageSB = new StringBuilder(); + messageSB.append(MessageManager.getString( "label.backupfiles_confirm_save_file_backupfiles_roll_wrong")); + if (rename) { - roll = true; + if (messageSB.length() > 0) + { + messageSB.append("\n"); + } + messageSB.append(MessageManager.getString( + "label.backupfiles_confirm_save_new_saved_file_ok")); + } + else + { + if (messageSB.length() > 0) + { + messageSB.append("\n"); + } + messageSB.append(MessageManager.getString( + "label.backupfiles_confirm_save_new_saved_file_not_ok")); } - } - boolean rename = false; - if (roll) + int confirmButton = JvOptionPane.showConfirmDialog(Desktop.desktop, + messageSB.toString(), + MessageManager + .getString("label.backupfiles_confirm_save_file"), + JvOptionPane.OK_OPTION, JvOptionPane.WARNING_MESSAGE); + okay = confirmButton == JvOptionPane.OK_OPTION; + } + if (okay) { - rename = this.renameTempFile(); + tidyUpFiles(); } return rename; @@ -516,4 +653,69 @@ public class BackupFiles return bfTreeMap; } + private boolean addDeleteFile(File oldFile, File newFile, boolean delete, + boolean newer, String info) + { + boolean ret = false; + int pos = deleteFiles.indexOf(oldFile); + 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; + } + else + { + deleteFiles + .add(new DeleteFile(oldFile, newFile, delete, newer, info)); + } + 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); + } } diff --git a/src/jalview/jbgui/GPreferences.java b/src/jalview/jbgui/GPreferences.java index dedcd10..c9e3629 100755 --- a/src/jalview/jbgui/GPreferences.java +++ b/src/jalview/jbgui/GPreferences.java @@ -536,9 +536,6 @@ public class GPreferences extends JPanel embbedBioJSON.setText(MessageManager.getString("label.embbed_biojson")); embbedBioJSON.setBounds(new Rectangle(228, 200, 250, 23)); - TitledBorder backupFilesBorder = new TitledBorder( - MessageManager.getString("label.backup_files")); - jPanel11.add(jLabel1); jPanel11.add(blcjv); jPanel11.add(clustaljv); @@ -1766,7 +1763,6 @@ public class GPreferences extends JPanel @Override public void actionPerformed(ActionEvent e) { - boolean selected = enableBackupFiles.isSelected(); // enable other options only when the first is checked backupsOptionsSetEnabled(); } @@ -1824,8 +1820,9 @@ public class GPreferences extends JPanel return backupsTab; } - private final int BACKUPFILESSCHEMECUSTOMISE = 0; - private final IntKeyStringValueEntry[] backupfilesPresetEntries = { + protected static final int BACKUPFILESSCHEMECUSTOMISE = 0; + + private static final IntKeyStringValueEntry[] backupfilesPresetEntries = { new IntKeyStringValueEntry(1, MessageManager.getString("label.default")), new IntKeyStringValueEntry(2, @@ -1835,14 +1832,17 @@ public class GPreferences extends JPanel new IntKeyStringValueEntry(4, MessageManager.getString("label.rolled_backups")), // ... - // new IntKeyStringValueEntry(255, - // MessageManager.getString("label.previously_saved_scheme")), // IMPORTANT, keep "Custom" entry with key 0 (even though it appears last) new IntKeyStringValueEntry(BACKUPFILESSCHEMECUSTOMISE, MessageManager.getString("label.customise")) }; - private final Map backupfilesPresetEntriesValues = new HashMap() + private static final Map backupfilesPresetEntriesValues = new HashMap() { + /** + * + */ + private static final long serialVersionUID = 125L; + { put(1, new BackupFilesPresetEntry( ".bak" + BackupFiles.NUM_PLACEHOLDER, 3, false, false, 3, @@ -1972,7 +1972,7 @@ public class GPreferences extends JPanel updateBackupFilesExampleLabel(); } - protected int getComboIntStringKey(JComboBox c) + protected int getComboIntStringKey(JComboBox c) { IntKeyStringValueEntry e = (IntKeyStringValueEntry) c.getSelectedItem(); return e != null ? e.getKey() : 0; @@ -1984,8 +1984,6 @@ public class GPreferences extends JPanel for (int i = 0; i < c.getItemCount(); i++) { IntKeyStringValueEntry e = c.getItemAt(i); - int k = e.getKey(); - String v = e.getValue(); if (e.getKey() == key) { c.setSelectedIndex(i); @@ -2170,7 +2168,6 @@ public class GPreferences extends JPanel @Override public void actionPerformed(ActionEvent e) { - boolean selected = backupfilesKeepAll.isSelected(); keepRollMaxOptionsEnabled(); updateBackupFilesExampleLabel(); } @@ -2774,18 +2771,20 @@ class BackupFilesPresetEntry class BackupFilesPresetsComboBoxRenderer extends DefaultListCellRenderer { + /** + * + */ + private static final long serialVersionUID = 88L; + @Override - public Component getListCellRendererComponent(JList list, Object value, int index, boolean isSelected, boolean cellHasFocus) { + public Component getListCellRendererComponent(JList list, Object value, + int index, boolean isSelected, boolean cellHasFocus) + { super.getListCellRendererComponent(list, value, index, isSelected, cellHasFocus); try { IntKeyStringValueEntry e = (IntKeyStringValueEntry) value; - if (e.getKey() == 255) - { - // "Previously saved scheme" item - this.setFont(this.getFont().deriveFont(Font.ITALIC)); - } - else if (e.getKey() == 0) + if (e.getKey() == GPreferences.BACKUPFILESSCHEMECUSTOMISE) { // "Customise" item this.setFont(this.getFont().deriveFont(Font.BOLD)); -- 1.7.10.2