GUACAMOLE-926: Finalize automatic cleanup on failure.

This commit is contained in:
James Muehlner
2023-03-14 02:20:51 +00:00
parent 9c31dba3cd
commit f8fb95fbc8
3 changed files with 104 additions and 118 deletions

View File

@@ -43,6 +43,7 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
// Required types
const DirectoryPatch = $injector.get('DirectoryPatch');
const Error = $injector.get('Error');
const ParseError = $injector.get('ParseError');
const PermissionSet = $injector.get('PermissionSet');
const User = $injector.get('User');
@@ -118,27 +119,27 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
$scope.patchFailure = null;
$scope.fileName = null;
// Broadcast an event to clear the file upload UI
$scope.$broadcast('clearFile');
}
// Indicate that data is currently being loaded / processed if the the file
// has been provided but not yet fully uploaded, or if the the file is
// fully loaded and is currently being processed.
$scope.isLoading = () => (
($scope.fileName && !$scope.dataReady) || $scope.processing);
($scope.fileName && !$scope.dataReady && !$scope.patchFailure)
|| $scope.processing);
/**
* Create all users and user groups mentioned in the import file that don't
* already exist in the current data source.
* already exist in the current data source. If either creation fails, any
* already-created entities will be cleaned up, and the returned promise
* will be rejected.
*
* @param {ParseResult} parseResult
* The result of parsing the user-supplied import file.
*
* @return {Object}
* An object containing the results of the calls to create the users
* and groups.
* @return {Promise.<Object>}
* A promise resolving to an object containing the results of the calls
* to create the users and groups.
*/
function createUsersAndGroups(parseResult) {
@@ -173,9 +174,30 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
value: new UserGroup({ identifier })
}));
return $q.all({
userResponse: userService.patchUsers(dataSource, userPatches),
groupResponse: userGroupService.patchUserGroups(dataSource, groupPatches)
// First, create any required users and groups, automatically cleaning
// up any created already-created entities if a call fails.
// NOTE: Generally we'd want to do these calls in parallel, using
// `$q.all()`. However, `$q.all()` rejects immediately if any of the
// wrapped promises reject, so the users may not be ready for cleanup
// at the time that the group promise rejects, or vice versa. While
// it would be possible to juggle promises and still do these calls
// in parallel, the code gets pretty complex, so for readability and
// simplicity, they are executed serially. The performance cost of
// doing so should be low.
return userService.patchUsers(dataSource, userPatches).then(userResponse => {
// Then, if that succeeds, create any required groups
return userGroupService.patchUserGroups(dataSource, groupPatches).then(
// If user group creation succeeds, resolve the returned promise
userGroupResponse => ({ userResponse, userGroupResponse}))
// If the group creation request fails, clean up any created users
.catch(groupFailure => {
cleanUpUsers(userResponse);
return groupFailure;
});
});
});
@@ -298,63 +320,6 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
}
/**
* Given a successful response to a user group PATCH request, make another
* request to delete every created user group in the provided request.
*
* @param {DirectoryPatchResponse} creationResponse
* The response to the user group PATCH creation request.
*
* @returns {DirectoryPatchResponse}
* The response to the PATCH deletion request.
*/
function cleanUpUserGroups(creationResponse) {
return userGroupService.patchUserGroups(
$routeParams.dataSource, createDeletionPatches(creationResponse));
}
/**
* Make requests to delete all connections, users, and/or groups from any
* provided PATCH API responses. If any responses are not provided, no
* cleanup will be attempted.
*
* @param {DirectoryPatchResponse} connectionResponse
* The response to the connection PATCH creation request.
*
* @param {DirectoryPatchResponse} userResponse
* The response to the user PATCH creation request.
*
* @param {DirectoryPatchResponse} userGroupResponse
* The response to the user group PATCH creation request.
*
* @returns {Promise.<Object>}
* A promise resolving to an object containing PATCH deletion responses
* corresponding to any provided connection, user, and/or user group
* creation responses.
*/
function cleanUpAll(connectionResponse, userResponse, userGroupResponse) {
// All cleanup requests that need to be made
const requests = {};
// If the connection response was provided, clean up connections
if (connectionResponse)
requests.connectionCleanup = cleanUpConnections(connectionResponse);
// If the user response was provided, clean up users
if (userResponse)
requests.userCleanup = cleanUpUsers(userResponse);
// If the user group response was provided, clean up user groups
if (connectionResponse)
requests.userGroupCleanup = cleanUpUserGroups(userGroupResponse);
// Return when all cleanup is complete
return $q.all(requests);
}
/**
* Process a successfully parsed import file, creating any specified
* connections, creating and granting permissions to any specified users
@@ -379,19 +344,14 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
// First, attempt to create the connections
connectionService.patchConnections(dataSource, parseResult.patches)
.then(connectionResponse => {
.then(connectionResponse =>
// If connection creation is successful, create users and groups
createUsersAndGroups(parseResult).then(
({userResponse, groupResponse}) =>
createUsersAndGroups(parseResult).then(() =>
grantConnectionPermissions(parseResult, connectionResponse)
.then(() => {
// TODON'T: Delete the stuff so we can test over and over
cleanUpAll(connectionResponse, userResponse, groupResponse)
.then(() => {
$scope.processing = false;
// Display a success message if everything worked
@@ -416,21 +376,31 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
$location.url('/settings/' + dataSource + '/connections');
}
}]
})
});
}));
})
}))
// If an error occurs while trying to users or groups, or while trying
// to assign permissions to users or groups, clean up the already-created
// connections, displaying an error to the user along with a blank slate
// so they can fix their problems and try again.
.catch(error => {
cleanUpConnections(connectionResponse);
handleError(error);
}))
// If an error occured when the call to create the connections was made,
// skip any further processing - the user will have a chance to fix the
// problems and try again
.catch(patchFailure => { $scope.patchFailure = patchFailure; });
.catch(patchFailure => {
$scope.processing = false;
$scope.patchFailure = patchFailure;
});
}
/**
* Display the provided error to the user in a dismissable dialog.
*
* @argument {ParseError} error
* @argument {ParseError|Error} error
* The error to display.
*/
const handleError = error => {
@@ -439,19 +409,33 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
// all upload state to allow for a fresh retry
resetUploadState();
let text;
// If it's a import file parsing error
if (error instanceof ParseError)
text = {
// Use the translation key if available
key: error.key || error.message,
variables: error.variables
};
// If it's a generic REST error
else if (error instanceof Error)
text = error.translatableMessage;
// If it's an unknown type, just use the message directly
else
text = { key: error };
guacNotification.showStatus({
className : 'error',
title : 'IMPORT.DIALOG_HEADER_ERROR',
// Use the translation key if available
text : {
key: error.key || error.message,
variables: error.variables
},
text,
// Add a button to hide the error
actions : [{
name : 'IMPORT.BUTTON_CLEAR',
name : 'IMPORT.ACTION_ACKNOWLEDGE',
callback : () => guacNotification.showStatus(false)
}]
})
@@ -546,6 +530,9 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$
};
/**
* Returns true if cancellation should be disabled, or false if
* cancellation should be allowed.
*
* @return {Boolean}
* True if cancellation should be disabled, or false if cancellation
* should be allowed.

View File

@@ -189,7 +189,6 @@
"ACTION_ACKNOWLEDGE" : "@:APP.ACTION_ACKNOWLEDGE",
"BUTTON_CANCEL": "Cancel",
"BUTTON_CLEAR" : "Clear",
"BUTTON_IMPORT": "Import Connections",
"CONNECTIONS_IMPORTED_SUCCESS": "{NUMBER} connections imported successfully.",

View File

@@ -95,14 +95,14 @@ module.exports = {
optimization: {
minimizer: [
// // Minify using Google Closure Compiler
// new ClosureWebpackPlugin({ mode: 'STANDARD' }, {
// languageIn: 'ECMASCRIPT_2020',
// languageOut: 'ECMASCRIPT5',
// compilationLevel: 'SIMPLE'
// }),
//
// new CssMinimizerPlugin()
// Minify using Google Closure Compiler
new ClosureWebpackPlugin({ mode: 'STANDARD' }, {
languageIn: 'ECMASCRIPT_2020',
languageOut: 'ECMASCRIPT5',
compilationLevel: 'SIMPLE'
}),
new CssMinimizerPlugin()
],
splitChunks: {