Boilerplate config changes 2 (#1328)

* added service-life-cycle doc * proposal * different implementation * worky proposal * testing and removal of local config member * unit test fixed and audiofile updated * Rework config generics so T is propagated through ConfigurableService correctly * stop compiler errors * all service templated with config * unit tests catches bug before release ... yesss.. * more fixes * dumb unit test fix * requested updates * removed casts fixed servo ui * fixed unit test --------- Co-authored-by: Branden Butler <[email protected]>
MyRobotLab · Aug 12, 2023 · 516f1d5 · 516f1d5
1 parent 244d6a7
commit 516f1d5
Show file tree

Hide file tree

Showing 165 changed files with 774 additions and 706 deletions.
diff --git a/doc/service-life-cycle.md b/doc/service-life-cycle.md
@@ -0,0 +1,52 @@
+# Service Life Cycle
+```mermaid
+stateDiagram
+
+    start: start(name, type)
+    load: load(name, type)
+    loadService: loadService(plan, name, type, true, 0)
+
+    [*] --> start
+
+    start --> load
+    load --> loadService
+    loadService --> getDefault
+    getDefault --> readServiceConfig
+    readServiceConfig --> loadService
+    loadService --> createServicesFromPlan
+    createServicesFromPlan --> createService
+    createService --> setConfig
+    setConfig --> apply
+    apply --> startService
+    startService --> stopService
+    stopService --> releaseService
+    releaseService --> release
+    release --> [*]
+
+    [*] --> create
+    create --> load
+
+```
+
+### start(name, type)
+Creates and starts a service with the given name and type
+
+### load
+Starts loading the hierarchy of configuration
+FIXME - this should not be the memory plan, but should exist on the filesystem
+Default config is used if no config found ~~in memory~~ on filesystem
+
+### loadService
+Recursively loads a service config into a plan
+
+### createServicesFromPlan(plan, createdServices, name)
+Loops through all "loaded" services in the plan and creates them all
+
+### createService
+Instantiates instance of service
+
+### setConfig
+Sets the config of the service
+
+### apply
+Applies the config to the service
diff --git a/src/main/java/org/myrobotlab/codec/CodecUtils.java b/src/main/java/org/myrobotlab/codec/CodecUtils.java
@@ -1619,14 +1619,4 @@ public static byte[] fromBase64(String input) {
     return Base64.getDecoder().decode(input);
   }
 
-  public static String removeEnd(final String str, final String remove) {
-    if (str == null || str.length() == 0 || remove == null || remove.length() == 0) {
-      return str;
-    }
-    if (str.endsWith(remove)) {
-      return str.substring(0, str.length() - remove.length());
-    }
-    return str;
-  }
-
 }
diff --git a/src/main/java/org/myrobotlab/document/connector/AbstractConnector.java b/src/main/java/org/myrobotlab/document/connector/AbstractConnector.java
@@ -7,8 +7,8 @@
 import org.myrobotlab.document.Document;
 import org.myrobotlab.document.transformer.ConnectorConfig;
 import org.myrobotlab.framework.Service;
+import org.myrobotlab.service.config.ServiceConfig;
 import org.myrobotlab.service.interfaces.DocumentConnector;
-import org.myrobotlab.service.interfaces.DocumentListener;
 import org.myrobotlab.service.interfaces.DocumentPublisher;
 
 /**
@@ -17,7 +17,7 @@
  * service.
  * 
  */
