From 77d366843268d7ea728901b568529fe292711d71 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Mon, 13 Jul 2015 14:47:10 -0400 Subject: [PATCH] feat(http): serialize search parameters from request options - Extends URLSearchParams API to include operations for combining different URLSearchParams objects: These new methods include: setAll(otherParams): performs `this.set(key, values[0])` for each key/value-list pair in `otherParams` appendAll(otherParams): performs `this.append(key, values)` for each key/value-list pair in `otherParams` replaceAll(otherParams): for each key/value-list pair in `otherParams`, replaces current set of values for `key` with a copy of the list of values. - RequestOptions do not merge search params automatically (because there are multiple ways to do this). Instead, they replace any existing `search` field if `search` is provided. Explicit merging is required if merging is desirable. - Some extra test coverage added. Closes #2417 Closes #3020 --- .../angular2/src/http/base_request_options.ts | 16 ++- modules/angular2/src/http/http.ts | 1 + modules/angular2/src/http/interfaces.ts | 5 + modules/angular2/src/http/static_request.ts | 15 ++- .../angular2/src/http/url_search_params.ts | 104 +++++++++++++++--- modules/angular2/test/http/http_spec.ts | 42 +++++++ .../test/http/url_search_params_spec.ts | 45 ++++++++ 7 files changed, 210 insertions(+), 18 deletions(-) diff --git a/modules/angular2/src/http/base_request_options.ts b/modules/angular2/src/http/base_request_options.ts index 5583ce582a..1e49e5fac7 100644 --- a/modules/angular2/src/http/base_request_options.ts +++ b/modules/angular2/src/http/base_request_options.ts @@ -1,8 +1,9 @@ -import {CONST_EXPR, CONST, isPresent} from 'angular2/src/facade/lang'; +import {CONST_EXPR, CONST, isPresent, isString} from 'angular2/src/facade/lang'; import {Headers} from './headers'; import {RequestModesOpts, RequestMethods, RequestCacheOpts, RequestCredentialsOpts} from './enums'; import {IRequestOptions} from './interfaces'; import {Injectable} from 'angular2/di'; +import {URLSearchParams} from './url_search_params'; /** * Creates a request options object similar to the `RequestInit` description @@ -33,7 +34,9 @@ export class RequestOptions implements IRequestOptions { credentials: RequestCredentialsOpts; cache: RequestCacheOpts; url: string; - constructor({method, headers, body, mode, credentials, cache, url}: IRequestOptions = {}) { + search: URLSearchParams; + constructor({method, headers, body, mode, credentials, cache, url, search}: + IRequestOptions = {}) { this.method = isPresent(method) ? method : null; this.headers = isPresent(headers) ? headers : null; this.body = isPresent(body) ? body : null; @@ -41,6 +44,9 @@ export class RequestOptions implements IRequestOptions { this.credentials = isPresent(credentials) ? credentials : null; this.cache = isPresent(cache) ? cache : null; this.url = isPresent(url) ? url : null; + this.search = isPresent(search) ? (isString(search) ? new URLSearchParams((search)) : + (search)) : + null; } /** @@ -56,7 +62,11 @@ export class RequestOptions implements IRequestOptions { credentials: isPresent(options) && isPresent(options.credentials) ? options.credentials : this.credentials, cache: isPresent(options) && isPresent(options.cache) ? options.cache : this.cache, - url: isPresent(options) && isPresent(options.url) ? options.url : this.url + url: isPresent(options) && isPresent(options.url) ? options.url : this.url, + search: isPresent(options) && isPresent(options.search) ? + (isString(options.search) ? new URLSearchParams((options.search)) : + ((options.search)).clone()) : + this.search }); } } diff --git a/modules/angular2/src/http/http.ts b/modules/angular2/src/http/http.ts index e962dc314d..0bdfd24881 100644 --- a/modules/angular2/src/http/http.ts +++ b/modules/angular2/src/http/http.ts @@ -17,6 +17,7 @@ function mergeOptions(defaultOpts, providedOpts, method, url): RequestOptions { newOptions = newOptions.merge(new RequestOptions({ method: providedOpts.method, url: providedOpts.url, + search: providedOpts.search, headers: providedOpts.headers, body: providedOpts.body, mode: providedOpts.mode, diff --git a/modules/angular2/src/http/interfaces.ts b/modules/angular2/src/http/interfaces.ts index d796542382..cc85cbb21c 100644 --- a/modules/angular2/src/http/interfaces.ts +++ b/modules/angular2/src/http/interfaces.ts @@ -12,6 +12,10 @@ import {Headers} from './headers'; import {BaseException} from 'angular2/src/facade/lang'; import {EventEmitter} from 'angular2/src/facade/async'; import {Request} from './static_request'; +import {URLSearchParamsUnionFixer, URLSearchParams} from './url_search_params'; + +// Work around Dartanalyzer problem :( +const URLSearchParams_UnionFixer = URLSearchParamsUnionFixer; /** * Abstract class from which real backends are derived. @@ -41,6 +45,7 @@ export class Connection { export interface IRequestOptions { url?: string; method?: RequestMethods; + search?: string | URLSearchParams; headers?: Headers; // TODO: Support Blob, ArrayBuffer, JSON, URLSearchParams, FormData body?: string; diff --git a/modules/angular2/src/http/static_request.ts b/modules/angular2/src/http/static_request.ts index e246a5ecb4..4d79ef762e 100644 --- a/modules/angular2/src/http/static_request.ts +++ b/modules/angular2/src/http/static_request.ts @@ -6,7 +6,8 @@ import { RegExpWrapper, CONST_EXPR, isPresent, - isJsObject + isJsObject, + StringWrapper } from 'angular2/src/facade/lang'; // TODO(jeffbcross): properly implement body accessors @@ -39,7 +40,19 @@ export class Request { cache: RequestCacheOpts; constructor(requestOptions: RequestOptions) { // TODO: assert that url is present + let url = requestOptions.url; this.url = requestOptions.url; + if (isPresent(requestOptions.search)) { + let search = requestOptions.search.toString(); + if (search.length > 0) { + let prefix = '?'; + if (StringWrapper.contains(this.url, '?')) { + prefix = (this.url[this.url.length - 1] == '&') ? '' : '&'; + } + // TODO: just delete search-query-looking string in url? + this.url = url + prefix + search; + } + } this._body = requestOptions.body; this.method = requestOptions.method; // TODO(jeffbcross): implement behavior diff --git a/modules/angular2/src/http/url_search_params.ts b/modules/angular2/src/http/url_search_params.ts index 596f54f076..034d2e4dde 100644 --- a/modules/angular2/src/http/url_search_params.ts +++ b/modules/angular2/src/http/url_search_params.ts @@ -1,4 +1,4 @@ -import {isPresent, isBlank, StringWrapper} from 'angular2/src/facade/lang'; +import {CONST_EXPR, isPresent, isBlank, StringWrapper} from 'angular2/src/facade/lang'; import { Map, MapWrapper, @@ -7,28 +7,42 @@ import { isListLikeIterable } from 'angular2/src/facade/collection'; -function paramParser(rawParams: string): Map> { +function paramParser(rawParams: string = ''): Map> { var map: Map> = new Map(); - var params: List = StringWrapper.split(rawParams, new RegExp('&')); - ListWrapper.forEach(params, (param: string) => { - var split: List = StringWrapper.split(param, new RegExp('=')); - var key = ListWrapper.get(split, 0); - var val = ListWrapper.get(split, 1); - var list = isPresent(map.get(key)) ? map.get(key) : []; - list.push(val); - map.set(key, list); - }); + if (rawParams.length > 0) { + var params: List = StringWrapper.split(rawParams, new RegExp('&')); + ListWrapper.forEach(params, (param: string) => { + var split: List = StringWrapper.split(param, new RegExp('=')); + var key = ListWrapper.get(split, 0); + var val = ListWrapper.get(split, 1); + var list = isPresent(map.get(key)) ? map.get(key) : []; + list.push(val); + map.set(key, list); + }); + } return map; } +// TODO(caitp): This really should not be needed. Issue with ts2dart. +export const URLSearchParamsUnionFixer: string = CONST_EXPR("UnionFixer"); + /** * Map-like representation of url search parameters, based on - * [URLSearchParams](https://url.spec.whatwg.org/#urlsearchparams) in the url living standard. - * + * [URLSearchParams](https://url.spec.whatwg.org/#urlsearchparams) in the url living standard, + * with several extensions for merging URLSearchParams objects: + * - setAll() + * - appendAll() + * - replaceAll() */ export class URLSearchParams { paramsMap: Map>; - constructor(public rawParams: string) { this.paramsMap = paramParser(rawParams); } + constructor(public rawParams: string = '') { this.paramsMap = paramParser(rawParams); } + + clone(): URLSearchParams { + var clone = new URLSearchParams(); + clone.appendAll(this); + return clone; + } has(param: string): boolean { return this.paramsMap.has(param); } @@ -46,6 +60,30 @@ export class URLSearchParams { return isPresent(mapParam) ? mapParam : []; } + set(param: string, val: string) { + var mapParam = this.paramsMap.get(param); + var list = isPresent(mapParam) ? mapParam : []; + ListWrapper.clear(list); + list.push(val); + this.paramsMap.set(param, list); + } + + // A merge operation + // For each name-values pair in `searchParams`, perform `set(name, values[0])` + // + // E.g: "a=[1,2,3], c=[8]" + "a=[4,5,6], b=[7]" = "a=[4], c=[8], b=[7]" + // + // TODO(@caitp): document this better + setAll(searchParams: URLSearchParams) { + MapWrapper.forEach(searchParams.paramsMap, (value, param) => { + var mapParam = this.paramsMap.get(param); + var list = isPresent(mapParam) ? mapParam : []; + ListWrapper.clear(list); + list.push(value[0]); + this.paramsMap.set(param, list); + }); + } + append(param: string, val: string): void { var mapParam = this.paramsMap.get(param); var list = isPresent(mapParam) ? mapParam : []; @@ -53,6 +91,44 @@ export class URLSearchParams { this.paramsMap.set(param, list); } + // A merge operation + // For each name-values pair in `searchParams`, perform `append(name, value)` + // for each value in `values`. + // + // E.g: "a=[1,2], c=[8]" + "a=[3,4], b=[7]" = "a=[1,2,3,4], c=[8], b=[7]" + // + // TODO(@caitp): document this better + appendAll(searchParams: URLSearchParams) { + MapWrapper.forEach(searchParams.paramsMap, (value, param) => { + var mapParam = this.paramsMap.get(param); + var list = isPresent(mapParam) ? mapParam : []; + for (var i = 0; i < value.length; ++i) { + list.push(value[i]); + } + this.paramsMap.set(param, list); + }); + } + + + // A merge operation + // For each name-values pair in `searchParams`, perform `delete(name)`, + // followed by `set(name, values)` + // + // E.g: "a=[1,2,3], c=[8]" + "a=[4,5,6], b=[7]" = "a=[4,5,6], c=[8], b=[7]" + // + // TODO(@caitp): document this better + replaceAll(searchParams: URLSearchParams) { + MapWrapper.forEach(searchParams.paramsMap, (value, param) => { + var mapParam = this.paramsMap.get(param); + var list = isPresent(mapParam) ? mapParam : []; + ListWrapper.clear(list); + for (var i = 0; i < value.length; ++i) { + list.push(value[i]); + } + this.paramsMap.set(param, list); + }); + } + toString(): string { var paramsList = []; MapWrapper.forEach(this.paramsMap, (values, k) => { diff --git a/modules/angular2/test/http/http_spec.ts b/modules/angular2/test/http/http_spec.ts index 1d2d7fbd35..5bba92a783 100644 --- a/modules/angular2/test/http/http_spec.ts +++ b/modules/angular2/test/http/http_spec.ts @@ -21,6 +21,7 @@ import {ResponseOptions} from 'angular2/src/http/base_response_options'; import {Request} from 'angular2/src/http/static_request'; import {EventEmitter, ObservableWrapper} from 'angular2/src/facade/async'; import {ConnectionBackend} from 'angular2/src/http/interfaces'; +import {URLSearchParams} from 'angular2/src/http/url_search_params'; class SpyObserver extends SpyObject { onNext: Function; @@ -202,6 +203,47 @@ export function main() { ObservableWrapper.subscribe(http.head(url), res => {}); })); }); + + + describe('searchParams', () => { + it('should append search params to url', inject([AsyncTestCompleter], async => { + var params = new URLSearchParams(); + params.append('q', 'puppies'); + ObservableWrapper.subscribe(backend.connections, c => { + expect(c.request.url).toEqual('https://www.google.com?q=puppies'); + backend.resolveAllConnections(); + async.done(); + }); + ObservableWrapper.subscribe( + http.get('https://www.google.com', new RequestOptions({search: params})), + res => {}); + })); + + + it('should append string search params to url', inject([AsyncTestCompleter], async => { + ObservableWrapper.subscribe(backend.connections, c => { + expect(c.request.url).toEqual('https://www.google.com?q=piggies'); + backend.resolveAllConnections(); + async.done(); + }); + ObservableWrapper.subscribe( + http.get('https://www.google.com', new RequestOptions({search: 'q=piggies'})), + res => {}); + })); + + + it('should produce valid url when url already contains a query', + inject([AsyncTestCompleter], async => { + ObservableWrapper.subscribe(backend.connections, c => { + expect(c.request.url).toEqual('https://www.google.com?q=angular&as_eq=1.x'); + backend.resolveAllConnections(); + async.done(); + }); + ObservableWrapper.subscribe(http.get('https://www.google.com?q=angular', + new RequestOptions({search: 'as_eq=1.x'})), + res => {}); + })); + }); }); }); } diff --git a/modules/angular2/test/http/url_search_params_spec.ts b/modules/angular2/test/http/url_search_params_spec.ts index 17093e9ce6..b690be724f 100644 --- a/modules/angular2/test/http/url_search_params_spec.ts +++ b/modules/angular2/test/http/url_search_params_spec.ts @@ -30,6 +30,51 @@ export function main() { expect(searchParams.toString()).toEqual("q=URLUtils.searchParams&topic=api&topic=webdev"); searchParams.delete("topic"); expect(searchParams.toString()).toEqual("q=URLUtils.searchParams"); + + // Test default constructor + expect(new URLSearchParams().toString()).toBe(""); + }); + + + it('should support map-like merging operation via setAll()', () => { + var mapA = new URLSearchParams('a=1&a=2&a=3&c=8'); + var mapB = new URLSearchParams('a=4&a=5&a=6&b=7'); + mapA.setAll(mapB); + expect(mapA.has('a')).toBe(true); + expect(mapA.has('b')).toBe(true); + expect(mapA.has('c')).toBe(true); + expect(mapA.getAll('a')).toEqual(['4']); + expect(mapA.getAll('b')).toEqual(['7']); + expect(mapA.getAll('c')).toEqual(['8']); + expect(mapA.toString()).toEqual('a=4&c=8&b=7'); + }); + + + it('should support multimap-like merging operation via appendAll()', () => { + var mapA = new URLSearchParams('a=1&a=2&a=3&c=8'); + var mapB = new URLSearchParams('a=4&a=5&a=6&b=7'); + mapA.appendAll(mapB); + expect(mapA.has('a')).toBe(true); + expect(mapA.has('b')).toBe(true); + expect(mapA.has('c')).toBe(true); + expect(mapA.getAll('a')).toEqual(['1', '2', '3', '4', '5', '6']); + expect(mapA.getAll('b')).toEqual(['7']); + expect(mapA.getAll('c')).toEqual(['8']); + expect(mapA.toString()).toEqual('a=1&a=2&a=3&a=4&a=5&a=6&c=8&b=7'); + }); + + + it('should support multimap-like merging operation via replaceAll()', () => { + var mapA = new URLSearchParams('a=1&a=2&a=3&c=8'); + var mapB = new URLSearchParams('a=4&a=5&a=6&b=7'); + mapA.replaceAll(mapB); + expect(mapA.has('a')).toBe(true); + expect(mapA.has('b')).toBe(true); + expect(mapA.has('c')).toBe(true); + expect(mapA.getAll('a')).toEqual(['4', '5', '6']); + expect(mapA.getAll('b')).toEqual(['7']); + expect(mapA.getAll('c')).toEqual(['8']); + expect(mapA.toString()).toEqual('a=4&a=5&a=6&c=8&b=7'); }); }); }