JS-41: make API objects immutable

README.md

@@ -54,9 +54,13 @@ OpenNMS.js 2.0 adds a few new APIs, contains a ton of refactoring and build syst
 #### Breaking Changes:
 
 * The `api/Log` module now only exports a single, simplified `log` object; `typescript-logging` was overly complicated and not really adding much in the way of value.  Use `.setDebug()`, `.setQuiet()`, and `.setSilent()` to change the logging level instead.
-* To remain compatible with loose JS requirements for object creation (and later hydration), a number of the TypeScript APIs have been clarified to be explicitly nullable (and/or `undefined`-able).
+* A number of the TypeScript APIs have been clarified to be explicitly nullable (and/or `undefined`-able) to make strict null- and type-checking validation pass.
 * `PropertiesCache` and its associated interface, `ISearchPropertyAccessor` are gone.  This only affects you if you have implemented custom DAOs, which is very unlikely.  :)
 * The previously deprecated `timeout` property in `AbstractHTTP` (and sub-classes) has been removed.  Access the `AbstractHTTP.options.timeout` property directly.
+* The `Client` no longer keeps a separate copy of the server object.  Instead you should access the `http.server` sub-property directly.
+* A number of API objects are now immutable/read-only to reduce side-effects: `OnmsAuthConfig`, `OnmsEnum`, `OnmsError`, `OnmsHTTPOptions`, `OnmsResult`, `OnmsServer`, `Operator`, `SearchPropertyType`, `ServerMetadata`, `TicketerConfig`.  For objects you are likely to modify, builder-style methods starting with `with` have been provided to make it easy to create new objects based on existing.
+* The `id` property on `OnmsServer` is no longer generated, it is computed based on the contents of the server object and should be repeatably equal if the contents are equal.
+* The `OnmsServer` object now expects an optional `auth` and and optional `metadata` field as the 3rd and 4th arguments.  The overloaded form where you could pass a string `username` and `password` is no longer supported.
 
 ### 1.5
 

package.json