-public abstract class AbstractConnector extends Service implements DocumentPublisher, DocumentConnector {
+public abstract class AbstractConnector extends Service<ServiceConfig> implements DocumentPublisher, DocumentConnector {
 
   private static final long serialVersionUID = 1L;
   protected ConnectorState state = ConnectorState.STOPPED;

diff --git a/src/main/java/org/myrobotlab/framework/Service.java b/src/main/java/org/myrobotlab/framework/Service.java
@@ -58,6 +58,7 @@
 import org.myrobotlab.codec.CodecUtils;
 import org.myrobotlab.framework.interfaces.Attachable;
 import org.myrobotlab.framework.interfaces.Broadcaster;
+import org.myrobotlab.framework.interfaces.ConfigurableService;
 import org.myrobotlab.framework.interfaces.FutureInvoker;
 import org.myrobotlab.framework.interfaces.NameProvider;
 import org.myrobotlab.framework.interfaces.ServiceInterface;
@@ -73,6 +74,7 @@
 import org.myrobotlab.service.interfaces.AuthorizationProvider;
 import org.myrobotlab.service.interfaces.QueueReporter;
 import org.myrobotlab.service.meta.abstracts.MetaData;
+import org.myrobotlab.string.StringUtil;
 import org.slf4j.Logger;
 
 /**
@@ -86,7 +88,7 @@
  * messages.
  * 
  */
-public abstract class Service implements Runnable, Serializable, ServiceInterface, Broadcaster, QueueReporter, FutureInvoker {
+public abstract class Service<T extends ServiceConfig> implements Runnable, Serializable, ServiceInterface, Broadcaster, QueueReporter, FutureInvoker, ConfigurableService<T> {
 
   // FIXME upgrade to ScheduledExecutorService
   // http://howtodoinjava.com/2015/03/25/task-scheduling-with-executors-scheduledthreadpoolexecutor-example/
@@ -99,6 +101,12 @@ public abstract class Service implements Runnable, Serializable, ServiceInterfac
    */
   protected MetaData serviceType;
 
+  /**
+   * Config member - configuration of type {ServiceType}Config
+   * Runtime applys either the default config or a saved config during service creation
+   */
+  protected T config;
+
   private static final long serialVersionUID = 1L;
 
   transient public final static Logger log = LoggerFactory.getLogger(Service.class);
@@ -145,11 +153,6 @@ public abstract class Service implements Runnable, Serializable, ServiceInterfac
    */
   protected Properties defaultLocalization = null;
 
-  /**
-   * the last config applied to this service
-   */
-  protected ServiceConfig config;
-
   /**
    * map of keys to localizations -
    * 
@@ -483,7 +486,7 @@ static public String getResourceDir(String serviceType, String additionalPath) {
 
     // stupid solution to get past static problem
     if (!"Runtime".equals(serviceType)) {
-      resourceDir = ((RuntimeConfig)Runtime.getInstance().getConfig()).resource + fs + serviceType;
+      resourceDir = Runtime.getInstance().getConfig().resource + fs + serviceType;
     } else {
       resourceDir = "resource";
     }
@@ -1127,10 +1130,10 @@ public boolean hasError() {
 
   @Override
   public Map<String, Peer> getPeers() {
-    if (config == null) {
+    if (getConfig() == null) {
       return null;
     }
-    return config.getPeers();
+    return getConfig().getPeers();
   }
 
   /**
@@ -1155,10 +1158,10 @@ public String getPeerKey(String name) {
 
   @Override
   public Set<String> getPeerKeys() {
-    if (config == null || config.peers == null) {
+    if (getConfig() == null || getConfig().peers == null) {
       return new HashSet<>();
     }
-    return config.peers.keySet();
+    return getConfig().peers.keySet();
   }
 
   public String help(String format, String level) {
@@ -1395,47 +1398,50 @@ public boolean isRunning() {
   }
 
   /**
-   * Default load config method, subclasses should override this to support
-   * service specific configuration in the service yaml files.
-   * 
-   * apply is the first function to be called after construction of a service,
-   * then startService will be called
-   * 
-   * construct -&gt; apply -&gt; startService
-   * 
+   * getConfig returns current config of the service. This default super method
+   * will also filter webgui subscriptions out, in addition for any local subscriptions it
+   * will remove the instance "id" from any service.  The reason it removes the webgui
+   * subscriptions is to avoid overwelming the user when modifying config.  UI subscriptions
+   * tend to be very numerous and not very useful to the user.  The reason it removes the
+   * instance id from local subscriptions is to allow the config to be used with any instance.
+   * Unless the user is controlling instance id, its random every restart.
    */
-  @Override
-  public ServiceConfig apply(ServiceConfig inConfig) {
-    log.info("Default service config loading for service: {} type: {}", getName(), getTypeKey());
-    /*
-     * We clone/serialize here because we don't want to use the same reference
-     * of of config in the plan. If configuration is applied through the plan,
-     * "or from anywhere else" we make a copy of it here. And the copy is
-     * applied to the actual service. This keeps the plan safe to modify without
-     * the worry of modifying a running service config.
-     */
+  public T getConfig() {
+    return config;
+  }
+
+  /**
+   * Super class apply using template type. The default assigns config of the templated type, and also
+   * add listeners from subscriptions found on the base class ServiceConfig.listeners 
+   */
+  public T apply(T c) {
+      config = c;
+      addConfigListeners(c);
+      return config;
+  }
 
-    String yaml = CodecUtils.toYaml(inConfig);
-    ServiceConfig copyOfConfig = CodecUtils.fromYaml(yaml, inConfig.getClass());
 
-    // TODO - handle subscriptions / listeners
-    if (copyOfConfig.listeners != null) {
-      for (Listener listener : copyOfConfig.listeners) {
+  /**
+   * The basic ServiceConfig has a list of listeners. These are definitions of
+   * other subscribers subscribing for data from this service. This method
+   * processes those listeners and adds them to the outbox notifyList.
+   */
+  public ServiceConfig addConfigListeners(ServiceConfig config) {
+    if (config.listeners != null) {
+      for (Listener listener : config.listeners) {
         addListener(listener.method, listener.listener, listener.callback);
       }
     }
-
-    this.config = copyOfConfig;
     return config;
   }
 
   /**
-   * Default getConfig returns name and type with null service specific config
-   * 
+   * Default filtered config, used when saving, can be overriden by concrete class
    */
   @Override
-  public ServiceConfig getConfig() {
-
+  public ServiceConfig getFilteredConfig() {
+    // Make a copy, because we don't want to modify the original
+    ServiceConfig sc = CodecUtils.fromYaml(CodecUtils.toYaml(getConfig()), config.getClass());
     Map<String, List<MRLListener>> listeners = getOutbox().notifyList;
     List<Listener> newListeners = new ArrayList<>();
 
@@ -1445,49 +1451,30 @@ public ServiceConfig getConfig() {
       for (MRLListener listener : list) {
         if (!listener.callbackName.endsWith("@webgui-client")) {
           // Removes the `@runtime-id` so configs still work with local IDs
-          // The StringUtils.removeEnd() call is a no-op when the ID is not our local ID,
+          // The StringUtils.removeEnd() call is a no-op when the ID is not our
+          // local ID,
           // so doesn't conflict with remote routes
-          Listener newConfigListener = new Listener(
-                  listener.topicMethod,
-                  CodecUtils.removeEnd(
-                          listener.callbackName,
-                          '@' + Platform.getLocalInstance().getId()
-                  ),
-                  listener.callbackMethod
-          );
+          Listener newConfigListener = new Listener(listener.topicMethod, StringUtil.removeEnd(listener.callbackName, '@' + Platform.getLocalInstance().getId()),
+              listener.callbackMethod);
           newListeners.add(newConfigListener);
         }
       }
     }
 
-
     if (newListeners.size() > 0) {
-      config.listeners = newListeners;
+      sc.listeners = newListeners;
     }
-    return config;
-  }
-
-  // FIXME - NEED A BETTER SOLUTION !!!
-  @Override
-  public ServiceConfig getFilteredConfig() {
-    ServiceConfig sc = getConfig();
-    // deep clone
-    sc = CodecUtils.fromYaml(CodecUtils.toYaml(sc), sc.getClass());
     return sc;
   }
 
-  @Override
-  public void setConfig(ServiceConfig config) {
-    this.config = config;
-  }
 
   @Override
   public void setConfigValue(String fieldname, Object value) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException {
-      log.info("setting field name fieldname {} to {}", fieldname, value);
+    log.info("setting field name fieldname {} to {}", fieldname, value);
 
-      Field field = config.getClass().getDeclaredField(fieldname);
-      // field.setAccessible(true); should not need this - it "should" be public
-      field.set(config, value);
+    Field field = getConfig().getClass().getDeclaredField(fieldname);
+    // field.setAccessible(true); should not need this - it "should" be public
+    field.set(getConfig(), value);
   }
 
   @Override
@@ -1913,7 +1900,7 @@ synchronized public ServiceInterface startPeer(String peerKey) {
     peerKey = peerKey.trim();
 
     // get current definition of config and peer
-    Peer peer = config.getPeer(peerKey);
+    Peer peer = getConfig().getPeer(peerKey);
 
     if (peer == null) {
       error("startPeer could not find peerKey of %s in %s", peerKey, getName());
@@ -1956,8 +1943,8 @@ synchronized public ServiceInterface startPeer(String peerKey) {
    */
   synchronized public void releasePeer(String peerKey) {
 
-    if (config != null && config.getPeer(peerKey) != null) {
-      Peer peer = config.getPeer(peerKey);
+    if (getConfig() != null && getConfig().getPeer(peerKey) != null) {
+      Peer peer = getConfig().getPeer(peerKey);
       ServiceConfig sc = Runtime.getPlan().get(peer.name);
       // peer recursive
       if (sc != null && sc.getPeers() != null) {
@@ -2732,10 +2719,10 @@ public int getCreationOrder() {
    */
   public String getPeerName(String peerKey) {
 
-    if (config == null) {
+    if (getConfig() == null) {
       return null;
     }
-    return config.getPeerName(peerKey);
+    return getConfig().getPeerName(peerKey);
   }
 
   /**
@@ -2778,7 +2765,7 @@ public void apply() {
     // updating plan
     Runtime.getPlan().put(getName(), sc);
     // applying config to self
-    apply(sc);
+    apply((T)sc);
   }
 
   /**
@@ -2790,7 +2777,7 @@ public void apply() {
    * @param fullName
    */
   public void setPeerName(String key, String fullName) {
-    Peer peer = config.getPeer(key);
+    Peer peer = getConfig().getPeer(key);
     String oldName = peer.name;
     peer.name = fullName;
     // update plan ?
@@ -2822,7 +2809,7 @@ public void updatePeerType(String key, String peerType) {
       sc.putPeerType(key, String.format("%s.%s", getName(), key), peerType);
     }
 
-    Peer peer = config.getPeer(key);
+    Peer peer = getConfig().getPeer(key);
     peer.type = peerType;
 
     // not Needed

diff --git a/src/main/java/org/myrobotlab/framework/Task.java b/src/main/java/org/myrobotlab/framework/Task.java
@@ -64,7 +64,7 @@ public void run() {
       Task t = new Task(this);
       // clear history list - becomes "new" message
       t.msg.historyList.clear();
-      Timer timer = myService.tasks.get(taskName);
+      Timer timer = (Timer) myService.tasks.get(taskName);
       if (timer != null) {
         // timer = new Timer(String.format("%s.timer", getName()));
         try {

diff --git a/src/main/java/org/myrobotlab/framework/interfaces/ConfigurableService.java b/src/main/java/org/myrobotlab/framework/interfaces/ConfigurableService.java
@@ -0,0 +1,8 @@
+package org.myrobotlab.framework.interfaces;
+
+import org.myrobotlab.service.config.ServiceConfig;
+
+public interface ConfigurableService<T extends ServiceConfig> {
+  T getConfig();
+  T apply(T c);
+}