Skip to content

Commit

Permalink
Fixed issue where SharePointClient would use a temp ID in change even…
Browse files Browse the repository at this point in the history 
…ts to a subscription that works with the remote ID instead.

Fixed race condition in _handleSet where the events emitted could contain the wrong model's properties
  • Loading branch information
@tjclement
tjclement committed Nov 25, 2015
1 parent 4025fa7 commit dc2687a
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 98 deletions.
86 changes: 48 additions & 38 deletions Worker/SharePointClient.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class SharePointClient extends EventEmitter {
});
}

subscribeToChanges(){
subscribeToChanges() {
if (!this.isChild) {
/* Don't monitor child item updates/removes. We only do that on parent arrays. */
this._refresh();
Expand Down Expand Up @@ -334,36 +334,38 @@ export class SharePointClient extends EventEmitter {
};

// initial initialisation of the datasource
soapClient.call(configuration, tempKeys)
.then((result)=> {
(function (newData) {
soapClient.call(configuration, tempKeys)
.then((result)=> {

let data = this._getResults(result.data);
if (data.length == 1) {
let remoteId = data[0].id;
let data = this._getResults(result.data);
if (data.length == 1) {
let remoteId = data[0].id;

// push ID mapping for given session to collection of temp keys
if (newData['_temporary-identifier']) {
tempKeys.push({localId: newData['_temporary-identifier'], remoteId: remoteId});
}
let messages = this._updateCache(data);
for (let message of messages) {
this.emit('message', message);
}
// push ID mapping for given session to collection of temp keys
if (newData['_temporary-identifier']) {
tempKeys.push({localId: newData['_temporary-identifier'], remoteId: remoteId, client: this});
}
let messages = this._updateCache(data);
for (let message of messages) {
this.emit('message', message);
}

/* Fire a value/child_changed event with the now available remoteId present */
let model = newData;
model.id = model['_temporary-identifier'] || model.id;
model.remoteId = remoteId;
if(this.isChild) {
this.emit('message', {event: 'value', result: model});
} else {
this.emit('message', {event: 'child_changed', result: model});
this.emit('message', {event: 'value', result: this.cache});
/* Fire a value/child_changed event with the now available remoteId present */
let model = newData;
model.id = model['_temporary-identifier'] || model.id;
model.remoteId = remoteId;
if (this.isChild) {
this.emit('message', {event: 'value', result: model});
} else {
this.emit('message', {event: 'child_changed', result: model});
this.emit('message', {event: 'value', result: this.cache});
}
}
}
}, (error) => {
console.log(error);
});
}, (error) => {
console.log(error);
});
}.bind(this))(newData);
}

/**
Expand Down Expand Up @@ -428,20 +430,26 @@ export class SharePointClient extends EventEmitter {
_updateCache(data) {
let messages = [];
for (let record in data) {
let shouldUseRemoteId = false;
let model = data[record];
model.remoteId = model.id;

let isLocal = _.findIndex(tempKeys, function (key) {
let localIndex = _.findIndex(tempKeys, function (key) {
return key.remoteId == model.id;
});

if (isLocal > -1) {
model.id = tempKeys[isLocal].localId;
if (localIndex > -1) {
let tempKey = tempKeys[localIndex];

/* If this SPClient instance created the temp ID, we need to use it in our events.
* Otherwise, we should use the remote ID that SharePoint generated. */
shouldUseRemoteId = tempKey.client !== this;
model.id = shouldUseRemoteId ? model.remoteId : tempKey.localId;
}

let cacheIndex = _.findIndex(this.cache, function (item) {
return model.id == item.id;
});
let cacheIndex = _.findIndex(this.cache, function (item) {
return model.id == item.id;
});

if (cacheIndex === -1) {
this.cache.push(model);
Expand All @@ -455,7 +463,8 @@ export class SharePointClient extends EventEmitter {
}
else {
if (!_.isEqual(model, this.cache[cacheIndex])) {
this.cache.splice(cacheIndex, 1, model);
this.cache[cacheIndex] = model;

let previousSibling = cacheIndex == 0 ? null : this.cache[cacheIndex - 1];
messages.push({
event: 'child_changed',
Expand All @@ -475,7 +484,6 @@ export class SharePointClient extends EventEmitter {
* @private
*/
_setLastUpdated(lastChangeToken) {

this.retriever.params.changeToken = lastChangeToken;
}

Expand All @@ -491,12 +499,14 @@ export class SharePointClient extends EventEmitter {

let recordId = changes[change]._;

let isLocal = _.findIndex(tempKeys, function (key) {
let localIndex = _.findIndex(tempKeys, function (key) {
return key.remoteId == recordId;
});

if (isLocal > -1) {
recordId = tempKeys[isLocal].localId;
if (localIndex > -1) {
let tempKey = tempKeys[localIndex];
let isOurTempKey = tempKey.client === this;
recordId = isOurTempKey ? tempKey.localId : tempKey.remoteId;
}

let cacheItem = _.findIndex(this.cache, function (item) {
Expand Down
126 changes: 67 additions & 59 deletions worker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18605,7 +18605,6 @@ $__System.register("8e", ["c", "e", "8b", "8c", "89", "8d"], function($__export)
}
},
_handleSet: function(newData) {
var $__3 = this;
var configuration = this._updateListItemsDefaultConfiguration();
configuration.url = this._parsePath(this.settings.endPoint, this._getListService()) + ("?update=" + this.settings.listName);
var fieldCollection = [];
Expand Down Expand Up @@ -18666,64 +18665,68 @@ $__System.register("8e", ["c", "e", "8b", "8c", "89", "8d"], function($__export)
"_ViewName": ""
}}
};
soapClient.call(configuration, tempKeys).then(function(result) {
var data = $__3._getResults(result.data);
if (data.length == 1) {
var remoteId = data[0].id;
if (newData['_temporary-identifier']) {
tempKeys.push({
localId: newData['_temporary-identifier'],
remoteId: remoteId
});
}
var messages = $__3._updateCache(data);
var $__7 = true;
var $__8 = false;
var $__9 = undefined;
try {
for (var $__5 = void 0,
$__4 = (messages)[Symbol.iterator](); !($__7 = ($__5 = $__4.next()).done); $__7 = true) {
var message = $__5.value;
{
$__3.emit('message', message);
}
(function(newData) {
var $__3 = this;
soapClient.call(configuration, tempKeys).then(function(result) {
var data = $__3._getResults(result.data);
if (data.length == 1) {
var remoteId = data[0].id;
if (newData['_temporary-identifier']) {
tempKeys.push({
localId: newData['_temporary-identifier'],
remoteId: remoteId,
client: $__3
});
}
} catch ($__10) {
$__8 = true;
$__9 = $__10;
} finally {
var messages = $__3._updateCache(data);
var $__7 = true;
var $__8 = false;
var $__9 = undefined;
try {
if (!$__7 && $__4.return != null) {
$__4.return();
for (var $__5 = void 0,
$__4 = (messages)[Symbol.iterator](); !($__7 = ($__5 = $__4.next()).done); $__7 = true) {
var message = $__5.value;
{
$__3.emit('message', message);
}
}
} catch ($__10) {
$__8 = true;
$__9 = $__10;
} finally {
if ($__8) {
throw $__9;
try {
if (!$__7 && $__4.return != null) {
$__4.return();
}
} finally {
if ($__8) {
throw $__9;
}
}
}
var model = newData;
model.id = model['_temporary-identifier'] || model.id;
model.remoteId = remoteId;
if ($__3.isChild) {
$__3.emit('message', {
event: 'value',
result: model
});
} else {
$__3.emit('message', {
event: 'child_changed',
result: model
});
$__3.emit('message', {
event: 'value',
result: $__3.cache
});
}
}
var model = newData;
model.id = model['_temporary-identifier'] || model.id;
model.remoteId = remoteId;
if ($__3.isChild) {
$__3.emit('message', {
event: 'value',
result: model
});
} else {
$__3.emit('message', {
event: 'child_changed',
result: model
});
$__3.emit('message', {
event: 'value',
result: $__3.cache
});
}
}
}, function(error) {
console.log(error);
});
}, function(error) {
console.log(error);
});
}.bind(this))(newData);
},
_handleRemove: function(record) {
var $__3 = this;
Expand Down Expand Up @@ -18767,13 +18770,16 @@ $__System.register("8e", ["c", "e", "8b", "8c", "89", "8d"], function($__export)
var messages = [];
var $__12 = this,
$__13 = function(record) {
var shouldUseRemoteId = false;
var model = data[record];
model.remoteId = model.id;
var isLocal = _.findIndex(tempKeys, function(key) {
var localIndex = _.findIndex(tempKeys, function(key) {
return key.remoteId == model.id;
});
if (isLocal > -1) {
model.id = tempKeys[isLocal].localId;
if (localIndex > -1) {
var tempKey = tempKeys[localIndex];
shouldUseRemoteId = tempKey.client !== $__12;
model.id = shouldUseRemoteId ? model.remoteId : tempKey.localId;
}
var cacheIndex = _.findIndex($__12.cache, function(item) {
return model.id == item.id;
Expand All @@ -18788,7 +18794,7 @@ $__System.register("8e", ["c", "e", "8b", "8c", "89", "8d"], function($__export)
});
} else {
if (!_.isEqual(model, $__12.cache[cacheIndex])) {
$__12.cache.splice(cacheIndex, 1, model);
$__12.cache[cacheIndex] = model;
var previousSibling = cacheIndex == 0 ? null : $__12.cache[cacheIndex - 1];
messages.push({
event: 'child_changed',
Expand All @@ -18813,11 +18819,13 @@ $__System.register("8e", ["c", "e", "8b", "8c", "89", "8d"], function($__export)
$__15 = function(change) {
if (changes[change].$.ChangeType == "Delete") {
var recordId = changes[change]._;
var isLocal = _.findIndex(tempKeys, function(key) {
var localIndex = _.findIndex(tempKeys, function(key) {
return key.remoteId == recordId;
});
if (isLocal > -1) {
recordId = tempKeys[isLocal].localId;
if (localIndex > -1) {
var tempKey = tempKeys[localIndex];
var isOurTempKey = tempKey.client === $__14;
recordId = isOurTempKey ? tempKey.localId : tempKey.remoteId;
}
var cacheItem = _.findIndex($__14.cache, function(item) {
return item.id == recordId;
Expand Down
2 changes: 1 addition & 1 deletion worker.js.map
View file

Large diffs are not rendered by default.

0 comments on commit dc2687a

Please sign in to comment.