Refactor admin props

* rename class to AdminPropertiesFactory
* rename factory method to "create"
* remove side effects from methods
* returned properties default to system properties

src/main/java/org/cryptomator/launcher/AdminPropertiesFactory.java

@@ -14,9 +14,11 @@ import java.util.Properties;
 import java.util.Set;
 
 /**
- * Class to overwrite system properties with an external properties file
+ * Factory to generate admin properties.
+ *
  * <p>
- * To overwrite system properties, the method {@link #adjustSystemProperties()} reads the properties file defined in the property {@value #ADMIN_PROP_FILE_KEY} and writes all supported properties to the {@link System} properties.
+ * Admin properties are {@link Properties} using system properties as defaults, but allow overwriting a specific set of properties with an external config file.
+ * Those properties are created by calling {@link #create()}. The method first reads system property {@value #ADMIN_PROP_FILE_KEY}. If it contains a path to a valid properties file, all overridable properties from the file are loaded into the returned admin properties.
  * <p>
  * The overridable properties are:
  * <ul>
@@ -27,9 +29,10 @@ import java.util.Set;
  *     <li>cryptomator.disableUpdateCheck</li>
  * </ul>
  *
+ * @see Properties
  * @see System#getProperties()
  */
-class AdminPropertiesSetter {
+class AdminPropertiesFactory {
 
 	private static final Logger LOG = EventualLogger.INSTANCE;
 	private static final long MAX_CONFIG_SIZE_BYTES = 8192;
@@ -43,39 +46,38 @@ class AdminPropertiesSetter {
 
 
 	/**
-	 * Adjusts the system properties by loading administrative properties from a predefined file location.
+	 * Creates new {@link Properties} containing overridable properties from the admin config.
 	 * <p>
-	 * If the file exists and is a valid properties file, its content will overwrite existing system properties.
-	 * Only some properties can be overridden, see {@link AdminPropertiesSetter}
+	 * The returned properties object uses as default the {@link System} properties.
+	 * For a list of overridable properties, see {@link AdminPropertiesFactory}
 	 *
-	 * @return The adjusted system properties.
+	 * @return {@link Properties} containing overridable properties from the admin config and defaulting to system properties.
 	 */
-	static Properties adjustSystemProperties() {
+	static Properties create() {
 		var systemProps = System.getProperties();
+		var adminProps = new Properties(systemProps);
 
 		final String adminCfgPath = System.getProperty(ADMIN_PROP_FILE_KEY);
 		if (adminCfgPath == null) {
-			LOG.debug("Path to admin properties file is not defined.");
-			return systemProps;
+			LOG.debug("Admin config property is not defined. Skipping.");
+			return adminProps;
 		}
-		var adminProps = loadAdminProperties(Path.of(adminCfgPath));
+		var propsFromFile = loadPropertiesFromFile(Path.of(adminCfgPath));
 
-		var newSystemProps = new Properties(systemProps);
-		for (var key : adminProps.stringPropertyNames()) {
+		for (var key : propsFromFile.stringPropertyNames()) {
 			if (ALLOWED_OVERRIDES.contains(key)) {
-				var value = adminProps.getProperty(key);
+				var value = propsFromFile.getProperty(key);
 				LOG.info("Overwriting {} with value {} from admin config.", key, value);
-				newSystemProps.setProperty(key, value);
+				adminProps.setProperty(key, value);
 			} else {
 				LOG.debug("Property {} in admin config is not supported for override.", key);
 			}
 		}
-		System.setProperties(newSystemProps);
-		return newSystemProps;
+		return adminProps;
 	}
 
 	//visible for testing
-	static Properties loadAdminProperties(Path adminPropertiesPath) {
+	static Properties loadPropertiesFromFile(Path adminPropertiesPath) {
 		var adminProps = new Properties();
 		try (FileChannel ch = FileChannel.open(adminPropertiesPath, StandardOpenOption.READ); //
 			 Reader reader = Channels.newReader(ch, StandardCharsets.UTF_8)) {

src/main/java/org/cryptomator/launcher/Cryptomator.java

@@ -35,8 +35,8 @@ public class Cryptomator {
 	private static final long STARTUP_TIME = System.currentTimeMillis();
 
 	static {
-		var adjustedSystemProps = AdminPropertiesSetter.adjustSystemProperties();
-		var lazyProcessedProps = new SubstitutingProperties(adjustedSystemProps, System.getenv());
+		var adminProps = AdminPropertiesFactory.create();
+		var lazyProcessedProps = new SubstitutingProperties(adminProps, System.getenv());
 		System.setProperties(lazyProcessedProps);
 		CRYPTOMATOR_COMPONENT = DaggerCryptomatorComponent.factory().create(STARTUP_TIME);
 		LOG = LoggerFactory.getLogger(Cryptomator.class);

src/test/java/org/cryptomator/launcher/AdminPropertiesFactoryTest.java

@@ -21,7 +21,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mockStatic;
 import static org.mockito.Mockito.never;
 
-public class AdminPropertiesSetterTest {
+public class AdminPropertiesFactoryTest {
 
 	private static final String PROPS = """
 			fruit=banana
@@ -37,7 +37,7 @@ public class AdminPropertiesSetterTest {
 			out.write(bytes);
 		}
 
-		var properties = AdminPropertiesSetter.loadAdminProperties(config);
+		var properties = AdminPropertiesFactory.loadPropertiesFromFile(config);
 		Assertions.assertAll(List.of( //
 				() -> MatcherAssert.assertThat(properties, hasEntry("fruit", "banana")), //
 				() -> MatcherAssert.assertThat(properties, hasEntry("vegetable", "kärrot")), //
@@ -48,7 +48,7 @@ public class AdminPropertiesSetterTest {
 	@DisplayName("Loading not existing file returns empty properties")
 	void loadNotExistingFile(@TempDir Path path) {
 		var config = path.resolve("config.properties");
-		var properties = AdminPropertiesSetter.loadAdminProperties(config);
+		var properties = AdminPropertiesFactory.loadPropertiesFromFile(config);
 		MatcherAssert.assertThat(properties, anEmptyMap());
 	}
 
@@ -61,7 +61,7 @@ public class AdminPropertiesSetterTest {
 			out.write(bytes);
 		}
 
-		var properties = AdminPropertiesSetter.loadAdminProperties(config);
+		var properties = AdminPropertiesFactory.loadPropertiesFromFile(config);
 		MatcherAssert.assertThat(properties, anEmptyMap());
 	}
 
@@ -74,22 +74,23 @@ public class AdminPropertiesSetterTest {
 			channel.write(ByteBuffer.wrap("test=test".getBytes()));
 		}
 
-		var properties = AdminPropertiesSetter.loadAdminProperties(config);
+		var properties = AdminPropertiesFactory.loadPropertiesFromFile(config);
 		MatcherAssert.assertThat(properties, anEmptyMap());
 	}
 
 	@Test
-	@DisplayName("If system property for config path is null, skip loading and replacing")
-	void skipAdjustSystemPropertiesOnUndefinedProperty() {
+	@DisplayName("If system properties do not contain config path, skip loading")
+	void skipLoadIfFilePathIsNotDefined() {
 		Assertions.assertNull(System.getProperty("cryptomator.adminConfigPath"));
 
-		try (var adminPropSetterMock = mockStatic(AdminPropertiesSetter.class)) {
-			adminPropSetterMock.when(AdminPropertiesSetter::adjustSystemProperties).thenCallRealMethod();
-			adminPropSetterMock.when(() -> AdminPropertiesSetter.loadAdminProperties(any())).thenReturn(new Properties());
+		try (var adminPropSetterMock = mockStatic(AdminPropertiesFactory.class)) {
+			adminPropSetterMock.when(AdminPropertiesFactory::create).thenCallRealMethod();
+			adminPropSetterMock.when(() -> AdminPropertiesFactory.loadPropertiesFromFile(any())).thenReturn(new Properties());
 
-			AdminPropertiesSetter.adjustSystemProperties();
+			var adminProps = AdminPropertiesFactory.create();
 
-			adminPropSetterMock.verify(() -> AdminPropertiesSetter.loadAdminProperties(any()), never());
+			adminPropSetterMock.verify(() -> AdminPropertiesFactory.loadPropertiesFromFile(any()), never());
+			Assertions.assertEquals(System.getProperty("user.home"), adminProps.getProperty("user.home"));
 		}
 	}