Refactor and simplify query tool connection error handling code. Fixes #3235

2025-02-25 18:55:31 -06:00 · 2018-04-04 11:20:36 +01:00
parent be055ce57d
commit a705fb46a8
8 changed files with 393 additions and 607 deletions
--- a/web/regression/javascript/sqleditor/execute_query_spec.js
+++ b/web/regression/javascript/sqleditor/execute_query_spec.js
@@ -8,7 +8,7 @@
 //////////////////////////////////////////////////////////////////////////

 import * as subject from 'sources/sqleditor/execute_query';
-import * as transaction from 'sources/sqleditor/is_new_transaction_required';
+import * as httpErrorHandler from 'sources/sqleditor/query_tool_http_error_handler';
 import axios from 'axios';
 import MockAdapter from 'axios-mock-adapter';
 import $ from 'jquery';
@@ -27,8 +27,8 @@ describe('ExecuteQuery', () => {
    networkMock = new MockAdapter(axios);
    jasmine.addMatchers({jQuerytoHaveBeenCalledWith: jQuerytoHaveBeenCalledWith});
    userManagementMock = jasmine.createSpyObj('UserManagement', [
-      'is_pga_login_required',
-      'pga_login',
+      'isPgaLoginRequired',
+      'pgaLogin',
    ]);

    sqlEditorMock = jasmine.createSpyObj('SqlEditor', [
@@ -40,14 +40,14 @@ describe('ExecuteQuery', () => {
      'update_msg_history',
      '_highlight_error',
      '_init_polling_flags',
-      'save_state',
-      'init_transaction',
+      'saveState',
+      'initTransaction',
      'handle_connection_lost',
    ]);
    sqlEditorMock.transId = 123;
    sqlEditorMock.rows_affected = 1000;
    executeQuery = new subject.ExecuteQuery(sqlEditorMock, userManagementMock);
-    isNewTransactionRequiredMock = spyOn(transaction, 'is_new_transaction_required');
+    isNewTransactionRequiredMock = spyOn(httpErrorHandler, 'httpResponseRequiresNewTransaction');
  });

  afterEach(() => {
@@ -272,7 +272,7 @@ describe('ExecuteQuery', () => {
          describe('when JSON response is available', () => {
            describe('when login is not required', () => {
              beforeEach(() => {
-                userManagementMock.is_pga_login_required.and.returnValue(false);
+                userManagementMock.isPgaLoginRequired.and.returnValue(false);
                response = {responseJSON: errorMessageJson};
                networkMock.onGet('/sqleditor/query_tool/poll/123').reply(401, response);

@@ -336,7 +336,7 @@ describe('ExecuteQuery', () => {
              it('should not login is displayed', (done) => {
                setTimeout(
                  () => {
-                    expect(userManagementMock.pga_login).not
+                    expect(userManagementMock.pgaLogin).not
                      .toHaveBeenCalled();
                    done();
                  }, 0);
@@ -345,7 +345,7 @@ describe('ExecuteQuery', () => {

            describe('when login is required', () => {
              beforeEach(() => {
-                userManagementMock.is_pga_login_required.and.returnValue(true);
+                userManagementMock.isPgaLoginRequired.and.returnValue(true);
                response = {responseJSON: errorMessageJson};
                networkMock.onGet('/sqleditor/query_tool/poll/123').reply(401, response);

@@ -409,7 +409,7 @@ describe('ExecuteQuery', () => {
              it('should login is displayed', (done) => {
                setTimeout(
                  () => {
-                    expect(userManagementMock.pga_login)
+                    expect(userManagementMock.pgaLogin)
                      .toHaveBeenCalled();
                    done();
                  }, 0);
@@ -420,7 +420,7 @@ describe('ExecuteQuery', () => {
          describe('when no JSON response is available', () => {
            describe('when login is not required', () => {
              beforeEach(() => {
-                userManagementMock.is_pga_login_required.and.returnValue(false);
+                userManagementMock.isPgaLoginRequired.and.returnValue(false);
                response = {
                  errormsg: errorMessageText,
                };
@@ -486,7 +486,7 @@ describe('ExecuteQuery', () => {
              it('should login is not displayed', (done) => {
                setTimeout(
                  () => {
-                    expect(userManagementMock.pga_login).not
+                    expect(userManagementMock.pgaLogin).not
                      .toHaveBeenCalled();
                    done();
                  }, 0);
@@ -495,7 +495,7 @@ describe('ExecuteQuery', () => {

            describe('when login is required', () => {
              beforeEach(() => {
-                userManagementMock.is_pga_login_required.and.returnValue(true);
+                userManagementMock.isPgaLoginRequired.and.returnValue(true);
                response = {
                  errormsg: errorMessageText,
                };
@@ -561,7 +561,7 @@ describe('ExecuteQuery', () => {
              it('should login is displayed', (done) => {
                setTimeout(
                  () => {
-                    expect(userManagementMock.pga_login)
+                    expect(userManagementMock.pgaLogin)
                      .toHaveBeenCalled();
                    done();
                  }, 0);
@@ -633,7 +633,7 @@ describe('ExecuteQuery', () => {
            it('should login is not displayed', (done) => {
              setTimeout(
                () => {
-                  expect(userManagementMock.pga_login).not
+                  expect(userManagementMock.pgaLogin).not
                    .toHaveBeenCalled();
                  done();
                }, 0);
@@ -1366,7 +1366,7 @@ describe('ExecuteQuery', () => {
      describe('when error is returned by the server', () => {
        describe('when login is not required', () => {
          beforeEach(() => {
-            userManagementMock.is_pga_login_required.and.returnValue(false);
+            userManagementMock.isPgaLoginRequired.and.returnValue(false);
            response.errormsg = 'some error message';
            networkMock.onAny('/sqleditor/query_tool/start/123').reply(500, response);

@@ -1422,19 +1422,19 @@ describe('ExecuteQuery', () => {

          it('should not save the state', () => {
            setTimeout(() => {
-              expect(sqlEditorMock.save_state).not.toHaveBeenCalled();
+              expect(sqlEditorMock.saveState).not.toHaveBeenCalled();
            }, 0);
          });

          it('should not display pga login', () => {
            setTimeout(() => {
-              expect(userManagementMock.pga_login).not.toHaveBeenCalled();
+              expect(userManagementMock.pgaLogin).not.toHaveBeenCalled();
            }, 0);
          });
        });
        describe('when login is required', () => {
          beforeEach(() => {
-            userManagementMock.is_pga_login_required.and.returnValue(true);
+            userManagementMock.isPgaLoginRequired.and.returnValue(true);
            response.errormsg = 'some error message';
            networkMock.onAny('/sqleditor/query_tool/start/123').reply(500, response);

@@ -1490,7 +1490,7 @@ describe('ExecuteQuery', () => {

          it('should save the state', () => {
            setTimeout(() => {
-              expect(sqlEditorMock.save_state).toHaveBeenCalledWith(
+              expect(sqlEditorMock.saveState).toHaveBeenCalledWith(
                'execute',
                ['']
              );
@@ -1499,7 +1499,7 @@ describe('ExecuteQuery', () => {

          it('should display pga login', () => {
            setTimeout(() => {
-              expect(userManagementMock.pga_login).toHaveBeenCalled();
+              expect(userManagementMock.pgaLogin).toHaveBeenCalled();
            }, 0);
          });
        });
@@ -1561,19 +1561,19 @@ describe('ExecuteQuery', () => {

          it('should not save the state', () => {
            setTimeout(() => {
-              expect(sqlEditorMock.save_state).not.toHaveBeenCalled();
+              expect(sqlEditorMock.saveState).not.toHaveBeenCalled();
            }, 0);
          });

          it('should not display pga login', () => {
            setTimeout(() => {
-              expect(userManagementMock.pga_login).not.toHaveBeenCalled();
+              expect(userManagementMock.pgaLogin).not.toHaveBeenCalled();
            }, 0);
          });

          it('should not initialize a new transaction', () => {
            setTimeout(() => {
-              expect(sqlEditorMock.init_transaction).not.toHaveBeenCalled();
+              expect(sqlEditorMock.initTransaction).not.toHaveBeenCalled();
            }, 0);
          });
        });
@@ -1635,7 +1635,7 @@ describe('ExecuteQuery', () => {

          it('should save the state', () => {
            setTimeout(() => {
-              expect(sqlEditorMock.save_state).toHaveBeenCalledWith(
+              expect(sqlEditorMock.saveState).toHaveBeenCalledWith(
                'execute',
                ['']
              );
@@ -1644,13 +1644,13 @@ describe('ExecuteQuery', () => {

          it('should not display pga login', () => {
            setTimeout(() => {
-              expect(userManagementMock.pga_login).not.toHaveBeenCalled();
+              expect(userManagementMock.pgaLogin).not.toHaveBeenCalled();
            }, 0);
          });

          it('should initialize a new transaction', () => {
            setTimeout(() => {
-              expect(sqlEditorMock.init_transaction).toHaveBeenCalled();
+              expect(sqlEditorMock.initTransaction).toHaveBeenCalled();
            }, 0);
          });
        });
@@ -1665,7 +1665,7 @@ describe('ExecuteQuery', () => {

          it('saves state', () => {
            setTimeout(() => {
-              expect(sqlEditorMock.save_state).toHaveBeenCalledWith(
+              expect(sqlEditorMock.saveState).toHaveBeenCalledWith(
                'execute',
                ['']
              );
--- a/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js
+++ b/web/regression/javascript/sqleditor/is_new_transaction_required_spec.js
@@ -1,65 +0,0 @@
-//////////////////////////////////////////////////////////////////////////
-//
-// pgAdmin 4 - PostgreSQL Tools
-//
-// Copyright (C) 2013 - 2018, The pgAdmin Development Team
-// This software is released under the PostgreSQL Licence
-//
-//////////////////////////////////////////////////////////////////////////
-
-import {is_new_transaction_required} from '../../../pgadmin/static/js/sqleditor/is_new_transaction_required';
-
-describe('#is_new_transaction_required', () => {
-  describe('when status is not 404', () => {
-    it('should return false', () => {
-      expect(is_new_transaction_required({
-        status: 300,
-      })).toBe(false);
-    });
-  });
-
-  describe('when status is 404', () => {
-    describe('when data is not present', () => {
-      it('should return false', () => {
-        expect(is_new_transaction_required({
-          status: 404,
-        })).toBeFalsy();
-      });
-    });
-
-    describe('when data is present', () => {
-      describe('when info is not present inside data', () => {
-        it('should return false', () => {
-          expect(is_new_transaction_required({
-            status: 404,
-            data: {},
-          })).toBeFalsy();
-        });
-      });
-
-      describe('when info is present inside data', () => {
-        describe('when info value is not "DATAGRID_TRANSACTION_REQUIRED"', () => {
-          it('should return false', () => {
-            expect(is_new_transaction_required({
-              status: 404,
-              data: {
-                info: 'some information',
-              },
-            })).toBe(false);
-          });
-        });
-
-        describe('when info value is "DATAGRID_TRANSACTION_REQUIRED"', () => {
-          it('should return false', () => {
-            expect(is_new_transaction_required({
-              status: 404,
-              data: {
-                info: 'DATAGRID_TRANSACTION_REQUIRED',
-              },
-            })).toBe(true);
-          });
-        });
-      });
-    });
-  });
-});
--- a/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js
+++ b/web/regression/javascript/sqleditor/query_tool_http_error_handler_spec.js
@@ -0,0 +1,191 @@
+//////////////////////////////////////////////////////////////////////////
+//
+// pgAdmin 4 - PostgreSQL Tools
+//
+// Copyright (C) 2013 - 2018, The pgAdmin Development Team
+// This software is released under the PostgreSQL Licence
+//
+//////////////////////////////////////////////////////////////////////////
+
+import {
+  httpResponseRequiresNewTransaction,
+  handleQueryToolAjaxError
+} from '../../../pgadmin/static/js/sqleditor/query_tool_http_error_handler';
+
+describe('#httpResponseRequiresNewTransaction', () => {
+  describe('when status is not 404', () => {
+    it('should return false', () => {
+      expect(httpResponseRequiresNewTransaction({
+        status: 300,
+      })).toBe(false);
+    });
+  });
+
+  describe('when status is 404', () => {
+    describe('when data is not present', () => {
+      it('should return false', () => {
+        expect(httpResponseRequiresNewTransaction({
+          status: 404,
+        })).toBeFalsy();
+      });
+    });
+
+    describe('when data is present', () => {
+      describe('when info is not present inside data', () => {
+        it('should return false', () => {
+          expect(httpResponseRequiresNewTransaction({
+            status: 404,
+            data: {},
+          })).toBeFalsy();
+        });
+      });
+
+      describe('when info is present inside data', () => {
+        describe('when info value is not "DATAGRID_TRANSACTION_REQUIRED"', () => {
+          it('should return false', () => {
+            expect(httpResponseRequiresNewTransaction({
+              status: 404,
+              data: {
+                info: 'some information',
+              },
+            })).toBe(false);
+          });
+        });
+
+        describe('when info value is "DATAGRID_TRANSACTION_REQUIRED"', () => {
+          it('should return false', () => {
+            expect(httpResponseRequiresNewTransaction({
+              status: 404,
+              data: {
+                info: 'DATAGRID_TRANSACTION_REQUIRED',
+              },
+            })).toBe(true);
+          });
+        });
+      });
+    });
+  });
+});
+
+
+describe('#handleQueryToolAjaxError', () => {
+  let sqlEditorHandler,
+    exceptionSpy, stateToSave,
+    stateParameters, checkTransaction, UserManagementMock,
+    pgBrowserMock;
+
+    beforeEach(() => {
+      stateToSave = 'testState';
+      stateParameters = [];
+      checkTransaction = false;
+      sqlEditorHandler = jasmine.createSpyObj(
+        'handler', ['initTransaction', 'saveState', 'handle_connection_lost']
+      );
+      exceptionSpy = {
+        readyState: 0,
+        status: 404,
+        data: {
+          info: 'CONNECTION_LOST',
+        },
+      };
+      pgBrowserMock = {
+        'Browser': {
+          'UserManagement': jasmine.createSpyObj('UserManagement', ['isPgaLoginRequired', 'pgaLogin'])
+        }
+      };
+    });
+
+  describe('when ready state is 0', () => {
+    it('should return connection', () => {
+      expect(
+        handleQueryToolAjaxError(
+          pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave,
+          stateParameters, checkTransaction
+        )
+      ).toBe('Not connected to the server or the connection to the server has been closed.');
+    });
+  });
+
+  describe('when there is an ajax error due to login is required', () => {
+    beforeEach(() => {
+      exceptionSpy.readyState = 1;
+      exceptionSpy.status = 401;
+      exceptionSpy.data.info = 'PGADMIN_LOGIN_REQUIRED';
+      pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(true);
+    });
+
+    it('should save the current state and call login handler', () => {
+      handleQueryToolAjaxError(
+        pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave,
+        stateParameters, checkTransaction
+      );
+      expect(sqlEditorHandler.saveState).toHaveBeenCalledWith(stateToSave, stateParameters);
+      expect(pgBrowserMock.Browser.UserManagement.pgaLogin).toHaveBeenCalled();
+    });
+  });
+
+  describe('when there is an ajax error and new transaction initialization required', () => {
+    beforeEach(() => {
+      exceptionSpy.readyState = 1;
+      exceptionSpy.status = 404;
+      exceptionSpy.data.info = 'DATAGRID_TRANSACTION_REQUIRED';
+      pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(false);
+      checkTransaction = true;
+    });
+
+    it('should save the current state and call login handler', () => {
+      handleQueryToolAjaxError(
+        pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave,
+        stateParameters, checkTransaction
+      );
+      expect(pgBrowserMock.Browser.UserManagement.pgaLogin).not.toHaveBeenCalled();
+      expect(sqlEditorHandler.saveState).toHaveBeenCalledWith(stateToSave, stateParameters);
+      expect(sqlEditorHandler.initTransaction).toHaveBeenCalled();
+    });
+  });
+
+  describe('when there is an ajax error due to database connection has been lost', () => {
+    beforeEach(() => {
+      exceptionSpy.readyState = 1;
+      exceptionSpy.status = 503;
+      exceptionSpy.responseJSON = {
+        'info': 'CONNECTION_LOST'
+      };
+      pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(false);
+      checkTransaction = false;
+    });
+
+    it('should save the current state and call connection lost handler', (done) => {
+      handleQueryToolAjaxError(
+        pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave,
+        stateParameters, checkTransaction
+      );
+      expect(pgBrowserMock.Browser.UserManagement.pgaLogin).not.toHaveBeenCalled();
+      setTimeout(() => {
+        expect(sqlEditorHandler.saveState).toHaveBeenCalledWith(stateToSave, stateParameters);
+        expect(sqlEditorHandler.handle_connection_lost).toHaveBeenCalledWith(false, exceptionSpy);
+        done();
+      }, 0);
+    });
+  });
+
+  describe('when there is an ajax error due to unknown reason', () => {
+    beforeEach(() => {
+      exceptionSpy.readyState = 1;
+      exceptionSpy.status = 803;
+      exceptionSpy.responseText = 'ajax failed with unknown reason';
+      pgBrowserMock.Browser.UserManagement.isPgaLoginRequired.and.returnValue(false);
+      checkTransaction = false;
+    });
+
+    it('should return proper error message from ajax exception', () => {
+      expect(
+        handleQueryToolAjaxError(
+          pgBrowserMock, sqlEditorHandler, exceptionSpy, stateToSave,
+          stateParameters, checkTransaction
+        )
+      ).toBe('ajax failed with unknown reason');
+    });
+  });
+
+});