From 1ad63871313ff9b27a7a493f4b5bacded433dd2a Mon Sep 17 00:00:00 2001 From: Cindy Qi Li Date: Tue, 13 Nov 2018 14:33:19 -0500 Subject: [PATCH 1/3] GPII-3529: Report NoConnection user error when no cloud connection. --- .../flowManager/src/UntrustedFlowManager.js | 6 +- .../gpii-userErrors-messageBundle_en.json5 | 1 - .../lifecycleManager/src/UserLogonRequest.js | 10 +- tests/ProductionConfigTests.js | 2 +- tests/UntrustedUserLogonRequestTests.js | 87 ++++++++++++++++ tests/shared/UserLogonRequestTestDefs.js | 98 +++++++++++-------- 6 files changed, 161 insertions(+), 43 deletions(-) diff --git a/gpii/node_modules/flowManager/src/UntrustedFlowManager.js b/gpii/node_modules/flowManager/src/UntrustedFlowManager.js index a078c72a1..c54ef3004 100644 --- a/gpii/node_modules/flowManager/src/UntrustedFlowManager.js +++ b/gpii/node_modules/flowManager/src/UntrustedFlowManager.js @@ -61,9 +61,13 @@ fluid.defaults("gpii.flowManager.untrusted", { }); gpii.flowManager.untrusted.reportSaveError = function (userErrorEvent, error) { + fluid.log("The save of preferences to the cloud fails with the error: ", error); + + var connectionErrCode = ["ECONNREFUSED", "ETIMEDOUT", "ECONNRESET"]; + userErrorEvent.fire({ isError: true, - messageKey: "SaveToCloudFail", + messageKey: connectionErrCode.indexOf(error.code) === -1 ? "SaveToCloudFail" : "NoConnection", originalError: error }); }; diff --git a/gpii/node_modules/gpii-user-errors/bundles/gpii-userErrors-messageBundle_en.json5 b/gpii/node_modules/gpii-user-errors/bundles/gpii-userErrors-messageBundle_en.json5 index a56501cda..96bb124f8 100644 --- a/gpii/node_modules/gpii-user-errors/bundles/gpii-userErrors-messageBundle_en.json5 +++ b/gpii/node_modules/gpii-user-errors/bundles/gpii-userErrors-messageBundle_en.json5 @@ -5,7 +5,6 @@ "gpii_userErrors_KeyInFail-title": "Oops.", "gpii_userErrors_KeyInFail-subhead": "Something went wrong.", "gpii_userErrors_KeyInFail-details": "Try restarting your computer. If this problem continues, contact Morphic Technical Support.", - // Note that the "NoConnection" error is currently not fired "gpii_userErrors_NoConnection-title": "No Internet connection", "gpii_userErrors_NoConnection-subhead": "Morphic cannot see the cloud to get your settings.", "gpii_userErrors_NoConnection-details": "Check your internet connection and try again.", diff --git a/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js b/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js index de65cab18..93a579d98 100644 --- a/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js +++ b/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js @@ -97,7 +97,15 @@ gpii.lifecycleManager.userLogonRequest.handleSuccessRequest = function (that, pa gpii.lifecycleManager.userLogonRequest.handleFailedRequest = function (that, userErrorEvent, err) { fluid.log("UserLogonRequest fails with the error: ", err); - if (!err.ignoreUserErrors) { + var connectionErrCode = ["ECONNREFUSED", "ETIMEDOUT", "ECONNRESET"]; + + if (connectionErrCode.indexOf(err.code) !== -1) { + userErrorEvent.fire({ + isError: true, + messageKey: "NoConnection", + originalError: err + }); + } else if (!err.ignoreUserErrors) { userErrorEvent.fire({ isError: true, messageKey: "KeyInFail", diff --git a/tests/ProductionConfigTests.js b/tests/ProductionConfigTests.js index a6e15dc98..64a6d5d8f 100644 --- a/tests/ProductionConfigTests.js +++ b/tests/ProductionConfigTests.js @@ -58,7 +58,7 @@ gpii.tests.productionConfigTesting.testDefs = fluid.transform(gpii.tests.develop }); // Override the original "kettle.test.testDefToServerEnvironment" function provided by kettle library to boil a new -// aggregate event "onAllReady" that listens to both "onServerReady" and "{flowManager}.events.noUserLoggedIn" events +// aggregate event "onAllReady" that listens to both "onServerReady" and "{flowManager}.events.resetAtStartSuccess" events kettle.test.testDefToServerEnvironment = function (testDef) { var configurationName = testDef.configType || kettle.config.createDefaults(testDef.config); return { diff --git a/tests/UntrustedUserLogonRequestTests.js b/tests/UntrustedUserLogonRequestTests.js index d0cd4a152..88a742630 100644 --- a/tests/UntrustedUserLogonRequestTests.js +++ b/tests/UntrustedUserLogonRequestTests.js @@ -13,6 +13,7 @@ "use strict"; var fluid = require("infusion"), + kettle = fluid.registerNamespace("kettle"), gpii = fluid.registerNamespace("gpii"); fluid.require("%gpii-universal"); @@ -23,4 +24,90 @@ gpii.loadTestingSupport(); fluid.registerNamespace("gpii.tests.untrusted.userLogonRequest"); +// 1. Tests for general user logon request tests gpii.test.bootstrapServer(gpii.tests.userLogonRequest.buildTestDefs(gpii.tests.userLogonRequest.testDefs, "untrusted")); + +// 2. Specific tests for untrusted environment only. +// Test the user report on NoConnection when the cloud cannot be accessed +gpii.tests.untrusted.userLogonRequest.buildTestDefs = function (testDefs) { + var config = { + configName: "gpii.config.untrusted.development", + configPath: "%gpii-universal/gpii/configs" + }; + + return fluid.transform(testDefs, function (testDef) { + return fluid.extend(true, { + config: config, + gpiiKey: testDefs.gpiiKey || gpii.tests.userLogonRequest.gpiiKey, + distributeOptions: { + "flowManager.escalate": { + "record": { + "resetAtStartSuccess.escalate": "{testEnvironment}.events.resetAtStartSuccess" + }, + "target": "{that gpii.flowManager.local}.options.listeners" + } + } + }, gpii.tests.userLogonRequest.commonTestConfig, testDef); + }); +}; + +// Override the original "kettle.test.testDefToServerEnvironment" function provided by kettle library to boil a new +// aggregate event "onAllReady" that listens to both "onServerReady" and "{flowManager}.events.resetAtStartSuccess" events +kettle.test.testDefToServerEnvironment = function (testDef) { + var configurationName = testDef.configType || kettle.config.createDefaults(testDef.config); + return { + type: "kettle.test.serverEnvironment", + options: { + configurationName: configurationName, + components: { + tests: { + options: kettle.test.testDefToCaseHolder(configurationName, testDef) + } + }, + events: { + resetAtStartSuccess: null + } + } + }; +}; + +gpii.tests.untrusted.userLogonRequest.untrustedSpecificTests = [{ + name: "GPII-3529: report NoConnection user error when no cloud connection", + expect: 2, + sequence: [{ + event: "{kettle.test.serverEnvironment}.events.resetAtStartSuccess", + listener: "fluid.identity" + }, { + // standard login without a cloud + func: "gpii.tests.invokePromiseProducer", + args: ["{lifecycleManager}.performLogin", [gpii.tests.userLogonRequest.gpiiKey], "{that}"] + }, { + event: "{that}.events.onError", + listener: "gpii.tests.userLogonRequest.testUserError", + args: ["{arguments}.0", + { + "code": "ECONNREFUSED", + "errno": "ECONNREFUSED", + "syscall": "connect", + "address": "127.0.0.1", + "port": 8084, + "isError": true + }, + "{lifecycleManager}.userErrors.options.trackedUserErrors", + { + "isError": true, + "messageKey": "NoConnection", + "originalError": { + "code": "ECONNREFUSED", + "errno": "ECONNREFUSED", + "syscall": "connect", + "address": "127.0.0.1", + "port": 8084, + "isError": true + } + } + ] + }] +}]; + +kettle.test.bootstrapServer(gpii.tests.untrusted.userLogonRequest.buildTestDefs(gpii.tests.untrusted.userLogonRequest.untrustedSpecificTests)); diff --git a/tests/shared/UserLogonRequestTestDefs.js b/tests/shared/UserLogonRequestTestDefs.js index 77bfce7d8..013c5b603 100644 --- a/tests/shared/UserLogonRequestTestDefs.js +++ b/tests/shared/UserLogonRequestTestDefs.js @@ -61,13 +61,26 @@ gpii.tests.userLogonRequest.modelChangeChecker = function (trackedLogonChange, e jqUnit.assertEquals("Checking gpiiKey of model change", expectedGpiiKey, result.gpiiKey); }; -gpii.tests.userLogonRequest.testLogoutError = function (actualError, userError, expectedError) { - jqUnit.assertDeepEq("onError fires with the expected message", expectedError, actualError); - if (actualError.ignoreUserErrors) { - jqUnit.assertDeepEq("User error has not be fired", {}, userError); - } else { - var userErrorMessage = userError.error.originalError; - jqUnit.assertDeepEq("User error has been fired with the expected message", actualError.message, userErrorMessage); +gpii.tests.userLogonRequest.testUserError = function (actualResponse, expectedResponse, userError, expectedUserError) { + jqUnit.assertDeepEq("onError fires with the expected message", expectedResponse, actualResponse); + jqUnit.assertDeepEq("User error has been fired with the expected content", expectedUserError, userError.error); +}; + +gpii.tests.userLogonRequest.commonTestConfig = { + gradeNames: ["gpii.tests.userLogonRequest.testCaseHolder", "gpii.test.integration.testCaseHolder.linux"], + distributeOptions: { + "lifecycleManager.userErrorsListener": { + "record": { + trackedUserErrors: {}, + listeners: { + "userError.trackReportedError": { + listener: "fluid.set", + args: ["{that}.options.trackedUserErrors", "error", "{arguments}.0"] + } + } + }, + "target": "{that gpii.flowManager.local userErrors}.options" + } } }; @@ -88,7 +101,6 @@ gpii.tests.userLogonRequest.buildTestDefs = function (testDefs, testType) { var extraTestDef = testType === "untrusted" ? testDef.untrustedExtras : {}; return fluid.extend(true, { config: config, - gradeNames: ["gpii.tests.userLogonRequest.testCaseHolder", "gpii.test.integration.testCaseHolder.linux"], gpiiKey: testDefs.gpiiKey || gpii.tests.userLogonRequest.gpiiKey, distributeOptions: { "lifecycleManager.logonChangeListener": { @@ -102,21 +114,9 @@ gpii.tests.userLogonRequest.buildTestDefs = function (testDefs, testType) { } }, "target": "{that gpii.flowManager.local lifecycleManager}.options" - }, - "lifecycleManager.userErrorsListener": { - "record": { - trackedUserErrors: {}, - listeners: { - "userError.trackReportedError": { - listener: "fluid.set", - args: ["{that}.options.trackedUserErrors", "error", "{arguments}.0"] - } - } - }, - "target": "{that gpii.flowManager.local userErrors}.options" } } - }, testDef, extraTestDef); + }, gpii.tests.userLogonRequest.commonTestConfig, testDef, extraTestDef); }); }; @@ -491,12 +491,20 @@ gpii.tests.userLogonRequest.testDefs = [{ args: ["{lifecycleManager}.performLogin", [gpii.tests.userLogonRequest.gpiiKey], "{that}"] }, { event: "{that}.events.onError", - listener: "gpii.tests.userLogonRequest.testLogoutError", - args: ["{arguments}.0", "{lifecycleManager}.userErrors.options.trackedUserErrors", { - "statusCode": 409, - "message": "Got log in request from user testUser1, but the user testUser1 is already logged in. So ignoring login request.", - "ignoreUserErrors": false - }] + listener: "gpii.tests.userLogonRequest.testUserError", + args: ["{arguments}.0", + { + "statusCode": 409, + "message": "Got log in request from user testUser1, but the user testUser1 is already logged in. So ignoring login request.", + "ignoreUserErrors": false + }, + "{lifecycleManager}.userErrors.options.trackedUserErrors", + { + "isError": true, + "messageKey": "KeyInFail", + "originalError": "Got log in request from user testUser1, but the user testUser1 is already logged in. So ignoring login request." + } + ] }, { // logout of different user func: "gpii.tests.invokePromiseProducer", @@ -504,12 +512,20 @@ gpii.tests.userLogonRequest.testDefs = [{ }, { event: "{that}.events.onError", priority: "last", - listener: "gpii.tests.userLogonRequest.testLogoutError", - args: ["{arguments}.0", "{lifecycleManager}.userErrors.options.trackedUserErrors", { - "statusCode": 409, - "message": "Got logout request from user sammy, but the user testUser1 is logged in. So ignoring the request.", - "ignoreUserErrors": false - }] + listener: "gpii.tests.userLogonRequest.testUserError", + args: ["{arguments}.0", + { + "statusCode": 409, + "message": "Got logout request from user sammy, but the user testUser1 is logged in. So ignoring the request.", + "ignoreUserErrors": false + }, + "{lifecycleManager}.userErrors.options.trackedUserErrors", + { + "isError": true, + "messageKey": "KeyInFail", + "originalError": "Got logout request from user sammy, but the user testUser1 is logged in. So ignoring the request." + } + ] }, { // logout of the correct user func: "gpii.tests.invokePromiseProducer", @@ -542,12 +558,16 @@ gpii.tests.userLogonRequest.testDefs = [{ args: ["{lifecycleManager}.performLogout", [gpii.tests.userLogonRequest.gpiiKey], "{that}"] }, { event: "{that}.events.onError", - listener: "gpii.tests.userLogonRequest.testLogoutError", - args: ["{arguments}.0", "{lifecycleManager}.userErrors.options.trackedUserErrors", { - "statusCode": 409, - "message": "Got logout request from user testUser1, but the user noUser is logged in. So ignoring the request.", - "ignoreUserErrors": true - }, "{arguments}.0"] + listener: "gpii.tests.userLogonRequest.testUserError", + args: ["{arguments}.0", + { + "statusCode": 409, + "message": "Got logout request from user testUser1, but the user noUser is logged in. So ignoring the request.", + "ignoreUserErrors": true + }, + "{lifecycleManager}.userErrors.options.trackedUserErrors", + undefined + ] }] }, { name: "Testing standard error handling: invalid user URLs", From 692c9a0c070d85758ca45b3e843c54748bb9bc35 Mon Sep 17 00:00:00 2001 From: Cindy Qi Li Date: Wed, 14 Nov 2018 10:25:45 -0500 Subject: [PATCH 2/3] GPII-3529: Improvements according to code review suggestions. --- .../flowManager/src/UntrustedFlowManager.js | 4 +- .../gpii-user-errors/src/UserErrors.js | 17 +++++++- .../test/html/UserErrorsTest.html | 39 +++++++++++++++++ .../test/js/UserErrorsTests.js | 43 +++++++++++++++++++ gpii/node_modules/lifecycleManager/index.js | 1 + .../lifecycleManager/src/UserLogonRequest.js | 4 +- tests/web/html/all-tests.html | 1 + 7 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html create mode 100644 gpii/node_modules/gpii-user-errors/test/js/UserErrorsTests.js diff --git a/gpii/node_modules/flowManager/src/UntrustedFlowManager.js b/gpii/node_modules/flowManager/src/UntrustedFlowManager.js index c54ef3004..9957994c8 100644 --- a/gpii/node_modules/flowManager/src/UntrustedFlowManager.js +++ b/gpii/node_modules/flowManager/src/UntrustedFlowManager.js @@ -63,11 +63,9 @@ fluid.defaults("gpii.flowManager.untrusted", { gpii.flowManager.untrusted.reportSaveError = function (userErrorEvent, error) { fluid.log("The save of preferences to the cloud fails with the error: ", error); - var connectionErrCode = ["ECONNREFUSED", "ETIMEDOUT", "ECONNRESET"]; - userErrorEvent.fire({ isError: true, - messageKey: connectionErrCode.indexOf(error.code) === -1 ? "SaveToCloudFail" : "NoConnection", + messageKey: gpii.userErrors.isConnectionError(error.code) ? "NoConnection" : "SaveToCloudFail", originalError: error }); }; diff --git a/gpii/node_modules/gpii-user-errors/src/UserErrors.js b/gpii/node_modules/gpii-user-errors/src/UserErrors.js index 5239b10bd..4a30a3b23 100644 --- a/gpii/node_modules/gpii-user-errors/src/UserErrors.js +++ b/gpii/node_modules/gpii-user-errors/src/UserErrors.js @@ -12,7 +12,9 @@ "use strict"; -var fluid = fluid || require("infusion"); +var fluid = fluid || require("infusion"), + gpii = fluid.registerNamespace("gpii"); + fluid.registerNamespace("gpii.userErrors"); /** The channel for receiving and transmitting errors from the core GPII architecture which are of interest to end @@ -37,3 +39,16 @@ fluid.defaults("gpii.userErrors", { userError: null } }); + +// The constant that defines the node.js error code indicating connection problems. +gpii.userErrors.connectionErrorCode = ["ECONNREFUSED", "ETIMEDOUT", "ECONNRESET"]; + +/** + * Check if the given error code indicates a connection problem. Return true if the code does indicate a connection + * problem. Otherwise, return false. + * @param {String} code - An node.js error code. + * @return {Boolean} Return true if the code does indicate a connection problem. Otherwise, return false. + */ +gpii.userErrors.isConnectionError = function (code) { + return gpii.userErrors.connectionErrorCode.indexOf(code) !== -1; +}; diff --git a/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html b/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html new file mode 100644 index 000000000..f8ce8562d --- /dev/null +++ b/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html @@ -0,0 +1,39 @@ + + + + + GPII User Errors Tests + + + + + + + + + + + + + + + + + + + + + + + + + + +