@@ -85,6 +85,7 @@
   "dependencies": {
     "@babel/runtime-corejs2": "^7.5.5",
     "@types/btoa": "^1.2.3",
+    "@types/object-hash": "^1.3.0",
     "axios": "^0.19.0",
     "btoa": "^1.2.1",
     "chalk": "^2.4.2",
@@ -95,6 +96,7 @@
     "ip-address": "^6.1.0",
     "lodash": "^4.17.15",
     "moment": "^2.19.3",
+    "object-hash": "^1.3.1",
     "qs": "^6.5.0",
     "table": "^5.4.4",
     "version_compare": "^0.0.3",

src/Client.ts

@@ -20,6 +20,7 @@ import {NodeDAO} from './dao/NodeDAO';
 import {SituationFeedbackDAO} from './dao/SituationFeedbackDAO';
 
 import {AxiosHTTP} from './rest/AxiosHTTP';
+import { OnmsAuthConfig } from './api/OnmsAuthConfig';
 
 /**
  * The OpenNMS client.  This is the primary interface to OpenNMS servers.
@@ -34,14 +35,14 @@ export class Client implements IHasHTTP {
    * @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, server);
+    let opts = new OnmsHTTPOptions(timeout, server);
     if (!httpImpl) {
       if (!Client.defaultHttp) {
         throw new OnmsError('No HTTP implementation is configured!');
       }
       httpImpl = Client.defaultHttp;
     }
-    opts.headers.accept = 'text/plain';
+    opts = opts.withHeader('Accept', 'text/plain');
 
     const infoUrl = server.resolveURL('rest/alarms/count');
     log.debug('checkServer: checking URL: ' + infoUrl);
@@ -59,49 +60,43 @@ export class Client implements IHasHTTP {
    */
   public static async getMetadata(server: OnmsServer, httpImpl?: IOnmsHTTP, timeout?: number):
     Promise<ServerMetadata> {
-    const opts = new OnmsHTTPOptions(timeout, server.auth, server);
-    opts.headers.accept = 'application/json';
+    let opts = new OnmsHTTPOptions(timeout, server).withHeader('Accept', 'application/json');
     if (!httpImpl) {
       if (!Client.defaultHttp) {
         throw new OnmsError('No default HTTP implementation is configured!');
       }
       httpImpl = Client.defaultHttp;
     }
     if (!timeout && httpImpl && httpImpl.options && httpImpl.options.timeout) {
-      opts.timeout = httpImpl.options.timeout;
+      opts = opts.withTimeout(httpImpl.options.timeout);
     }
 
     const infoUrl = server.resolveURL('rest/info');
     log.debug('getMetadata: checking URL: ' + infoUrl);
 
     const response = await httpImpl.get(infoUrl, opts);
     const version = new OnmsVersion(response.data.version, response.data.displayVersion);
-    const metadata = new ServerMetadata(version);
+    let type = ServerTypes.HORIZON;
     if (response.data.packageName) {
       if (response.data.packageName.toLowerCase() === 'meridian') {
-        metadata.type = ServerTypes.MERIDIAN;
+        type = ServerTypes.MERIDIAN;
       }
     }
-    if (metadata.ticketer()) {
-      metadata.ticketerConfig = new TicketerConfig();
-      metadata.ticketerConfig.enabled = false;
-      if (response.data.ticketerConfig) {
-          metadata.ticketerConfig.plugin = response.data.ticketerConfig.plugin;
-          metadata.ticketerConfig.enabled = response.data.ticketerConfig.enabled === true;
-      }
+
+    if (response.data.ticketerConfig) {
+      const config = response.data.ticketerConfig;
+      return new ServerMetadata(version, type, new TicketerConfig(config.plugin, config.enabled));
     }
-    return metadata;
+
+    return new ServerMetadata(version, type);
   }
 
   /** The default OnmsHTTP implementation to be used when making requests */
-  private static defaultHttp: IOnmsHTTP;
+  private static defaultHttp: IOnmsHTTP = new AxiosHTTP();
 
   /** the OnmsHTTP implementation that will be used when making requests */
   public http: IOnmsHTTP;
 
-  /** The remote server to connect to */
-  public server?: OnmsServer;
-
   /**
    * A cache of initialized DAOs, kept until server configuration changes
    * @hidden
@@ -116,32 +111,31 @@ export class Client implements IHasHTTP {
    *        based on the environment.
    */
   constructor(httpImpl?: IOnmsHTTP) {
-    if (httpImpl) {
-      Client.defaultHttp = httpImpl;
-    } else {
-      Client.defaultHttp = new AxiosHTTP();
-    }
-    this.http = Client.defaultHttp;
+    this.http = httpImpl || Client.defaultHttp;
   }
 
   /**
-   * Connect to an OpenNMS server, check what capabilities it has, and return an [[OnmsServer]]
-   * for that connection.
+   * Connect to an OpenNMS server.
+   *
+   * NOTE: This method will connect to the server using the provided
+   * information, get the server metadata, and then _assign_ that
+   * server to the _existing_ [[IOnmsHTTP]] implementation associated
+   * with this client (or the default impl, if one has not yet been provided).
    */
   public async connect(name: string, url: string, username: string, password: string, timeout?: number) {
     const self = this;
-    const server = new OnmsServer(name, url, username, password);
+    let server = new OnmsServer(name, url, new OnmsAuthConfig(username, password));
 
-    await Client.checkServer(server, undefined, timeout);
-    server.metadata = await Client.getMetadata(server, undefined, timeout);
+    await Client.checkServer(server, this.http, timeout);
+    const metadata = await Client.getMetadata(server, this.http, timeout);
+    server = server.withMetadata(metadata);
 
     if (!self.http) {
       self.http = Client.defaultHttp;
     }
     if (!self.http.server) {
       self.http.server = server;
     }
-    self.server = server;
 
     return self;
   }
@@ -183,7 +177,7 @@ export class Client implements IHasHTTP {
   ) {
     const existing = this.daos.get(key);
     if (existing) {
-      if (existing.server && existing.server.equals(this.server)) {
+      if (existing.server && existing.server.equals(this.http.server)) {
         return existing;
       }
     }

src/api/OnmsAuthConfig.ts

@@ -4,24 +4,25 @@
  */
 export class OnmsAuthConfig {
   /** The password to authenticate with. */
-  public password?: string;
+  public readonly password: string | null;
 
   /** The username to connect as. */
-  public username?: string;
+  public readonly username: string | null;
 
   /**
    * Construct an auth configuration object.
    * @constructor
    */
-  constructor(username?: string, password?: string) {
-    this.username = username;
-    this.password = password;
+  constructor(username?: string | null, password?: string | null) {
+    this.username = username || null;
+    this.password = password || null;
+    Object.freeze(this);
   }
 
   /**
    * Whether this auth object is the same as another.
    */
-  public equals(that?: OnmsAuthConfig) {
+  public equals(that?: OnmsAuthConfig | null) {
     return that
       && this.username === that.username
       && this.password === that.password;

src/api/OnmsError.ts

@@ -7,17 +7,17 @@ export class OnmsError extends Error {
    * The response status code, if any.
    * @hidden
    */
-  private statusCode?: number;
+  private readonly statusCode?: number;
 
   /**
    * The data (payload) associated with a response.
    */
-  private data: any;
+  private readonly data: any;
 
   /**
    * The options provided as part of the request that resulted in this erro.
    */
-  private options: any;
+  private readonly options: any;
 
   /** The error code associated with this error. */
   public get code() {