Skip to content

Commit

Permalink
Fix #18637: Prevent aspect.before/after errors after removing handles
Browse files Browse the repository at this point in the history
  • Loading branch information
Kenneth G. Franqueiro committed Jul 9, 2015
1 parent 01b03b3 commit 89034c5
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 7 deletions.
18 changes: 11 additions & 7 deletions aspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ define([], function(){
var args = arguments;
var before = dispatcher.before;
while(before){
args = before.advice.apply(this, args) || args;
if(before.advice){
args = before.advice.apply(this, args) || args;
}
before = before.next;
}
// around advice
Expand All @@ -94,12 +96,14 @@ define([], function(){
// after advice
var after = dispatcher.after;
while(after && after.id < executionId){
if(after.receiveArguments){
var newResults = after.advice.apply(this, args);
// change the return value only if a new value was returned
results = newResults === undefined ? results : newResults;
}else{
results = after.advice.call(this, results, args);
if(after.advice){
if(after.receiveArguments){
var newResults = after.advice.apply(this, args);
// change the return value only if a new value was returned
results = newResults === undefined ? results : newResults;
}else{
results = after.advice.call(this, results, args);
}
}
after = after.next;
}
Expand Down
38 changes: 38 additions & 0 deletions tests/unit/aspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,25 @@ define([
assert.equal(aspectSpy1.lastCall.args[0], 6);
assert.equal(methodSpy.lastCall.args[0], 7);
assert.equal(methodSpy.returnValues[0], 8);
},

'multiple aspect.before() with removal inside handler': function () {
var count = 0;

var handle1 = aspect.before(obj, 'method', function () {
count++;
});

var handle2 = aspect.before(obj, 'method', function () {
count++;
handle2.remove();
handle1.remove();
});

assert.doesNotThrow(function () {
obj.method();
});
assert.strictEqual(count, 1, 'Only one advising function should be called');
}
},

Expand All @@ -80,6 +99,25 @@ define([
assert.isTrue(aspectStub2.calledAfter(aspectStub1));
},

'multiple aspect.after() with removal inside handler': function () {
var count = 0;

var handle1 = aspect.after(obj, 'method', function () {
handle1.remove();
handle2.remove();
count++;
});

var handle2 = aspect.after(obj, 'method', function () {
count++;
});

assert.doesNotThrow(function () {
obj.method();
});
assert.strictEqual(count, 1, 'Only one advising function should be called');
},

'recieveArguments is true': {
'provides the original arguments to the aspect method': function () {
var expected = 'expected';
Expand Down

0 comments on commit 89034c5

Please sign in to comment.