refactor(rest): clean up rest inheritance

* avoid dependency loops
* avoid exposing OnmsResult objects to "public" APIs
* pass the OnmsServer object in OnmsHTTPOptions for 1-off calls
* be more careful about making sure filter processor is init'd
* allow overriding axios and superagent impls

.babelrc

@@ -17,13 +17,9 @@
     "transform-class-properties",
     "transform-es2015-destructuring",
     "transform-es2015-for-of",
+    "transform-es2015-modules-commonjs",
     "transform-object-rest-spread",
     "transform-regenerator",
     "syntax-async-functions",
-  ],
-  env: {
-    test: {
-      plugins: ["transform-es2015-modules-commonjs"]
-    }
-  }
+  ]
 }

HOWTO.md

@@ -26,7 +26,8 @@ new Client().connect('Demo', 'https://demo.opennms.org/opennms', 'demo', 'demo')
   const idRestriction = new Restriction('id', Comparators.GE, 1);
   const filter = new Filter(idRestriction);
   // query all alarms with an ID greater than or equal to 1
-  client.alarms().find(filter).then((alarms) => {
+  return client.alarms().find(filter).then((alarms) => {
+    console.log('got ' + alarms.length + ' alarms.');
     // return all the node IDs associated with the matching alarms
     return alarms.map((alarm) => {
       return alarm.nodeId;
@@ -45,6 +46,11 @@ new Client().connect('Demo', 'https://demo.opennms.org/opennms', 'demo', 'demo')
       console.log('node ' + node.id + ' (' + node.label + ') has '
         + node.ipInterfaces.length + ' IP interfaces');
     });
+    return true;
   });
+}).catch((err) => {
+  console.log('error:',err);
+  console.log(err.stack);
+  throw err;
 });
 ```

src/CLI.ts

@@ -96,15 +96,15 @@ function CLI() {
       const http = new Rest.AxiosHTTP(server);
       return Client.getMetadata(server, http).then((res) => {
         let c = colors.green;
-        if (res.data.type === API.ServerTypes.MERIDIAN) {
-          console.log(colors.blue('OpenNMS Meridian ' + res.data.version.displayVersion + ' Capabilities:'));
+        if (res.type === API.ServerTypes.MERIDIAN) {
+          console.log(colors.blue('OpenNMS Meridian ' + res.version.displayVersion + ' Capabilities:'));
           c = colors.blue;
         } else {
-          console.log(colors.green('OpenNMS Horizon ' + res.data.version.displayVersion + ' Capabilities:'));
+          console.log(colors.green('OpenNMS Horizon ' + res.version.displayVersion + ' Capabilities:'));
         }
         console.log('');
 
-        const caps = res.data.capabilities();
+        const caps = res.capabilities();
         const rows = [];
         for (const cap in caps) {
           if (cap === 'type') {

src/Client.ts

@@ -3,6 +3,7 @@ import * as axios from 'axios';
 import {log, catRoot} from './api/Log';
 import {Category} from 'typescript-logging';
 
+import {IHasHTTP} from './api/IHasHTTP';
 import {IOnmsHTTP} from './api/IOnmsHTTP';
 
 import {OnmsAuthConfig} from './api/OnmsAuthConfig';
@@ -28,7 +29,7 @@ const catClient = new Category('client', catRoot);
  * The OpenNMS client.  This is the primary interface to OpenNMS servers.
  * @module Client
  */ /** */
-export class Client {
+export class Client implements IHasHTTP {
   /**
    * Given an OnmsServer object, check that it can be connected to.
    *
@@ -37,20 +38,19 @@ export class Client {
    * @param timeout - how long to wait before giving up when making ReST calls
    */
   public static async checkServer(server: OnmsServer, httpImpl?: IOnmsHTTP, timeout?: number): Promise<boolean> {
-    const opts = new OnmsHTTPOptions(timeout, server.auth);
+    const opts = new OnmsHTTPOptions(timeout, server.auth, server);
     if (!httpImpl) {
-      if (!Client.http) {
+      if (!Client.defaultHttp) {
         throw new OnmsError('No HTTP implementation is configured!');
       }
-      httpImpl = Client.http;
+      httpImpl = Client.defaultHttp;
     }
     opts.accept = 'text/plain';
 
     const infoUrl = server.resolveURL('rest/alarms/count');
-    log.debug('checking URL: ' + infoUrl, catClient);
-    return httpImpl.get(infoUrl, opts).then((ret) => {
-      return true;
-    });
+    log.debug('checkServer: checking URL: ' + infoUrl, catClient);
+    await httpImpl.get(infoUrl, opts);
+    return true;
   }
 
   /**
@@ -62,33 +62,35 @@ export class Client {
    * @param timeout - how long to wait before giving up when making ReST calls
    */
   public static async getMetadata(server: OnmsServer, httpImpl?: IOnmsHTTP, timeout?: number):
-    Promise<OnmsResult<ServerMetadata>> {
-    const opts = new OnmsHTTPOptions(timeout, server.auth);
+    Promise<ServerMetadata> {
+    const opts = new OnmsHTTPOptions(timeout, server.auth, server);
     opts.accept = 'application/json';
     if (!httpImpl) {
-      if (!Client.http) {
-        throw new OnmsError('No HTTP implementation is configured!');
+      if (!Client.defaultHttp) {
+        throw new OnmsError('No default HTTP implementation is configured!');
       }
-      httpImpl = Client.http;
+      httpImpl = Client.defaultHttp;
     }
 
     const infoUrl = server.resolveURL('rest/info');
-    log.debug('checking URL: ' + infoUrl, catClient);
-    return httpImpl.get(infoUrl, opts).then((ret) => {
-      const version = new OnmsVersion(ret.data.version, ret.data.displayVersion);
-      const metadata = new ServerMetadata(version);
-
-      if (ret.data.packageName) {
-        if (ret.data.packageName.toLowerCase() === 'meridian') {
-          metadata.type = ServerTypes.MERIDIAN;
-        }
+    log.debug('getMetadata: checking URL: ' + infoUrl, catClient);
+
+    const response = await httpImpl.get(infoUrl, opts);
+    const version = new OnmsVersion(response.data.version, response.data.displayVersion);
+    const metadata = new ServerMetadata(version);
+    if (response.data.packageName) {
+      if (response.data.packageName.toLowerCase() === 'meridian') {
+        metadata.type = ServerTypes.MERIDIAN;
       }
-      return OnmsResult.ok(metadata, ret.message, ret.code);
-    });
+    }
+    return metadata;
   }
 
-  /** The OnmsHTTP implementation to be used when making requests */
-  private static http: IOnmsHTTP;
+  /** The default OnmsHTTP implementation to be used when making requests */
+  private static defaultHttp: IOnmsHTTP;
+
+  /** the OnmsHTTP implementation that will be used when making requests */
+  public http: IOnmsHTTP;
 
   /** The remote server to connect to */
   public server: OnmsServer;
@@ -102,10 +104,11 @@ export class Client {
    */
   constructor(httpImpl?: IOnmsHTTP) {
     if (httpImpl) {
-      Client.http = httpImpl;
+      Client.defaultHttp = httpImpl;
     } else {
-      Client.http = new AxiosHTTP();
+      Client.defaultHttp = new AxiosHTTP();
     }
+    this.http = Client.defaultHttp;
   }
 
   /**
@@ -115,12 +118,20 @@ export class Client {
   public async connect(name: string, url: string, username: string, password: string, timeout?: number) {
     const self = this;
     const server = new OnmsServer(name, url, username, password);
+
     await Client.checkServer(server, undefined, timeout);
-    return Client.getMetadata(server, undefined, timeout).then((result) => {
-      self.server = server;
-      server.metadata = result.data;
-      return self;
-    });
+    const metadata = await Client.getMetadata(server, undefined, timeout);
+
+    if (!self.http) {
+      self.http = Client.defaultHttp;
+    }
+    if (!self.http.server) {
+      self.http.server = server;
+    }
+    self.server = server;
+    server.metadata = metadata;
+
+    return self;
   }
 
   /** Get an alarm DAO for querying alarms. */

src/api/IHasHTTP.ts

@@ -0,0 +1,16 @@
+import {IOnmsHTTP} from './IOnmsHTTP';
+
+/**
+ * Interface for a class that has an HTTP object.
+ *
+ * This exists to avoid import loops between the DAOs (that need to easily access {@link IOnmsHTTP})
+ * and the {@link Client} which needs to contain an {@link IOnmsHTTP}.
+ *
+ * @interface
+ * @module IHasHTTP
+ */ /** */
+
+export interface IHasHTTP {
+  /** the HTTP implementation this object should contain */
+  http: IOnmsHTTP;
+}

src/api/IOnmsHTTP.ts

@@ -10,7 +10,7 @@ import {OnmsServer} from './OnmsServer';
 /**
  * Interface for making ReST calls to an HTTP server.
  * @interface
- * @module OnmsHTTP
+ * @module IOnmsHTTP
  *
  * Notes to implementors:
  * - Implementations of this interface MUST have a constructor that allows an empty

src/api/OnmsHTTPOptions.ts

@@ -1,4 +1,5 @@
 import {OnmsAuthConfig} from './OnmsAuthConfig';
+import {OnmsServer} from './OnmsServer';
 import {IHash} from '../internal/IHash';
 
 /**
@@ -9,6 +10,9 @@ export class OnmsHTTPOptions {
   /** the authentication config that should be used when no server auth is configured */
   public auth: OnmsAuthConfig;
 
+  /** the server to use if no server is set on the HTTP implementation */
+  public server: OnmsServer;
+
   /** how long to wait for ReST calls to time out */
   public timeout = 10000;
 
@@ -22,12 +26,15 @@ export class OnmsHTTPOptions {
    * Construct a new OnmsHTTPOptions object.
    * @constructor
    */
-  constructor(timeout?: number, auth?: OnmsAuthConfig) {
+  constructor(timeout?: number, auth?: OnmsAuthConfig, server?: OnmsServer) {
     if (timeout !== undefined) {
       this.timeout = timeout;
     }
     if (auth !== undefined) {
       this.auth = auth;
     }
+    if (server !== undefined) {
+      this.server = server;
+    }
   }
 }

src/dao/AbstractDAO.ts

@@ -1,7 +1,6 @@
+import {IHasHTTP} from '../api/IHasHTTP';
 import {IOnmsHTTP} from '../api/IOnmsHTTP';
 
-import {Client} from '../Client';
-
 import {Filter} from '../api/Filter';
 import {OnmsHTTPOptions} from '../api/OnmsHTTPOptions';
 
@@ -23,12 +22,11 @@ export abstract class AbstractDAO<K, T> {
   protected http: IOnmsHTTP;
 
   /** construct a DAO instance */
-  constructor(impl: Client | IOnmsHTTP) {
-    if (impl instanceof Client) {
-      this.http = (impl as any).http;
-    } else {
-      this.http = impl;
+  constructor(impl: IHasHTTP | IOnmsHTTP) {
+    if ((impl as IHasHTTP).http) {
+      impl = (impl as IHasHTTP).http;
     }
+    this.http = impl as IOnmsHTTP;
   }
 
   /** create a model object given a JSON data structure */

src/dao/AlarmDAO.ts

@@ -1,20 +1,18 @@
 import {AbstractDAO} from './AbstractDAO';
 import {EventDAO} from './EventDAO';
 
-import {Client} from '../Client';
+import {Filter} from '../api/Filter';
+import {IHasHTTP} from '../api/IHasHTTP';
+import {IOnmsHTTP} from '../api/IOnmsHTTP';
+import {OnmsError} from '../api/OnmsError';
 
 import {OnmsAlarm} from '../model/OnmsAlarm';
+import {AlarmTypes} from '../model/OnmsAlarmType';
 import {OnmsParm} from '../model/OnmsParm';
 import {OnmsServiceType} from '../model/OnmsServiceType';
-
-import {AlarmTypes} from '../model/OnmsAlarmType';
 import {Severities} from '../model/OnmsSeverity';
 import {TroubleTicketStates} from '../model/OnmsTroubleTicketState';
 
-import {Filter} from '../api/Filter';
-import {IOnmsHTTP} from '../api/IOnmsHTTP';
-import {OnmsError} from '../api/OnmsError';
-
 import {log, catDao} from '../api/Log';
 import {Category} from 'typescript-logging';
 
@@ -29,7 +27,7 @@ export class AlarmDAO extends AbstractDAO<number, OnmsAlarm> {
   /** an event DAO to be used for creating events attached to alarms from API/JSON data */
   private eventDao: EventDAO;
 
-  constructor(impl: Client | IOnmsHTTP) {
+  constructor(impl: IHasHTTP | IOnmsHTTP) {
     super(impl);
     this.eventDao = new EventDAO(impl);
   }

src/dao/EventDAO.ts

@@ -1,6 +1,8 @@
 import {AbstractDAO} from './AbstractDAO';
 
 import {Filter} from '../api/Filter';
+import {IHasHTTP} from '../api/IHasHTTP';
+import {IOnmsHTTP} from '../api/IOnmsHTTP';
 import {OnmsError} from '../api/OnmsError';
 
 import {Util} from '../internal/Util';
@@ -21,6 +23,10 @@ const cat = new Category('events', catDao);
  * @module EventDAO
  */ /** */
 export class EventDAO extends AbstractDAO<number, OnmsEvent> {
+  constructor(impl: IHasHTTP | IOnmsHTTP) {
+    super(impl);
+  }
+
   /**
    * create an event object from a JSON object
    * @hidden

src/dao/NodeDAO.ts

@@ -1,6 +1,8 @@
 import {AbstractDAO} from './AbstractDAO';
 
 import {Filter} from '../api/Filter';
+import {IHasHTTP} from '../api/IHasHTTP';
+import {IOnmsHTTP} from '../api/IOnmsHTTP';
 import {OnmsError} from '../api/OnmsError';
 
 import {Util} from '../internal/Util';
@@ -31,6 +33,10 @@ const cat = new Category('nodes', catDao);
  * @module NodeDAO
  */ /** */
 export class NodeDAO extends AbstractDAO<number, OnmsNode> {
+  constructor(impl: IHasHTTP | IOnmsHTTP) {
+    super(impl);
+  }
+
   /**
    * create a node object from a JSON object
    * @hidden

src/model/OnmsIpInterface.ts

@@ -3,7 +3,6 @@ import {Moment} from 'moment';
 
 import {OnmsManagedType} from './OnmsManagedType';
 import {OnmsMonitoredService} from './OnmsMonitoredService';
-import {OnmsNode} from './OnmsNode';
 import {OnmsPrimaryType} from './OnmsPrimaryType';
 
 /**
@@ -44,7 +43,7 @@ export class OnmsIpInterface {
   }
 
   /** the node this interface is associated with */
-  public node: OnmsNode;
+  public node: any;
 
   /** the services on this interface */
   public services = [] as OnmsMonitoredService[];

src/model/OnmsMonitoredService.ts

@@ -1,8 +1,6 @@
 import {Address4, Address6} from 'ip-address';
 import {Moment} from 'moment';
 
-import {OnmsIpInterface} from './OnmsIpInterface';
-import {OnmsNode} from './OnmsNode';
 import {OnmsServiceType} from './OnmsServiceType';
 import {OnmsServiceStatusType} from './OnmsServiceStatusType';
 
@@ -21,10 +19,10 @@ export class OnmsMonitoredService {
   public lastGood: Moment;
 
   /** the node associated with this service */
-  public node: OnmsNode;
+  public node: any;
 
   /** the ipInterface associated with this service */
-  public ipInterface: OnmsIpInterface;
+  public ipInterface: any;
 
   /** the service type associated with this service */
   public type: OnmsServiceType;