Fix #111: Add error/failure logging and centralize package configs (#113)

Logging:
We want to to log errors and failures by default to prevent those
problems from being swallowed up since they may contain important
information about underlying issues.

EPackageConfigurations:
Centralize EPackageConfiguration management and apply it on the Java
model server client to ensure proper setup. We also ensure that the
ChangePackage is properly initialized on the client-side. We already use
the RecordingModelResourceManager as default on the server so we need
the ChangePackage to be initialized.

Author: martin-fleck-at

bundles/org.eclipse.emfcloud.modelserver.client/META-INF/MANIFEST.MF

@@ -14,7 +14,8 @@ Require-Bundle: com.fasterxml.jackson.core.jackson-core;bundle-version="[2.9.0,3
  org.eclipse.emfcloud.modelserver.common;bundle-version="[0.7.0,1.0.0)",
  org.eclipse.emfcloud.modelserver.edit;bundle-version="[0.7.0,1.0.0)",
  org.eclipse.emfcloud.modelserver.lib;bundle-version="[0.7.0,1.0.0)",
- org.eclipse.emfcloud.modelserver.emf;bundle-version="[0.7.0,1.0.0)"
+ org.eclipse.emfcloud.modelserver.emf;bundle-version="[0.7.0,1.0.0)",
+ org.eclipse.emf.ecore.change;bundle-version="[2.14.0,3.0.0)"
 Export-Package: org.eclipse.emfcloud.modelserver.client,
  org.eclipse.emfcloud.modelserver.internal.client;x-internal:=true
 Bundle-Vendor: EclipseSource

bundles/org.eclipse.emfcloud.modelserver.client/src/org/eclipse/emfcloud/modelserver/client/ModelServerClient.java

@@ -36,6 +36,10 @@
 import org.eclipse.emfcloud.modelserver.common.codecs.XmiCodec;
 import org.eclipse.emfcloud.modelserver.emf.common.JsonResponseMember;
 import org.eclipse.emfcloud.modelserver.emf.common.JsonResponseType;
+import org.eclipse.emfcloud.modelserver.emf.configuration.ChangePackageConfiguration;
+import org.eclipse.emfcloud.modelserver.emf.configuration.CommandPackageConfiguration;
+import org.eclipse.emfcloud.modelserver.emf.configuration.EPackageConfiguration;
+import org.eclipse.emfcloud.modelserver.emf.configuration.EcorePackageConfiguration;
 import org.eclipse.emfcloud.modelserver.internal.client.EditingContextImpl;
 import org.eclipse.emfcloud.modelserver.jsonschema.Json;
 import org.jetbrains.annotations.NotNull;
@@ -64,20 +68,39 @@ public class ModelServerClient implements ModelServerClientApi<EObject>, ModelSe
    public static final String PATCH = "PATCH";
    public static final String POST = "POST";
 
-   private static Logger LOG = Logger.getLogger(ModelServerClient.class.getSimpleName());
+   protected static Logger LOG = Logger.getLogger(ModelServerClient.class.getSimpleName());
 
    protected final OkHttpClient client;
    protected final String baseUrl;
    protected final Map<String, WebSocket> openSockets = new LinkedHashMap<>();
    protected final Map<EditingContextImpl, WebSocket> openEditingSockets = new LinkedHashMap<>();
+   protected final EPackageConfiguration[] configurations;
 
