Tighten up PropertiesLauncher's contract

The main changes are:

- Switch to `loader.properties` instead of `application.properties`
- Search for `loader.properties` in `loader.home` as well as in
  the classpath
- Placeholder replacements in MANIFEST.MF (using `loader.properties`
  or system/env vars)

See gh-7221
Closes gh-8346

Author: Dave Syer

spring-boot-docs/src/main/asciidoc/appendix-executable-jar-format.adoc

@@ -147,7 +147,7 @@ files in directories (as opposed to explicitly on the classpath). In the case of
 you just add extra jars in those locations if you want more. The `PropertiesLauncher`
 looks in `BOOT-INF/lib/` in your application archive by default, but you can add
 additional locations by setting an environment variable `LOADER_PATH` or `loader.path`
-in `application.properties` (comma-separated list of directories or archives).
+in `loader.properties` (comma-separated list of directories or archives).
 
 
 
@@ -198,7 +198,13 @@ the appropriate launcher:
 
 `PropertiesLauncher` has a few special features that can be enabled with external
 properties (System properties, environment variables, manifest entries or
-`application.properties`).
+`loader.properties`).
+
+NOTE: `PropertiesLauncher` supports loading properties from
+`loader.properties` and also (for historic reasons)
+`application.properties`. We recommend using
+`loader.properties` exclusively, as support for
+`application.properties` is deprecated and may be removed in the future.
 
 |===
 |Key |Purpose
