From 65ed1da9a1710a9ce132c7edfce54955dc1ddf99 Mon Sep 17 00:00:00 2001
From: Ben Soares <b.soares@dundee.ac.uk>
Date: Thu, 1 Jun 2023 15:07:18 +0100
Subject: [PATCH] JAL-4193 JAL-4194 sessionProperties layer in Cache, CLI arg
 Opts to deal with -Pkey=val and set sessionProperties.
 Documentation for it.

---
 help/help/html/features/clarguments-reference.html |   12 ++++
 src/jalview/bin/Cache.java                         |   59 +++++++++++++++++++-
 src/jalview/bin/Jalview.java                       |   42 +++++++++++---
 src/jalview/bin/argparser/Arg.java                 |   41 +++++++++++---
 src/jalview/bin/argparser/ArgParser.java           |   37 +++++++++---
 src/jalview/bin/argparser/BootstrapArgs.java       |   19 +++++++
 src/jalview/gui/Preferences.java                   |   30 ++++++++++
 test/jalview/bin/CommandLineOperationsNG.java      |    8 +--
 8 files changed, 219 insertions(+), 29 deletions(-)

diff --git a/help/help/html/features/clarguments-reference.html b/help/help/html/features/clarguments-reference.html
index fc4f0a0..0221a67 100644
--- a/help/help/html/features/clarguments-reference.html
+++ b/help/help/html/features/clarguments-reference.html
@@ -103,6 +103,11 @@
     </tr>
 
     <tr valign="top">
