Reinstate env var substitution in appbase.

Also added tests for base init of Application & versioned appbase creation.

CHANGELOG.md

@@ -1,12 +1,16 @@
 # Getdown Releases
 
+## 1.8.7 - Unreleased
+
+* Reinstated env var support in `appbase` property.
+
 ## 1.8.6 - June 4, 2019
 
 * Fixed issues with PAC proxy support: added `myIpAddress()`, fixed `dnsResolve()`, fixed crash
   when detecting PAC proxy.
 
-* Reverted env var support in `appbase` and `latest` properties. It's causing problems that need to
+* Reverted env var support in `appbase` property. It's causing problems that need to be
-  be investigated.
+  investigated.
 
 ## 1.8.5 - May 29, 2019
 

core/src/main/java/com/threerings/getdown/data/Application.java

@@ -605,7 +605,7 @@ public class Application
         }
 
         // check if we're overriding the domain in the appbase, and sub envvars
-        _appbase = SysProps.overrideAppbase(_appbase);
+        _appbase = resolveEnvVars(SysProps.overrideAppbase(_appbase));
 
         // make sure there's a trailing slash
         if (!_appbase.endsWith("/")) {
@@ -1130,27 +1130,31 @@ public class Application
         }
     }
 
-    /** Replaces the application directory and version in any argument. */
+    /** Replaces the application directory, version and env vars in any argument. */
     protected String processArg (String arg)
     {
         arg = arg.replace("%APPDIR%", getAppDir().getAbsolutePath());
         arg = arg.replace("%VERSION%", String.valueOf(_version));
+        arg = resolveEnvVars(arg);
+        return arg;
+    }
 
-        // if this argument contains %ENV.FOO% replace those with the associated values looked up
+    /** Resolves env var substitutions in {@code text}. */
-        // from the environment
+    protected String resolveEnvVars (String text) {
-        if (arg.contains(ENV_VAR_PREFIX)) {
+        // if the text contains %ENV.FOO% replace it with FOO looked up in the environment
+        if (text.contains(ENV_VAR_PREFIX)) {
             StringBuffer sb = new StringBuffer();
-            Matcher matcher = ENV_VAR_PATTERN.matcher(arg);
+            Matcher matcher = ENV_VAR_PATTERN.matcher(text);
             while (matcher.find()) {
                 String varName = matcher.group(1), varValue = System.getenv(varName);
                 String repValue = varValue == null ? "MISSING:"+varName : varValue;
                 matcher.appendReplacement(sb, Matcher.quoteReplacement(repValue));
             }
             matcher.appendTail(sb);
-            arg = sb.toString();
+            return sb.toString();
+        } else {
+            return text;
         }
-
-        return arg;
     }
 
     /**

core/src/main/java/com/threerings/getdown/util/Config.java

@@ -57,27 +57,11 @@ public class Config
     }
 
     /**
-     * Parses a configuration file containing key/value pairs. The file must be in the UTF-8
+     * Parses configuration text containing key/value pairs.
-     * encoding.
-     *
      * @param opts options that influence the parsing. See {@link #createOpts}.
-     *
      * @return a list of {@code String[]} instances containing the key/value pairs in the
      * order they were parsed from the file.
      */
-    public static List<String[]> parsePairs (File source, ParseOpts opts)
-        throws IOException
-    {
-        // annoyingly FileReader does not allow encoding to be specified (uses platform default)
-        try (FileInputStream fis = new FileInputStream(source);
-             InputStreamReader input = new InputStreamReader(fis, StandardCharsets.UTF_8)) {
-            return parsePairs(input, opts);
-        }
-    }
-
-    /**
-     * See {@link #parsePairs(File,ParseOpts)}.
-     */
     public static List<String[]> parsePairs (Reader source, ParseOpts opts) throws IOException
     {
         List<String[]> pairs = new ArrayList<>();
@@ -131,6 +115,21 @@ public class Config
         return pairs;
     }
 
+    /**
+     * Parses configuration file containing key/value pairs.
+     * @param source the file containing the config text. Must be in the UTF-8 encoding.
+     * @param opts options that influence the parsing. See {@link #createOpts}.
+     */
+    public static List<String[]> parsePairs (File source, ParseOpts opts)
+        throws IOException
+    {
+        // annoyingly FileReader does not allow encoding to be specified (uses platform default)
+        try (FileInputStream fis = new FileInputStream(source);
+             InputStreamReader input = new InputStreamReader(fis, StandardCharsets.UTF_8)) {
+            return parsePairs(input, opts);
+        }
+    }
+
     /**
      * Takes a comma-separated String of four integers and returns a rectangle using those ints as
      * the its x, y, width, and height.
@@ -166,13 +165,10 @@ public class Config
     }
 
     /**
-     * Parses a configuration file containing key/value pairs. The file must be in the UTF-8
+     * Parses the data for a config instance from the supplied {@code source} reader.
-     * encoding.
+     * @return a map that can be used to create a {@link #Config}.
-     *
-     * @return a map from keys to values, where a value will be an array of strings if more than
-     * one key/value pair in the config file was associated with the same key.
      */
-    public static Config parseConfig (File source, ParseOpts opts)
+    public static Map<String, Object> parseData (Reader source, ParseOpts opts)
         throws IOException
     {
         Map<String, Object> data = new HashMap<>();
@@ -195,9 +191,27 @@ public class Config
             }
         }
 
