Improve handling for remove and setting retrieval

Improve handling of remove patches without an index
but which have an container.

When retrieving settings from emf like paths, the code
should first check whether an object can be retrieved and
only then do some path checks.
This way paths containing only an object id won't fail.

Author: eneufeld

bundles/org.eclipse.emfcloud.modelserver.common/src/org/eclipse/emfcloud/modelserver/common/patch/AbstractJsonPatchHelper.java

@@ -187,47 +187,38 @@ protected Command getRemoveCommand(final String modelURI, final ResourceSet reso
       String jsonPath = path.asText();
       SettingValue setting = getSetting(modelURI, resourceSet, jsonPath);
 
+      // If neither the index nor the feature are present, we delete an Object
+      EObject eObject = setting.getEObject();
+
       // If the index is present, we remove a value from a list
       if (setting.getIndex().isPresent()) {
          int index = setting.getIndex().get();
          if (index == CommandParameter.NO_INDEX) {
             // If the index is "-", remove the last value from the list
-            Object currentValue = setting.getEObject().eGet(setting.getFeature());
+            Object currentValue = eObject.eGet(setting.getFeature());
             if (currentValue instanceof Collection) {
                index = ((Collection<?>) currentValue).size() - 1;
             }
          }
-         return RemoveCommand.create(getEditingDomain(setting.getEObject()), setting.getEObject(),
-            setting.getFeature(), index);
+         return RemoveCommand.create(getEditingDomain(eObject), eObject, setting.getFeature(), index);
+      }
+
+      // if we are in a feature which is a list, remove the element from said feature
+      if (eObject.eContainmentFeature() != null && eObject.eContainmentFeature().isMany()) {
+         return RemoveCommand.create(getEditingDomain(eObject), eObject.eContainer(), eObject.eContainmentFeature(),
+            eObject);
       }
 
       // If the feature is present, but index is absent, we unset the value (i.e. set it to default value)
       if (setting.getFeature() != null) {
-         Object defaultValue = getDefaultValue(setting.getFeature());
-         return SetCommand.create(getEditingDomain(setting.getEObject()), setting.getEObject(), setting.getFeature(),
-            defaultValue);
+         return SetCommand.create(getEditingDomain(eObject), eObject, setting.getFeature(), SetCommand.UNSET_VALUE);
       }
 
-      // If neither the index nor the feature are present, we delete an Object
-      EObject eObjectToDelete = setting.getEObject();
-
-      Command deleteCommand = DeleteCommand.create(getEditingDomain(eObjectToDelete), eObjectToDelete);
+      Command deleteCommand = DeleteCommand.create(getEditingDomain(eObject), eObject);
 
       return deleteCommand;
    }
 
-   protected Object getDefaultValue(final EStructuralFeature feature) {
-      Object defaultValue = feature.getDefaultValue();
-      if (defaultValue == null && feature.isMany()) {
-         // Special case for lists default value.
-         // If the feature represents a collection, and 'null' is specified as the default
-         // value, we need to use an empty collection instead. Otherwise, SetCommand(object, feature, null)
-         // will cause a NPE.
-         return Collections.emptyList();
-      }
-      return defaultValue;
-   }
-
    protected Command getAddCommand(final String modelURI, final ResourceSet resourceSet, final JsonNode patchAction)
       throws JsonPatchException {
       JsonNode path = patchAction.get(PATH);
@@ -599,18 +590,17 @@ protected Object getValueFromList(final String jsonPath, final String[] segments
     */
    protected SettingValue getSettingFromCustomPath(final String modelURI, final String jsonPath)
       throws JsonPatchException {
-      int lastSegmentPos = jsonPath.lastIndexOf('/');
-      if (lastSegmentPos < 0 || lastSegmentPos >= jsonPath.length()) {
-         throw new JsonPatchException("Failed to parse Json Path: " + jsonPath);
-      }
-
       // The path may represent an EObject (without feature or index). In that case,
       // directly return it.
       EObject eObject = getEObject(modelURI, URI.createURI(jsonPath));
       if (eObject != null) {
          return new SettingValue(eObject);
       }
 
+      int lastSegmentPos = jsonPath.lastIndexOf('/');
+      if (lastSegmentPos < 0 || lastSegmentPos >= (jsonPath.length() - 1)) {
+         throw new JsonPatchException("Failed to parse Json Path: " + jsonPath);
+      }
       // XXX if the edited object is the root element, its positional URI will be '/'
       // Since we expect path as <id>/<feature>, the expected path in that case is
       // "//feature" and not "/feature" (Although "/feature" would be a valid Json Pointer path)