fix: skip mssql tables with dots in their names instead of crashing t…

…he agent (#870)

* fix: skip mssql tables with dots in their names instead of crashing the agent 2

* test: add unit test

* refactor: update according to review suggestion

* fix: remove useless filter

* test: add broken relation  integration test

* test: add integration test

* test: fix integration test

* test: implement review suggestions

---------

Co-authored-by: Nicolas Moreau <nicolas.moreau76@gmail.com>

packages/datasource-sql/src/introspection/introspector.ts

@@ -14,8 +14,22 @@ import { Table } from './types';
 
 export default class Introspector {
   static async introspect(sequelize: Sequelize, logger?: Logger): Promise<Table[]> {
-    const tableNames = await this.getTableNames(sequelize as SequelizeWithOptions);
-    const promises = tableNames.map(name => this.getTable(sequelize, logger, name));
+    const tableNamesAndSchemas = await this.getTableNames(sequelize as SequelizeWithOptions);
+    const validTableNames = tableNamesAndSchemas.filter(
+      name => sequelize.getDialect() !== 'mssql' || !name.tableName.includes('.'),
+    );
+
+    if (validTableNames.length < tableNamesAndSchemas.length) {
+      const diff = tableNamesAndSchemas.filter(name => !validTableNames.includes(name));
+      logger?.(
+        'Warn',
+        `Skipping table(s): ${diff
+          .map(tableNameAndSchema => `'${tableNameAndSchema.tableName}'`)
+          .join(', ')}. MSSQL tables with dots are not supported`,
+      );
+    }
+
+    const promises = validTableNames.map(name => this.getTable(sequelize, logger, name));
     const tables = await Promise.all(promises);
 
     this.sanitizeInPlace(tables);

packages/datasource-sql/test/_helpers/create-database-if-not-exist.ts

@@ -1,6 +1,10 @@
 import { Sequelize } from 'sequelize';
 
-export default async function createDatabaseIfNotExist(baseUri: string, database: string) {
+export default async function createDatabaseIfNotExist(
+  baseUri: string,
+  database: string,
+  closeConnection = false,
+) {
   const sequelize = new Sequelize(baseUri, { logging: false });
 
   try {
@@ -10,6 +14,8 @@ export default async function createDatabaseIfNotExist(baseUri: string, database
 
     throw e;
   } finally {
-    await sequelize.close();
+    if (closeConnection) await sequelize.close();
   }
+
+  return sequelize;
 }

packages/datasource-sql/test/introspection/introspector.integration.test.ts

@@ -0,0 +1,54 @@
+/* eslint-disable max-len */
+import { Sequelize } from 'sequelize';
+
+import Introspector from '../../src/introspection/introspector';
+import CONNECTION_DETAILS from '../_helpers/connection-details';
+
+const db = 'database_introspector';
+const URL = CONNECTION_DETAILS.find(connection => connection.dialect === 'mssql')?.url() as string;
+
+describe('Introspector > Integration', () => {
+  it('should skip MSSQL tables with dots in their names and their relations', async () => {
+    let sequelize = new Sequelize(URL, { logging: false });
+    await sequelize.getQueryInterface().dropDatabase(db);
+    await sequelize.getQueryInterface().createDatabase(db);
+    sequelize.close();
+
+    sequelize = new Sequelize(`${URL}/${db}`, { logging: false });
+
+    await sequelize.query(`
+CREATE TABLE "Employees.oldVersion" (
+	"EmployeeID" "int" IDENTITY (1, 1) NOT NULL ,
+	"Name" nvarchar (20) NOT NULL ,
+	CONSTRAINT "PK_Employees" PRIMARY KEY  CLUSTERED ("EmployeeID"),
+)
+      `);
+    await sequelize.query(`
+CREATE TABLE "Customers" (
+	"CustomerID" nchar (5) NOT NULL ,
+	"Name" nvarchar (40) NOT NULL ,
+	CONSTRAINT "PK_Customers" PRIMARY KEY  CLUSTERED ("CustomerID")
+)
+      `);
+    await sequelize.query(`
+CREATE TABLE "Orders" (
+	"OrderID" "int" IDENTITY (1, 1) NOT NULL ,
+	"CustomerID" nchar (5) NULL ,
+	"EmployeeID" "int" NULL ,
+	CONSTRAINT "PK_Orders" PRIMARY KEY  CLUSTERED ("OrderID"),
+    CONSTRAINT "FK_Orders_Customers" FOREIGN KEY ("CustomerID") REFERENCES "dbo"."Customers" ("CustomerID"),
+	CONSTRAINT "FK_Orders_Employees" FOREIGN KEY ("EmployeeID") REFERENCES "dbo"."Employees.oldVersion" ("EmployeeID"),
+)
+      `);
+    const tables = await Introspector.introspect(sequelize);
+    expect(tables).toHaveLength(2);
+    const orders = tables.find(table => table.name === 'Orders');
+    expect(orders?.columns[1].constraints).toStrictEqual([
+      {
+        column: 'CustomerID',
+        table: 'Customers',
+      },
+    ]);
+    expect(orders?.columns[2].constraints).toStrictEqual([]);
+  });
+});

packages/datasource-sql/test/introspection/intropector.test.ts → packages/datasource-sql/test/introspection/introspector.test.ts

@@ -88,7 +88,6 @@ describe('Introspector', () => {
         "Failed to load constraints on relation 'fk_unknown_column' on table 'table1'. The relation will be ignored.",
       );
     });
-
     it('should log errors for missing constraint names for sqlite datasources', async () => {
       // Mock the Sequelize methods
       const mockDescribeTable = jest.fn().mockResolvedValue({
@@ -137,5 +136,33 @@ describe('Introspector', () => {
         "Failed to load constraints on relation 'fk_unknown_column' on table 'table1'. The relation will be ignored.",
       );
     });
+    it('should skip mssql tables with dots in their names and log a warning', async () => {
+      mockQueryInterface.showAllTables = jest
+        .fn()
+        .mockResolvedValue([{ tableName: 'table.1' }, { tableName: 'table2' }]);
+
+      const getForeignKeyReferencesForTable = jest.fn().mockResolvedValue([]);
+
+      mockQueryInterface.getForeignKeyReferencesForTable = getForeignKeyReferencesForTable;
+      mockSequelize.query = jest.fn().mockResolvedValue([
+        {
+          constraint_name: 'fk_column1',
+          table_name: 'table.1',
+        },
+      ]);
+      mockSequelize.getDialect = jest.fn().mockReturnValue('mssql');
+
+      const result = await Introspector.introspect(mockSequelize, logger);
+      expect(result).toStrictEqual([expect.objectContaining({ name: 'table2' })]);
+      expect(logger).toHaveBeenCalledWith(
+        'Warn',
+        "Skipping table(s): 'table.1'. MSSQL tables with dots are not supported",
+      );
+      expect(logger).toHaveBeenCalledWith(
+        'Error',
+        // eslint-disable-next-line max-len
+        "Failed to load constraints on relation 'fk_column1' on table 'table.1'. The relation will be ignored.",
+      );
+    });
   });
 });