Repository: hazelcast/hazelcast-kubernetes
Pull request #201 (2020-04-10 13:59:45)

empty properties treated as null

devOpsHazelcast: [![CLA assistant check](http://ec2-54-211-157-189.compute-1.amazonaws.com:5000/pull/badge/not_signed)](http://ec2-54-211-157-189.compute-1.amazonaws.com:5000/hazelcast/hazelcast-kubernetes?pullRequest=201) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](http://ec2-54-211-157-189.compute-1.amazonaws.com:5000/hazelcast/hazelcast-kubernetes?pullRequest=201) before we can accept your contribution.<br/><hr/>**jawitcher** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](http://ec2-54-211-157-189.compute-1.amazonaws.com:5000/check/hazelcast/hazelcast-kubernetes?pullRequest=201) it.</sub>
devOpsHazelcast: Can one of the admins verify this patch?
jawitcher: > 
> 
> @jawitcher Thanks. I had a look at the PR and it looks like what we need.
> 
> Could you also do the following and then change it from DRAFT to the PR?
> 
>     1. Add the same logic for `getMode()`
> 
>     2. Add unit tests
> 
>     3. Format the code
> 
> 
> Thanks!

I revised the code in accordance with your notes
mesutcelik: @jawitcher Can you please sign the CLA before the merge?
jawitcher: @leszko , I changed it
jawitcher: > 
> 
> @jawitcher Can you please sign the CLA before the merge?

Hi. I signed it :). My name is on the list https://hazelcast.atlassian.net/wiki/spaces/COM/pages/6357071/Hazelcast+Contributor+Agreement
jawitcher: > 
> 
> @jawitcher Can you please sign the CLA before the merge?

Hi. I signed it :)
leszko: Looks great 💯 

Thanks a lot for the change @jawitcher !

Patch:
diff --git a/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java b/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java
index 811a703..4980ee7 100644
--- a/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java
+++ b/src/main/java/com/hazelcast/kubernetes/KubernetesConfig.java
@@ -219,22 +219,23 @@ final class KubernetesConfig {
     }
 
     private void validateConfig() {
-        if (serviceDns != null && (serviceName != null || serviceLabelName != null || podLabelName != null)) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceDns) && (!StringUtil.isNullOrEmptyAfterTrim(serviceName)
+                || !StringUtil.isNullOrEmptyAfterTrim(serviceLabelName) || !StringUtil.isNullOrEmptyAfterTrim(podLabelName))) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and ('%s' or '%s' or %s) cannot be defined at the same time",
                             SERVICE_DNS.key(), SERVICE_NAME.key(), SERVICE_LABEL_NAME.key(), POD_LABEL_NAME.key()));
         }
-        if (serviceName != null && serviceLabelName != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceName) && !StringUtil.isNullOrEmptyAfterTrim(serviceLabelName)) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and '%s' cannot be defined at the same time",
                             SERVICE_NAME.key(), SERVICE_LABEL_NAME.key()));
         }
-        if (serviceName != null && podLabelName != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceName) && !StringUtil.isNullOrEmptyAfterTrim(podLabelName)) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and '%s' cannot be defined at the same time",
                             SERVICE_NAME.key(), POD_LABEL_NAME.key()));
         }
-        if (serviceLabelName != null && podLabelName != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceLabelName) && !StringUtil.isNullOrEmptyAfterTrim(podLabelName)) {
             throw new InvalidConfigurationException(
                     String.format("Properties '%s' and '%s' cannot be defined at the same time",
                             SERVICE_LABEL_NAME.key(), POD_LABEL_NAME.key()));
@@ -254,7 +255,7 @@ final class KubernetesConfig {
     }
 
     DiscoveryMode getMode() {
-        if (serviceDns != null) {
+        if (!StringUtil.isNullOrEmptyAfterTrim(serviceDns)) {
             return DiscoveryMode.DNS_LOOKUP;
         } else {
             return DiscoveryMode.KUBERNETES_API;


Test patch:
diff --git a/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java b/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java
index f8370d1..d3a753c 100644
--- a/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java
+++ b/src/test/java/com/hazelcast/kubernetes/KubernetesConfigTest.java
@@ -275,4 +275,52 @@ public class KubernetesConfigTest {
         }
         return temp.getAbsolutePath();
     }
+
+    @Test
+    public void propertyServiceNameIsEmpty() {
+        // given
+        Map<String, Comparable> properties = createProperties();
+        properties.put(SERVICE_NAME.key(), "  ");
+        String serviceDns = "service-dns";
+        properties.put(SERVICE_DNS.key(), serviceDns);
+
+        //when
+        KubernetesConfig config = new KubernetesConfig(properties);
+
+        //then
+        assertEquals(serviceDns, config.getServiceDns());
+
+    }
+
+    @Test
+    public void propertyServiceDnsIsNull() {
+        // given
+        Map<String, Comparable> properties = createProperties();
+        String serviceName = "service-name";
+        properties.put(SERVICE_NAME.key(), serviceName);
+        properties.put(SERVICE_DNS.key(), null);
+
+        //when
+        KubernetesConfig config = new KubernetesConfig(properties);
+
+        //then
+        assertEquals(serviceName, config.getServiceName());
+
+    }
+
+    @Test
+    public void emptyProperties() {
+        // given
+        Map<String, Comparable> properties = createProperties();
+        properties.put(SERVICE_LABEL_NAME.key(), "  ");
+        String serviceLabelValue = "service-label-value";
+        properties.put(SERVICE_LABEL_VALUE.key(), serviceLabelValue);
+        properties.put(SERVICE_DNS.key(), "");
+
+        //when
+        KubernetesConfig config = new KubernetesConfig(properties);
+
+        //then
+        assertEquals(serviceLabelValue, config.getServiceLabelValue());
+    }
 }