Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Commit

Permalink
Expose Path.get and support Paths as arguments to PathObserver and Co…
Browse files Browse the repository at this point in the history 
…mpoundPath observer

R=arv
BUG=

Review URL: https://codereview.appspot.com/13272046
  • Loading branch information
@rafaelw
rafaelw committed Sep 3, 2013
1 parent 6c7b0cf commit 4190641
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 102 deletions.
68 changes: 34 additions & 34 deletions src/observe.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,26 +105,17 @@
return pathRegExp.test(s);
}

// TODO(rafaelw): Make simple LRU cache
var pathCache = {};
var constructorIsPrivate = {};

function getPath(str) {
var path = pathCache[str];
if (path)
return path;
if (!isPathValid(str))
return;
var path = new Path(str);
pathCache[str] = path;
return path;
}
function Path(s, privateToken) {
if (privateToken !== constructorIsPrivate)
throw Error('Use Path.get to retrieve path objects');

function Path(s) {
if (s.trim() == '')
return this;

if (isIndex(s)) {
this.push(String(s));
this.push(s);
return this;
}

Expand All @@ -139,7 +130,30 @@
}
}

Path.isValid = isPathValid;
// TODO(rafaelw): Make simple LRU cache
var pathCache = {};

function getPath(pathString) {
if (pathString instanceof Path)
return pathString;

if (pathString == null)
pathString = '';

if (typeof pathString !== 'string')
pathString = String(pathString);

var path = pathCache[pathString];
if (path)
return path;
if (!isPathValid(pathString))
return;
var path = new Path(pathString, constructorIsPrivate);
pathCache[pathString] = path;
return path;
}

Path.get = getPath;

Path.prototype = createObject({
__proto__: [],
Expand Down Expand Up @@ -588,9 +602,10 @@
}
};

function PathObserver(object, pathString, callback, target, token, valueFn,
function PathObserver(object, path, callback, target, token, valueFn,
setValueFn) {
var path = getPath(pathString);
var path = path instanceof Path ? path : getPath(path);

if (!path) {
// Invalid path.
this.closed = true;
Expand Down Expand Up @@ -686,21 +701,6 @@
}
});

PathObserver.getValueAtPath = function(obj, pathString) {
var path = getPath(pathString);
if (!path)
return;
return path.getValueFrom(obj);
}

PathObserver.setValueAtPath = function(obj, pathString, value) {
var path = getPath(pathString);
if (!path)
return;

path.setValueFrom(obj, value);
};