+    <td><code>&#8209;&#8209;nostartupfile</code></td>
+    <td>Don't show the default startup file.</td>
+    </tr>
+
+    <tr valign="top">
     <td><code>&#8209;&#8209;webservicediscovery / &#8209;&#8209;nowebservicediscovery</code></td>
     <td>Attempt (/ or don't attempt) to connect to JABAWS web services.</td>
     </tr>
@@ -124,6 +129,13 @@
 
 <!--
     <tr valign="top">
+    <td><code>&#8209;&#8209;P<em>PREFERENCE=VALUE</em></code></td>
+    <td>Set a Jalview preference for this session only.  Experimental.</td>
+    </tr>
+-->
+
+<!--
+    <tr valign="top">
     <td><code>&#8209;&#8209;initsubstitutions / &#8209;&#8209;noinitsubstitutions</code></td>
     <td>Set <code>&#8209;&#8209;substitutions</code> to be initially enabled (or initially disabled).</td>
     </tr>
diff --git a/src/jalview/bin/Cache.java b/src/jalview/bin/Cache.java
index f4c8854..b966af6 100755
--- a/src/jalview/bin/Cache.java
+++ b/src/jalview/bin/Cache.java
@@ -42,7 +42,9 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.Enumeration;
+import java.util.HashMap;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Properties;
 import java.util.StringTokenizer;
 import java.util.TreeSet;
@@ -310,6 +312,24 @@ public class Cache
   // in-memory only storage of proxy password, safer to use char array
   public static char[] proxyAuthPassword = null;
 
+  /**
+   * Session properties, set by command line, try not to affect stored
+   * properties!
+   */
+  private static Map<String, String> sessionProperties = new HashMap<>();
+
+  private static boolean bypassSessionProperties = false;
+
+  public static void enableSessionProperties()
+  {
+    bypassSessionProperties = false;
+  }
+
+  public static void disableSessionProperties()
+  {
+    bypassSessionProperties = true;
+  }
+
   /** Jalview Properties */
   public static Properties applicationProperties = new Properties()
   {
@@ -712,7 +732,21 @@ public class Cache
    */
   public static String getProperty(String key)
   {
-    String prop = applicationProperties.getProperty(key);
+    return getProperty(key, false);
+  }
+
+  public static String getProperty(String key,
+          boolean skipSessionProperties)
+  {
+    String prop = null;
+    if (!(skipSessionProperties || bypassSessionProperties))
+    {
+      prop = getSessionProperty(key);
+    }
+    if (prop == null)
+    {
+      prop = applicationProperties.getProperty(key);
+    }
     if (prop == null && Platform.isJS())
     {
       prop = applicationProperties.getProperty(Platform.getUniqueAppletID()
@@ -782,8 +816,16 @@ public class Cache
     try
     {
       oldValue = applicationProperties.setProperty(key, obj);
-      if (propertiesFile != null && !propsAreReadOnly)
+      if (propertiesFile != null && !propsAreReadOnly
+      // don't rewrite if new value is same as old value
+              && !((obj == null && oldValue == null)
+                      || (obj != null && obj.equals(oldValue))))
       {
+        // reset the session property too
+        if (sessionProperties.containsKey(key))
+        {
+          sessionProperties.remove(key);
+        }
         FileOutputStream out = new FileOutputStream(propertiesFile);
         applicationProperties.store(out, "---JalviewX Properties File---");
         out.close();
@@ -1695,4 +1737,17 @@ public class Cache
     }
     return bootstrapProps;
   }
+
+  public static void setSessionProperty(String key, String val)
+  {
+    if (key != null)
+    {
+      sessionProperties.put(key, val);
+    }
+  }
+
+  public static String getSessionProperty(String key)
+  {
+    return key == null ? null : sessionProperties.get(key);
+  }
 }
diff --git a/src/jalview/bin/Jalview.java b/src/jalview/bin/Jalview.java
index dc97549..b29d9d7 100755
--- a/src/jalview/bin/Jalview.java
+++ b/src/jalview/bin/Jalview.java
@@ -343,6 +343,25 @@ public class Jalview
       }
     }
 
+    // set individual session preferences
+    if (bootstrapArgs.contains(Arg.P))
+    {
+      for (String kev : bootstrapArgs.getValueList(Arg.P))
+      {
+        if (kev == null)
+        {
+          continue;
+        }
+        int equalsIndex = kev.indexOf(ArgParser.EQUALS);
+        if (equalsIndex > -1)
+        {
+          String key = kev.substring(0, equalsIndex);
+          String val = kev.substring(equalsIndex + 1);
+          Cache.setSessionProperty(key, val);
+        }
+      }
+    }
+
     // Move any new getdown-launcher-new.jar into place over old
     // getdown-launcher.jar
     String appdirString = System.getProperty("getdownappdir");
@@ -720,18 +739,18 @@ public class Jalview
           testoutput(argparser, Arg.WEBSERVICEDISCOVERY);
         }
 
-        boolean usagestats = bootstrapArgs.getBoolean(Arg.USAGESTATS);
+        boolean usagestats = !bootstrapArgs.getBoolean(Arg.NOUSAGESTATS);
         if (aparser.contains("nousagestats"))
           usagestats = false;
         if (usagestats)
         {
           startUsageStats(desktop);
-          testoutput(argparser, Arg.USAGESTATS);
+          testoutput(argparser, Arg.NOUSAGESTATS);
         }
         else
         {
           System.out.println("CMD [-nousagestats] executed successfully!");
-          testoutput(argparser, Arg.USAGESTATS);
+          testoutput(argparser, Arg.NOUSAGESTATS);
         }
 
         boolean questionnaire = bootstrapArgs.getBoolean(Arg.QUESTIONNAIRE);
@@ -1114,7 +1133,8 @@ public class Jalview
 
     if (!Platform.isJS() && !headless && file == null
             && Cache.getDefault("SHOW_STARTUP_FILE", true)
-            && !cmds.commandArgsProvided())
+            && !cmds.commandArgsProvided()
+            && !bootstrapArgs.getBoolean(Arg.NOSTARTUPFILE))
     // don't open the startup file if command line args have been processed
     // (&& !Commands.commandArgsProvided())
     /**
@@ -1840,7 +1860,7 @@ public class Jalview
   }
 
   /*
-   * testoutput for boolean values
+   * testoutput for boolean and unary values
    */
   protected static void testoutput(ArgParser ap, Arg a)
   {
@@ -1883,7 +1903,15 @@ public class Jalview
 
   private static void testoutput(boolean yes, Arg a)
   {
-    System.out.println("[TESTOUTPUT] arg "
-            + (yes ? a.argString() : a.negateArgString()) + " was set");
+    String message = null;
+    if (a.hasOption(Opt.BOOLEAN))
+    {
+      message = (yes ? a.argString() : a.negateArgString()) + " was set";
+    }
+    else if (a.hasOption(Opt.UNARY))
+    {
+      message = a.argString() + (yes ? " was set" : " was not set");
+    }
+    System.out.println("[TESTOUTPUT] arg " + message);
   }
 }