@@ -208,8 +214,7 @@ properties (System properties, environment variables, manifest entries or
  just like a regular `-classpath` on the `javac` command line.
 
 |`loader.home`
-|Location of additional properties file, e.g. `file:///opt/app`
- (defaults to `${user.dir}`)
+|Used to resolve relative paths in `loader.path`. E.g. `loader.path=lib` then `${loader.home}/lib` is a classpath location (along with all jar files in that directory). Also used to locate a `loader.properties file`. Example `file:///opt/app` (defaults to `${user.dir}`).
 
 |`loader.args`
 |Default arguments for the main method (space separated)
@@ -218,11 +223,11 @@ properties (System properties, environment variables, manifest entries or
 |Name of main class to launch, e.g. `com.app.Application`.
 
 |`loader.config.name`
-|Name of properties file, e.g. `loader` (defaults to `application`).
+|Name of properties file, e.g. `launcher` (defaults to `loader`).
 
 |`loader.config.location`
 |Path to properties file, e.g. `classpath:loader.properties` (defaults to
- `application.properties`).
+ `loader.properties`).
 
 |`loader.system`
 |Boolean flag to indicate that all properties should be added to System properties
@@ -241,7 +246,7 @@ be used:
 |`LOADER_PATH`
 
 |`loader.home`
-|
+|`Loader-Home`
 |`LOADER_HOME`
 
 |`loader.args`
@@ -253,11 +258,11 @@ be used:
 |`LOADER_MAIN`
 
 |`loader.config.location`
-|
+|`Loader-Config-Location`
 |`LOADER_CONFIG_LOCATION`
 
 |`loader.system`
-|
+|`Loader-System`
 |`LOADER_SYSTEM`
 
 |===
@@ -266,15 +271,21 @@ TIP: Build plugins automatically move the `Main-Class` attribute to `Start-Class
 the fat jar is built. If you are using that, specify the name of the class to launch using
 the `Main-Class` attribute and leave out `Start-Class`.
 
-* `loader.home` is the directory location of an additional properties file (overriding
+* `loader.properties` are searched for in `loader.home` then in the root of the classpath,
+  then in `classpath:/BOOT-INF/classes`. The first location that exists is used.
+* `loader.home` is only the directory location of an additional properties file (overriding
   the default) as long as `loader.config.location` is not specified.
 * `loader.path` can contain directories (scanned recursively for jar and zip files),
   archive paths, or wildcard patterns (for the default JVM behavior).
 * `loader.path` (if empty) defaults to `BOOT-INF/lib` (meaning a local directory or a
   nested one if running from an archive). Because of this `PropertiesLauncher` behaves the
   same as `JarLauncher` when no additional configuration is provided.
+* `loader.path` can not be used to configure the location of `loader.properties` (the classpath
+  used to search for the latter is the JVM classpath when `PropertiesLauncher` is launched).
 * Placeholder replacement is done from System and environment variables plus the
   properties file itself on all values before use.
+* The search order for properties (where it makes sense to look in more than one place)
+  is env vars, system properties, `loader.properties`, exploded archive manifest, archive manifest.
 
 
 

spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/PropertiesLauncher.java

@@ -142,48 +142,64 @@ public PropertiesLauncher() {
 	}
 
 	protected File getHomeDirectory() {
-		return new File(SystemPropertyUtils
-				.resolvePlaceholders(System.getProperty(HOME, "${user.dir}")));
+		try {
+			return new File(getPropertyWithDefault(HOME, "${user.dir}"));
+		}
+		catch (Exception ex) {
+			throw new IllegalStateException(ex);
+		}
 	}
 
 	private void initializeProperties() throws Exception, IOException {
-		String config = "classpath:BOOT-INF/classes/"
-				+ SystemPropertyUtils.resolvePlaceholders(
-						SystemPropertyUtils.getProperty(CONFIG_NAME, "application"))
-				+ ".properties";
-		config = SystemPropertyUtils.resolvePlaceholders(
-				SystemPropertyUtils.getProperty(CONFIG_LOCATION, config));
-		InputStream resource = getResource(config);
-		if (resource != null) {
-			log("Found: " + config);
-			try {
-				this.properties.load(resource);
-			}
-			finally {
-				resource.close();
+		List<String> configs = new ArrayList<String>();
+		if (getProperty(CONFIG_LOCATION) != null) {
+			configs.add(getProperty(CONFIG_LOCATION));
+		}
+		else {
+			String[] names = getPropertyWithDefault(CONFIG_NAME, "loader,application")
+					.split(",");
+			for (String name : names) {
+				configs.add("file:" + getHomeDirectory() + "/" + name + ".properties");
+				configs.add("classpath:" + name + ".properties");
+				configs.add("classpath:BOOT-INF/classes/" + name + ".properties");
 			}
-			for (Object key : Collections.list(this.properties.propertyNames())) {
-				String text = this.properties.getProperty((String) key);
-				String value = SystemPropertyUtils.resolvePlaceholders(this.properties,
-						text);
-				if (value != null) {
-					this.properties.put(key, value);
+		}
+		for (String config : configs) {
+			InputStream resource = getResource(config);
+			if (resource != null) {
+				log("Found: " + config);
+				try {
+					this.properties.load(resource);
+				}
+				finally {
+					resource.close();
 				}
-			}
-			if (SystemPropertyUtils
-					.resolvePlaceholders("${" + SET_SYSTEM_PROPERTIES + ":false}")
-					.equals("true")) {
-				log("Adding resolved properties to System properties");
 				for (Object key : Collections.list(this.properties.propertyNames())) {
-					String value = this.properties.getProperty((String) key);
-					System.setProperty((String) key, value);
+					if (config.endsWith("application.properties")
+							&& ((String) key).startsWith("loader.")) {
+						warn("WARNING: use of application.properties for PropertiesLauncher is deprecated");
+					}
+					String text = this.properties.getProperty((String) key);
+					String value = SystemPropertyUtils
+							.resolvePlaceholders(this.properties, text);
+					if (value != null) {
+						this.properties.put(key, value);
+					}
+				}
+				if ("true".equals(getProperty(SET_SYSTEM_PROPERTIES))) {
+					log("Adding resolved properties to System properties");
+					for (Object key : Collections.list(this.properties.propertyNames())) {
+						String value = this.properties.getProperty((String) key);
+						System.setProperty((String) key, value);
+					}
 				}
+				// Load the first one we find
+				return;
+			}
+			else {
+				log("Not found: " + config);
 			}
 		}
-		else {
-			log("Not found: " + config);
-		}
-
 	}
 
 	private InputStream getResource(String config) throws Exception {
@@ -354,34 +370,50 @@ private ClassLoader wrapWithCustomClassLoader(ClassLoader parent,
 	}
 
 	private String getProperty(String propertyKey) throws Exception {
-		return getProperty(propertyKey, null);
+		return getProperty(propertyKey, null, null);
 	}
 
 	private String getProperty(String propertyKey, String manifestKey) throws Exception {
+		return getProperty(propertyKey, manifestKey, null);
+	}
+
+	private String getPropertyWithDefault(String propertyKey, String defaultValue)
+			throws Exception {
+		return getProperty(propertyKey, null, defaultValue);
+	}
+
+	private String getProperty(String propertyKey, String manifestKey,
+			String defaultValue) throws Exception {
 		if (manifestKey == null) {
 			manifestKey = propertyKey.replace('.', '-');
 			manifestKey = toCamelCase(manifestKey);
 		}
 		String property = SystemPropertyUtils.getProperty(propertyKey);
 		if (property != null) {
-			String value = SystemPropertyUtils.resolvePlaceholders(property);
+			String value = SystemPropertyUtils.resolvePlaceholders(this.properties,
+					property);
 			log("Property '" + propertyKey + "' from environment: " + value);
 			return value;
 		}
 		if (this.properties.containsKey(propertyKey)) {
-			String value = SystemPropertyUtils
-					.resolvePlaceholders(this.properties.getProperty(propertyKey));
+			String value = SystemPropertyUtils.resolvePlaceholders(this.properties,
+					this.properties.getProperty(propertyKey));
 			log("Property '" + propertyKey + "' from properties: " + value);
 			return value;
 		}
 		try {
-			// Prefer home dir for MANIFEST if there is one
-			Manifest manifest = new ExplodedArchive(this.home, false).getManifest();
-			if (manifest != null) {
-				String value = manifest.getMainAttributes().getValue(manifestKey);
-				log("Property '" + manifestKey + "' from home directory manifest: "
-						+ value);
-				return value;
+			if (this.home != null) {
+				// Prefer home dir for MANIFEST if there is one
+				Manifest manifest = new ExplodedArchive(this.home, false).getManifest();
+				if (manifest != null) {
+					String value = manifest.getMainAttributes().getValue(manifestKey);
+					if (value != null) {
+						log("Property '" + manifestKey
+								+ "' from home directory manifest: " + value);
+						return SystemPropertyUtils.resolvePlaceholders(this.properties,
+								value);
+					}
+				}
 			}
 		}
 		catch (IllegalStateException ex) {
@@ -393,10 +425,11 @@ private String getProperty(String propertyKey, String manifestKey) throws Except
 			String value = manifest.getMainAttributes().getValue(manifestKey);
 			if (value != null) {
 				log("Property '" + manifestKey + "' from archive manifest: " + value);
-				return value;
+				return SystemPropertyUtils.resolvePlaceholders(this.properties, value);
 			}
 		}
-		return null;
+		return defaultValue == null ? defaultValue
+				: SystemPropertyUtils.resolvePlaceholders(this.properties, defaultValue);
 	}
 
 	@Override
@@ -436,10 +469,10 @@ private List<Archive> getClassPathArchives(String path) throws Exception {
 			log("Adding classpath entries from archive " + archive.getUrl() + root);
 			lib.add(archive);
 		}
-		Archive nested = getNestedArchive(root);
+		List<Archive> nested = getNestedArchive(root);
 		if (nested != null) {
-			log("Adding classpath entries from nested " + nested.getUrl() + root);
-			lib.add(nested);
+			log("Adding classpath entries from nested " + root);
+			lib.addAll(nested);
 		}
 		return lib;
 	}
@@ -457,19 +490,21 @@ private Archive getArchive(File file) throws IOException {
 		return null;
 	}
 
-	private Archive getNestedArchive(String root) throws Exception {
+	private List<Archive> getNestedArchive(String root) throws Exception {
+		List<Archive> list = new ArrayList<Archive>();
 		if (root.startsWith("/")
 				|| this.parent.getUrl().equals(this.home.toURI().toURL())) {
 			// If home dir is same as parent archive, no need to add it twice.
-			return null;
+			return list;
 		}
 		EntryFilter filter = new PrefixMatchingArchiveFilter(root);
 		if (this.parent.getNestedArchives(filter).isEmpty()) {
-			return null;
+			return list;
 		}
 		// If there are more archives nested in this subdirectory (root) then create a new
 		// virtual archive for them, and have it added to the classpath
-		return new FilteredArchive(this.parent, filter);
+		list.add(new FilteredArchive(this.parent, filter));
+		return list;
 	}
 
 	private void addNestedEntries(List<Archive> lib) {
@@ -548,6 +583,11 @@ private void log(String message) {
 		}
 	}
 
+	private void warn(String message) {
+		// We shouldn't use java.util.logging because of classpath issues
+		System.out.println(message);
+	}
+
 	/**
 	 * Convenience class for finding nested archives that have a prefix in their file path
 	 * (e.g. "lib/").

spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/PropertiesLauncherTests.java

@@ -31,6 +31,7 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.MockitoAnnotations;
 
@@ -50,6 +51,9 @@ public class PropertiesLauncherTests {
 	@Rule
 	public InternalOutputCapture output = new InternalOutputCapture();
 
+	@Rule
+	public ExpectedException expected = ExpectedException.none();
+
 	@Rule
 	public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
@@ -72,9 +76,28 @@ public void close() {
 
 	@Test
 	public void testDefaultHome() {
+		System.clearProperty("loader.home");
+		PropertiesLauncher launcher = new PropertiesLauncher();
+		assertThat(launcher.getHomeDirectory())
+				.isEqualTo(new File(System.getProperty("user.dir")));
+	}
+
+	@Test
+	public void testAlternateHome() throws Exception {
+		System.setProperty("loader.home", "src/test/resources/home");
 		PropertiesLauncher launcher = new PropertiesLauncher();
 		assertThat(launcher.getHomeDirectory())
 				.isEqualTo(new File(System.getProperty("loader.home")));
+		assertThat(launcher.getMainClass()).isEqualTo("demo.HomeApplication");
+	}
+
+	@Test
+	public void testNonExistentHome() throws Exception {
+		System.setProperty("loader.home", "src/test/resources/nonexistent");
+		this.expected.expectMessage("Invalid source folder");
+		PropertiesLauncher launcher = new PropertiesLauncher();
+		assertThat(launcher.getHomeDirectory())
+				.isNotEqualTo(new File(System.getProperty("loader.home")));
 	}
 
 	@Test
@@ -93,6 +116,13 @@ public void testUserSpecifiedConfigName() throws Exception {
 				.isEqualTo("[etc/]");
 	}
 
+	@Test
+	public void testRootOfClasspathFirst() throws Exception {
+		System.setProperty("loader.config.name", "bar");
+		PropertiesLauncher launcher = new PropertiesLauncher();
+		assertThat(launcher.getMainClass()).isEqualTo("my.BarApplication");
+	}
+
 	@Test
 	public void testUserSpecifiedDotPath() throws Exception {
 		System.setProperty("loader.path", ".");
@@ -232,6 +262,13 @@ public void testLoadPathCustomizedUsingManifest() throws Exception {
 				.containsExactly("/foo.jar", "/bar/");
 	}
 
+	@Test
+	public void testManifestWithPlaceholders() throws Exception {
+		System.setProperty("loader.home", "src/test/resources/placeholders");
+		PropertiesLauncher launcher = new PropertiesLauncher();
+		assertThat(launcher.getMainClass()).isEqualTo("demo.FooApplication");
+	}
+
 	private void waitFor(String value) throws Exception {
 		int count = 0;
 		boolean timeout = false;

spring-boot-tools/spring-boot-loader/src/test/resources/BOOT-INF/classes/application.properties

@@ -1 +1 @@
-loader.main: demo.Application
+loader.main: demo.NoSuchApplication

spring-boot-tools/spring-boot-loader/src/test/resources/BOOT-INF/classes/bar.properties

@@ -0,0 +1 @@
+loader.main: my.BootInfBarApplication