From 9495cd9a452073db15529f4844dc9888a6847d00 Mon Sep 17 00:00:00 2001 From: Ben Soares Date: Wed, 29 Jul 2020 19:48:02 +0100 Subject: [PATCH] JAL-3633 Small changes to allow future fitting with ApplicationSingletonI/Jalview-JS, Most NPE fixes for tests --- src/jalview/bin/Cache.java | 49 +++++++++----- src/jalview/gui/Preferences.java | 29 +++++--- src/jalview/io/BackupFiles.java | 127 +++++++++++++++++------------------ src/jalview/jbgui/GPreferences.java | 31 +++++---- 4 files changed, 132 insertions(+), 104 deletions(-) diff --git a/src/jalview/bin/Cache.java b/src/jalview/bin/Cache.java index e96fef1..a6f05db 100755 --- a/src/jalview/bin/Cache.java +++ b/src/jalview/bin/Cache.java @@ -1400,11 +1400,11 @@ public class Cache { try { + char[] displayHttpPw = new char[httpPassword == null ? 0 + : httpPassword.length]; + Arrays.fill(displayHttpPw, '*'); Cache.debug("CACHE Proxy: setting new Authenticator with httpUser='" - + httpUser + "' httpPassword='" - + (proxyAuthPassword == null ? "null" - : new String(proxyAuthPassword)) - + "'"); // DELETE THIS LINE (password in logs) + + httpUser + "' httpPassword='" + displayHttpPw + "'"); Authenticator.setDefault(new Authenticator() { @Override @@ -1432,7 +1432,7 @@ public class Cache .getString("label.proxy_password_required"); Preferences.openPreferences(Preferences.CONNECTIONS_TAB, message); - Preferences.getPreferences().proxyAuthPasswordFocus(); + Preferences.getInstance().proxyAuthPasswordHighlight(true); } else { @@ -1443,8 +1443,6 @@ public class Cache && getRequestingPort() == Integer .valueOf(httpPort)) { - char[] displayHttpPw = new char[httpPassword.length]; - Arrays.fill(displayHttpPw, '*'); Cache.debug( "AUTHENTICATOR returning PasswordAuthentication(\"" + httpUser + "\", '" @@ -1495,6 +1493,9 @@ public class Cache { // reset the Authenticator to protect http.proxyUser and // http.proxyPassword Just In Case + /* as noted above, due to bug in java this doesn't work if the sun.net.www.protocol.http.AuthCache + * has working credentials. No workaround for Java 11. + */ Cache.debug("AUTHENTICATOR setting default Authenticator to null"); Authenticator.setDefault(null); } @@ -1527,25 +1528,32 @@ public class Cache } } - public final static int DEBUG = 10; + public final static int TRACE = 10; - public final static int INFO = 20; + public final static int DEBUG = 20; - public final static int WARN = 30; + public final static int INFO = 30; - public final static int ERROR = 40; + public final static int WARN = 40; + + public final static int ERROR = 50; public static boolean println(int level, String message) { if (Cache.log == null) { - if (level >= ERROR) + if (level >= WARN) System.err.println(message); - else + else if (level >= INFO) System.out.println(message); + // not printing debug or trace messages return false; } - if (level >= WARN) + if (level >= ERROR) + { + Cache.log.error(message); + } + else if (level >= WARN) { Cache.log.warn(message); } @@ -1553,13 +1561,22 @@ public class Cache { Cache.log.info(message); } - else + else if (level >= DEBUG) { Cache.log.debug(message); } + else + { + Cache.log.trace(message); + } return true; } + public static void trace(String message) + { + println(TRACE, message); + } + public static void debug(String message) { println(DEBUG, message); @@ -1579,4 +1596,4 @@ public class Cache { println(ERROR, message); } -} +} \ No newline at end of file diff --git a/src/jalview/gui/Preferences.java b/src/jalview/gui/Preferences.java index 65b333e..b447214 100755 --- a/src/jalview/gui/Preferences.java +++ b/src/jalview/gui/Preferences.java @@ -81,6 +81,10 @@ import jalview.ws.sifts.SiftsSettings; * @author $author$ * @version $Revision$ */ +/* + * for merge with Jalview-JS + public class Preferences extends GPreferences implements ApplicationSingletonI + */ public class Preferences extends GPreferences { public static final String ENABLE_SPLIT_FRAME = "ENABLE_SPLIT_FRAME"; @@ -191,21 +195,26 @@ public class Preferences extends GPreferences MessageManager.getString("action.text"), "Text"); // get singleton Preferences instance - public static Preferences getPreferences() - { - return getPreferences(0, null); - } - - public static Preferences getPreferences(int selectTab, String message) + public static Preferences getInstance() { if (INSTANCE == null || INSTANCE.frame == null || INSTANCE.frame.isClosed()) { INSTANCE = new Preferences(); } - INSTANCE.selectTab(selectTab); - INSTANCE.setMessage(message); return INSTANCE; + + /* + * Replace code with the following for Jalvew-JS + Preferences INSTANCE = ApplicationSingletonProvider.getInstance(Preferences.class); + if (INSTANCE == null || INSTANCE.frame == null + || INSTANCE.frame.isClosed()) + { + ApplicationSingletonProvider.remove(Preferences.class); + INSTANCE = ApplicationSingletonProvider.getInstance(Preferences.class); + } + return INSTANCE; + */ } public static void openPreferences() @@ -215,7 +224,9 @@ public class Preferences extends GPreferences public static void openPreferences(int selectTab, String message) { - Preferences p = getPreferences(selectTab, message); + Preferences p = getInstance(); + p.selectTab(selectTab); + p.setMessage(message); p.frame.show(); p.frame.moveToFront(); p.frame.grabFocus(); diff --git a/src/jalview/io/BackupFiles.java b/src/jalview/io/BackupFiles.java index 6fdbf2c..6e80578 100644 --- a/src/jalview/io/BackupFiles.java +++ b/src/jalview/io/BackupFiles.java @@ -131,17 +131,17 @@ public class BackupFiles { String tempfilename = file.getName(); File tempdir = file.getParentFile(); - Cache.log.trace( + Cache.trace( "BACKUPFILES [file!=null] attempting to create temp file for " + tempfilename + " in dir " + tempdir); temp = File.createTempFile(tempfilename, TEMP_FILE_EXT + newTempFileSuffix, tempdir); - Cache.log.debug( + Cache.debug( "BACKUPFILES using temp file " + temp.getAbsolutePath()); } else { - Cache.log.trace( + Cache.trace( "BACKUPFILES [file==null] attempting to create default temp file " + DEFAULT_TEMP_FILE + " with extension " + TEMP_FILE_EXT); @@ -149,14 +149,13 @@ public class BackupFiles } } catch (IOException e) { - Cache.log - .error("Could not create temp file to save to (IOException)"); - Cache.log.error(e.getMessage()); - Cache.log.debug(Cache.getStackTraceString(e)); + Cache.error("Could not create temp file to save to (IOException)"); + Cache.error(e.getMessage()); + Cache.debug(Cache.getStackTraceString(e)); } catch (Exception e) { - Cache.log.error("Exception creating temp file for saving"); - Cache.log.debug(Cache.getStackTraceString(e)); + Cache.error("Exception creating temp file for saving"); + Cache.debug(Cache.getStackTraceString(e)); } this.setTempFile(temp); } @@ -164,15 +163,15 @@ public class BackupFiles public static void classInit() { Cache.initLogger(); - Cache.log.trace("BACKUPFILES classInit"); + Cache.trace("BACKUPFILES classInit"); boolean e = Cache.getDefault(ENABLED, !Platform.isJS()); setEnabled(e); - Cache.log.trace("BACKUPFILES " + (e ? "enabled" : "disabled")); + Cache.trace("BACKUPFILES " + (e ? "enabled" : "disabled")); BackupFilesPresetEntry bfpe = BackupFilesPresetEntry .getSavedBackupEntry(); - Cache.log.trace("BACKUPFILES preset scheme " + bfpe.toString()); + Cache.trace("BACKUPFILES preset scheme " + bfpe.toString()); setConfirmDelete(bfpe.confirmDelete); - Cache.log.trace("BACKUPFILES confirm delete " + bfpe.confirmDelete); + Cache.trace("BACKUPFILES confirm delete " + bfpe.confirmDelete); } public static void setEnabled(boolean flag) @@ -216,10 +215,9 @@ public class BackupFiles path = this.getTempFile().getCanonicalPath(); } catch (IOException e) { - Cache.log.error( - "IOException when getting Canonical Path of temp file '" - + this.getTempFile().getName() + "'"); - Cache.log.debug(Cache.getStackTraceString(e)); + Cache.error("IOException when getting Canonical Path of temp file '" + + this.getTempFile().getName() + "'"); + Cache.debug(Cache.getStackTraceString(e)); } return path; } @@ -255,7 +253,7 @@ public class BackupFiles || suffix.length() == 0) { // nothing to do - Cache.log.debug("BACKUPFILES rollBackupFiles nothing to do." + ", " + Cache.debug("BACKUPFILES rollBackupFiles nothing to do." + ", " + "filename: " + (file != null ? file.getName() : "null") + ", " + "file exists: " + file.exists() + ", " + "enabled: " + enabled + ", " + "max: " + max + ", " + "suffix: '" + suffix @@ -263,7 +261,7 @@ public class BackupFiles return true; } - Cache.log.trace("BACKUPFILES rollBackupFiles starting"); + Cache.trace("BACKUPFILES rollBackupFiles starting"); String dir = ""; File dirFile; @@ -271,19 +269,18 @@ public class BackupFiles { dirFile = file.getParentFile(); dir = dirFile.getCanonicalPath(); - Cache.log.trace("BACKUPFILES dir: " + dir); + Cache.trace("BACKUPFILES dir: " + dir); } catch (Exception e) { - Cache.log.error( - "Could not get canonical path for file '" + file + "'"); - Cache.log.error(e.getMessage()); - Cache.log.debug(Cache.getStackTraceString(e)); + Cache.error("Could not get canonical path for file '" + file + "'"); + Cache.error(e.getMessage()); + Cache.debug(Cache.getStackTraceString(e)); return false; } String filename = file.getName(); String basename = filename; - Cache.log.trace("BACKUPFILES filename is " + filename); + Cache.trace("BACKUPFILES filename is " + filename); boolean ret = true; // Create/move backups up one @@ -295,12 +292,11 @@ public class BackupFiles File[] backupFiles = dirFile.listFiles(bff); int nextIndexNum = 0; - Cache.log - .trace("BACKUPFILES backupFiles.length: " + backupFiles.length); + Cache.trace("BACKUPFILES backupFiles.length: " + backupFiles.length); if (backupFiles.length == 0) { // No other backup files. Just need to move existing file to backupfile_1 - Cache.log.trace( + Cache.trace( "BACKUPFILES no existing backup files, setting index to 1"); nextIndexNum = 1; } @@ -314,7 +310,7 @@ public class BackupFiles if (reverseOrder) { // backup style numbering - Cache.log.trace("BACKUPFILES rolling files in reverse order"); + Cache.trace("BACKUPFILES rolling files in reverse order"); int tempMax = noMax ? -1 : max; // noMax == true means no limits @@ -346,7 +342,7 @@ public class BackupFiles // no "oldest" file to delete previousFile = backupfile_n; fileToBeDeleted = null; - Cache.log.trace("BACKUPFILES No oldest file to delete"); + Cache.trace("BACKUPFILES No oldest file to delete"); continue; } @@ -357,9 +353,9 @@ public class BackupFiles File replacementFile = backupfile_n; long fileToBeDeletedLMT = fileToBeDeleted.lastModified(); long replacementFileLMT = replacementFile.lastModified(); - Cache.log.trace("BACKUPFILES fileToBeDeleted is " + Cache.trace("BACKUPFILES fileToBeDeleted is " + fileToBeDeleted.getAbsolutePath()); - Cache.log.trace("BACKUPFILES replacementFile is " + Cache.trace("BACKUPFILES replacementFile is " + backupfile_n.getAbsolutePath()); try @@ -373,7 +369,7 @@ public class BackupFiles .format(fileToBeDeletedLMT); String replacementFileLMTString = sdf .format(replacementFileLMT); - Cache.log.warn("WARNING! I am set to delete backupfile " + Cache.warn("WARNING! I am set to delete backupfile " + fileToBeDeleted.getName() + " has modification time " + fileToBeDeletedLMTString @@ -384,7 +380,7 @@ public class BackupFiles boolean delete = confirmNewerDeleteFile(fileToBeDeleted, replacementFile, true); - Cache.log.trace("BACKUPFILES " + Cache.trace("BACKUPFILES " + (delete ? "confirmed" : "not") + " deleting file " + fileToBeDeleted.getAbsolutePath() + " which is newer than " @@ -397,7 +393,7 @@ public class BackupFiles } else { - Cache.log.debug("BACKUPFILES moving " + Cache.debug("BACKUPFILES moving " + fileToBeDeleted.getAbsolutePath() + " to " + oldestTempFile.getAbsolutePath()); moveFileToFile(fileToBeDeleted, oldestTempFile); @@ -405,7 +401,7 @@ public class BackupFiles } else { - Cache.log.debug("BACKUPFILES going to move " + Cache.debug("BACKUPFILES going to move " + fileToBeDeleted.getAbsolutePath() + " to " + oldestTempFile.getAbsolutePath()); moveFileToFile(fileToBeDeleted, oldestTempFile); @@ -414,10 +410,10 @@ public class BackupFiles } catch (Exception e) { - Cache.log.error( + Cache.error( "Error occurred, probably making new temp file for '" + fileToBeDeleted.getName() + "'"); - Cache.log.error(Cache.getStackTraceString(e)); + Cache.error(Cache.getStackTraceString(e)); } // reset @@ -459,12 +455,12 @@ public class BackupFiles } bfsb.append(backupFiles[i].getName()); } - Cache.log.trace("BACKUPFILES backupFiles: " + bfsb.toString()); + Cache.trace("BACKUPFILES backupFiles: " + bfsb.toString()); // noMax == true means keep all backup files if ((!noMax) && bfTreeMap.size() >= max) { - Cache.log.trace("BACKUPFILES noMax: " + noMax + ", " + "max: " + Cache.trace("BACKUPFILES noMax: " + noMax + ", " + "max: " + max + ", " + "bfTreeMap.size(): " + bfTreeMap.size()); // need to delete some files to keep number of backups to designated // max. @@ -474,7 +470,7 @@ public class BackupFiles int numToDelete = suffix.indexOf(NUM_PLACEHOLDER) > -1 ? bfTreeMap.size() - max + 1 : 0; - Cache.log.trace("BACKUPFILES numToDelete: " + numToDelete); + Cache.trace("BACKUPFILES numToDelete: " + numToDelete); // the "replacement" file is the latest backup file being kept (it's // not replacing though) File replacementFile = numToDelete < backupFiles.length @@ -487,8 +483,7 @@ public class BackupFiles File fileToBeDeleted = backupFiles[i]; boolean delete = true; - Cache.log.trace( - "BACKUPFILES fileToBeDeleted: " + fileToBeDeleted); + Cache.trace("BACKUPFILES fileToBeDeleted: " + fileToBeDeleted); boolean newer = false; if (replacementFile != null) @@ -504,7 +499,7 @@ public class BackupFiles String replacementFileLMTString = sdf .format(replacementFileLMT); - Cache.log.warn("WARNING! I am set to delete backupfile '" + Cache.warn("WARNING! I am set to delete backupfile '" + fileToBeDeleted.getName() + "' has modification time " + fileToBeDeletedLMTString @@ -519,14 +514,14 @@ public class BackupFiles { // User has confirmed delete -- no need to add it to the list fileToBeDeleted.delete(); - Cache.log.debug("BACKUPFILES deleting fileToBeDeleted: " + Cache.debug("BACKUPFILES deleting fileToBeDeleted: " + fileToBeDeleted); delete = false; } else { // keeping file, nothing to do! - Cache.log.debug("BACKUPFILES keeping fileToBeDeleted: " + Cache.debug("BACKUPFILES keeping fileToBeDeleted: " + fileToBeDeleted); } } @@ -534,7 +529,7 @@ public class BackupFiles if (delete) { addDeleteFile(fileToBeDeleted); - Cache.log.debug("BACKUPFILES addDeleteFile(fileToBeDeleted): " + Cache.debug("BACKUPFILES addDeleteFile(fileToBeDeleted): " + fileToBeDeleted); } @@ -550,17 +545,16 @@ public class BackupFiles String latestBackupFilename = dir + File.separatorChar + BackupFilenameParts.getBackupFilename(nextIndexNum, basename, suffix, digits); - Cache.log.trace("BACKUPFILES Moving old file [" + file + Cache.trace("BACKUPFILES Moving old file [" + file + "] to latestBackupFilename [" + latestBackupFilename + "]"); // using boolean '&' instead of '&&' as don't want moveFileToFile attempt to // be conditional (short-circuit) ret = ret & moveFileToFile(file, new File(latestBackupFilename)); - Cache.log.debug( - "BACKUPFILES moving " + file + " to " + latestBackupFilename - + " was " + (ret ? "" : "NOT ") + "successful"); + Cache.debug("BACKUPFILES moving " + file + " to " + latestBackupFilename + + " was " + (ret ? "" : "NOT ") + "successful"); if (tidyUp) { - Cache.log.debug("BACKUPFILES tidying up files"); + Cache.debug("BACKUPFILES tidying up files"); tidyUpFiles(); } @@ -616,7 +610,7 @@ public class BackupFiles saveFile = nextTempFile(ftbd.getName(), ftbd.getParentFile()); } catch (Exception e) { - Cache.log.error( + Cache.error( "Error when confirming to keep backup file newer than other backup files."); e.printStackTrace(); } @@ -726,10 +720,10 @@ public class BackupFiles for (int i = 0; i < deleteFiles.size(); i++) { File fileToDelete = deleteFiles.get(i); - Cache.log.trace("BACKUPFILES about to delete fileToDelete:" + Cache.trace("BACKUPFILES about to delete fileToDelete:" + fileToDelete); fileToDelete.delete(); - Cache.log.warn("deleted '" + fileToDelete.getName() + "'"); + Cache.warn("deleted '" + fileToDelete.getName() + "'"); } } @@ -838,8 +832,7 @@ public class BackupFiles dirFile = file.getParentFile(); } catch (Exception e) { - Cache.log.error( - "Could not get canonical path for file '" + file + "'"); + Cache.error("Could not get canonical path for file '" + file + "'"); return new TreeMap<>(); } @@ -880,14 +873,14 @@ public class BackupFiles int pos = deleteFiles.indexOf(fileToBeDeleted); if (pos > -1) { - Cache.log.debug("BACKUPFILES not adding file " + Cache.debug("BACKUPFILES not adding file " + fileToBeDeleted.getAbsolutePath() + " to the delete list (already at index" + pos + ")"); return true; } else { - Cache.log.debug("BACKUPFILES adding file " + Cache.debug("BACKUPFILES adding file " + fileToBeDeleted.getAbsolutePath() + " to the delete list"); deleteFiles.add(fileToBeDeleted); } @@ -903,24 +896,24 @@ public class BackupFiles try { // delete destination file - not usually necessary but Just In Case... - Cache.log.trace("BACKUPFILES deleting " + newFile.getAbsolutePath()); + Cache.trace("BACKUPFILES deleting " + newFile.getAbsolutePath()); newFile.delete(); - Cache.log.trace("BACKUPFILES moving " + oldFile.getAbsolutePath() + Cache.trace("BACKUPFILES moving " + oldFile.getAbsolutePath() + " to " + newFile.getAbsolutePath()); Files.move(oldPath, newPath, StandardCopyOption.REPLACE_EXISTING); ret = true; - Cache.log.trace("BACKUPFILES move seems to have succeeded"); + Cache.trace("BACKUPFILES move seems to have succeeded"); } catch (IOException e) { - Cache.log.warn("Could not move file '" + oldPath.toString() + "' to '" + Cache.warn("Could not move file '" + oldPath.toString() + "' to '" + newPath.toString() + "'"); - Cache.log.error(e.getMessage()); - Cache.log.debug(Cache.getStackTraceString(e)); + Cache.error(e.getMessage()); + Cache.debug(Cache.getStackTraceString(e)); ret = false; } catch (Exception e) { - Cache.log.error(e.getMessage()); - Cache.log.debug(Cache.getStackTraceString(e)); + Cache.error(e.getMessage()); + Cache.debug(Cache.getStackTraceString(e)); ret = false; } return ret; diff --git a/src/jalview/jbgui/GPreferences.java b/src/jalview/jbgui/GPreferences.java index ab60f90..ddef7a4 100755 --- a/src/jalview/jbgui/GPreferences.java +++ b/src/jalview/jbgui/GPreferences.java @@ -998,25 +998,19 @@ public class GPreferences extends JPanel @Override public void changedUpdate(DocumentEvent e) { - if (!proxyAuthPasswordPB.getBackground() - .equals(Color.WHITE)) - proxyAuthPasswordPB.setBackground(Color.WHITE); + proxyAuthPasswordHighlight(true); } @Override public void insertUpdate(DocumentEvent e) { - if (!proxyAuthPasswordPB.getBackground() - .equals(Color.WHITE)) - proxyAuthPasswordPB.setBackground(Color.WHITE); + proxyAuthPasswordHighlight(true); } @Override public void removeUpdate(DocumentEvent e) { - if (!proxyAuthPasswordPB.getBackground() - .equals(Color.WHITE)) - proxyAuthPasswordPB.setBackground(Color.WHITE); + proxyAuthPasswordHighlight(true); } }); @@ -1238,10 +1232,21 @@ public class GPreferences extends JPanel return proxyPanel; } - public void proxyAuthPasswordFocus() + public void proxyAuthPasswordHighlight(boolean enabled) { - proxyAuthPasswordPB.grabFocus(); - proxyAuthPasswordPB.setBackground(Color.PINK); + if (enabled && proxyType.isSelected(customProxy.getModel()) + && proxyAuth.isSelected() + && !proxyAuthUsernameTB.getText().isEmpty() + && proxyAuthPasswordPB.getDocument().getLength() == 0) + { + + proxyAuthPasswordPB.grabFocus(); + proxyAuthPasswordPB.setBackground(Color.PINK); + } + else + { + proxyAuthPasswordPB.setBackground(Color.WHITE); + } } public void saveProxySettings() @@ -3190,11 +3195,13 @@ public class GPreferences extends JPanel public void proxyType_actionPerformed() { setCustomProxyEnabled(); + proxyAuthPasswordHighlight(true); } public void proxyAuth_actionPerformed() { setProxyAuthEnabled(); + proxyAuthPasswordHighlight(true); } /** -- 1.7.10.2