diff --git a/src/jalview/bin/argparser/Arg.java b/src/jalview/bin/argparser/Arg.java
index 0d6e3b1..f7588cb 100644
--- a/src/jalview/bin/argparser/Arg.java
+++ b/src/jalview/bin/argparser/Arg.java
@@ -44,9 +44,10 @@ public enum Arg
   QUESTIONNAIRE(Type.CONFIG,
           "Show (or don't show) the questionnaire if one is available.",
           true, Opt.BOOLEAN, Opt.BOOTSTRAP),
-  USAGESTATS(Type.CONFIG,
-          "Send (or don't send) initial launch usage stats.", true,
-          Opt.BOOLEAN, Opt.BOOTSTRAP),
+  NOUSAGESTATS(Type.CONFIG, "Don't send initial launch usage stats.",
+          Opt.UNARY, Opt.BOOTSTRAP),
+  NOSTARTUPFILE(Type.CONFIG, "Don't show the default startup file.",
+          Opt.UNARY, Opt.BOOTSTRAP),
   WEBSERVICEDISCOVERY(Type.CONFIG,
           "Attempt (or don't attempt) to connect to JABAWS web services.",
           true, Opt.BOOLEAN, Opt.BOOTSTRAP),
@@ -65,6 +66,9 @@ public enum Arg
   INITSUBSTITUTIONS(Type.CONFIG,
           "Set ‑‑substitutions to be initially enabled (or initially disabled).",
           true, Opt.BOOLEAN, Opt.BOOTSTRAP, Opt.NOACTION, Opt.SECRET),
+  P(Type.CONFIG, "Set a Jalview preference value for this session.",
+          Opt.PREFIXKEV, Opt.PRESERVECASE, Opt.STRING, Opt.BOOTSTRAP,
+          Opt.MULTI, Opt.NOACTION, Opt.SECRET), // keep this secret for now.
 
   // Opening an alignment
   OPEN(Type.OPENING,
@@ -399,6 +403,14 @@ public enum Arg
      * A LAST arg gets moved to appear last in the usage statement (within type)
      */
     LAST(null),
+    /*
+     * After other args are checked, the following args can prefix a KEY=VALUE argument
+     */
+    PREFIXKEV("prefixes key=value"),
+    /*
+     * do not lowercase the name when getting the arg name or arg string
+     */
+    PRESERVECASE(null),
     //
     ;
 
@@ -486,15 +498,15 @@ public enum Arg
   private Arg(Type type, String alternativeName, String description,
           boolean defaultBoolean, Opt... options)
   {
+    this.type = type;
+    this.description = description;
+    this.defaultBoolValue = defaultBoolean;
+    this.setOptions(options);
     this.argNames = alternativeName != null
             ? new String[]
             { this.getName(), alternativeName }
             : new String[]
             { this.getName() };
-    this.type = type;
-    this.description = description;
-    this.defaultBoolValue = defaultBoolean;
-    this.setOptions(options);
   }
 
   public String argString()
@@ -544,7 +556,9 @@ public enum Arg
 
   public String getName()
   {
-    return this.name().toLowerCase(Locale.ROOT).replace('_', '-');
+    String name = hasOption(Opt.PRESERVECASE) ? this.name()
+            : this.name().toLowerCase(Locale.ROOT);
+    return name.replace('_', '-');
   }
 
   @Override
@@ -794,7 +808,16 @@ public enum Arg
     argSb.append(
             a.hasOption(Opt.BOOLEAN) ? booleanArgString(a) : a.argString());
     if (a.hasOption(Opt.STRING))
-      argSb.append("=value");
+    {
+      if (a.hasOption(Opt.PREFIXKEV))
+      {
+        argSb.append("key=value");
+      }
+      else
+      {
+        argSb.append("=value");
+      }
+    }
     return argSb.toString();
   }
 
diff --git a/src/jalview/bin/argparser/ArgParser.java b/src/jalview/bin/argparser/ArgParser.java
index f08e678..1558065c 100644
--- a/src/jalview/bin/argparser/ArgParser.java
+++ b/src/jalview/bin/argparser/ArgParser.java
@@ -48,7 +48,7 @@ public class ArgParser
 
   protected static final String DOUBLEDASH = "--";
 
-  protected static final char EQUALS = '=';
+  public static final char EQUALS = '=';
 
   protected static final String NEGATESTRING = "no";
 
@@ -302,12 +302,34 @@ public class ArgParser
         Arg a = argMap.get(argName);
         // check for boolean prepended by "no" e.g. --nowrap
         boolean negated = false;