-   public ModelServerClient(final String baseUrl) throws MalformedURLException {
-      this(new OkHttpClient(), baseUrl);
+   public ModelServerClient(final String baseUrl, final EPackageConfiguration... configurations)
+      throws MalformedURLException {
+      this(new OkHttpClient(), baseUrl, configurations);
    }
 
-   public ModelServerClient(final OkHttpClient client, final String baseUrl) throws MalformedURLException {
+   public ModelServerClient(final OkHttpClient client, final String baseUrl,
+      final EPackageConfiguration... configurations) throws MalformedURLException {
       this.client = client;
       this.baseUrl = new URL(baseUrl).toString();
+      this.configurations = configurations;
+      this.init();
+   }
+
+   protected void init() {
+      initEPackages();
+   }
+
+   private void initEPackages() {
+      // default packages that should always be present
+      EPackageConfiguration.setup(
+         new EcorePackageConfiguration(),
+         new CommandPackageConfiguration(),
+         new ChangePackageConfiguration());
+      // custom packages adding or overriding default configurations
+      EPackageConfiguration.setup(this.configurations);
    }
 
    @Override

bundles/org.eclipse.emfcloud.modelserver.client/src/org/eclipse/emfcloud/modelserver/client/TypedSubscriptionListener.java

@@ -13,9 +13,12 @@
 import java.util.Optional;
 import java.util.function.Function;
 
+import org.apache.log4j.Logger;
 import org.eclipse.emfcloud.modelserver.emf.common.JsonResponseType;
 
 public class TypedSubscriptionListener<T> implements NotificationSubscriptionListener<T> {
+   private static Logger LOG = Logger.getLogger(TypedSubscriptionListener.class.getSimpleName());
+
    private final Function<? super String, Optional<T>> updateFunction;
 
    public TypedSubscriptionListener(final Function<String, Optional<T>> updateFunction) {
@@ -55,7 +58,9 @@ public void onNotification(final ModelServerNotification notification) {
    public void onSuccess(final Optional<String> message) {}
 
    @Override
-   public void onError(final Optional<String> message) {}
+   public void onError(final Optional<String> message) {
+      LOG.error("Error: " + message.orElse("Unknown"));
+   }
 
    @Override
    public void onDirtyChange(final boolean isDirty) {}
@@ -67,7 +72,9 @@ public void onFullUpdate(final T fullUpdate) {}
    public void onIncrementalUpdate(final T incrementalUpdate) {}
 
    @Override
-   public void onUnknown(final ModelServerNotification notification) {}
+   public void onUnknown(final ModelServerNotification notification) {
+      LOG.warn("Unknown notification type: " + notification.getType());
+   }
 
    @Override
    public void onOpen(final Response<String> response) {}
@@ -79,8 +86,12 @@ public void onClosing(final int code, final String reason) {}
    public void onClosed(final int code, final String reason) {}
 
    @Override
-   public void onFailure(final Throwable t, final Response<String> response) {}
+   public void onFailure(final Throwable throwable, final Response<String> response) {
+      LOG.error("Failure: " + response.getMessage(), throwable);
+   }
 
    @Override
-   public void onFailure(final Throwable t) {}
+   public void onFailure(final Throwable throwable) {
+      LOG.error("Failure: ", throwable);
+   }
 }

bundles/org.eclipse.emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/common/DefaultModelResourceManager.java

@@ -29,23 +29,21 @@
 import org.eclipse.emf.ecore.resource.ResourceSet;
 import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl;
 import org.eclipse.emf.ecore.util.EcoreUtil;
-import org.eclipse.emf.ecore.xmi.impl.XMIResourceFactoryImpl;
 import org.eclipse.emfcloud.modelserver.command.CCommand;
 import org.eclipse.emfcloud.modelserver.command.CCommandExecutionResult;
 import org.eclipse.emfcloud.modelserver.command.CCommandFactory;
 import org.eclipse.emfcloud.modelserver.common.codecs.DecodingException;
-import org.eclipse.emfcloud.modelserver.common.codecs.EMFJsonConverter;
 import org.eclipse.emfcloud.modelserver.common.codecs.EncodingException;
 import org.eclipse.emfcloud.modelserver.edit.CommandCodec;
 import org.eclipse.emfcloud.modelserver.edit.CommandExecutionType;
 import org.eclipse.emfcloud.modelserver.edit.ModelServerCommand;
 import org.eclipse.emfcloud.modelserver.edit.command.UpdateModelCommandContribution;
 import org.eclipse.emfcloud.modelserver.emf.configuration.EPackageConfiguration;
 import org.eclipse.emfcloud.modelserver.emf.configuration.ServerConfiguration;
-import org.emfjson.jackson.resource.JsonResourceFactory;
 
 import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import com.google.inject.Inject;
 
 public class DefaultModelResourceManager implements ModelResourceManager {
@@ -66,16 +64,15 @@ public class DefaultModelResourceManager implements ModelResourceManager {
    public DefaultModelResourceManager(final Set<EPackageConfiguration> configurations,
       final AdapterFactory adapterFactory, final ServerConfiguration serverConfiguration) {
 
-      this.configurations = configurations;
+      this.configurations = Sets.newLinkedHashSet(configurations);
       this.adapterFactory = adapterFactory;
       this.serverConfiguration = serverConfiguration;
       initialize();
    }
 
    @Override
    public void initialize() {
-      registerExtensions(configurations);
-      configurations.forEach(EPackageConfiguration::registerEPackage);
+      EPackageConfiguration.setup(configurations.toArray(EPackageConfiguration[]::new));
 
       String workspacePath = this.serverConfiguration.getWorkspaceRootURI().toFileString();
       if (workspacePath != null) {
@@ -87,24 +84,6 @@ public void initialize() {
       }
    }
 
-   protected void registerExtensions(final Set<EPackageConfiguration> configurations) {
-      Map<String, Object> map = Resource.Factory.Registry.INSTANCE.getExtensionToFactoryMap();
-      // register default ResourceFactories (XMI and JSON)
-      map.put("*", new XMIResourceFactoryImpl());
-      map.put("json", new JsonResourceFactory(EMFJsonConverter.setupDefaultMapper()));
-      // register additional ResourceFactories
-      configurations.forEach(conf -> map.putAll(registerExtensions(conf)));
-   }
-
-   protected Map<String, Object> registerExtensions(final EPackageConfiguration configuration) {
-      Map<String, Object> map = Maps.newHashMap();
-      configuration.getFileExtensions().forEach(ext -> {
-         configuration.getResourceFactory(ext).ifPresent(fac -> map.put(ext, fac));
-      });
-
-      return map;
-   }
-
    @Override
    public ResourceSet getResourceSet(final String modeluri) {
       return resourceSets.get(createURI(modeluri));

bundles/org.eclipse.emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/common/RecordingModelResourceManager.java

@@ -19,6 +19,7 @@
 import org.eclipse.emf.ecore.change.util.ChangeRecorder;
 import org.eclipse.emfcloud.modelserver.command.CCommand;
 import org.eclipse.emfcloud.modelserver.command.CCommandExecutionResult;
+import org.eclipse.emfcloud.modelserver.emf.configuration.ChangePackageConfiguration;
 import org.eclipse.emfcloud.modelserver.emf.configuration.EPackageConfiguration;
 import org.eclipse.emfcloud.modelserver.emf.configuration.ServerConfiguration;
 
@@ -37,6 +38,12 @@ public RecordingModelResourceManager(final Set<EPackageConfiguration> configurat
       super(configurations, adapterFactory, serverConfiguration);
    }
 
+   @Override
+   public void initialize() {
+      this.configurations.add(new ChangePackageConfiguration());
+      super.initialize();
+   }
+
    @Override
    protected CommandExecutionContext executeCommand(final ModelServerEditingDomain domain, final Command serverCommand,
       final CCommand clientCommand) {

bundles/org.eclipse.emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/configuration/ChangePackageConfiguration.java

@@ -0,0 +1,31 @@
+/********************************************************************************
+ * Copyright (c) 2019 EclipseSource and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * https://www.eclipse.org/legal/epl-2.0, or the MIT License which is
+ * available at https://opensource.org/licenses/MIT.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR MIT
+ ********************************************************************************/
+package org.eclipse.emfcloud.modelserver.emf.configuration;
+
+import java.util.Collection;
+import java.util.Collections;
+
+import org.eclipse.emf.ecore.change.ChangePackage;
+
+public class ChangePackageConfiguration implements EPackageConfiguration {
+
+   @Override
+   public Collection<String> getFileExtensions() { return Collections.emptyList(); }
+
+   @Override
+   public void registerEPackage() {
+      ChangePackage.eINSTANCE.eClass();
+   }
+
+   @Override
+   public String getId() { return ChangePackage.eNS_URI; }
+
+}

bundles/org.eclipse.emfcloud.modelserver.emf/src/org/eclipse/emfcloud/modelserver/emf/configuration/EPackageConfiguration.java

@@ -10,10 +10,15 @@
  ********************************************************************************/
 package org.eclipse.emfcloud.modelserver.emf.configuration;
 
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Map;
 import java.util.Optional;
 
 import org.eclipse.emf.ecore.resource.Resource;
+import org.eclipse.emf.ecore.xmi.impl.XMIResourceFactoryImpl;
+import org.eclipse.emfcloud.modelserver.common.codecs.EMFJsonConverter;
+import org.emfjson.jackson.resource.JsonResourceFactory;
 
 public interface EPackageConfiguration {
    /**
@@ -48,4 +53,36 @@ public interface EPackageConfiguration {
    default Optional<Resource.Factory> getResourceFactory(final String extension) {
       return Optional.empty();
    }
+
+   default void registerFileExtensions() {
+      registerFileExtensions(Resource.Factory.Registry.INSTANCE.getExtensionToFactoryMap());
+   }
+
+   default void registerFileExtensions(final Map<String, Object> extensionToFactoryMap) {
+      getFileExtensions().forEach(fileExtension -> {
+         getResourceFactory(fileExtension).ifPresent(factory -> extensionToFactoryMap.put(fileExtension, factory));
+      });
+   }
+
+   default void register() {
+      register(Resource.Factory.Registry.INSTANCE.getExtensionToFactoryMap());
+   }
+
+   default void register(final Map<String, Object> extensionToFactoryMap) {
+      registerEPackage();
+      registerFileExtensions(extensionToFactoryMap);
+   }
+
+   static void setup(final EPackageConfiguration... configurations) {
+      setup(Resource.Factory.Registry.INSTANCE.getExtensionToFactoryMap(), configurations);
+   }
+
+   static void setup(final Map<String, Object> extensionToFactoryMap, final EPackageConfiguration... configurations) {
+      // default initialization as XMI resources
+      extensionToFactoryMap.put("*", new XMIResourceFactoryImpl());
+      // default json mapping
+      extensionToFactoryMap.put("json", new JsonResourceFactory(EMFJsonConverter.setupDefaultMapper()));
+      // custom configurations: may add or override default configurations (order matters)
+      Arrays.stream(configurations).forEach(configuration -> configuration.register(extensionToFactoryMap));
+   }
 }

examples/org.eclipse.emfcloud.modelserver.example/src/main/java/org/eclipse/emfcloud/modelserver/example/client/ExampleModelServerClient.java

@@ -22,9 +22,9 @@
 import org.eclipse.emfcloud.modelserver.client.SubscriptionListener;
 import org.eclipse.emfcloud.modelserver.coffee.model.coffee.CoffeePackage;
 import org.eclipse.emfcloud.modelserver.command.CCommand;
-import org.eclipse.emfcloud.modelserver.command.CCommandPackage;
 import org.eclipse.emfcloud.modelserver.edit.command.SetCommandContribution;
 import org.eclipse.emfcloud.modelserver.edit.util.CommandUtil;
+import org.eclipse.emfcloud.modelserver.example.CoffeePackageConfiguration;
 import org.eclipse.emfcloud.modelserver.example.UpdateTaskNameCommandContribution;
 import org.eclipse.emfcloud.modelserver.jsonschema.Json;
 
@@ -48,8 +48,9 @@ public final class ExampleModelServerClient {
    private ExampleModelServerClient() {}
 
    public static void main(final String[] args) {
-      registerPackages();
-      try (ModelServerClient client = new ModelServerClient("http://localhost:8081/api/v1/");
+      try (
+         ModelServerClient client = new ModelServerClient("http://localhost:8081/api/v1/",
+            new CoffeePackageConfiguration());
          Scanner userInput = new Scanner(System.in)) {
          System.out.println("Simple Model Server Client Interface");
          System.out.println("====================================");
@@ -166,12 +167,6 @@ private static void handleUnsubscribe(final ModelServerClient client, final Stri
       System.out.println("< OK");
    }
 
-   private static void registerPackages() {
-      EcorePackage.eINSTANCE.eClass();
-      CCommandPackage.eINSTANCE.eClass();
-      CoffeePackage.eINSTANCE.eClass();
-   }
-
    private static void printHelp() {
       System.out.println("Supported commands:");
       System.out.println("- " + CMD_SUBSCRIBE + " <modelUri> <format>");