From 27d570578d281fc43b2d7386ff6a440bc9a65e92 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 4 Sep 2019 11:59:36 -0400 Subject: [PATCH] quicksave: Remove saving and restoring of state The quicksave feature was added back in 2011 when the framework was somewhat unstable---we were taking calls from users and it wasn't a pleasent experience to tell them that they had to refresh the page to work around some issue with bad state, and lose all unsaved data. This feature is now more trouble than it's worth, since it causes a number of bugs and can even cause data corruption. If this is reintroduced in the future, I'd prefer it be done by periodically saving quote state after every or a few bucket modifications, to permit Meteor-like features. This keeps the feature available on the client as a heartbeat for quote locking; this can be removed in the future when we handle distributed locking. * src/client/Client.js (_changeQuote): Stop setting quicksave data from request. (_mergeQuickSaveData): Remove now-unused method. (_createUi): Stop merging quicksave data on render. (saveStaging): Add note that this method is now only used as a heartbeat. * src/client/quote/ClientQuote.js (saveStaging): Add comment indicating that this is now used for a heartbeat. (setQuickSaveData, getQuickSaveData): Remove methods. * src/quote/BaseQuote.js (_quickSaveData): Remove field. (setQuickSaveData, getQuickSaveData): Remove methods. * src/server/Server.js (initQuote): Do not create or initialize quicksave data. (handleQuickSave): Remove method. * src/server/daemon/controller.js (doRoute): Return empty reply without processing quicksave data. Continue touching session to retain quote lock. * src/server/db/MongoServerDao.js (saveQuote): Do not clear quicksave. (quickSaveQuote): Remove method. --- src/client/Client.js | 65 +++------------------------ src/client/quote/ClientQuote.js | 24 +++------- src/quote/BaseQuote.js | 31 ------------- src/server/Server.js | 78 --------------------------------- src/server/daemon/controller.js | 23 +++------- src/server/db/MongoServerDao.js | 44 ------------------- 6 files changed, 18 insertions(+), 247 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index 879a38b..0adccc2 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -507,8 +507,6 @@ module.exports = Class( 'Client' ) client._monitorQuote( client._quote ); - client._quote.setQuickSaveData( data.content.quicksave || {} ); - client._cmatch.hookClassifier( client._dataValidator ); // store internal status @@ -689,57 +687,6 @@ module.exports = Class( 'Client' ) }, - /** - * Merge quick save data with bucket data - * - * This has the wonderful consequence of allowing the user to refresh the - * page and retain the majority of the data. This is useful if the broker - * experiences problems that require a refresh to resolve. This also allows - * us (developers) to jump into a quote to aid the broker before the step - * is saved. - * - * @return {undefined} - */ - 'private _mergeQuickSaveData': function() - { - var merge = {}, - qs_data = this._quote.getQuickSaveData(); - - // merge quick save data with bucket - for ( var name in qs_data ) - { - var values = qs_data[ name ], - i = values.length || 0; - - merge[ name ] = []; - - // merge each of the values individually, skipping unchanged (null) - // values - while ( i-- ) - { - var val = values[ i ]; - - // ignore null values, as they are unchanged (well, this isn't - // 100% true (removing tabs/rows), but it will suffice for now) - if ( val !== null ) - { - merge[ name ][ i ] = val; - } - }; - } - - this._quote.setData( merge ); - - // empty quick save data - this._quote.setQuickSaveData( {} ); - - // force UI cmatch update, since we may have fields that have been added - // that need to be shown/hidden based on the current set of - // classifications - this._cmatch.forceCmatchAction(); - }, - - /** * Retrieves the program ID from the URL * @@ -1141,10 +1088,6 @@ module.exports = Class( 'Client' ) // care about a response). This will allow the server to save our // current step even if it's cached client-side. client.dataProxy.get( url ); - - // merge any quick save data *after* the UI is rendered, or we will - // run into validation/display issues - client._mergeQuickSaveData(); } ).on( 'preRenderStep', function( step, $content ) { client.elementStyler.setContext( $content ); @@ -1743,7 +1686,13 @@ module.exports = Class( 'Client' ) /** - * Save the staging bucket to the server (for debug/recovery purposes) + * Heartbeat + * + * Previously, this would save the staging bucket to the server (for + * debug/recovery purposes). While data are still sent, it should be + * ignored by the server. + * + * TODO: Remove once we have a proper heartbeat route. * * @return {Client} self */ diff --git a/src/client/quote/ClientQuote.js b/src/client/quote/ClientQuote.js index 0b1ee3e..997876c 100644 --- a/src/client/quote/ClientQuote.js +++ b/src/client/quote/ClientQuote.js @@ -460,6 +460,12 @@ module.exports = Class( 'ClientQuote' ) }, + /** + * Save staging bucket + * + * TODO: This is now used as a heartbeat and should be removed in the + * future. + */ 'public saveStaging': function( transport, callback ) { this._doSave( transport, callback ); @@ -715,24 +721,6 @@ module.exports = Class( 'ClientQuote' ) 'public proxy setImported': '_quote', - /** - * Set quicksave data - * - * @param {Object} data quicksave data, diff format - * - * @return {Quote} self - */ - 'public proxy setQuickSaveData': '_quote', - - - /** - * Retrieve quicksave data - * - * @return {Object} quicksave data - */ - 'public proxy getQuickSaveData': '_quote', - - /** * Sets an explicit lock, providing a reason for doing so * diff --git a/src/quote/BaseQuote.js b/src/quote/BaseQuote.js index bbe275b..9e31383 100644 --- a/src/quote/BaseQuote.js +++ b/src/quote/BaseQuote.js @@ -127,12 +127,6 @@ module.exports = Class( 'BaseQuote' ) */ 'private _error': '', - /** - * Quick save data (diff format) - * @type {Object} - */ - 'private _quickSaveData': {}, - /** * Allows quote to be locked for any reason * @type {string} @@ -710,31 +704,6 @@ module.exports = Class( 'BaseQuote' ) }, - /** - * Set quicksave data - * - * @param {Object} data quicksave data, diff format - * - * @return {Quote} self - */ - 'public setQuickSaveData': function( data ) - { - this._quickSaveData = data; - return this; - }, - - - /** - * Retrieve quicksave data - * - * @return {Object} quicksave data - */ - 'public getQuickSaveData': function() - { - return this._quickSaveData; - }, - - /** * Sets an explicit lock, providing a reason for doing so * diff --git a/src/server/Server.js b/src/server/Server.js index cd9e18e..6592faf 100644 --- a/src/server/Server.js +++ b/src/server/Server.js @@ -329,7 +329,6 @@ module.exports = Class( 'Server' ) quote .setData( default_bucket ) .setMetadata( quote_data.meta || {} ) - .setQuickSaveData( quote_data.quicksave || {} ) .setAgentId( quote_data.agentId || agent_id ) .setAgentName( quote_data.agentName || agent_name ) .setAgentEntityId( quote_data.agentEntityId || "" ) @@ -765,8 +764,6 @@ module.exports = Class( 'Server' ) initialRatedDate: quote.getInitialRatedDate(), lastPremDate: quote.getLastPremiumDate(), - quicksave: quote.getQuickSaveData(), - // set to undefined if not internal so it's not included in the // JSON response internal: ( ( request.getSession().isInternal() === true ) @@ -1644,81 +1641,6 @@ module.exports = Class( 'Server' ) }, - /** - * Handle quick save request - * - * @param {UserRequest} request user request - * @param {Quote} quote quote to save - * @param {Program} program quote program - * - * @return {Server} self - */ - 'public handleQuickSave': function( request, quote, program ) - { - var _self = this; - - // do not allow quote modification if locked unless logged in as an - // internal user (FS#5772) and the program is unlockable - if ( quote.isImported() ) - { - //return this; - } - - request.getPostData( function( post_data ) - { - // sanitize, permitting nulls (since the diff will have them) - try - { - var data = JSON.parse( post_data.data ); - - var filtered = _self._dataProcessor.sanitizeDiff( - data, request, program, true - ); - } - catch ( e ) - { - _self.logger.log( server.logger.PRIORITY_ERROR, - "Invalid quicksave data string (%s): %s", - e.message, - post_data.data - ); - - return; - } - - var secure = program.secureFields, - i = secure.length; - - // strip out secure fields (we can encrypt them later; this is just - // a quick solution to prevent sensitive data in plain text) - while ( i-- ) - { - delete filtered[ secure[ i ] ]; - } - - // attempt to save the diff - _self.dao.quickSaveQuote( quote, filtered, function( err ) - { - if ( !err ) - { - return; - } - - _self.logger.log( server.logger.PRIORITY_DB, - "[Quick Save] Quick Save Failed: " + err - ); - } ); - } ); - - // just send the response immediately, as they do not need feedback if - // the quick-save fails (it is for debugging/backup in the event of a - // problem, so we need only concern ourselves if there is an issue) - this.sendEmptyReply( request, quote ); - - return this; - }, - - 'public createRevision': function( request, quote ) { var _self = this; diff --git a/src/server/daemon/controller.js b/src/server/daemon/controller.js index 7cc230a..680fde7 100644 --- a/src/server/daemon/controller.js +++ b/src/server/daemon/controller.js @@ -567,27 +567,14 @@ function doRoute( program, request, data, resolve, reject ) } else if ( cmd === 'quicksave' ) { - // attempt to acquire the write lock, aborting immediately if we - // cannot (instead of queueing) - acquireWriteLockImmediate( quote_id, request, function( free ) + // TODO: we keep this route around only as a heartbeat, for now; the + // original purpose of this route (to save staged data) has been + // removed + handleRequest( function( quote ) { - handleRequest( function( quote ) - { - // if we could not immediately obtain the lock, then something - // is currently saving, meaning that the quicksave data is - // probably irrelevant (since it would just be wiped out anyway - // if we had issued this request moments earlier); abort - if ( !free ) - { - server.sendEmptyReply( request, quote ); - return; - } - - server.handleQuickSave( request, quote, program ); - } ); + server.sendEmptyReply( request, quote ); } ); - // keep the session alive touchSession( quote_id, session ); } else if ( /^log\//.test( cmd ) ) diff --git a/src/server/db/MongoServerDao.js b/src/server/db/MongoServerDao.js index 2347164..2cc2001 100644 --- a/src/server/db/MongoServerDao.js +++ b/src/server/db/MongoServerDao.js @@ -313,10 +313,6 @@ module.exports = Class( 'MongoServerDao' ) meta = quote.getMetabucket().getData(); } - // when we update the quote data, clear quick save data (this data - // should take precedence) - save_data.quicksave = {}; - var id = quote.getId(); // some data should always be saved because the quote will be created if @@ -457,46 +453,6 @@ module.exports = Class( 'MongoServerDao' ) }, - /** - * Perform a "quick save" - * - * A quick save simply saves the given diff to the database for recovery - * purposes - * - * @param {Quote} quote quote being saved - * @param {Object} diff staged changes - * - * @param {function(*)} callback callback to call when complete - * - * @return {MongoServerDao} self - */ - 'public quickSaveQuote': function( quote, diff, callback ) - { - // unlikely, but possible for a request to come in before we're ready - // since this system is asynchronous - if ( this._ready === false ) - { - callback( Error( 'Database server not ready' ) ); - return; - } - - var id = quote.getId(); - - this._collection.update( - { id: id }, - { $set: { quicksave: diff } }, - - // on complete - function( err, docs ) - { - callback( err ); - } - ); - - return this; - }, - - /** * Saves the quote state to the database *