Merge pull request #464 from ONLYOFFICE/feature/get-table-columns-usage-refactoring

Feature/get table columns usage refactoring

.github/workflows/damengDatabaseTests.yml

@@ -2,6 +2,7 @@ name: Dameng database tests
 on:
   push:
     paths:
+      - 'tests/integration/databaseTests/**'
       - 'DocService/sources/databaseConnectors/baseConnector.js'
       - 'DocService/sources/databaseConnectors/damengConnector.js'
 jobs:

.github/workflows/mssqlDatabaseTests.yml

@@ -2,6 +2,7 @@ name: MSSQL database tests
 on:
   push:
     paths:
+      - 'tests/integration/databaseTests/**'
       - 'DocService/sources/databaseConnectors/baseConnector.js'
       - 'DocService/sources/databaseConnectors/mssqlConnector.js'
 jobs:

.github/workflows/mysqlDatabaseTests.yml

@@ -2,6 +2,7 @@ name: MYSQL database tests
 on:
   push:
     paths:
+      - 'tests/integration/databaseTests/**'
       - 'DocService/sources/databaseConnectors/baseConnector.js'
       - 'DocService/sources/databaseConnectors/mysqlConnector.js'
 jobs:

.github/workflows/oracleDatabaseTests.yml

@@ -2,6 +2,7 @@ name: Oracle database tests
 on:
   push:
     paths:
+      - 'tests/integration/databaseTests/**'
       - 'DocService/sources/databaseConnectors/baseConnector.js'
       - 'DocService/sources/databaseConnectors/oracleConnector.js'
 jobs:

.github/workflows/postgreDatabaseTests.yml

@@ -2,6 +2,7 @@ name: Postgre database tests
 on:
   push:
     paths:
+      - 'tests/integration/databaseTests/**'
       - 'DocService/sources/databaseConnectors/baseConnector.js'
       - 'DocService/sources/databaseConnectors/postgreConnector.js'
 jobs:

Common/sources/constants.js

@@ -293,4 +293,28 @@ exports.SUPPORTED_TEMPLATES_EXTENSIONS = {
   'Word': ['docx', 'docxf'],
   'Excel': ['xlsx'],
   'PowerPoint': ['pptx']
-};
+};
+exports.TABLE_RESULT_SCHEMA = [
+  'tenant',
+  'id',
+  'status',
+  'status_info',
+  'created_at',
+  'last_open_date',
+  'user_index',
+  'change_id',
+  'callback',
+  'baseurl',
+  'password',
+  'additional'
+];
+exports.TABLE_CHANGES_SCHEMA = [
+  'tenant',
+  'id',
+  'change_id',
+  'user_id',
+  'user_id_original',
+  'user_name',
+  'change_data',
+  'change_date',
+];

DocService/sources/DocsCoServer.js

