From 4164ffef04d44ee2e3e99134b643b32a3c5516a3 Mon Sep 17 00:00:00 2001 From: danielghost Date: Tue, 21 Jul 2020 16:17:16 +0100 Subject: [PATCH] Fixed issue where it was possible for State API calls fired in quick succession to the same object, to persist the older data, even if the calls where fired in the correct order. State calls are now queued, so it is no longer possible for the LRS to save the states in the wrong order due to the time it takes to execute the asynchronous requests. This also required a move from the XHR requests used in the ADLWrapper as these do not work with the callback functionality used with Async.queue. Moved to using the Fetch API with a modified polyfill for browsers which don't support the API or the keepalive attribute, to resolve https://github.com/adlnet/xAPIWrapper/issues/166. --- js/adapt-xapi.js | 7 +- js/stateModel.js | 305 +++++++++++++++--------- js/statementModel.js | 51 ++-- js/statements/abstractStatementModel.js | 6 +- 4 files changed, 222 insertions(+), 147 deletions(-) diff --git a/js/adapt-xapi.js b/js/adapt-xapi.js index 60fccb3..cc4a87a 100644 --- a/js/adapt-xapi.js +++ b/js/adapt-xapi.js @@ -5,8 +5,9 @@ define([ './launchModel', './statementModel', './stateModel', - 'libraries/url', - 'libraries/xapiwrapper.min' + 'libraries/xapiwrapper.min', + 'libraries/url-polyfill', + 'libraries/fetch-polyfill' ], function(Adapt, OfflineStorage, ErrorNotificationModel, LaunchModel, StatementModel, StateModel) { var xAPI = Backbone.Controller.extend({ @@ -57,7 +58,7 @@ define([ initializeStatement: function() { var config = { activityId: this.getActivityId(), - //registration: this.launchModel.get('registration'), + registration: this.launchModel.get('registration'), actor: this.launchModel.get('actor'), contextActivities: this.launchModel.get('contextActivities') }; diff --git a/js/stateModel.js b/js/stateModel.js index bedcf50..5cf0881 100644 --- a/js/stateModel.js +++ b/js/stateModel.js @@ -6,7 +6,6 @@ define([ var COMPONENTS_KEY = 'components'; var DURATIONS_KEY = 'durations'; - var LOCATION_KEY = 'location'; var StateModel = Backbone.Model.extend({ @@ -26,6 +25,7 @@ define([ _isInitialized: false, _isLoaded: false, _isRestored: false, + _queues: {}, initialize: function(attributes, options) { this.listenTo(Adapt, { @@ -77,76 +77,64 @@ define([ }, load: function() { - var activityId = this.get('activityId'); - var actor = this.get('actor'); - var registration = this.get('registration'); - var states = this.xAPIWrapper.getState(activityId, actor, null, registration); - - if (states === null) { - this.showErrorNotification(); - } else { - var scope = this; - - Async.each(states, function(id, callback) { - scope.xAPIWrapper.getState(activityId, actor, id, registration, null, function(request) { - Adapt.log.debug(request.response); - - switch (request.status) { - case 200: - var state; - var response = request.response; - - // account for invalid JSON string? - try { - state = JSON.parse(response); - } catch(e) { - state = response; - } - - scope.set(id, state); - break; - case 404: - Adapt.log.error("Could not find " + id + " in State API."); - break; - } + var scope = this; - callback(); + this._getStates(function(err, data) { + if (err) { + scope.showErrorNotification(); + } else { + var states = data; + + Async.each(states, function(id, callback) { + scope._fetchState(id, function(err, data) { + if (err) { + scope.showErrorNotification(); + } else { + // all data is now saved and retrieved as JSON, so no need for try/catch anymore + scope.set(id, data); + } + + callback(); + }); + }, function(err) { + if (err) { + scope.showErrorNotification(); + } else { + scope._isLoaded = true; + + Adapt.trigger('xapi:stateLoaded'); + + scope.listenToOnce(Adapt, 'app:dataReady', scope.onDataReady); + } }); - }, function(err) { - if (err) { - scope.showErrorNotification(); - } else { - scope._isLoaded = true; - - Adapt.trigger('xapi:stateLoaded'); - - scope.listenTo(Adapt, 'app:dataReady', scope.onDataReady); - } - }); - } + } + }); }, reset: function() { - var states = this._getStates(); var scope = this; - - this._isRestored = false; - - Adapt.wait.begin(); - - Async.each(states, function(id, callback) { - scope.delete(id, callback); - }, function(err) { + + this._getStates(function(err, data) { if (err) { scope.showErrorNotification(); - } + } else { + Adapt.wait.begin(); - var data = {}; - data[COMPONENTS_KEY] = []; - data[DURATIONS_KEY] = []; - scope.set(data, { silent: true }); + var states = data; - Adapt.wait.end(); + Async.each(states, function(id, callback) { + scope.delete(id, callback); + }, function(err) { + if (err) scope.showErrorNotification(); + + var data = {}; + data[COMPONENTS_KEY] = []; + data[DURATIONS_KEY] = []; + scope.set(data, { silent: true }); + + Adapt.wait.end(); + }); + } }); }, @@ -162,63 +150,142 @@ define([ set: function(id, value) { Backbone.Model.prototype.set.apply(this, arguments); - // delete location if empty - xAPIWrapper returns early from empty values, meaning once a location has been set, it doesn't reset on returning to the menu - if (id === LOCATION_KEY && this.has(LOCATION_KEY) && value === "") { - this.unset(id, { silent: true }); - this.delete(id); - - return; - } - // @todo: save every time the value changes, or only on specific events? - if (this._isLoaded) this.save(id); + if (this._isLoaded) { + if (Adapt.terminate) { + this.save(id); + } else { + var queue = this._getQueueById(id); + queue.push(id); + } + } }, - save: function(id) { - this.xAPIWrapper.sendState(this.get('activityId'), this.get('actor'), id, this.get('registration'), this.get(id), null, null, function(request) { - Adapt.log.debug(request.response); - - switch (request.status) { - case 204: - // no content - break; - case 401: - // @todo: add a session expired notification? - case 404: - this.showErrorNotification(); - break; - } + save: function(id, callback) { + var scope = this; + var state = this.get(id); + var data = JSON.stringify(state); + + fetch(this._getStateURL(id), { + keepalive: Adapt.terminate || false, + method: "PUT", + headers: { + "Content-Type": "application/json", + "Authorization": this.xAPIWrapper.lrs.auth, + "X-Experience-API-Version": this.xAPIWrapper.xapiVersion + }, + body: data + }).then(function(response) { + //if (response) Adapt.log.debug(response); + + if (!response.ok) throw Error(response.statusText); + + if (callback) callback(); + + return response; + }).catch(function(error) { + scope.showErrorNotification(); + + if (callback) callback(); }); }, delete: function(id, callback) { this.unset(id, { silent: true }); - this.xAPIWrapper.deleteState(this.get('activityId'), this.get('actor'), id, this.get('registration'), null, null, function(request) { - Adapt.log.debug(request.response); - - switch (request.status) { - case 204: - // no content - break; - case 401: - // @todo: add a session expired notification? - case 404: - this.showErrorNotification(); - break; + var scope = this; + + fetch(this._getStateURL(id), { + method: "DELETE", + headers: { + "Authorization": this.xAPIWrapper.lrs.auth, + "X-Experience-API-Version": this.xAPIWrapper.xapiVersion } + }).then(function(response) { + if (!response.ok) throw Error(response.statusText); + + if (callback) callback(); + + return response; + }).catch(function(error) { + scope.showErrorNotification(); if (callback) callback(); }); }, - _getStates: function() { + _getStateURL: function(stateId) { var activityId = this.get('activityId'); - var actor = this.get('actor'); + var agent = this.get('actor'); var registration = this.get('registration'); - var states = this.xAPIWrapper.getState(activityId, actor, null, registration); + var url = this.xAPIWrapper.lrs.endpoint + "activities/state?activityId=" + encodeURIComponent(activityId) + "&agent="+ encodeURIComponent(JSON.stringify(agent)); + + if (registration) url += "®istration=" + encodeURIComponent(registration); + if (stateId) url += "&stateId=" + encodeURIComponent(stateId); + + return url; + }, + + _fetchState: function(stateId, callback) { + var scope = this; + + fetch(this._getStateURL(stateId), { + //cache: "no-cache", + method: "GET", + headers: { + "Content-Type": "application/json", + "Authorization": this.xAPIWrapper.lrs.auth, + "X-Experience-API-Version": this.xAPIWrapper.xapiVersion, + "Cache-Control": "no-cache", + "Pragma": "no-cache" + } + }).then(function(response) { + if (!response.ok) throw Error(response.statusText); + + return response.json(); + }).then(function(data) { + //if (data) Adapt.log.debug(data); + + if (callback) callback(null, data); + }).catch(function(error) { + scope.showErrorNotification(); + + if (callback) callback(); + }); + }, + + _getStates: function(callback) { + var scope = this; + + Adapt.wait.begin(); + + this._fetchState(null, function(err, data) { + if (err) { + scope.showErrorNotification(); - return states; + if (callback) callback(err, null); + } else { + if (callback) callback(null, data); + } + + Adapt.wait.end(); + }); + }, + + _getQueueById: function(id) { + var queue = this._queues[id]; + + if (!queue) { + queue = this._queues[id] = Async.queue(_.bind(function(id, callback) { + this.save(id, callback); + }, this), 1); + + queue.drain = function() { + Adapt.log.debug("State API queue cleared for " + id); + }; + } + + return queue; }, _restoreComponentsData: function() { @@ -329,29 +396,35 @@ define([ this._isRestored = false; - var states = this._getStates(); + var scope = this; - var statesToReset = states.filter(function(id) { - return id !== 'lang'; - }); + this._getStates(function(err, data) { + if (err) { + scope.showErrorNotification(); + } else { + Adapt.wait.begin(); - var scope = this; + var states = data; - Adapt.wait.begin(); + var statesToReset = states.filter(function(id) { + return id !== 'lang'; + }); - Async.each(statesToReset, function(id, callback) { - scope.delete(id, callback); - }, function(err) { - if (err) scope.showErrorNotification(); + Async.each(statesToReset, function(id, callback) { + scope.delete(id, callback); + }, function(err) { + if (err) scope.showErrorNotification(); - var data = {}; - data[COMPONENTS_KEY] = []; - data[DURATIONS_KEY] = []; - scope.set(data, { silent: true }); + var data = {}; + data[COMPONENTS_KEY] = []; + data[DURATIONS_KEY] = []; + scope.set(data, { silent: true }); - Adapt.wait.end(); + Adapt.wait.end(); + }); + } }); - }, + } }); diff --git a/js/statementModel.js b/js/statementModel.js index 788acc9..7f7781d 100644 --- a/js/statementModel.js +++ b/js/statementModel.js @@ -208,30 +208,33 @@ define([ this.send(statement); }, + /* + * @todo: Add Fetch API into xAPIWrapper - https://github.com/adlnet/xAPIWrapper/issues/166 + */ send: function(statement) { - // don't run asynchronously when terminating as statements may not be executed before browser closes - no longer supported by Chrome - look into sendBeacon (in xAPIWrapper) - if (this._terminate) { - this.xAPIWrapper.sendStatement(statement); - } else { - this.xAPIWrapper.sendStatement(statement, _.bind(function(request, obj) { - Adapt.log.debug("[" + obj.id + "]: " + request.status + " - " + request.statusText); - - switch (request.status) { - case 200: - // OK - break; - case 400: - // bad request - invalid statement - break; - case 401: - // add a session expired notification? - case 404: - // LRS not found - this.showErrorNotification(); - break; - } - }, this)); - } + var lrs = this.xAPIWrapper.lrs; + var url = lrs.endpoint + "statements"; + var data = JSON.stringify(statement); + var scope = this; + + fetch(url, { + keepalive: this._terminate, + method: "POST", + headers: { + "Content-Type": "application/json", + "Authorization": lrs.auth, + "X-Experience-API-Version": this.xAPIWrapper.xapiVersion + }, + body: data + }).then(function(response) { + Adapt.log.debug("[" + statement.id + "]: " + response.status + " - " + response.statusText); + + if (!response.ok) throw Error(response.statusText); + + return response; + }).catch(function(error) { + scope.showErrorNotification(); + }); }, setModelSessionStartTime: function(model) { @@ -393,7 +396,7 @@ define([ $(window).off('beforeunload unload', this._onWindowUnload); if (!this._terminate) { - this._terminate = true; + Adapt.terminate = this._terminate = true; var model = Adapt.findById(Adapt.location._currentId); diff --git a/js/statements/abstractStatementModel.js b/js/statements/abstractStatementModel.js index 54483ad..52f0a85 100644 --- a/js/statements/abstractStatementModel.js +++ b/js/statements/abstractStatementModel.js @@ -9,7 +9,7 @@ define([ recipeLang: "en", lang: "en", activityId: null, - //registration: null, + registration: null, actor: null, contextActivities: { grouping: [] @@ -18,6 +18,7 @@ define([ getData: function(model) { var statement = new ADL.XAPIStatement(); + statement.id = ADL.ruuid(); statement.actor = new ADL.XAPIStatement.Agent(this.get('actor')); statement.verb = this.getVerb(model); statement.object = this.getObject(model); @@ -68,11 +69,8 @@ define([ language: this.get('lang') }; - /** - * ADL xAPIWrapper adds registration var registration = this.get('registration'); if (registration) context.registration = registration; - */ return context; },