-        if (a == null && argName.startsWith(NEGATESTRING) && argMap
-                .containsKey(argName.substring(NEGATESTRING.length())))
+        if (a == null)
         {
-          argName = argName.substring(NEGATESTRING.length());
-          a = argMap.get(argName);
-          negated = true;
+          if (argName.startsWith(NEGATESTRING) && argMap
+                  .containsKey(argName.substring(NEGATESTRING.length())))
+          {
+            argName = argName.substring(NEGATESTRING.length());
+            a = argMap.get(argName);
+            negated = true;
+          }
+          else
+          {
+            // after all other args, look for Opt.PREFIXKEV args if still not
+            // found
+            for (Arg potentialArg : EnumSet.allOf(Arg.class))
+            {
+              if (potentialArg.hasOption(Opt.PREFIXKEV) && argName != null
+                      && argName.startsWith(potentialArg.getName())
+                      && equalPos > -1)
+              {
+                val = argName.substring(potentialArg.getName().length())
+                        + EQUALS + val;
+                argName = argName.substring(0,
+                        potentialArg.getName().length());
+                a = potentialArg;
+                break;
+              }
+            }
+          }
         }
 
         // check for config errors
@@ -464,7 +486,8 @@ public class ArgParser
               val = LINKEDIDDIRNAME + File.separator + LINKEDIDBASENAME
                       + val.substring(MATCHALLLINKEDIDS.length());
             }
-            else if (a.hasOption(Opt.OUTPUTFILE) && a.hasOption(Opt.ALLOWALL)
+            else if (a.hasOption(Opt.OUTPUTFILE)
+                    && a.hasOption(Opt.ALLOWALL)
                     && val.startsWith(MATCHOPENEDLINKEDIDS))
             {
               // --output=open*.ext is shorthand for --opened --output
diff --git a/src/jalview/bin/argparser/BootstrapArgs.java b/src/jalview/bin/argparser/BootstrapArgs.java
index e1ad1d7..4b7b180 100644
--- a/src/jalview/bin/argparser/BootstrapArgs.java
+++ b/src/jalview/bin/argparser/BootstrapArgs.java
@@ -4,6 +4,7 @@ import java.io.File;
 import java.util.AbstractMap;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -108,6 +109,24 @@ public class BootstrapArgs
           }
         }
 
+        // after all other args, look for Opt.PREFIX args if still not found
+        if (!ArgParser.argMap.containsKey(argName))
+        {
+          for (Arg potentialArg : EnumSet.allOf(Arg.class))
+          {
+            if (potentialArg.hasOption(Opt.PREFIXKEV) && argName != null
+                    && argName.startsWith(potentialArg.getName())
+                    && val != null)
+            {
+              val = argName.substring(potentialArg.getName().length())
+                      + ArgParser.EQUALS + val;
+              argName = argName.substring(0,
+                      potentialArg.getName().length());
+              break;
+            }
+          }
+        }
+
         if (ArgParser.argMap.containsKey(argName) && val == null)
         {
           val = "true";
diff --git a/src/jalview/gui/Preferences.java b/src/jalview/gui/Preferences.java
index 35fdf6a..c4f32c3 100755
--- a/src/jalview/gui/Preferences.java
+++ b/src/jalview/gui/Preferences.java
@@ -156,6 +156,9 @@ public class Preferences extends GPreferences
   public static List<String> groupURLLinks;
   static
   {
+    // don't populate with session properties
+    Cache.disableSessionProperties();
+
     // get links selected to be in the menu (SEQUENCE_LINKS)
     // and links entered by the user but not selected (STORED_LINKS)
     String inMenuString = Cache.getDefault("SEQUENCE_LINKS", "");
@@ -180,6 +183,9 @@ public class Preferences extends GPreferences
      */
 
     groupURLLinks = new ArrayList<>();
+
+    // reenable
+    Cache.enableSessionProperties();
   }
 
   JInternalFrame frame;
@@ -248,6 +254,9 @@ public class Preferences extends GPreferences
   private Preferences()
   {
     super();
+    // don't populate with session properties
+    Cache.disableSessionProperties();
+
     frame = new JInternalFrame();
     frame.setFrameIcon(null);
     frame.setContentPane(this);
@@ -689,6 +698,8 @@ public class Preferences extends GPreferences
      * Set Startup tab defaults
      */
 
+    // re-enable
+    Cache.enableSessionProperties();
   }
 
   /**
@@ -701,6 +712,8 @@ public class Preferences extends GPreferences
   protected void setupOutputCombo(JComboBox<Object> comboBox,
           String propertyKey)
   {
+    Cache.disableSessionProperties();
+
     comboBox.addItem(promptEachTimeOpt);
     comboBox.addItem(lineArtOpt);
     comboBox.addItem(textOpt);
@@ -722,6 +735,8 @@ public class Preferences extends GPreferences
     {
       comboBox.setSelectedItem(promptEachTimeOpt);
     }
+
+    Cache.enableSessionProperties();
   }
 
   /**
@@ -733,6 +748,8 @@ public class Preferences extends GPreferences
   @Override
   public void ok_actionPerformed(ActionEvent e)
   {
+    Cache.disableSessionProperties();
+
     if (!validateSettings())
     {
       return;
@@ -746,6 +763,7 @@ public class Preferences extends GPreferences
     /*
      * Save Visual settings
      */
+
     Cache.applicationProperties.setProperty("SHOW_JVSUFFIX",
             Boolean.toString(seqLimit.isSelected()));
     Cache.applicationProperties.setProperty("RIGHT_ALIGN_IDS",
@@ -1020,10 +1038,14 @@ public class Preferences extends GPreferences
     } catch (Exception ex)
     {
     }
+
+    Cache.enableSessionProperties();
   }
 
   public void saveProxySettings()
   {
+    Cache.disableSessionProperties();
+
     String newProxyType = customProxy.isSelected() ? Cache.PROXYTYPE_CUSTOM
             : noProxy.isSelected() ? Cache.PROXYTYPE_NONE
                     : Cache.PROXYTYPE_SYSTEM;
@@ -1044,6 +1066,8 @@ public class Preferences extends GPreferences
       wsPrefs.update++;
     }
     previousProxyType = newProxyType;