function CompoundPathObserver(callback, target, token, valueFn) {
Observer.call(this, undefined, callback, target, token);
this.valueFn = valueFn;
Expand All @@ -713,11 +713,11 @@
CompoundPathObserver.prototype = createObject({
__proto__: PathObserver.prototype,

addPath: function(object, pathString) {
addPath: function(object, path) {
if (this.started)
throw Error('Cannot add more paths once started.');

var path = getPath(pathString);
var path = path instanceof Path ? path : getPath(path);
var value = undefined;

if (!path) {
Expand Down
199 changes: 131 additions & 68 deletions tests/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,112 @@ var createObject = ('__proto__' in {}) ?
return newObject;
};

suite('Path', function() {
test('constructor throws', function() {
assert.throws(function() {
new Path('foo')
});
});

test('valid paths', function() {
assert.isDefined(Path.get('a'));
assert.isDefined(Path.get('a.b'));
assert.isDefined(Path.get('a. b'));
assert.isDefined(Path.get('a .b'));
assert.isDefined(Path.get('a . b'));
assert.isDefined(Path.get(''));
assert.isDefined(Path.get(' '));
assert.isDefined(Path.get(null));
assert.isDefined(Path.get(undefined));
assert.isDefined(Path.get());
assert.isDefined(Path.get(42));
});

test('invalid paths', function() {
assert.isUndefined(Path.get('a b'));
assert.isUndefined(Path.get('.'));
assert.isUndefined(Path.get(' . '));
assert.isUndefined(Path.get('..'));
});

test('Paths are interned', function() {
var p = Path.get('foo.bar');
var p2 = Path.get('foo.bar');
assert.strictEqual(p, p2);

var p3 = Path.get('');
var p4 = Path.get('');
assert.strictEqual(p3, p4);
});

test('null is empty path', function() {
assert.strictEqual(Path.get(''), Path.get(null));
});

test('undefined is empty path', function() {
assert.strictEqual(Path.get(undefined), Path.get(null));
});

test('Path.getValueFrom', function() {
var obj = {
a: {
b: {
c: 1
}
}
};

var p1 = Path.get('a');
var p2 = Path.get('a.b');
var p3 = Path.get('a.b.c');

assert.strictEqual(obj.a, p1.getValueFrom(obj));
assert.strictEqual(obj.a.b, p2.getValueFrom(obj));
assert.strictEqual(1, p3.getValueFrom(obj));

obj.a.b.c = 2;
assert.strictEqual(2, p3.getValueFrom(obj));

obj.a.b = {
c: 3
};
assert.strictEqual(3, p3.getValueFrom(obj));

obj.a = {
b: 4
};
assert.strictEqual(undefined, p3.getValueFrom(obj));
assert.strictEqual(4, p2.getValueFrom(obj));
});

test('Path.setValueFrom', function() {
var obj = {};
var p2 = Path.get('bar');

Path.get('foo').setValueFrom(obj, 3);
assert.equal(3, obj.foo);

var bar = { baz: 3 };

Path.get('bar').setValueFrom(obj, bar);
assert.equal(bar, obj.bar);

var p = Path.get('bar.baz.bat');
p.setValueFrom(obj, 'not here');
assert.equal(undefined, p.getValueFrom(obj));
});

test('Degenerate Values', function() {
var emptyPath = Path.get();
var foo = {};

assert.equal(null, emptyPath.getValueFrom(null));
assert.equal(foo, emptyPath.getValueFrom(foo));
assert.equal(3, emptyPath.getValueFrom(3));
assert.equal(undefined, Path.get('a').getValueFrom(undefined));
});
});

suite('Basic Tests', function() {

test('Exception Doesnt Stop Notification', function() {
Expand Down Expand Up @@ -233,31 +339,27 @@ suite('PathObserver Tests', function() {
});

test('Degenerate Values', function() {
var emptyPath = Path.get();
observer = new PathObserver(null, '', callback);
assert.equal(null, observer.value);
assert.equal(null, PathObserver.getValueAtPath(null, ''));
observer.close();

var foo = {};
observer = new PathObserver(foo, '', callback);
assert.equal(foo, observer.value);
assert.equal(foo, PathObserver.getValueAtPath(foo, ''));
observer.close();

observer = new PathObserver(3, '', callback);
assert.equal(3, observer.value);
assert.equal(3, PathObserver.getValueAtPath(3, ''));
observer.close();

observer = new PathObserver(undefined, 'a', callback);
assert.equal(undefined, observer.value);
assert.equal(undefined, PathObserver.getValueAtPath(undefined, 'a'));
observer.close();

var bar = { id: 23 };
observer = new PathObserver(undefined, 'a/3!', callback);
assert.equal(undefined, observer.value);
assert.equal(undefined, PathObserver.getValueAtPath(bar, 'a/3!'));
observer.close();
});

Expand All @@ -274,61 +376,21 @@ suite('PathObserver Tests', function() {
observer.close();
});

test('Path GetValueAtPath', function() {
var obj = {
a: {
b: {
c: 1
}
}
};

assert.strictEqual(obj.a, PathObserver.getValueAtPath(obj, 'a'));
assert.strictEqual(obj.a.b, PathObserver.getValueAtPath(obj, 'a.b'));
assert.strictEqual(1, PathObserver.getValueAtPath(obj, 'a.b.c'));

obj.a.b.c = 2;
assert.strictEqual(2, PathObserver.getValueAtPath(obj, 'a.b.c'));

obj.a.b = {
c: 3
};
assert.strictEqual(3, PathObserver.getValueAtPath(obj, 'a.b.c'));

obj.a = {
b: 4
};
assert.strictEqual(undefined, PathObserver.getValueAtPath(obj, 'a.b.c'));
assert.strictEqual(4, PathObserver.getValueAtPath(obj, 'a.b'));
});

test('Path SetValueAtPath', function() {
var obj = {};
PathObserver.setValueAtPath(obj, 'foo', 3);
assert.equal(3, obj.foo);

var bar = { baz: 3 };

PathObserver.setValueAtPath(obj, 'bar', bar);
assert.equal(bar, obj.bar);

PathObserver.setValueAtPath(obj, 'bar.baz.bat', 'not here');
assert.equal(undefined, PathObserver.getValueAtPath(obj, 'bar.baz.bat'));
});

test('Path Set Value Back To Same', function() {
var obj = {};
PathObserver.setValueAtPath(obj, 'foo', 3);
var path = Path.get('foo');

path.setValueFrom(obj, 3);
assert.equal(3, obj.foo);

observer = new PathObserver(obj, 'foo', callback);
assert.equal(3, observer.value);

PathObserver.setValueAtPath(obj, 'foo', 2);
path.setValueFrom(obj, 2);
observer.reset();
assert.equal(2, observer.value);

PathObserver.setValueAtPath(obj, 'foo', 3);
path.setValueFrom(obj, 3);
observer.reset();
assert.equal(3, observer.value);

Expand Down Expand Up @@ -368,6 +430,24 @@ suite('PathObserver Tests', function() {
observer.close();
});

test('Path Simple - path object', function() {
var model = { };

var path = Path.get('foo');
observer = new PathObserver(model, path, callback);

model.foo = 1;
assertPathChanges(1, undefined);

model.foo = 2;
assertPathChanges(2, 1);

delete model.foo;
assertPathChanges(undefined, 2);

observer.close();
});

test('valueFn', function() {
var model = { };

Expand Down Expand Up @@ -752,7 +832,7 @@ suite('CompoundPathObserver Tests', function() {
valueFn);
observer.addPath(model, 'a');
observer.addPath(model, 'b');
observer.addPath(model, 'c');
observer.addPath(model, Path.get('c'));
observer.start();

assert.strictEqual(6, observer.value);
Expand Down Expand Up @@ -1701,21 +1781,4 @@ suite('ObjectObserver Tests', function() {

observer.close();
});

test('Path.isValid', function() {
assert.isTrue(Path.isValid('a'));
assert.isTrue(Path.isValid('a.b'));
assert.isTrue(Path.isValid('a. b'));
assert.isTrue(Path.isValid('a .b'));
assert.isTrue(Path.isValid('a . b'));

assert.isFalse(Path.isValid(42));
assert.isTrue(Path.isValid(''));
assert.isTrue(Path.isValid(' '));

assert.isFalse(Path.isValid('a b'));
assert.isFalse(Path.isValid('.'));
assert.isFalse(Path.isValid(' . '));
assert.isFalse(Path.isValid('..'));
});
});

2 comments on commit 4190641

@peterwmwong
Copy link
Contributor

Choose a reason for hiding this comment

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

@rafaelw
Looks like polymer/polymer is busted due to the removal of PathObserver.getValueAtPath() and PathObserver.setValueAtPath() at https://github.com/Polymer/polymer/blob/318a26011c3f001463bad7f2864cd4a88a4cc909/src/instance/properties.js#L84-87

var path = Path.get(inPath);
var v = path.getValueFrom(inB);
if (v === null || v === undefined) {
  path.setValueFrom(inB, inA[inProperty]);
}

Sorry about the nag if you already know :)

@rafaelw
Copy link
Contributor Author

@rafaelw rafaelw commented on 4190641 Sep 4, 2013

Choose a reason for hiding this comment

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

Dan & Steve know. They are going to fix it. Thanks for the attention, though!

Please sign in to comment.