-        // special magic for the getdown.txt config: if the parsed data contains 'strict_comments =
+        return data;
-        // true' then we reparse the file with strict comments (i.e. # is only assumed to start a
+    }
-        // comment in column 0)
+
+    /**
+     * Parses a configuration file containing key/value pairs. The file must be in the UTF-8
+     * encoding.
+     *
+     * @return a map from keys to values, where a value will be an array of strings if more than
+     * one key/value pair in the config file was associated with the same key.
+     */
+    public static Config parseConfig (File source, ParseOpts opts)
+        throws IOException
+    {
+        // annoyingly FileReader does not allow encoding to be specified (uses platform default)
+        try (FileInputStream fis = new FileInputStream(source);
+             InputStreamReader input = new InputStreamReader(fis, StandardCharsets.UTF_8)) {
+            Map<String, Object> data = parseData(input, opts);
+
+            // special magic for the getdown.txt config: if the parsed data contains
+            // 'strict_comments = true' then we reparse the file with strict comments (i.e. # is
+            // only assumed to start a comment in column 0)
             if (!opts.strictComments && Boolean.parseBoolean((String)data.get("strict_comments"))) {
                 opts.strictComments = true;
                 return parseConfig(source, opts);
@@ -205,6 +219,7 @@ public class Config
 
             return new Config(data);
         }
+    }
 
     public Config (Map<String,  Object> data) {
         _data = data;

core/src/test/java/com/threerings/getdown/data/ApplicationTest.java

@@ -0,0 +1,75 @@
+//
+// Getdown - application installer, patcher and launcher
+// Copyright (C) 2004-2018 Getdown authors
+// https://github.com/threerings/getdown/blob/master/LICENSE
+
+package com.threerings.getdown.data;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.*;
+import static org.junit.Assert.*;
+
+import com.threerings.getdown.util.Config;
+
+public class ApplicationTest {
+
+    Application createApp () {
+        List<EnvConfig.Note> notes = new ArrayList<>();
+        EnvConfig env = EnvConfig.create(new String[0], notes);
+        EnvConfigTest.checkNoNotes(notes);
+        return new Application(env);
+    }
+
+    @Test public void testBaseConfig () throws Exception {
+        Application app = createApp();
+        URL appbase = new URL("https://test.com/foo/bar/");
+        Config config = new Config(Config.parseData(toReader(
+            "appbase", appbase.toString()
+        ), Config.createOpts(true)));
+        app.initBase(config);
+
+        assertEquals(appbase, app.getRemoteURL(""));
+    }
+
+    @Test public void testVersionedBase () throws Exception {
+        Application app = createApp();
+        String rootAppbase = "https://test.com/foo/bar/";
+        Config config = new Config(Config.parseData(toReader(
+            "appbase", rootAppbase + "%VERSION%",
+            "version", "42"
+        ), Config.createOpts(true)));
+        app.initBase(config);
+
+        assertEquals(new URL(rootAppbase + "42/"), app.getRemoteURL(""));
+    }
+
+    @Test public void testEnvVarBase () throws Exception {
+        // fiddling to make test work on Windows or Unix
+        String evar = System.getenv("USER") == null ? "USERNAME" : "USER";
+        Application app = createApp();
+        String rootAppbase = "https://test.com/foo/%ENV." + evar + "%/";
+        Config config = new Config(Config.parseData(toReader(
+            "appbase", rootAppbase + "%VERSION%",
+            "version", "42"
+        ), Config.createOpts(true)));
+        app.initBase(config);
+
+        String expectAppbase = "https://test.com/foo/" + System.getenv(evar) + "/42/";
+        assertEquals(new URL(expectAppbase), app.getRemoteURL(""));
+    }
+
+    protected static StringReader toReader (String... pairs)
+    {
+        StringBuilder builder = new StringBuilder();
+        for (int ii = 0; ii < pairs.length; ii += 2) {
+            builder.append(pairs[ii]).append("=").append(pairs[ii+1]).append("\n");
+        }
+        return new StringReader(builder.toString());
+    }
+
+}

core/src/test/java/com/threerings/getdown/data/EnvConfigTest.java

@@ -19,13 +19,13 @@ public class EnvConfigTest {
     static String TESTID = "testid";
     static String TESTBASE = "https://test.com/test";
 
-    private void debugNotes(List<EnvConfig.Note> notes) {
+    private void debugNotes (List<EnvConfig.Note> notes) {
         for (EnvConfig.Note note : notes) {
             System.out.println(note.message);
         }
     }
 
-    private void checkNoNotes (List<EnvConfig.Note> notes) {
+    static void checkNoNotes (List<EnvConfig.Note> notes) {
         StringBuilder msg = new StringBuilder();
         for (EnvConfig.Note note : notes) {
             if (note.level != EnvConfig.Note.Level.INFO) {