GPII User Errors Tests

+

+
+

+
    + + + diff --git a/gpii/node_modules/gpii-user-errors/test/js/UserErrorsTests.js b/gpii/node_modules/gpii-user-errors/test/js/UserErrorsTests.js new file mode 100644 index 000000000..f366bd737 --- /dev/null +++ b/gpii/node_modules/gpii-user-errors/test/js/UserErrorsTests.js @@ -0,0 +1,43 @@ +/*! +GPII User Errors Tests + +Copyright 2018 OCAD University + +Licensed under the New BSD license. You may not use this file except in +compliance with this License. + +You may obtain a copy of the License at +https://github.com/GPII/universal/blob/master/LICENSE.txt +*/ + +/* eslint-env browser */ +/* eslint strict: ["error", "function"] */ + +/* global jqUnit, fluid */ + +(function () { + "use strict"; + + var gpii = fluid.registerNamespace("gpii"); + + jqUnit.test("Test gpii.userErrors.isConnectionError()", function () { + var testCases = [{ + code: "ECONNREFUSED", + expected: true + }, { + code: "ETIMEDOUT", + expected: true + }, { + code: "ECONNRESET", + expected: true + }, { + code: "NONERROR", + expected: false + }]; + + fluid.each(testCases, function (oneTestCase) { + jqUnit.assertEquals("The return on the connection error code is expected", oneTestCase.expected, gpii.userErrors.isConnectionError(oneTestCase.code)); + }); + }); + +})(); diff --git a/gpii/node_modules/lifecycleManager/index.js b/gpii/node_modules/lifecycleManager/index.js index 04b7f1d09..60ab4d53a 100644 --- a/gpii/node_modules/lifecycleManager/index.js +++ b/gpii/node_modules/lifecycleManager/index.js @@ -4,6 +4,7 @@ var fluid = require("infusion"); fluid.module.register("lifecycleManager", __dirname, require); +require("gpii-user-errors"); // MatchMaking.js is used in both flowManager and lifecycleManager modules for: // * Cloud Based Flow Manager: to perform matchmaking in the cloud without the present of lifecycleManager; // * lifecycleManager: to handle user logon requests. diff --git a/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js b/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js index 93a579d98..4520b24ff 100644 --- a/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js +++ b/gpii/node_modules/lifecycleManager/src/UserLogonRequest.js @@ -97,9 +97,7 @@ gpii.lifecycleManager.userLogonRequest.handleSuccessRequest = function (that, pa gpii.lifecycleManager.userLogonRequest.handleFailedRequest = function (that, userErrorEvent, err) { fluid.log("UserLogonRequest fails with the error: ", err); - var connectionErrCode = ["ECONNREFUSED", "ETIMEDOUT", "ECONNRESET"]; - - if (connectionErrCode.indexOf(err.code) !== -1) { + if (gpii.userErrors.isConnectionError(err.code)) { userErrorEvent.fire({ isError: true, messageKey: "NoConnection", diff --git a/tests/web/html/all-tests.html b/tests/web/html/all-tests.html index bc8437a9e..e66b908b6 100644 --- a/tests/web/html/all-tests.html +++ b/tests/web/html/all-tests.html @@ -17,6 +17,7 @@ "/gpii/node_modules/gpii-oauth2/gpii-oauth2-authz-server/test/html/AuthGrantFinderTests.html", "/gpii/node_modules/gpii-oauth2/gpii-oauth2-authz-server/test/html/AuthorizationServiceTests.html", "/gpii/node_modules/gpii-oauth2/gpii-oauth2-utilities/test/html/OAuth2UtilitiesTests.html", + "/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html", "/gpii/node_modules/journal/test/html/JournalIdParserTests.html", "/gpii/node_modules/lifecycleManager/test/html/LifecycleManagerTest.html", "/gpii/node_modules/lifecycleManager/test/html/LifecycleManagerResolverTest.html", From 99bee58b5bbcd80c10d9fc3d9a987e595cce9a24 Mon Sep 17 00:00:00 2001 From: Cindy Qi Li Date: Wed, 14 Nov 2018 12:35:05 -0500 Subject: [PATCH 3/3] GPII-3529: Fixed a typo. --- .../node_modules/gpii-user-errors/test/html/UserErrorsTest.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html b/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html index f8ce8562d..1a4fe3829 100644 --- a/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html +++ b/gpii/node_modules/gpii-user-errors/test/html/UserErrorsTest.html @@ -19,7 +19,7 @@ - +