+
+    Cache.enableSessionProperties();
   }
 
   /**
@@ -1075,6 +1099,8 @@ public class Preferences extends GPreferences
   @Override
   public void startupFileTextfield_mouseClicked()
   {
+    Cache.disableSessionProperties();
+
     // TODO: JAL-3048 not needed for Jalview-JS
     String fileFormat = Cache.getProperty("DEFAULT_FILE_FORMAT");
     JalviewFileChooser chooser = JalviewFileChooser
@@ -1096,6 +1122,8 @@ public class Preferences extends GPreferences
       startupFileTextfield
               .setText(chooser.getSelectedFile().getAbsolutePath());
     }
+
+    Cache.enableSessionProperties();
   }
 
   /**
@@ -1405,6 +1433,7 @@ public class Preferences extends GPreferences
      */
     String viewerPath = "";
     List<String> paths = null;
+    Cache.disableSessionProperties();
     try
     {
       ViewerType viewerType = ViewerType.valueOf(selectedItem);
@@ -1430,6 +1459,7 @@ public class Preferences extends GPreferences
     {
       // only valid entries should be in the drop-down
     }
+    Cache.enableSessionProperties();
     structureViewerPath.setText(viewerPath);
 
     paths.add(0, structureViewerPath.getText());
diff --git a/test/jalview/bin/CommandLineOperationsNG.java b/test/jalview/bin/CommandLineOperationsNG.java
index 049e9a8..30a1b9f 100644
--- a/test/jalview/bin/CommandLineOperationsNG.java
+++ b/test/jalview/bin/CommandLineOperationsNG.java
@@ -333,9 +333,8 @@ public class CommandLineOperationsNG
   public void testAllInputOperations(String expectedString,
           String failureMsg)
   {
-    if ("[TESTOUTPUT] arg --nousagestats was set".equals(expectedString))
-      Assert.assertTrue(successfulCMDs.contains(expectedString),
-              failureMsg);
+    Assert.assertTrue(successfulCMDs.contains(expectedString),
+            failureMsg + "; was expecting '" + expectedString + "'");
   }
 
   @Test(
@@ -352,7 +351,8 @@ public class CommandLineOperationsNG
     file.deleteOnExit();
     Worker worker = getJalviewDesktopRunner(withAWT, cmd, timeout);
     assertNotNull(worker, "worker is null");
-    String msg = "Didn't create an output" + type + " file.[" + cmd + "]";
+    String msg = "Didn't create an output" + type + " file '" + fileName
+            + "'. [" + cmd + "]";
     assertTrue(file.exists(), msg);
     FileAssert.assertFile(file, msg);
     FileAssert.assertMinLength(file, expectedMinFileSize);
-- 
1.7.10.2