Minor YAML config improvements (#15389)

- Make hazelcast-specific root-nodes optional
- Remove usages of locateFromSystemPropertyOrFailOnUnacceptedSuffix
config resolution codes, since it is confusing
- Align comments with the actual behavior
- Update tests with the changes

hazelcast/src/main/java/com/hazelcast/client/config/XmlClientConfigLocator.java

@@ -33,7 +33,7 @@ public boolean locateFromSystemProperty() {
     }
 
     @Override
-    public boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
+    protected boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
         return loadFromSystemPropertyOrFailOnUnacceptedSuffix(SYSPROP_CLIENT_CONFIG, XML_ACCEPTED_SUFFIXES);
     }
 

hazelcast/src/main/java/com/hazelcast/client/config/XmlClientFailoverConfigLocator.java

@@ -33,7 +33,7 @@ public boolean locateFromSystemProperty() {
     }
 
     @Override
-    public boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
+    protected boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
         return loadFromSystemPropertyOrFailOnUnacceptedSuffix(SYSPROP_CLIENT_FAILOVER_CONFIG, XML_ACCEPTED_SUFFIXES);
     }
 

hazelcast/src/main/java/com/hazelcast/client/config/YamlClientConfigBuilder.java

@@ -140,8 +140,7 @@ private void parseAndBuildConfig(ClientConfig config) throws Exception {
 
         YamlNode clientRoot = yamlRootNode.childAsMapping(ClientConfigSections.HAZELCAST_CLIENT.name);
         if (clientRoot == null) {
-            throw new InvalidConfigurationException("No mapping with hazelcast-client key is found in the provided "
-                    + "configuration");
+            clientRoot = yamlRootNode;
         }
 
         YamlDomChecker.check(clientRoot);

hazelcast/src/main/java/com/hazelcast/client/config/YamlClientConfigLocator.java

@@ -33,7 +33,7 @@ public boolean locateFromSystemProperty() {
     }
 
     @Override
-    public boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
+    protected boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
         return loadFromSystemPropertyOrFailOnUnacceptedSuffix(SYSPROP_CLIENT_CONFIG, YAML_ACCEPTED_SUFFIXES);
     }
 

hazelcast/src/main/java/com/hazelcast/client/config/YamlClientFailoverConfigBuilder.java

@@ -143,8 +143,7 @@ private void parseAndBuildConfig(ClientFailoverConfig config) throws Exception {
         String configRoot = getConfigRoot();
         YamlNode clientFailoverRoot = yamlRootNode.childAsMapping(configRoot);
         if (clientFailoverRoot == null) {
-            String message = String.format("No mapping with %s key is found in the provided configuration", configRoot);
-            throw new InvalidConfigurationException(message);
+            clientFailoverRoot = yamlRootNode;
         }
 
         YamlDomChecker.check(clientFailoverRoot);

hazelcast/src/main/java/com/hazelcast/client/config/YamlClientFailoverConfigLocator.java

@@ -33,7 +33,7 @@ public boolean locateFromSystemProperty() {
     }
 
     @Override
-    public boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
+    protected boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
         return loadFromSystemPropertyOrFailOnUnacceptedSuffix(SYSPROP_CLIENT_FAILOVER_CONFIG, YAML_ACCEPTED_SUFFIXES);
     }
 

hazelcast/src/main/java/com/hazelcast/client/impl/clientside/FailoverClientConfigSupport.java

