Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

chore(Http): remove deprecated getString method #930

Closed
wants to merge 1 commit into from
Closed

chore(Http): remove deprecated getString method #930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
127 changes: 41 additions & 86 deletions lib/core_dom/http.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,16 +387,6 @@ class Http {
Http(this._cookies, this._location, this._rewriter, this._backend,
this.defaults, this._interceptors);

/**
* DEPRECATED
*/
async.Future<String> getString(String url, {bool withCredentials,
void onProgress(dom.ProgressEvent e), Cache cache}) =>
request(url,
withCredentials: withCredentials,
onProgress: onProgress,
cache: cache).then((HttpResponse xhr) => xhr.responseText);

/**
* Parse a [requestUrl] and determine whether this is a same-origin request as
* the application document.
Expand Down Expand Up @@ -442,6 +432,7 @@ class Http {
throw ['timeout not implemented'];
}

url = _rewriter(url);
method = method.toUpperCase();

if (headers == null) headers = {};
Expand All @@ -461,8 +452,7 @@ class Http {
});

var serverRequest = (HttpResponseConfig config) {
assert(config.data == null || config.data is String ||
config.data is dom.File);
assert(config.data == null || config.data is String || config.data is dom.File);

// Strip content-type if data is undefined
if (config.data == null) {
Expand All @@ -471,12 +461,45 @@ class Http {
.forEach((h) => headers.remove(h));
}

return request(null,
config: config,
method: method,
sendData: config.data,
requestHeaders: config.headers,
cache: cache);
url = _buildUrl(config.url, config.params);

if (cache == false) {
cache = null;
} else if (cache == null) {
cache = defaults.cache;
}

// We return a pending request only if caching is enabled.
if (cache != null && _pendingRequests.containsKey(url)) {
return _pendingRequests[url];
}
var cachedResponse = (cache != null && method == 'GET') ? cache.get(url) : null;
if (cachedResponse != null) {
return new async.Future.value(new HttpResponse.copy(cachedResponse));
}

var result = _backend.request(url,
method: method,
requestHeaders: config.headers,
sendData: config.data).then((dom.HttpRequest value) {
// TODO: Uncomment after apps migrate off of this class.
// assert(value.status >= 200 && value.status < 300);

var response = new HttpResponse(value.status, value.responseText,
parseHeaders(value), config);

if (cache != null) cache.put(url, response);
_pendingRequests.remove(url);
return response;
}, onError: (error) {
if (error is! dom.ProgressEvent) throw error;
dom.ProgressEvent event = error;
_pendingRequests.remove(url);
dom.HttpRequest request = event.currentTarget;
return new async.Future.error(
new HttpResponse(request.status, request.response, parseHeaders(request), config));
});
return _pendingRequests[url] = result;
};

var chain = [[serverRequest, null]];
Expand Down Expand Up @@ -632,81 +655,13 @@ class Http {
});
return parsed;
}

/**
* Returns an [Iterable] of [Future] [HttpResponse]s for the requests
* that the [Http] service is currently waiting for.
*/
Iterable<async.Future<HttpResponse> > get pendingRequests =>
_pendingRequests.values;

/**
* DEPRECATED
*/
async.Future<HttpResponse> request(String rawUrl,
{ HttpResponseConfig config,
String method: 'GET',
bool withCredentials: false,
String responseType,
String mimeType,
Map<String, String> requestHeaders,
sendData,
void onProgress(dom.ProgressEvent e),
/*Cache<String, HttpResponse> or false*/ cache }) {
String url;

if (config == null) {
url = _rewriter(rawUrl);
config = new HttpResponseConfig(url: url);
} else {
url = _buildUrl(config.url, config.params);
}

if (cache == false) {
cache = null;
} else if (cache == null) {
cache = defaults.cache;
}
// We return a pending request only if caching is enabled.
if (cache != null && _pendingRequests.containsKey(url)) {
return _pendingRequests[url];
}
var cachedResponse = (cache != null && method == 'GET')
? cache.get(url)
: null;
if (cachedResponse != null) {
return new async.Future.value(new HttpResponse.copy(cachedResponse));
}

var result = _backend.request(url,
method: method,
withCredentials: withCredentials,
responseType: responseType,
mimeType: mimeType,
requestHeaders: requestHeaders,
sendData: sendData,
onProgress: onProgress).then((dom.HttpRequest value) {
// TODO: Uncomment after apps migrate off of this class.
// assert(value.status >= 200 && value.status < 300);

var response = new HttpResponse(value.status, value.responseText,
parseHeaders(value), config);

if (cache != null) cache.put(url, response);
_pendingRequests.remove(url);
return response;
}, onError: (error) {
if (error is! dom.ProgressEvent) throw error;
dom.ProgressEvent event = error;
_pendingRequests.remove(url);
dom.HttpRequest request = event.currentTarget;
return new async.Future.error(
new HttpResponse(request.status, request.response,
parseHeaders(request), config));
});
return _pendingRequests[url] = result;
}

_buildUrl(String url, Map<String, dynamic> params) {
if (params == null) return url;
var parts = [];
Expand Down
8 changes: 4 additions & 4 deletions lib/core_dom/view_factory.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ class ViewCache {
}

async.Future<ViewFactory> fromUrl(String url, DirectiveMap directives) {
return http.getString(url, cache: templateCache).then(
(html) => fromHtml(html, directives));
return http.get(url, cache: templateCache).then(
(resp) => fromHtml(resp.responseText, directives));
}
}

Expand Down Expand Up @@ -182,8 +182,8 @@ class _ComponentFactory implements Function {
var cssUrls = []..addAll(_baseCss.urls)..addAll(component.cssUrls);
if (cssUrls.isNotEmpty) {
cssUrls.forEach((css) => cssFutures.add(http
.getString(css, cache: templateCache)
.catchError((e) => '/*\n$e\n*/\n')
.get(css, cache: templateCache).then((resp) => resp.responseText,
onError: (e) => '/*\n$e\n*/\n')
));
} else {
cssFutures.add(new async.Future.value(null));
Expand Down
30 changes: 19 additions & 11 deletions test/core/templateurl_spec.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,12 @@ void main() {
it('should replace element with template from url', async(inject(
(Http http, Compiler compile, Scope rootScope, Logger log,
Injector injector, MockHttpBackend backend, DirectiveMap directives) {
backend.expectGET('simple.html').respond('<div log="SIMPLE">Simple!</div>');
backend.expectGET('simple.html').respond(200, '<div log="SIMPLE">Simple!</div>');

var element = es('<div><simple-url log>ignore</simple-url><div>');
compile(element, directives)(injector, element);

microLeap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did new microLeaps needed to got injected? I thought this change should not have change the external behavior.

backend.flush();
microLeap();

Expand All @@ -107,7 +108,7 @@ void main() {
it('should load template from URL once', async(inject(
(Http http, Compiler compile, Scope rootScope, Logger log,
Injector injector, MockHttpBackend backend, DirectiveMap directives) {
backend.whenGET('simple.html').respond('<div log="SIMPLE">Simple!</div>');
backend.whenGET('simple.html').respond(200, '<div log="SIMPLE">Simple!</div>');

var element = es(
'<div>'
Expand All @@ -116,6 +117,7 @@ void main() {
'<div>');
compile(element, directives)(injector, element);

microLeap();
backend.flush();
microLeap();

Expand All @@ -130,12 +132,13 @@ void main() {
(Http http, Compiler compile, Scope rootScope, Logger log,
Injector injector, MockHttpBackend backend, DirectiveMap directives) {
backend
..expectGET('simple.css').respond('.hello{}')
..expectGET('simple.html').respond('<div log="SIMPLE">Simple!</div>');
..expectGET('simple.css').respond(200, '.hello{}')
..expectGET('simple.html').respond(200, '<div log="SIMPLE">Simple!</div>');

var element = e('<div><html-and-css log>ignore</html-and-css><div>');
compile([element], directives)(injector, [element]);

microLeap();
backend.flush();
microLeap();

Expand All @@ -152,9 +155,10 @@ void main() {
(Http http, Compiler compile, Scope rootScope, Injector injector,
MockHttpBackend backend, DirectiveMap directives) {
var element = es('<div><inline-with-css log>ignore</inline-with-css><div>');
backend.expectGET('simple.css').respond('.hello{}');
backend.expectGET('simple.css').respond(200, '.hello{}');
compile(element, directives)(injector, element);

microLeap();
backend.flush();
microLeap();
expect(element[0]).toHaveText('.hello{}inline!');
Expand All @@ -167,6 +171,7 @@ void main() {
backend.expectGET('simple.css').respond(500, 'some error');
compile(element, directives)(injector, element);

microLeap();
backend.flush();
microLeap();
expect(element.first).toHaveText(
Expand All @@ -180,9 +185,10 @@ void main() {
(Http http, Compiler compile, Scope rootScope, Injector injector,
MockHttpBackend backend, DirectiveMap directives) {
var element = es('<div><only-css log>ignore</only-css><div>');
backend.expectGET('simple.css').respond('.hello{}');
backend.expectGET('simple.css').respond(200, '.hello{}');
compile(element, directives)(injector, element);

microLeap();
backend.flush();
microLeap();
expect(element[0]).toHaveText('.hello{}');
Expand All @@ -192,12 +198,13 @@ void main() {
(Http http, Compiler compile, Scope rootScope, Injector injector,
MockHttpBackend backend, DirectiveMap directives) {
backend
..expectGET('simple.css').respond('.hello{}')
..expectGET('simple.html').respond('<div>Simple!</div>');
..expectGET('simple.css').respond(200, '.hello{}')
..expectGET('simple.html').respond(200, '<div>Simple!</div>');

var element = es('<html-and-css>ignore</html-and-css>');
compile(element, directives)(injector, element);

microLeap();
backend.flush();
microLeap();
expect(element.first).toHaveText('.hello{}Simple!');
Expand All @@ -215,13 +222,14 @@ void main() {
(Http http, Compiler compile, Scope rootScope, Logger log,
Injector injector, MockHttpBackend backend, DirectiveMap directives) {
backend
..expectGET('simple.css').respond('.hello{}')
..expectGET('another.css').respond('.world{}')
..expectGET('simple.html').respond('<div log="SIMPLE">Simple!</div>');
..expectGET('simple.css').respond(200, '.hello{}')
..expectGET('another.css').respond(200, '.world{}')
..expectGET('simple.html').respond(200, '<div log="SIMPLE">Simple!</div>');

var element = e('<div><html-and-css log>ignore</html-and-css><div>');
compile([element], directives)(injector, [element]);

microLeap();
backend.flush();
microLeap();

Expand Down
3 changes: 2 additions & 1 deletion test/core_dom/compiler_spec.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ void main() {
});

it('should fire onTemplate method', async((Compiler compile, Logger logger, MockHttpBackend backend) {
backend.whenGET('some/template.url').respond('<div>WORKED</div>');
backend.whenGET('some/template.url').respond(200, '<div>WORKED</div>');
var scope = _.rootScope.createChild({});
scope.context['isReady'] = 'ready';
scope.context['logger'] = logger;
Expand All @@ -503,6 +503,7 @@ void main() {
expect(logger).toEqual(expected);
logger.clear();

microLeap();
backend.flush();
microLeap();
expect(logger).toEqual(['templateLoaded', _.rootScope.context['shadowRoot']]);
Expand Down
Loading