fix routing priority, add tests

Signed-off-by: Sebastian Malton <sebastian@malton.name>

extensions/example-extension/package-lock.json

@@ -1,5 +1,5 @@
 {
-  "name": "example-extension",
+  "name": "@mirantis/example-extension",
   "version": "1.0.0",
   "lockfileVersion": 1,
   "requires": true,

extensions/example-extension/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "example-extension",
+  "name": "@mirantis/example-extension",
   "version": "1.0.0",
   "description": "Example extension",
   "main": "dist/main.js",

extensions/kube-object-event-status/package-lock.json

@@ -1,5 +1,5 @@
 {
-  "name": "kube-object-event-status",
+  "name": "@mirantis/kube-object-event-status",
   "version": "0.1.0",
   "lockfileVersion": 1,
   "requires": true,

extensions/kube-object-event-status/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "kube-object-event-status",
+  "name": "@mirantis/kube-object-event-status",
   "version": "0.1.0",
   "description": "Adds kube object status from events",
   "renderer": "dist/renderer.js",

extensions/license-menu-item/package-lock.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-license",
+  "name": "@mirantis/lens-license",
   "version": "0.1.0",
   "lockfileVersion": 1,
   "requires": true,

extensions/license-menu-item/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-license",
+  "name": "@mirantis/lens-license",
   "version": "0.1.0",
   "description": "License menu item",
   "main": "dist/main.js",

extensions/metrics-cluster-feature/package-lock.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-metrics-cluster-feature",
+  "name": "@mirantis/lens-metrics-cluster-feature",
   "version": "0.1.0",
   "lockfileVersion": 1,
   "requires": true,

extensions/metrics-cluster-feature/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-metrics-cluster-feature",
+  "name": "@mirantis/lens-metrics-cluster-feature",
   "version": "0.1.0",
   "description": "Lens metrics cluster feature",
   "renderer": "dist/renderer.js",

extensions/node-menu/package-lock.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-node-menu",
+  "name": "@mirantis/lens-node-menu",
   "version": "0.1.0",
   "lockfileVersion": 1,
   "requires": true,

extensions/node-menu/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-node-menu",
+  "name": "@mirantis/lens-node-menu",
   "version": "0.1.0",
   "description": "Lens node menu",
   "renderer": "dist/renderer.js",

extensions/pod-menu/package-lock.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-pod-menu",
+  "name": "@mirantis/lens-pod-menu",
   "version": "0.1.0",
   "lockfileVersion": 1,
   "requires": true,

extensions/pod-menu/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-pod-menu",
+  "name": "@mirantis/lens-pod-menu",
   "version": "0.1.0",
   "description": "Lens pod menu",
   "renderer": "dist/renderer.js",

extensions/telemetry/package-lock.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-telemetry",
+  "name": "@mirantis/lens-telemetry",
   "version": "0.1.0",
   "lockfileVersion": 1,
   "requires": true,

extensions/telemetry/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "lens-telemetry",
+  "name": "@mirantis/lens-telemetry",
   "version": "0.1.0",
   "description": "Lens telemetry",
   "main": "dist/main.js",

src/extensions/lens-extension.ts

@@ -16,6 +16,8 @@ export interface LensExtensionManifest {
   lens?: object; // fixme: add more required fields for validation
 }
 
+const ExtensionNameSchema = /^@[a-z0-9][_-a-z0-9]*\/[a-z0-9][_-a-z0-9]*$/i;
+
 export class LensExtension {
   readonly id: LensExtensionId;
   readonly manifest: LensExtensionManifest;
@@ -25,6 +27,10 @@ export class LensExtension {
   @observable isEnabled = false;
 
   constructor({ id, manifest, manifestPath, isBundled }: InstalledExtension) {
+    if (!manifest.name.match(ExtensionNameSchema)) {
+      throw new TypeError("extension name must be '@<org>/<name>'");
+    }
+
     this.id = id;
     this.manifest = manifest;
     this.manifestPath = manifestPath;

src/main/protocol-handler/__test__/router.test.ts

@@ -11,26 +11,26 @@ describe("protocol router tests", () => {
   });
 
   it("should throw on non-lens URLS", () => {
-    expect(() => lpr.route(Url("https://google.ca"))).toThrowError();
+    expect(lpr.route(Url("https://google.ca"))).rejects.toThrowError();
   });
 
   it("should throw when host not internal or extension", () => {
-    expect(() => lpr.route(Url("lens://foobar"))).toThrowError();
+    expect(lpr.route(Url("lens://foobar"))).rejects.toThrowError();
   });
 
   it("should not throw when has valid host", () => {
     lpr.on("/", noop);
-    lpr.extensionOn("minikube", "/", noop);
+    lpr.extensionOn("@mirantis/minikube", "/", noop);
 
-    expect(() => lpr.route(Url("lens://internal"))).not.toThrowError();
-    expect(() => lpr.route(Url("lens://extension/minikube"))).not.toThrowError();
+    expect(lpr.route(Url("lens://internal"))).resolves.toBeUndefined();
+    expect(lpr.route(Url("lens://extension/@mirantis/minikube"))).resolves.toBeUndefined();
   });
 
   it("should call handler if matches", () => {
     let called = false;
 
     lpr.on("/page", () => { called = true; });
-    expect(() => lpr.route(Url("lens://internal/page"))).not.toThrowError();
+    expect(lpr.route(Url("lens://internal/page"))).resolves.toBeUndefined();
     expect(called).toBe(true);
   });
 
@@ -39,21 +39,42 @@ describe("protocol router tests", () => {
 
     lpr.on("/page", () => { called = 1; });
     lpr.on("/page/:id", params => { called = params.pathname.id; });
-    expect(() => lpr.route(Url("lens://internal/page/foo"))).not.toThrowError();
+    expect(lpr.route(Url("lens://internal/page/foo"))).resolves.toBeUndefined();
     expect(called).toBe("foo");
   });
 
   it("should call most exact handler for an extensions", () => {
     let called: any = 0;
 
-    lpr.extensionOn("foobar", "/page", () => { called = 1; });
-    lpr.extensionOn("foobar", "/page/:id", params => { called = params.pathname.id; });
-    expect(() => lpr.route(Url("lens://extension/foobar/page/foob"))).not.toThrowError();
+    lpr.extensionOn("@foobar/icecream", "/page", () => { called = 1; });
+    lpr.extensionOn("@foobar/icecream", "/page/:id", params => { called = params.pathname.id; });
+    expect(lpr.route(Url("lens://extension/@foobar/icecream/page/foob"))).resolves.toBeUndefined();
     expect(called).toBe("foob");
   });
 
   it("should throw if urlSchema is invalid", () => {
     expect(() => lpr.on("/:@", noop)).toThrowError();
-    expect(() => lpr.extensionOn("foobar", "/page/:@", noop)).toThrowError();
+    expect(() => lpr.extensionOn("@foobar/icecream", "/page/:@", noop)).toThrowError();
+  });
+
+  it("should call most exact handler with 3 found handlers", () => {
+    let called: any = 0;
+
+    lpr.on("/", () => { called = 2; });
+    lpr.on("/page", () => { called = 1; });
+    lpr.on("/page/foo", () => { called = 3; });
+    lpr.on("/page/bar", () => { called = 4; });
+    expect(lpr.route(Url("lens://internal/page/foo/bar/bat"))).resolves.toBeUndefined();
+    expect(called).toBe(3);
+  });
+
+  it("should call most exact handler with 2 found handlers", () => {
+    let called: any = 0;
+
+    lpr.on("/", () => { called = 2; });
+    lpr.on("/page", () => { called = 1; });
+    lpr.on("/page/bar", () => { called = 4; });
+    expect(lpr.route(Url("lens://internal/page/foo/bar/bat"))).resolves.toBeUndefined();
+    expect(called).toBe(1);
   });
 });

src/main/protocol-handler/router.ts

@@ -4,6 +4,7 @@ import { match, matchPath } from "react-router";
 import { pathToRegexp } from "path-to-regexp";
 import { subscribeToBroadcast } from "../../common/ipc";
 import logger from "../logger";
+import { countBy } from "lodash";
 
 export enum RoutingErrorType {
   INVALID_PROTOCOL = "invalid-protocol",
@@ -57,6 +58,18 @@ interface ExtensionUrlMatch {
   [EXTENSION_NAME_MATCH]: string;
 }
 
+function compareMatches<T>(a: match<T>, b: match<T>): number {
+  if (a.path === "/") {
+    return 1;
+  }
+
+  if (b.path === "/") {
+    return -1;
+  }
+
+  return countBy(b.path)["/"] - countBy(a.path)["/"];
+}
+
 export class LensProtocolRouter extends Singleton {
   private extentionRoutes = new Map<ExtensionId, Map<string, RouteHandler>>();
   private internalRoutes = new Map<string, RouteHandler>();
@@ -141,16 +154,15 @@ export class LensProtocolRouter extends Singleton {
     const matches = Array.from(routes.entries())
       .map(([schema, handler]): [match<Record<string, string>>, RouteHandler] => {
         if (matchExtension) {
-          const joinChar = schema.startsWith("/") ? "" : "/";
-
-          schema = `${LensProtocolRouter.ExtensionUrlSchema}${joinChar}${schema}`;
+          schema = `${LensProtocolRouter.ExtensionUrlSchema}/${schema}`.replace(/\/?\//g, "/");
         }
 
         return [matchPath(url.pathname, { path: schema }), handler];
       })
       .filter(([match]) => match);
     // prefer an exact match, but if not pick the first route registered
-    const route = matches.find(([match]) => match.isExact) ?? matches[0];
+    const route = matches.find(([match]) => match.isExact)
+      ?? matches.sort(([a], [b]) => compareMatches(a, b))[0];
 
     if (!route) {
       throw new RoutingError(RoutingErrorType.NO_HANDLER, url);