error check route creation and observable button bar instance

4 files changed, 81 insertions, 34 deletions

diff --git a/src/client/views/DocumentButtonBar.tsx b/src/client/views/DocumentButtonBar.tsx
index 15cbd2788..7f125dd34 100644
--- a/src/client/views/DocumentButtonBar.tsx
+++ b/src/client/views/DocumentButtonBar.tsx
@@ -21,7 +21,6 @@ import React = require("react");
 import { DocumentView } from './nodes/DocumentView';
 import { ParentDocSelector } from './collections/ParentDocumentSelector';
 import { CollectionDockingView } from './collections/CollectionDockingView';
-import { DocumentDecorations } from './DocumentDecorations';
 const higflyout = require("@hig/flyout");
 export const { anchorPoints } = higflyout;
 export const Flyout = higflyout.default;
@@ -57,13 +56,13 @@ export class DocumentButtonBar extends React.Component<{ views: DocumentView[],
 
     @observable private openHover = false;
 
-    public static Instance: DocumentButtonBar;
+    @observable public static Instance: DocumentButtonBar;
     public static hasPushedHack = false;
     public static hasPulledHack = false;
 
     constructor(props: { views: DocumentView[] }) {
         super(props);
-        DocumentButtonBar.Instance = this;
+        runInAction(() => DocumentButtonBar.Instance = this);
     }
 
     public startPullOutcome = action((success: boolean) => {
diff --git a/src/client/views/nodes/FormattedTextBox.tsx b/src/client/views/nodes/FormattedTextBox.tsx
index 57321de49..a9f8ea315 100644
--- a/src/client/views/nodes/FormattedTextBox.tsx
+++ b/src/client/views/nodes/FormattedTextBox.tsx
@@ -34,7 +34,6 @@ import { TooltipTextMenu } from "../../util/TooltipTextMenu";
 import { undoBatch, UndoManager } from "../../util/UndoManager";
 import { DocAnnotatableComponent } from "../DocComponent";
 import { DocumentButtonBar } from '../DocumentButtonBar';
-import { DocumentDecorations } from '../DocumentDecorations';
 import { InkingControl } from "../InkingControl";
 import { FieldView, FieldViewProps } from "./FieldView";
 import "./FormattedTextBox.scss";
@@ -92,6 +91,7 @@ export class FormattedTextBox extends DocAnnotatableComponent<(FieldViewProps &
     private _proxyReactionDisposer: Opt<IReactionDisposer>;
     private _pullReactionDisposer: Opt<IReactionDisposer>;
     private _pushReactionDisposer: Opt<IReactionDisposer>;
+    private _buttonBarReactionDisposer: Opt<IReactionDisposer>;
     private dropDisposer?: DragManager.DragDropDisposer;
 
     @observable private _ruleFontSize = 0;
@@ -472,10 +472,15 @@ export class FormattedTextBox extends DocAnnotatableComponent<(FieldViewProps &
     }
 
     componentDidMount() {
-        if (DocumentButtonBar.Instance) {
-            this.pullFromGoogleDoc(this.checkState);
-            this.dataDoc[GoogleRef] && this.dataDoc.unchanged && runInAction(() => DocumentButtonBar.Instance.isAnimatingFetch = true);
-        }
+        this._buttonBarReactionDisposer = reaction(
+            () => DocumentButtonBar.Instance,
+            instance => {
+                if (instance) {
+                    this.pullFromGoogleDoc(this.checkState);
+                    this.dataDoc[GoogleRef] && this.dataDoc.unchanged && runInAction(() => instance.isAnimatingFetch = true);
+                }
+            }
+        );
 
         this._reactionDisposer = reaction(
             () => {
@@ -835,6 +840,7 @@ export class FormattedTextBox extends DocAnnotatableComponent<(FieldViewProps &
         this._pullReactionDisposer && this._pullReactionDisposer();
         this._heightReactionDisposer && this._heightReactionDisposer();
         this._searchReactionDisposer && this._searchReactionDisposer();
+        this._buttonBarReactionDisposer && this._buttonBarReactionDisposer();
         this._editorView && this._editorView.destroy();
     }
     onPointerDown = (e: React.PointerEvent): void => {
diff --git a/src/server/RouteManager.ts b/src/server/RouteManager.ts
index 3af36d879..347be1952 100644
--- a/src/server/RouteManager.ts
+++ b/src/server/RouteManager.ts
@@ -1,7 +1,7 @@
 import RouteSubscriber from "./RouteSubscriber";
 import { DashUserModel } from "./authentication/models/user_model";
 import * as express from 'express';
-import { yellow, cyan, red } from 'colors';
+import { cyan, red, green } from 'colors';
 
 export enum Method {
     GET,
@@ -28,9 +28,15 @@ export interface RouteInitializer {
 
 const registered = new Map<string, Set<Method>>();
 
+enum RegistrationError {
+    Malformed,
+    Duplicate
+}
+
 export default class RouteManager {
     private server: express.Express;
     private _isRelease: boolean;
+    private failedRegistrations: { route: string, reason: RegistrationError }[] = [];
 
     public get isRelease() {
         return this._isRelease;
@@ -41,10 +47,35 @@ export default class RouteManager {
         this._isRelease = isRelease;
     }
 
-    log = () => {
-        console.log(yellow("\nthe following server routes have been registered:"));
-        Array.from(registered.keys()).sort().forEach(route => console.log(cyan(route)));
-        console.log();
+    logRegistrationOutcome = () => {
+        if (this.failedRegistrations.length) {
+            let duplicateCount = 0;
+            let malformedCount = 0;
+            this.failedRegistrations.forEach(({ reason, route }) => {
+                let error: string;
+                if (reason === RegistrationError.Duplicate) {
+                    error = `duplicate registration error: ${route} is already registered `;
+                    duplicateCount++;
+                } else {
+                    error = `malformed route error: ${route} is invalid`;
+                    malformedCount++;
+                }
+                console.log(red(error));
+            });
+            console.log();
+            if (duplicateCount) {
+                console.log('please remove all duplicate routes before continuing');
+            }
+            if (malformedCount) {
+                console.log(`please ensure all routes adhere to ^\/[A-Za-z]+(\/\:[A-Za-z]+)*$`);
+            }
+            console.log();
+            process.exit(0);
+        } else {
+            console.log(green("all server routes have been successfully registered:"));
+            Array.from(registered.keys()).sort().forEach(route => console.log(cyan(route)));
+            console.log();
+        }
     }
 
     /**
@@ -84,7 +115,7 @@ export default class RouteManager {
             }
             setTimeout(() => {
                 if (!res.headersSent) {
-                    console.log(`Initiating fallback for ${target}`);
+                    console.log(red(`Initiating fallback for ${target}. Please remove dangling promise from route handler`));
                     const warning = `request to ${target} fell through - this is a fallback response`;
                     res.send({ warning });
                 }
@@ -97,25 +128,34 @@ export default class RouteManager {
             } else {
                 route = subscriber.build;
             }
-            const existing = registered.get(route);
-            if (existing) {
-                if (existing.has(method)) {
-                    console.log(red(`\nDuplicate registration error: already registered ${route} with Method[${method}]`));
-                    console.log('Please remove duplicate registrations before continuing...\n');
-                    process.exit(0);
-                }
+            if (!/^\/[A-Za-z]+(\/\:[A-Za-z]+)*$/g.test(route)) {
+                this.failedRegistrations.push({
+                    reason: RegistrationError.Malformed,
+                    route
+                });
             } else {
-                const specific = new Set<Method>();
-                specific.add(method);
-                registered.set(route, specific);
-            }
-            switch (method) {
-                case Method.GET:
-                    this.server.get(route, supervised);
-                    break;
-                case Method.POST:
-                    this.server.post(route, supervised);
-                    break;
+                const existing = registered.get(route);
+                if (existing) {
+                    if (existing.has(method)) {
+                        this.failedRegistrations.push({
+                            reason: RegistrationError.Duplicate,
+                            route
+                        });
+                        return;
+                    }
+                } else {
+                    const specific = new Set<Method>();
+                    specific.add(method);
+                    registered.set(route, specific);
+                }
+                switch (method) {
+                    case Method.GET:
+                        this.server.get(route, supervised);
+                        break;
+                    case Method.POST:
+                        this.server.post(route, supervised);
+                        break;
+                }
             }
         };
         if (Array.isArray(subscription)) {
diff --git a/src/server/index.ts b/src/server/index.ts
index 0e0bd0bd6..2921ad2df 100644
--- a/src/server/index.ts
+++ b/src/server/index.ts
@@ -22,6 +22,7 @@ import { log_execution } from "./ActionUtilities";
 import GeneralGoogleManager from "./ApiManagers/GeneralGoogleManager";
 import GooglePhotosManager from "./ApiManagers/GooglePhotosManager";
 import DiagnosticManager from "./ApiManagers/DiagnosticManager";
+import { yellow } from "colors";
 
 export const publicDirectory = path.resolve(__dirname, "public");
 export const filesDirectory = path.resolve(publicDirectory, "files");
@@ -51,7 +52,7 @@ async function preliminaryFunctions() {
  * that will manage the registration of new routes
  * with the server
  */
-function routeSetter({ isRelease, addSupervisedRoute, log }: RouteManager) {
+function routeSetter({ isRelease, addSupervisedRoute, logRegistrationOutcome }: RouteManager) {
     const managers = [
         new UserManager(),
         new UploadManager(),
@@ -66,8 +67,9 @@ function routeSetter({ isRelease, addSupervisedRoute, log }: RouteManager) {
     ];
 
     // initialize API Managers
+    console.log(yellow("\nregistering server routes..."));
     managers.forEach(manager => manager.register(addSupervisedRoute));
-    log();
+    logRegistrationOutcome();
 
     // initialize the web socket (bidirectional communication: if a user changes
     // a field on one client, that change must be broadcast to all other clients)