Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#113 Build URL to determine if request is cached #297

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

#113 Build URL to determine if request is cached #297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f65f60
Build URL to determine if request is cached
May 9, 2016
fab2d44
Build URL only with angular >= 1.4
May 11, 2016
a03296f
Merge branch 'master' of github.com:chieffancypants/angular-loading-bar
Apr 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/loading-bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ angular.module('cfp.loadingBarInterceptor', ['cfp.loadingBar'])
var cache;
var defaultCache = $cacheFactory.get('$http');
var defaults = $httpProvider.defaults;
var url = config.url;

if (config.paramSerializer !== undefined) {
url = buildUrl(config.url, config.paramSerializer(config.params));
}

// Choose the proper cache source. Borrowed from angular: $http service
if ((config.cache || defaults.cache) && config.cache !== false &&
Expand All @@ -79,7 +84,7 @@ angular.module('cfp.loadingBarInterceptor', ['cfp.loadingBar'])
}

var cached = cache !== undefined ?
cache.get(config.url) !== undefined : false;
cache.get(url) !== undefined : false;

if (config.cached !== undefined && cached !== config.cached) {
return config.cached;
Expand All @@ -88,6 +93,12 @@ angular.module('cfp.loadingBarInterceptor', ['cfp.loadingBar'])
return cached;
}

function buildUrl(url, serializedParams) {
if (serializedParams.length > 0) {
url += ((url.indexOf('?') == -1) ? '?' : '&') + serializedParams;
}
return url;
}

return {
'request': function(config) {
Expand Down
41 changes: 41 additions & 0 deletions test/loading-bar-interceptor.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,47 @@ describe 'loadingBarInterceptor Service', ->
expect(cfpLoadingBar.status()).toBe 1
$timeout.flush()

it 'should not increment if the response is cached with different params', inject (cfpLoadingBar) ->
return unless angular.version.major >= 1 && angular.version.minor >= 4

$httpBackend.expectGET(endpoint).respond response
$http.get(endpoint, cache: true).then (data) ->
result = data

expect(cfpLoadingBar.status()).toBe 0
$timeout.flush()
$timeout.flush()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are 2 timeout flushes necessary?

Copy link
Author

Choose a reason for hiding this comment

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

Good question.
I must admit that I stupidly copied the structure from existing tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've got some really odd looking stuff in some of my tests, I wasn't criticising you.

Do the tests work if you remove one of them?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the delay.
I've just tried removing one flush, but both are required to pass the tests (mine and yours).
I guess that two are enough to complete timeouts in _set and _complete.

FYI, I just pushed source code changes. Not builds. Do you need them now ?

$httpBackend.flush(1)
expect(cfpLoadingBar.status()).toBe 1
cfpLoadingBar.complete() # set as complete
$timeout.flush()
flush()

$httpBackend.verifyNoOutstandingRequest()


$httpBackend.expectGET(endpoint + "?q=angular").respond response
$http.get(endpoint, params: { q: 'angular' }, cache: true).then (data) ->
result = data

$httpBackend.verifyNoOutstandingExpectation()
expect(cfpLoadingBar.status()).toBe 0
$timeout.flush()
$timeout.flush()
$httpBackend.flush(1)
expect(cfpLoadingBar.status()).toBe 1
cfpLoadingBar.complete() # set as complete
$timeout.flush()
flush()


$http.get(endpoint, params: { q: 'angular' }, cache: true).then (data) ->
result = data
# no need to flush $httpBackend since the response is cached
expect(cfpLoadingBar.status()).toBe 0
$httpBackend.verifyNoOutstandingRequest()
$timeout.flush() # loading bar is animated, so flush timeout


it 'should increment the loading bar when not all requests have been recieved', inject (cfpLoadingBar) ->
$httpBackend.expectGET(endpoint).respond response
Expand Down