@@ -105,11 +105,11 @@ private static ClientFailoverConfig locateAndCreateClientFailoverConfig() {
         YamlClientFailoverConfigLocator yamlConfigLocator = new YamlClientFailoverConfigLocator();
 
         if (xmlConfigLocator.locateFromSystemProperty()) {
-            // 1. Try loading config if provided in system property and it is an XML file
+            // 1. Try loading XML config from the configuration provided in system property
             config = new XmlClientFailoverConfigBuilder(xmlConfigLocator).build();
 
-        } else if (yamlConfigLocator.locateFromSystemPropertyOrFailOnUnacceptedSuffix()) {
-            // 2. Try loading config if provided in system property and it is an YAML file
+        } else if (yamlConfigLocator.locateFromSystemProperty()) {
+            // 2. Try loading YAML config from the configuration provided in system property
             config = new YamlClientFailoverConfigBuilder(yamlConfigLocator).build();
 
         } else if (xmlConfigLocator.locateInWorkDirOrOnClasspath()) {
@@ -135,11 +135,11 @@ private static ClientConfig locateAndCreateClientConfig() {
         YamlClientConfigLocator yamlConfigLocator = new YamlClientConfigLocator();
 
         if (xmlConfigLocator.locateFromSystemProperty()) {
-            // 1. Try loading config if provided in system property and it is an XML file
+            // 1. Try loading XML config from the configuration provided in system property
             config = new XmlClientConfigBuilder(xmlConfigLocator).build();
 
-        } else if (yamlConfigLocator.locateFromSystemPropertyOrFailOnUnacceptedSuffix()) {
-            // 2. Try loading config if provided in system property and it is an YAML file
+        } else if (yamlConfigLocator.locateFromSystemProperty()) {
+            // 2. Try loading YAML config from the configuration provided in system property
             config = new YamlClientConfigBuilder(yamlConfigLocator).build();
 
         } else if (xmlConfigLocator.locateInWorkDirOrOnClasspath()) {

hazelcast/src/main/java/com/hazelcast/config/AbstractConfigLocator.java

@@ -82,7 +82,7 @@ public boolean isConfigPresent() {
      *                            file referenced in the system property
      *                            is not an accepted suffix
      */
-    public abstract boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix();
+    protected abstract boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix();
 
     /**
      * Locates the configuration file in the working directory.

hazelcast/src/main/java/com/hazelcast/config/AbstractYamlConfigBuilder.java

@@ -108,7 +108,7 @@ protected void importDocuments(YamlNode imdgRoot) throws Exception {
 
             YamlNode imdgRootLoaded = asMapping(rootLoaded).child(getConfigRoot());
             if (imdgRootLoaded == null) {
-                return;
+                imdgRootLoaded = rootLoaded;
             }
 
             replaceVariables(asW3cNode(imdgRootLoaded));

hazelcast/src/main/java/com/hazelcast/config/XmlConfigLocator.java

@@ -36,7 +36,7 @@ public boolean locateFromSystemProperty() {
     }
 
     @Override
-    public boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
+    protected boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
         return loadFromSystemPropertyOrFailOnUnacceptedSuffix(SYSPROP_MEMBER_CONFIG, XML_ACCEPTED_SUFFIXES);
     }
 

hazelcast/src/main/java/com/hazelcast/config/YamlConfigBuilder.java

@@ -144,7 +144,7 @@ private void parseAndBuildConfig(Config config) throws Exception {
 
         YamlNode imdgRoot = yamlRootNode.childAsMapping(ConfigSections.HAZELCAST.name);
         if (imdgRoot == null) {
-            throw new InvalidConfigurationException("No mapping with hazelcast key is found in the provided configuration");
+            imdgRoot = yamlRootNode;
         }
 
         YamlDomChecker.check(imdgRoot);

hazelcast/src/main/java/com/hazelcast/config/YamlConfigLocator.java

@@ -36,7 +36,7 @@ public boolean locateFromSystemProperty() {
     }
 
     @Override
-    public boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
+    protected boolean locateFromSystemPropertyOrFailOnUnacceptedSuffix() {
         return loadFromSystemPropertyOrFailOnUnacceptedSuffix(SYSPROP_MEMBER_CONFIG, YAML_ACCEPTED_SUFFIXES);
     }
 

hazelcast/src/main/java/com/hazelcast/instance/impl/HazelcastInstanceFactory.java

@@ -134,11 +134,11 @@ public static HazelcastInstance newHazelcastInstance(Config config) {
             YamlConfigLocator yamlConfigLocator = new YamlConfigLocator();
 
             if (xmlConfigLocator.locateFromSystemProperty()) {
-                // 1. Try loading XML config if provided in system property with any extension
+                // 1. Try loading XML config from the configuration provided in system property
                 config = new XmlConfigBuilder(xmlConfigLocator).build();
 
-            } else if (yamlConfigLocator.locateFromSystemPropertyOrFailOnUnacceptedSuffix()) {
-                // 2. Try loading YAML config if provided in system property with .yaml or .yml extension
+            } else if (yamlConfigLocator.locateFromSystemProperty()) {
+                // 2. Try loading YAML config from the configuration provided in system property
                 config = new YamlConfigBuilder(yamlConfigLocator).build();
 
             } else if (xmlConfigLocator.locateInWorkDirOrOnClasspath()) {

hazelcast/src/test/java/com/hazelcast/client/config/AbstractClientConfigBuilderTest.java

@@ -403,9 +403,6 @@ public void testSecurityConfig() {
         assertEquals("value", properties.getProperty("property"));
     }
 
-    @Test(expected = InvalidConfigurationException.class)
-    public abstract void testInvalidRootElement();
-
     @Test(expected = HazelcastException.class)
     public abstract void loadingThroughSystemProperty_nonExistingFile() throws IOException;
 

hazelcast/src/test/java/com/hazelcast/client/config/AbstractClientConfigImportVariableReplacementTest.java

@@ -34,7 +34,7 @@ public abstract class AbstractClientConfigImportVariableReplacementTest extends
     public TemporaryFolder tempFolder = new TemporaryFolder();
 
     @Test(expected = InvalidConfigurationException.class)
-    public abstract void testImportElementOnlyAppersInTopLevel();
+    public abstract void testImportElementOnlyAppearsInTopLevel() throws IOException;
 
     @Test(expected = InvalidConfigurationException.class)
     public abstract void testHazelcastElementOnlyAppearsOnce();

hazelcast/src/test/java/com/hazelcast/client/config/XmlClientConfigBuilderTest.java

@@ -85,7 +85,6 @@ public void beforeAndAfter() {
         System.clearProperty("hazelcast.client.config");
     }
 
-    @Override
     @Test(expected = InvalidConfigurationException.class)
     public void testInvalidRootElement() {
         String xml = "<hazelcast>"

hazelcast/src/test/java/com/hazelcast/client/config/XmlClientConfigImportVariableReplacementTest.java

@@ -48,7 +48,7 @@ public class XmlClientConfigImportVariableReplacementTest extends AbstractClient
 
     @Override
     @Test(expected = InvalidConfigurationException.class)
-    public void testImportElementOnlyAppersInTopLevel() {
+    public void testImportElementOnlyAppearsInTopLevel() {
         String xml = HAZELCAST_CLIENT_START_TAG
                 + "   <network>\n"
                 + "        <import resource=\"\"/>\n"

hazelcast/src/test/java/com/hazelcast/client/config/YamlClientConfigBuilderTest.java

@@ -76,15 +76,12 @@ public void beforeAndAfter() {
         System.clearProperty("hazelcast.client.config");
     }
 
-    @Override
-    @Test(expected = InvalidConfigurationException.class)
-    public void testInvalidRootElement() {
+    @Test
+    public void testNoHazelcastClientRootElement() {
         String yaml = ""
-                + "hazelcast:\n"
-                + "  group:\n"
-                + "    name: dev\n"
-                + "    password: clusterpass";
-        buildConfig(yaml);
+                + "instance-name: my-instance";
+        ClientConfig clientConfig = buildConfig(yaml);
+        assertEquals("my-instance", clientConfig.getInstanceName());
     }
 
     @Override

hazelcast/src/test/java/com/hazelcast/client/config/YamlClientConfigImportVariableReplacementTest.java

@@ -41,6 +41,7 @@
 import static com.hazelcast.client.config.YamlClientConfigBuilderTest.buildConfig;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(HazelcastParallelClassRunner.class)
@@ -50,15 +51,45 @@ public class YamlClientConfigImportVariableReplacementTest extends AbstractClien
     public ExpectedException rule = ExpectedException.none();
 
     @Override
-    @Test(expected = InvalidConfigurationException.class)
-    public void testImportElementOnlyAppersInTopLevel() {
+    @Test
+    public void testImportElementOnlyAppearsInTopLevel() throws IOException {
+        File config1 = createConfigFile("hz1", ".yaml");
+        FileOutputStream os1 = new FileOutputStream(config1);
+        String config1Yaml = ""
+                + "hazelcast-client:\n"
+                + "  instance-name: my-instance";
+        writeStringToStreamAndClose(os1, config1Yaml);
         String yaml = ""
-                + "hazelcast:\n"
+                + "hazelcast-client:\n"
                 + "  network:\n"
                 + "    import:\n"
-                + "      resource: \"\"";
+                + "      - file:///" + config1.getAbsolutePath() + "\"";
 
-        buildConfig(yaml);
+        ClientConfig clientConfig = buildConfig(yaml);
+
+        // verify that instance-name is not set, because the import is not
+        // processed when defined at this level
+        assertNull(clientConfig.getInstanceName());
+    }
+
+    @Test
+    public void testImportNoHazelcastClientRootNode() throws Exception {
+        File file = createConfigFile("foo", "bar");
+        FileOutputStream os = new FileOutputStream(file);
+        String importedYaml = ""
+                + "properties:\n"
+                + "  prop1: value1\n"
+                + "  prop2: value2\n";
+        writeStringToStreamAndClose(os, importedYaml);
+
+        String yaml = ""
+                + "import:\n"
+                + "  - file:///" + "${file}\n"
+                + "instance-name: my-instance";
+        ClientConfig clientConfig = buildConfig(yaml, "file", file.getAbsolutePath());
+        assertEquals("my-instance", clientConfig.getInstanceName());
+        assertEquals("value1", clientConfig.getProperty("prop1"));
+        assertEquals("value2", clientConfig.getProperty("prop2"));
     }
 
     @Override

hazelcast/src/test/java/com/hazelcast/client/config/YamlClientFailoverConfigBuilderTest.java

@@ -47,14 +47,11 @@ public void init() throws Exception {
         fullClientConfig = new YamlClientFailoverConfigBuilder(schemaResource).build();
     }
 
-    @Test(expected = InvalidConfigurationException.class)
-    public void testInvalidRootElement() {
-        String yaml = ""
-                + "hazelcast:\n"
-                + "  group:\n"
-                + "    name: dev\n"
-                + "    password: clusterpass";
-        buildConfig(yaml);
+    public void testNoHazelcastClientFailoverRootElement() {
+        String yaml = "try-count: 2";
+
+        ClientFailoverConfig clientFailoverConfig = buildConfig(yaml);
+        assertEquals(2, clientFailoverConfig.getTryCount());
     }
 
     @Test

hazelcast/src/test/java/com/hazelcast/config/AbstractConfigBuilderTest.java

@@ -63,9 +63,6 @@ public abstract class AbstractConfigBuilderTest extends HazelcastTestSupport {
     @Test(expected = IllegalArgumentException.class)
     public abstract void testConfiguration_withNullInputStream();
 
-    @Test(expected = InvalidConfigurationException.class)
-    public abstract void testInvalidRootElement();
-
     @Test(expected = InvalidConfigurationException.class)
     public abstract void testJoinValidation();
 

hazelcast/src/test/java/com/hazelcast/config/AbstractConfigImportVariableReplacementTest.java

@@ -18,7 +18,6 @@
 
 import com.hazelcast.config.replacer.PropertyReplacer;
 import com.hazelcast.config.replacer.spi.ConfigReplacer;
-import com.hazelcast.core.HazelcastException;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -79,9 +78,6 @@ protected static void writeStringToStreamAndClose(FileOutputStream os, String st
     @Test
     public abstract void testImportNotExistingResourceThrowsException();
 
-    @Test(expected = HazelcastException.class)
-    public abstract void testImportFromNonHazelcastConfigThrowsException() throws Exception;
-
     @Test
     public abstract void testImportNetworkConfigFromFile() throws Exception;
 

hazelcast/src/test/java/com/hazelcast/config/XMLConfigBuilderTest.java

@@ -139,18 +139,6 @@ public void testConfiguration_withNullInputStream() {
         new XmlConfigBuilder((InputStream) null);
     }
 
-    @Override
-    @Test(expected = InvalidConfigurationException.class)
-    public void testInvalidRootElement() {
-        String xml = "<hazelcast-client>"
-                + "<group>"
-                + "<name>dev</name>"
-                + "<password>clusterpass</password>"
-                + "</group>"
-                + "</hazelcast-client>";
-        buildConfig(xml);
-    }
-
     @Override
     @Test(expected = InvalidConfigurationException.class)
     public void testJoinValidation() {

hazelcast/src/test/java/com/hazelcast/config/XmlConfigImportVariableReplacementTest.java

@@ -210,7 +210,6 @@ public void testImportNotExistingResourceThrowsException() {
         buildConfig(xml, null);
     }
 
-    @Override
     @Test(expected = HazelcastException.class)
     public void testImportFromNonHazelcastConfigThrowsException() throws Exception {
         File file = createConfigFile("mymap", "config");

hazelcast/src/test/java/com/hazelcast/config/XmlOnlyConfigBuilderTest.java

@@ -188,6 +188,17 @@ public void testCacheConfig_withInvalidEvictionConfig_failsFast() {
         buildConfig(xml);
     }
 
+    @Test(expected = InvalidConfigurationException.class)
+    public void testInvalidRootElement() {
+        String xml = "<hazelcast-client>"
+                + "<group>"
+                + "<name>dev</name>"
+                + "<password>clusterpass</password>"
+                + "</group>"
+                + "</hazelcast-client>";
+        buildConfig(xml);
+    }
+
     @Test
     public void testAddWhitespaceToNonSpaceStrings() throws Exception {
         // parse the default config file

hazelcast/src/test/java/com/hazelcast/config/YamlConfigBuilderTest.java

@@ -123,17 +123,6 @@ public void testConfiguration_withNullInputStream() {
         new YamlConfigBuilder((InputStream) null);
     }
 
-    @Override
-    @Test(expected = InvalidConfigurationException.class)
-    public void testInvalidRootElement() {
-        String yaml = ""
-                + "hazelcast-client:\n"
-                + "  group:\n"
-                + "    name: dev\n"
-                + "    password: clusterpass";
-        buildConfig(yaml);
-    }
-
     @Override
     @Test(expected = InvalidConfigurationException.class)
     public void testJoinValidation() {