Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Pr 11547 #14367

Merged
merged 2 commits into from
Apr 8, 2016
Merged

Pr 11547 #14367

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion src/ng/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,12 @@ function $HttpProvider() {
* - **headers** – `{Object}` – Map of strings or functions which return strings representing
* HTTP headers to send to the server. If the return value of a function is null, the
* header will not be sent. Functions accept a config object as an argument.
* - **eventHandlers** - `{Object}` - Event listeners to be bound to the XMLHttpRequest object.
* To bind events to the XMLHttpRequest upload object, use `uploadEventHandlers`.
* The handler will be called in the context of a `$apply` block.
* - **uploadEventHandlers** - `{Object}` - Event listeners to be bound to the XMLHttpRequest upload
* object. To bind events to the XMLHttpRequest object, use `eventHandlers`.
* The handler will be called in the context of a `$apply` block.
* - **xsrfHeaderName** – `{string}` – Name of HTTP header to populate with the XSRF token.
* - **xsrfCookieName** – `{string}` – Name of cookie containing the XSRF token.
* - **transformRequest** –
Expand Down Expand Up @@ -1259,11 +1265,31 @@ function $HttpProvider() {
}

$httpBackend(config.method, url, reqData, done, reqHeaders, config.timeout,
config.withCredentials, config.responseType);
config.withCredentials, config.responseType,
createApplyHandlers(config.eventHandlers),
createApplyHandlers(config.uploadEventHandlers));
}

return promise;

function createApplyHandlers(eventHandlers) {
if (eventHandlers) {
var applyHandlers = {};
forEach(eventHandlers, function(eventHandler, key) {
applyHandlers[key] = function() {
if (useApplyAsync) {
$rootScope.$applyAsync(eventHandler);
} else if ($rootScope.$$phase) {
eventHandler();
} else {
$rootScope.$apply(eventHandler);
}
};
});
return applyHandlers;
}
}


/**
* Callback registered to $httpBackend():
Expand Down
10 changes: 9 additions & 1 deletion src/ng/httpBackend.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function $HttpBackendProvider() {

function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDocument) {
// TODO(vojta): fix the signature
return function(method, url, post, callback, headers, timeout, withCredentials, responseType) {
return function(method, url, post, callback, headers, timeout, withCredentials, responseType, eventHandlers, uploadEventHandlers) {
$browser.$$incOutstandingRequestCount();
url = url || $browser.url();

Expand Down Expand Up @@ -114,6 +114,14 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
xhr.onerror = requestError;
xhr.onabort = requestError;

forEach(eventHandlers, function(value, key) {
xhr.addEventListener(key, value);
});

forEach(uploadEventHandlers, function(value, key) {
xhr.upload.addEventListener(key, value);
});

if (withCredentials) {
xhr.withCredentials = true;
}
Expand Down
19 changes: 18 additions & 1 deletion src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1322,12 +1322,15 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
}

// TODO(vojta): change params to: method, url, data, headers, callback
function $httpBackend(method, url, data, callback, headers, timeout, withCredentials, responseType) {
function $httpBackend(method, url, data, callback, headers, timeout, withCredentials, responseType, eventHandlers, uploadEventHandlers) {

var xhr = new MockXhr(),
expectation = expectations[0],
wasExpected = false;

xhr.$$events = eventHandlers;
xhr.upload.$$events = uploadEventHandlers;

function prettyPrint(data) {
return (angular.isString(data) || angular.isFunction(data) || data instanceof RegExp)
? data
Expand Down Expand Up @@ -2010,6 +2013,20 @@ function MockXhr() {
};

this.abort = angular.noop;

// This section simulates the events on a real XHR object (and the upload object)
// When we are testing $httpBackend (inside the angular project) we make partial use of this
// but store the events directly ourselves on `$$events`, instead of going through the `addEventListener`
this.$$events = {};
this.addEventListener = function(name, listener) {
if (angular.isUndefined(this.$$events[name])) this.$$events[name] = [];
this.$$events[name].push(listener);
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am kind of confused with this snippet:

When we create a MockXhr, we set this.$$events = {}.
Then inside the mock $httpBackend, right after we create xhr = new MockXhr(), we overwrite xhr.$$events = eventHandlers (where eventHandlers is of type {[key: string]: function}).
Then, when addEventListener is called on a MockXhr instance, we check create an array on this.$$events[name] and push the listener.

I would expect the tests to fail, but they obviously don't, so what do I miss ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is not optimal.
The MockXhr is used in two different scenarios:

  • In the $httpBackend tests we instantiate MockXhr objects and pass them through to the $httpBackend service, which does call addEventListener. This allows us to test the "real" use of the $httpBackend service.
  • In the $http tests, we have a mock $httpBackend service (from ngMock), which happens to hold its own instance of MockXhr. In these tests we never call addEventListener and I just chose to use the same $$events object for holding our mock event handlers for the test

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see 😕
(Clarification comments welcome 😄


this.upload = {
$$events: {},
addEventListener: this.addEventListener
};
}


Expand Down
11 changes: 11 additions & 0 deletions test/ng/httpBackendSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,17 @@ describe('$httpBackend', function() {
});


it('should set up event listeners', function() {
var progressFn = function() {};
var uploadProgressFn = function() {};
$backend('GET', '/url', null, callback, {}, null, null, null,
{progress: progressFn}, {progress: uploadProgressFn});
xhr = MockXhr.$$lastInstance;
expect(xhr.$$events.progress[0]).toBe(progressFn);
expect(xhr.upload.$$events.progress[0]).toBe(uploadProgressFn);
});


describe('responseType', function() {

it('should set responseType and return xhr.response', function() {
Expand Down
31 changes: 30 additions & 1 deletion test/ng/httpSpec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

/* global MockXhr: false */

describe('$http', function() {

var callback, mockedCookies;
Expand Down Expand Up @@ -1019,7 +1021,7 @@ describe('$http', function() {
});


describe('scope.$apply', function() {
describe('callbacks', function() {

it('should $apply after success callback', function() {
$httpBackend.when('GET').respond(200);
Expand Down Expand Up @@ -1047,6 +1049,33 @@ describe('$http', function() {

$exceptionHandler.errors = [];
}));


it('should pass the event handlers through to the backend', function() {
var progressFn = jasmine.createSpy('progressFn');
var uploadProgressFn = jasmine.createSpy('uploadProgressFn');
$httpBackend.when('GET').respond(200);
$http({
method: 'GET',
url: '/some',
eventHandlers: {progress: progressFn},
uploadEventHandlers: {progress: uploadProgressFn}
});
$rootScope.$apply();
var mockXHR = MockXhr.$$lastInstance;
expect(mockXHR.$$events.progress).toEqual(jasmine.any(Function));
expect(mockXHR.upload.$$events.progress).toEqual(jasmine.any(Function));

spyOn($rootScope, '$digest');

mockXHR.$$events.progress();
expect(progressFn).toHaveBeenCalledOnce();
expect($rootScope.$digest).toHaveBeenCalledTimes(1);

mockXHR.upload.$$events.progress();
expect(uploadProgressFn).toHaveBeenCalledOnce();
expect($rootScope.$digest).toHaveBeenCalledTimes(2);
});
});


Expand Down