@@ -144,6 +144,7 @@ const cfgExpDocumentsCron = config.get('services.CoAuthoring.expire.documentsCro
 const cfgRefreshLockInterval = ms(config.get('wopi.refreshLockInterval'));
 const cfgSocketIoConnection = config.get('services.CoAuthoring.socketio.connection');
 const cfgTableResult = config.get('services.CoAuthoring.sql.tableResult');
+const cfgTableChanges = config.get('services.CoAuthoring.sql.tableChanges');
 
 const EditorTypes = {
   document : 0,
@@ -1475,6 +1476,26 @@ function getOpenFormatByEditor(editorType) {
   return res;
 }
 
+async function isSchemaCompatible([tableName, tableSchema]) {
+  const resultSchema = await sqlBase.getTableColumns(operationContext.global, tableName);
+
+  if (resultSchema.length === 0) {
+    operationContext.global.logger.error('DB table "%s" does not exist', tableName);
+    return false;
+  }
+
+  const columnArray = resultSchema.map(row => row['column_name']);
+  const hashedResult = new Set(columnArray);
+  const schemaDiff = tableSchema.filter(column => !hashedResult.has(column));
+
+  if (schemaDiff.length > 0) {
+    operationContext.global.logger.error(`DB table "${tableName}" does not contain columns: ${schemaDiff}, columns info: ${columnArray}`);
+    return false;
+  }
+
+  return true;
+}
+
 exports.c_oAscServerStatus = c_oAscServerStatus;
 exports.editorData = editorData;
 exports.sendData = sendData;
@@ -3933,29 +3954,28 @@ exports.install = function(server, callbackFunction) {
       }
       gc.startGC();
 
-      let tableName = cfgTableResult;
-      const tableRequiredColumn = 'tenant';
       //check data base compatibility
-      sqlBase.getTableColumns(operationContext.global, tableName).then(function(res) {
-        let index = res.findIndex((currentValue) => {
-          for (let key in currentValue) {
-            if (currentValue.hasOwnProperty(key) && 'column_name' === key.toLowerCase()) {
-              return tableRequiredColumn === currentValue[key];
-            }
+      const tables = [
+        [cfgTableResult, constants.TABLE_RESULT_SCHEMA],
+        [cfgTableChanges, constants.TABLE_CHANGES_SCHEMA]
+      ];
+      const requestPromises = tables.map(table => isSchemaCompatible(table));
+
+      Promise.all(requestPromises).then(
+        checkResult => {
+          if (checkResult.includes(false)) {
+            return;
           }
-        });
-        if (-1 !== index || 0 === res.length) {
-          return editorData.connect().then(function() {
-            callbackFunction();
-          }).catch(err => {
-            operationContext.global.logger.error('editorData error: %s', err.stack);
-          });
-        } else {
-          operationContext.global.logger.error('DB table "%s" does not contain %s column, columns info: %j', tableName, tableRequiredColumn, res);
-        }
-      }).catch(err => {
-        operationContext.global.logger.error('getTableColumns error: %s', err.stack);
-      });
+
+          editorData.connect().then(
+            () => {
+                callbackFunction();
+              },
+            error => operationContext.global.logger.error('editorData error: %s', error.stack)
+          );
+        },
+        error => operationContext.global.logger.error('getTableColumns error: %s', error.stack)
+      );
     });
   });
 };

tests/integration/databaseTests/baseConnector.tests.js

@@ -218,30 +218,8 @@ describe('Base database connector', function () {
 
   describe('DB tables existence', function () {
     const tables = {
-      [cfgTableResult]: [
-        { column_name: 'tenant' },
-        { column_name: 'id' },
-        { column_name: 'status' },
-        { column_name: 'status_info' },
-        { column_name: 'created_at' },
-        { column_name: 'last_open_date' },
-        { column_name: 'user_index' },
-        { column_name: 'change_id' },
-        { column_name: 'callback' },
-        { column_name: 'baseurl' },
-        { column_name: 'password' },
-        { column_name: 'additional' }
-      ],
-      [cfgTableChanges]: [
-        { column_name: 'tenant' },
-        { column_name: 'id' },
-        { column_name: 'change_id' },
-        { column_name: 'user_id' },
-        { column_name: 'user_id_original' },
-        { column_name: 'user_name' },
-        { column_name: 'change_data' },
-        { column_name: 'change_date' }
-      ]
+      [cfgTableResult]: constants.TABLE_RESULT_SCHEMA.map(column => { return { column_name: column } }),
+      [cfgTableChanges]: constants.TABLE_CHANGES_SCHEMA.map(column => { return { column_name: column } })
     };
 
     for (const table in tables) {
@@ -252,6 +230,12 @@ describe('Base database connector', function () {
         }
       });
     }
+    
+    const table = "unused_table";
+    test(`${table} table absence`, async function () {
+      const result = await baseConnector.getTableColumns(ctx, table);
+      expect(result).toEqual([]);
+    });
   });
 
   describe('Changes manipulations', function () {
@@ -437,4 +421,4 @@ describe('Base database connector', function () {
       expect(updatedRow).toEqual(expectedUrlChanges);
     });
   });
-});
+});