From 12ccc1574b9f7a2a21c4e86a55e0a5c176a01e30 Mon Sep 17 00:00:00 2001
From: Phillipus <p.beauvoir@dadabeatnik.com>
Date: Sat, 30 Sep 2023 16:04:16 +0100
Subject: [PATCH] [Features] Allow a null value

---
 .../editor/model/commands/FeatureCommand.java |  8 +++++---
 .../archimatetool/model/IFeaturesEList.java   |  2 +-
 .../model/impl/FeaturesEList.java             | 10 ++++++----
 .../model/impl/FeaturesEListTests.java        | 20 +++++++++++++++++++
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/com.archimatetool.editor/src/com/archimatetool/editor/model/commands/FeatureCommand.java b/com.archimatetool.editor/src/com/archimatetool/editor/model/commands/FeatureCommand.java
index 3b4e09f9d..0c4569004 100644
--- a/com.archimatetool.editor/src/com/archimatetool/editor/model/commands/FeatureCommand.java
+++ b/com.archimatetool.editor/src/com/archimatetool/editor/model/commands/FeatureCommand.java
@@ -5,6 +5,8 @@
  */
 package com.archimatetool.editor.model.commands;
 
+import java.util.Objects;
+
 import org.eclipse.gef.commands.Command;
 
 import com.archimatetool.model.IFeatures;
@@ -24,8 +26,8 @@ public FeatureCommand(String label, IFeatures featuresObject, String name, Objec
         setLabel(label);
         features = featuresObject.getFeatures();
         this.name = name;
-        this.value = value.toString();
-        this.defaultValue = defaultValue.toString();
+        this.value = value != null ? value.toString() : null;
+        this.defaultValue = defaultValue != null ? defaultValue.toString() : null;
         oldValue = features.getString(name, this.defaultValue);
     }
 
@@ -41,7 +43,7 @@ public void undo() {
     
     @Override
     public boolean canExecute() {
-        return value != null && !value.equals(oldValue);
+        return !Objects.equals(value, oldValue);
     }
 
     @Override
diff --git a/com.archimatetool.model/src/com/archimatetool/model/IFeaturesEList.java b/com.archimatetool.model/src/com/archimatetool/model/IFeaturesEList.java
index 1524378b7..12ebe55cc 100644
--- a/com.archimatetool.model/src/com/archimatetool/model/IFeaturesEList.java
+++ b/com.archimatetool.model/src/com/archimatetool/model/IFeaturesEList.java
@@ -17,7 +17,7 @@ public interface IFeaturesEList extends EList<IFeature> {
     /**
      * Put or update a name/value as an IFeature
      * @param name The name, non-null. If it already exists the value will be updated with the new value.
-     * @param value The value, non-null string.
+     * @param value The value, can be null
      * @return the updated or new feature
      */
     IFeature putString(String name, String value);
diff --git a/com.archimatetool.model/src/com/archimatetool/model/impl/FeaturesEList.java b/com.archimatetool.model/src/com/archimatetool/model/impl/FeaturesEList.java
index 8852ac5cd..4b2d8935a 100644
--- a/com.archimatetool.model/src/com/archimatetool/model/impl/FeaturesEList.java
+++ b/com.archimatetool.model/src/com/archimatetool/model/impl/FeaturesEList.java
@@ -5,6 +5,8 @@
  */
 package com.archimatetool.model.impl;
 
+import java.util.Objects;
+
 import org.eclipse.emf.ecore.InternalEObject;
 import org.eclipse.emf.ecore.util.EObjectContainmentEList;
 
@@ -17,6 +19,7 @@
  * 
  * @author Phillip Beauvoir
  */
+@SuppressWarnings("nls")
 public class FeaturesEList extends EObjectContainmentEList<IFeature> implements IFeaturesEList {
     
     public FeaturesEList(Class<?> dataClass, InternalEObject owner, int featureID) {
@@ -31,10 +34,9 @@ public IFeature putString(String name, String value) {
     @Override
     public IFeature putString(String name, String value, String defaultValue) {
         checkNull(name);
-        checkNull(value);
         
         // value == default value so remove it or don't add it and return null
-        if(value.equals(defaultValue)) {
+        if(Objects.equals(value, defaultValue)) {
             remove(name);
             return null;
         }
@@ -49,7 +51,7 @@ public IFeature putString(String name, String value, String defaultValue) {
             add(feature);
         }
         // Different value
-        else if(!value.equals(feature.getValue())) {
+        else if(!Objects.equals(value, feature.getValue())) {
             feature.setValue(value);
         }
         
@@ -131,7 +133,7 @@ public IFeature getFeature(String name) {
 
     private void checkNull(String s) {
         if(s == null) {
-            throw new IllegalArgumentException("key or value cannot be null"); //$NON-NLS-1$
+            throw new IllegalArgumentException("name cannot be null");
         }
     }
     
diff --git a/tests/com.archimatetool.model.tests/src/com/archimatetool/model/impl/FeaturesEListTests.java b/tests/com.archimatetool.model.tests/src/com/archimatetool/model/impl/FeaturesEListTests.java
index 3c11bfe1f..5f56a17f2 100644
--- a/tests/com.archimatetool.model.tests/src/com/archimatetool/model/impl/FeaturesEListTests.java
+++ b/tests/com.archimatetool.model.tests/src/com/archimatetool/model/impl/FeaturesEListTests.java
@@ -73,6 +73,26 @@ public void putStringWithDefaultValueShouldNotAddFeature() {
         assertTrue(list.isEmpty());
     }
     
+    @Test
+    public void putStringNullWithDefault() {
+        IFeature feature = list.putString("name1", "value", "default");
+        assertNotNull(feature);
+        
+        feature = list.putString("name1", null, "default");
+        assertEquals("name1", list.get(0).getName());
+        assertNull(list.get(0).getValue());
+    }
+    
+    @Test
+    public void putStringNullWithNullDefault() {
+        IFeature feature = list.putString("name1", "value", null);
+        assertNotNull(feature);
+        
+        feature = list.putString("name1", null, null);
+        assertNull(feature);
+        assertTrue(list.isEmpty());
+    }
+    
     @Test
     public void putInt() {
         IFeature feature = list.putInt("name1", 1);