Merge pull request #8346 from Dave Syer

* gh-7221:
  Polish "Tighten up PropertiesLauncher's contract"
  Tighten up PropertiesLauncher's contract

Author: wilkinsona

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,10 @@ 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 +226,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 +249,7 @@ be used:
 |`LOADER_PATH`
 
 |`loader.home`
-|
+|`Loader-Home`
 |`LOADER_HOME`
 
 |`loader.args`
@@ -253,11 +261,11 @@ be used:
 |`LOADER_MAIN`
 
 |`loader.config.location`
-|
+|`Loader-Config-Location`
 |`LOADER_CONFIG_LOCATION`
 
 |`loader.system`
-|
+|`Loader-System`
 |`LOADER_SYSTEM`
 
 |===
@@ -266,15 +274,24 @@ 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
-  the default) as long as `loader.config.location` is not specified.
+* `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) {
+				debug("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("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))) {
+					debug("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 {
+				debug("Not found: " + config);
 			}
 		}
-		else {
-			log("Not found: " + config);
-		}
-
 	}
 
 	private InputStream getResource(String config) throws Exception {
@@ -216,13 +232,13 @@ private InputStream getClasspathResource(String config) {
 			config = config.substring(1);
 		}
 		config = "/" + config;
-		log("Trying classpath: " + config);
+		debug("Trying classpath: " + config);
 		return getClass().getResourceAsStream(config);
 	}
 
 	private InputStream getFileResource(String config) throws Exception {
 		File file = new File(config);
-		log("Trying file: " + config);
+		debug("Trying file: " + config);
 		if (file.canRead()) {
 			return new FileInputStream(file);
 		}
@@ -278,7 +294,7 @@ private void initializePaths() throws Exception {
 		if (path != null) {
 			this.paths = parsePathsProperty(path);
 		}
-		log("Nested archive paths: " + this.paths);
+		debug("Nested archive paths: " + this.paths);
 	}
 
 	private List<String> parsePathsProperty(String commaSeparatedPaths) {
@@ -326,7 +342,7 @@ protected ClassLoader createClassLoader(List<Archive> archives) throws Exception
 		String customLoaderClassName = getProperty("loader.classLoader");
 		if (customLoaderClassName != null) {
 			loader = wrapWithCustomClassLoader(loader, customLoaderClassName);
-			log("Using custom class loader: " + customLoaderClassName);
+			debug("Using custom class loader: " + customLoaderClassName);
 		}
 		return loader;
 	}
@@ -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);
-			log("Property '" + propertyKey + "' from environment: " + value);
+			String value = SystemPropertyUtils.resolvePlaceholders(this.properties,
+					property);
+			debug("Property '" + propertyKey + "' from environment: " + value);
 			return value;
 		}
 		if (this.properties.containsKey(propertyKey)) {
-			String value = SystemPropertyUtils
-					.resolvePlaceholders(this.properties.getProperty(propertyKey));
-			log("Property '" + propertyKey + "' from properties: " + value);
+			String value = SystemPropertyUtils.resolvePlaceholders(this.properties,
+					this.properties.getProperty(propertyKey));
+			debug("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) {
+						debug("Property '" + manifestKey
+								+ "' from home directory manifest: " + value);
+						return SystemPropertyUtils.resolvePlaceholders(this.properties,
+								value);
+					}
+				}
 			}
 		}
 		catch (IllegalStateException ex) {
@@ -392,11 +424,12 @@ private String getProperty(String propertyKey, String manifestKey) throws Except
 		if (manifest != null) {
 			String value = manifest.getMainAttributes().getValue(manifestKey);
 			if (value != null) {
-				log("Property '" + manifestKey + "' from archive manifest: " + value);
-				return value;
+				debug("Property '" + manifestKey + "' from archive manifest: " + value);
+				return SystemPropertyUtils.resolvePlaceholders(this.properties, value);
 			}
 		}
-		return null;
+		return defaultValue == null ? defaultValue
+				: SystemPropertyUtils.resolvePlaceholders(this.properties, defaultValue);
 	}
 
 	@Override
@@ -427,18 +460,18 @@ private List<Archive> getClassPathArchives(String path) throws Exception {
 			file = new File(this.home, root);
 		}
 		if (file.isDirectory()) {
-			log("Adding classpath entries from " + file);
+			debug("Adding classpath entries from " + file);
 			Archive archive = new ExplodedArchive(file, false);
 			lib.add(archive);
 		}
 		Archive archive = getArchive(file);
 		if (archive != null) {
-			log("Adding classpath entries from archive " + archive.getUrl() + root);
+			debug("Adding classpath entries from archive " + archive.getUrl() + root);
 			lib.add(archive);
 		}
 		Archive nested = getNestedArchive(root);
 		if (nested != null) {
-			log("Adding classpath entries from nested " + nested.getUrl() + root);
+			debug("Adding classpath entries from nested " + root);
 			lib.add(nested);
 		}
 		return lib;
@@ -540,14 +573,21 @@ private static String capitalize(String str) {
 		return Character.toUpperCase(str.charAt(0)) + str.substring(1);
 	}
 
-	private void log(String message) {
+	private void debug(String message) {
 		if (Boolean.getBoolean(DEBUG)) {
-			// We shouldn't use java.util.logging because of classpath issues so we
-			// just sysout log messages when "loader.debug" is true
-			System.out.println(message);
+			log(message);
 		}
 	}
 
+	private void warn(String message) {
+		log("WARNING: " + message);
+	}
+
+	private void log(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/").