Skip to content

Commit 075a9e6

Browse files
vicbvicb
committed
refactor(Cookies): code cleanup
1 parent 06fdc47 commit 075a9e6

File tree

2 files changed

+71
-99
lines changed

2 files changed

+71
-99
lines changed

‎lib/core_dom/cookies.dart

+46-60
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,44 @@
11
part of angular.core.dom_internal;
22

33
/**
4-
* This class provides low-level acces to the browser's cookies.
5-
* It is not meant to be used directly by applications. Instead
6-
* use the Cookies service.
7-
*
8-
*/
4+
* This class provides low-level access to the browser's cookies. It is not meant to be used
5+
* directly by applications. Use the [Cookies] service instead.
6+
*/
97
@Injectable()
108
class BrowserCookies {
9+
String cookiePath;
10+
1111
ExceptionHandler _exceptionHandler;
1212
dom.Document _document;
13-
14-
var lastCookies = {};
15-
var lastCookieString = '';
16-
var cookiePath;
17-
var baseElement;
13+
var _lastCookies = <String, String>{};
14+
var _lastCookieString = '';
15+
var _baseElement;
1816

1917
BrowserCookies(this._exceptionHandler) {
20-
// Injecting document produces the error 'Caught Compile-time error during mirrored execution:
21-
// <'file:///mnt/data/b/build/slave/dartium-lucid32-full-trunk/build/src/out/Release/gen/blink/
22-
// bindings/dart/dart/html/Document.dart': Error: line 7 pos 3: expression must be a compile-time constant
23-
// @ DocsEditable '
24-
// I have not had time to debug it yet.
2518
_document = dom.document;
26-
2719
var baseElementList = _document.getElementsByName('base');
2820
if (baseElementList.isEmpty) return;
29-
baseElement = baseElementList.first;
21+
_baseElement = baseElementList.first;
3022
cookiePath = _baseHref();
3123
}
3224

33-
var URL_PROTOCOL = new RegExp(r'^https?\:\/\/[^\/]*');
34-
_baseHref() {
35-
var href = baseElement != null ? baseElement.attr('href') : null;
25+
final URL_PROTOCOL = new RegExp(r'^https?\:\/\/[^\/]*');
26+
27+
String _baseHref() {
28+
var href = _baseElement != null ? _baseElement.attr('href') : null;
3629
return href != null ? href.replace(URL_PROTOCOL, '') : '';
3730
}
3831

3932
// NOTE(deboer): This is sub-optimal, see dartbug.com/14281
40-
_unescape(s) => Uri.decodeFull(s);
41-
_escape(s) =>
42-
Uri.encodeFull(s)
43-
.replaceAll('=', '%3D')
44-
.replaceAll(';', '%3B');
45-
46-
_updateLastCookies() {
47-
if (_document.cookie != lastCookieString) {
48-
lastCookieString = _document.cookie;
49-
List<String> cookieArray = lastCookieString.split("; ");
50-
lastCookies = {};
33+
String _unescape(s) => Uri.decodeFull(s);
34+
35+
String _escape(s) => Uri.encodeFull(s).replaceAll('=', '%3D').replaceAll(';', '%3B');
36+
37+
Map<String, String> _updateLastCookies() {
38+
if (_document.cookie != _lastCookieString) {
39+
_lastCookieString = _document.cookie;
40+
List<String> cookieArray = _lastCookieString.split("; ");
41+
_lastCookies = {};
5142

5243
// The first value that is seen for a cookie is the most specific one.
5344
// Values for the same cookie name that follow are for less specific paths.
@@ -56,64 +47,59 @@ class BrowserCookies {
5647
var index = cookie.indexOf('=');
5748
if (index > 0) { //ignore nameless cookies
5849
var name = _unescape(cookie.substring(0, index));
59-
lastCookies[name] = _unescape(cookie.substring(index + 1));
50+
_lastCookies[name] = _unescape(cookie.substring(index + 1));
6051
}
6152
});
6253
}
63-
return lastCookies;
54+
return _lastCookies;
6455
}
6556

66-
/**
67-
* Returns a cookie.
68-
*/
69-
operator[](key) => _updateLastCookies()[key];
57+
/// Return a cookie by name
58+
String operator[](key) => _updateLastCookies()[key];
7059

71-
/**
72-
* Sets a cookie. Setting a cookie to [null] deletes the cookie.
73-
*/
74-
operator[]=(name, value) {
75-
if (identical(value, null)) {
60+
/// Sets a cookie. Setting a cookie to [null] deletes the cookie.
61+
void operator[]=(name, value) {
62+
if (value == null) {
7663
_document.cookie = "${_escape(name)}=;path=$cookiePath;expires=Thu, 01 Jan 1970 00:00:00 GMT";
7764
} else {
7865
if (value is String) {
79-
var cookieLength = (_document.cookie = "${_escape(name)}=${_escape(value)};path=$cookiePath").length + 1;
66+
var cookie = "${_escape(name)}=${_escape(value)};path=$cookiePath";
67+
_document.cookie = cookie;
68+
var cookieLength = cookie.length + 1;
8069

8170
// per http://www.ietf.org/rfc/rfc2109.txt browser must allow at minimum:
8271
// - 300 cookies
8372
// - 20 cookies per unique domain
8473
// - 4096 bytes per cookie
8574
if (cookieLength > 4096) {
8675
_exceptionHandler("Cookie '$name' possibly not set or overflowed because it was " +
87-
"too large ($cookieLength > 4096 bytes)!", null);
76+
"too large ($cookieLength > 4096 bytes)!", null);
8877
}
8978
}
9079
}
9180
}
9281

93-
get all => _updateLastCookies();
82+
Map<String, String> get all => _updateLastCookies();
9483
}
9584

96-
/**
97-
* Cookies service
98-
*/
85+
/// Handling of browser cookies
9986
@Injectable()
10087
class Cookies {
10188
BrowserCookies _browserCookies;
89+
10290
Cookies(this._browserCookies);
10391

104-
/**
105-
* Returns the value of given cookie key
106-
*/
107-
operator[](name) => this._browserCookies[name];
92+
/// Returns the value of given cookie key
93+
String operator[](name) => _browserCookies[name];
10894

109-
/**
110-
* Sets a value for given cookie key
111-
*/
112-
operator[]=(name, value) => this._browserCookies[name] = value;
95+
/// Sets a value for given cookie key
96+
void operator[]=(name, value) {
97+
_browserCookies[name] = value;
98+
}
11399

114-
/**
115-
* Remove given cookie
116-
*/
117-
remove(name) => this._browserCookies[name] = null;
100+
/// Remove given cookie
101+
void remove(name) {
102+
_browserCookies[name] = null;
103+
}
118104
}
119105

‎test/core_dom/cookies_spec.dart

+25-39
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ void main() {
99
var cookies = document.cookie.split(";");
1010
var path = window.location.pathname;
1111

12-
for (var i = 0; i < cookies.length; i++) {
13-
var cookie = cookies[i];
12+
for (var cookie in cookies) {
1413
var eqPos = cookie.indexOf("=");
1514
var name = eqPos > -1 ? cookie.substring(0, eqPos) : '';
1615
var parts = path.split('/');
17-
while (!parts.isEmpty) {
16+
while (parts.isNotEmpty) {
1817
var joinedParts = parts.join('/');
19-
document.cookie = name + "=;path=" + (joinedParts.isEmpty ? '/': joinedParts) +
20-
";expires=Thu, 01 Jan 1970 00:00:00 GMT";
18+
document.cookie = name + "=;path=" +
19+
(joinedParts.isEmpty ? '/': joinedParts) +
20+
";expires=Thu, 01 Jan 1970 00:00:00 GMT";
2121
parts.removeLast();
2222
}
2323
}
@@ -94,43 +94,37 @@ void main() {
9494
});
9595

9696
it('should log warnings when 4kb per cookie storage limit is reached',
97-
(ExceptionHandler exceptionHandler) {
98-
var i, longVal = '', cookieStr;
97+
(ExceptionHandler exceptionHandler) {
98+
var i, cookieStr;
9999

100-
for (i=0; i<4083; i++) {
101-
longVal += 'r'; // Can't do + due to dartbug.com/14281
102-
}
100+
var longVal = 'r' * 4083;
103101

104102
cookieStr = document.cookie;
105-
cookies['x'] = longVal; //total size 4093-4096, so it should go through
103+
cookies['x'] = longVal;
106104
expect(document.cookie).not.toEqual(cookieStr);
107105
expect(cookies['x']).toEqual(longVal);
108-
//expect(logs.warn).toEqual([]);
109106
var overflow = 'xxxxxxxxxxxxxxxxxxxx';
110-
cookies['x'] = longVal + overflow; //total size 4097-4099, a warning should be logged
111-
//expect(logs.warn).toEqual(
112-
// [[ "Cookie 'x' possibly not set or overflowed because it was too large (4097 > 4096 " +
113-
// "bytes)!" ]]);
114-
expect(document.cookie).not.toContain(overflow);
107+
cookies['x'] = longVal + overflow;
115108

116-
//force browser to dropped a cookie and make sure that the cache is not out of sync
117109
cookies['x'] = 'shortVal';
118-
expect(cookies['x']).toEqual('shortVal'); //needed to prime the cache
110+
expect(cookies['x']).toEqual('shortVal');
119111
cookieStr = document.cookie;
120-
cookies['x'] = longVal + longVal + longVal; //should be too long for all browsers
112+
113+
// should be too long for all browsers
114+
cookies['x'] = longVal * 3;
121115

122116
if (document.cookie != cookieStr) {
123117
throw "browser didn't drop long cookie when it was expected. make the " +
124-
"cookie in this test longer";
118+
"cookie in this test longer";
125119
}
126120

127121
expect(cookies['x']).toEqual('shortVal');
128122
var errors = (exceptionHandler as LoggingExceptionHandler).errors;
129123
expect(errors.length).toEqual(2);
130-
expect(errors[0].error).
131-
toEqual("Cookie 'x' possibly not set or overflowed because it was too large (4113 > 4096 bytes)!");
132-
expect(errors[1].error).
133-
toEqual("Cookie 'x' possibly not set or overflowed because it was too large (12259 > 4096 bytes)!");
124+
expect(errors[0].error).toEqual("Cookie 'x' possibly not set or overflowed because it was"
125+
" too large (4113 > 4096 bytes)!");
126+
expect(errors[1].error).toEqual("Cookie 'x' possibly not set or overflowed because it was"
127+
" too large (12259 > 4096 bytes)!");
134128
errors.clear();
135129
});
136130
});
@@ -149,12 +143,10 @@ void main() {
149143
});
150144

151145
describe('get via cookies[cookieName]', () {
152-
153146
it('should return null for nonexistent cookie', () {
154147
expect(cookies['nonexistent']).toBe(null);
155148
});
156149

157-
158150
it ('should return a value for an existing cookie', () {
159151
document.cookie = "foo=bar=baz;path=/";
160152
expect(cookies['foo']).toEqual('bar=baz');
@@ -173,30 +165,25 @@ void main() {
173165
expect(cookies['cookie2=bar;baz']).toEqual('val=ue');
174166
});
175167

176-
177168
it('should preserve leading & trailing spaces in names and values', () {
178169
cookies[' cookie name '] = ' cookie value ';
179170
expect(cookies[' cookie name ']).toEqual(' cookie value ');
180171
expect(cookies['cookie name']).toBe(null);
181172
});
182173
});
183174

184-
185175
describe('getAll via cookies(', () {
186-
187176
it('should return cookies as hash', () {
188177
document.cookie = "foo1=bar1;path=/";
189178
document.cookie = "foo2=bar2;path=/";
190179
expect(cookies.all).toEqual({'foo1':'bar1', 'foo2':'bar2'});
191180
});
192181

193-
194182
it('should return empty hash if no cookies exist', () {
195183
expect(cookies.all).toEqual({});
196184
});
197185
});
198186

199-
200187
it('should pick up external changes made to browser cookies', () {
201188
cookies['oatmealCookie'] = 'drool';
202189
expect(cookies.all).toEqual({'oatmealCookie':'drool'});
@@ -205,7 +192,6 @@ void main() {
205192
expect(cookies['oatmealCookie']).toEqual('changed');
206193
});
207194

208-
209195
it('should initialize cookie cache with existing cookies', () {
210196
document.cookie = "existingCookie=existingValue;path=/";
211197
expect(cookies.all).toEqual({'existingCookie':'existingValue'});
@@ -214,8 +200,8 @@ void main() {
214200

215201
describe('cookies service', () {
216202
var cookiesService;
217-
beforeEach((Cookies iCookies) {
218-
cookiesService = iCookies;
203+
beforeEach((Cookies cookies, BrowserCookies bc) {
204+
cookiesService = cookies;
219205
document.cookie = 'oatmealCookie=fresh;path=/';
220206
});
221207

@@ -233,11 +219,11 @@ void main() {
233219
cookiesService["oatmealCookie"] = "stale";
234220
expect(document.cookie).toContain("oatmealCookie=stale");
235221
});
236-
});
237222

238-
it('should remove cookie', () {
239-
cookiesService.remove("oatmealCookie");
240-
expect(document.cookie).not.toContain("oatmealCookie");
223+
it('should remove cookie', () {
224+
cookiesService.remove("oatmealCookie");
225+
expect(document.cookie).not.toContain("oatmealCookie");
226+
});
241227
});
242228
});
243229
});

0 commit comments

Comments
 (0)