fix: Fix engine status parsing for engine get metods (#95)

Author: stepansergeevitch

src/service/engine/index.ts

@@ -2,7 +2,12 @@ import { ConnectionError, DeprecationError } from "../../common/errors";
 import { ResourceManagerContext } from "../../types";
 import { DatabaseModel } from "../database/model";
 import { EngineModel } from "./model";
-import { EngineStatusSummary, CreateEngineOptions, EngineType } from "./types";
+import {
+  EngineStatusSummary,
+  CreateEngineOptions,
+  EngineType,
+  processEngineStatus
+} from "./types";
 
 export class EngineService {
   context: ResourceManagerContext;
@@ -31,6 +36,17 @@ export class EngineService {
     this.context = context;
   }
 
+  private parseStatusForEngine(
+    value: string,
+    name: string
+  ): EngineStatusSummary {
+    const status = processEngineStatus(value);
+    if (status === undefined) {
+      throw new Error(`Engine ${name} has an unexpected status ${value}`);
+    }
+    return status;
+  }
+
   async getById(engineId: string) {
     throw new DeprecationError({
       message: "Can't call getById as engine IDs are deprecated"
@@ -52,7 +68,7 @@ export class EngineService {
     return new EngineModel(this.context.connection, {
       name,
       endpoint,
-      current_status_summary: status as EngineStatusSummary
+      current_status_summary: this.parseStatusForEngine(status, name)
     });
   }
 
@@ -71,7 +87,7 @@ export class EngineService {
         new EngineModel(this.context.connection, {
           name,
           endpoint,
-          current_status_summary: summary as EngineStatusSummary
+          current_status_summary: this.parseStatusForEngine(summary, name)
         })
       );
     }
@@ -93,7 +109,7 @@ export class EngineService {
         new EngineModel(this.context.connection, {
           name,
           endpoint,
-          current_status_summary: summary as EngineStatusSummary
+          current_status_summary: this.parseStatusForEngine(summary, name)
         })
       );
     }

src/service/engine/model.ts

@@ -72,7 +72,7 @@ export class EngineModel {
     }
     const firstRow = data[0] as unknown[];
     const status = processEngineStatus(firstRow[0] as string);
-    if (!status) {
+    if (status === undefined) {
       throw new Error(
         `Engine ${this.name} has an unexpected status ${firstRow[0]}`
       );

src/service/engine/types.ts

@@ -7,9 +7,12 @@ export type Engine = {
 };
 
 export function processEngineStatus(
-  value: string
+  value: string | undefined
 ): EngineStatusSummary | undefined {
   // Translate status from db to an EngineStatusSummary object
+  if (value === undefined) {
+    return undefined;
+  }
   const enumKey = Object.keys(EngineStatusSummary).find(
     key =>
       EngineStatusSummary[

test/unit/v2/database.test.ts

@@ -4,6 +4,7 @@ import { QUERY_URL } from "../../../src/common/api";
 import { Firebolt } from "../../../src";
 import { ConnectionError, DeprecationError } from "../../../src/common/errors";
 import { CreateDatabaseOptions } from "../../../src/service/database/types";
+import { selectEngineResponse, selectEnginesResponse } from "./engine.test";
 
 const apiEndpoint = "api.fake.firebolt.io";
 
@@ -40,6 +41,25 @@ const selectDbsResponse = {
   rows: 2
 };
 
+const selectOtherEngineResponse = {
+  meta: [
+    {
+      name: "engine_name",
+      type: "text"
+    },
+    {
+      name: "url",
+      type: "text"
+    },
+    {
+      name: "status",
+      type: "text"
+    }
+  ],
+  data: [["some_other_engine", "https://some_other_engine.com", "Running"]],
+  rows: 1
+};
+
 describe("database service", () => {
   const server = setupServer();
 
@@ -186,6 +206,31 @@ describe("database service", () => {
   });
 
   it("create and delete database", async () => {
+    server.use(
+      // Query against system engine
+      rest.post(
+        `https://some_system_engine.com/${QUERY_URL}`,
+        (req, res, ctx) => {
+          const body = (String(req.body) ?? "").toLowerCase();
+          if (
+            body.includes("information_schema.engines") &&
+            body.includes("some_engine")
+          ) {
+            return res(ctx.json(selectEngineResponse));
+          } else if (
+            body.includes("information_schema.engines") &&
+            body.includes("some_other_engine")
+          ) {
+            return res(ctx.json(selectOtherEngineResponse));
+          } else if (body.includes("information_schema.engines")) {
+            return res(ctx.json(selectEnginesResponse));
+          } else {
+            return res(ctx.json(selectDbResponse));
+          }
+        }
+      )
+    );
+
     const firebolt = Firebolt({ apiEndpoint });
     await firebolt.connect({
       account: "my_account",

test/unit/v2/engine.test.ts

@@ -14,7 +14,7 @@ import {
 
 const apiEndpoint = "api.fake.firebolt.io";
 
-const selectEngineResponse = {
+export const selectEngineResponse = {
   meta: [
     {
       name: "engine_name",
@@ -33,7 +33,7 @@ const selectEngineResponse = {
   rows: 1
 };
 
-const selectEnginesResponse = {
+export const selectEnginesResponse = {
   meta: [
     {
       name: "engine_name",
@@ -119,6 +119,36 @@ describe("engine service", () => {
     expect(engine).toBeTruthy();
     expect(engine.endpoint).toEqual("https://some_engine.com");
   });
+
+  it("gets engine by name v2 status", async () => {
+    // Copy
+    const engineResponse = JSON.parse(JSON.stringify(selectEngineResponse));
+    engineResponse.data.forEach((row: string[]) => {
+      row[2] = "RUNNING";
+    });
+    server.use(
+      rest.post(
+        `https://some_system_engine.com/${QUERY_URL}`,
+        (req, res, ctx) => {
+          return res(ctx.json(engineResponse));
+        }
+      )
+    );
+
+    const firebolt = Firebolt({ apiEndpoint });
+    await firebolt.connect({
+      account: "my_account",
+      auth: {
+        client_id: "id",
+        client_secret: "secret"
+      }
+    });
+    const resourceManager = firebolt.resourceManager;
+    const engine = await resourceManager.engine.getByName("some_engine");
+    expect(engine).toBeTruthy();
+    expect(engine.endpoint).toEqual("https://some_engine.com");
+  });
+
   it("starts engine", async () => {
     let startEngineCalled = false;
     const expectedEngine = "some_engine";
@@ -256,6 +286,58 @@ describe("engine service", () => {
     expect(engines[1].endpoint).toEqual("https://some_other_engine.com");
   });
 
+  it("gets all engines v2 status", async () => {
+    // Copy
+    const engineResponse = JSON.parse(JSON.stringify(selectEnginesResponse));
+    engineResponse.data.forEach((row: string[]) => {
+      row[2] = "RUNNING";
+    });
+    server.use(
+      rest.post(
+        `https://some_system_engine.com/${QUERY_URL}`,
+        (req, res, ctx) => {
+          return res(ctx.json(engineResponse));
+        }
+      )
+    );
+
+    const connectionOptions = {
+      account: "my_account",
+      auth: {
+        client_id: "id",
+        client_secret: "secret"
+      }
+    };
+    const firebolt = Firebolt({ apiEndpoint });
+    const connection = await firebolt.connect(connectionOptions);
+    // Also test diffrent way of instantiating a resource manager
+    const logger = new Logger();
+    const httpClient = new NodeHttpClient();
+    new Authenticator(
+      {
+        httpClient,
+        logger,
+        apiEndpoint,
+        queryFormatter: new QueryFormatter()
+      },
+      connectionOptions
+    );
+    const resourceManager = new ResourceManager({
+      logger,
+      connection,
+      apiEndpoint,
+      httpClient
+    });
+    const engines = await resourceManager.engine.getAll();
+    expect(engines).toBeTruthy();
+    expect(engines[0].current_status_summary).toEqual(
+      EngineStatusSummary.RUNNING
+    );
+    expect(engines[1].current_status_summary).toEqual(
+      EngineStatusSummary.RUNNING
+    );
+  });
+
   it("does not start engine that errors out", async () => {
     let startEngineCalled = false;
     const expectedEngine = "some_engine";
@@ -298,6 +380,7 @@ describe("engine service", () => {
     const engine = await resourceManager.engine.getByName(expectedEngine);
     expect(engine.start()).rejects.toThrowError(ConnectionError);
   });
+
   it("does not start engine that has unexpected status", async () => {
     let startEngineCalled = false;
     const expectedEngine = "some_engine";
@@ -463,6 +546,7 @@ describe("engine service", () => {
     expect(engine).toBeTruthy();
     expect(engine.endpoint).toEqual("https://some_engine.com");
   });
+
   it("create engine with environment variable", async () => {
     const engine_version = "20.1.1";
     server.use(
@@ -495,10 +579,13 @@ describe("engine service", () => {
 
     delete process.env.ENGINE_NAME;
   });
+
   it("Parses different engine statuses correctly", async () => {
     const statuses = ["RUNNING", "Running", "running"];
     for (const status of statuses) {
       expect(processEngineStatus(status)).toEqual(EngineStatusSummary.RUNNING);
     }
+    expect(processEngineStatus(undefined)).toBe(undefined);
+    expect(processEngineStatus("unexisting")).not.toBeTruthy();
   });
 });