Skip to content

Commit

Permalink
Fixed falsey atom values being treated missing paths
Browse files Browse the repository at this point in the history 
Paths which returned falsey values (null, 0, false, '') were being
treated as missing paths, and hence being returned as empty atoms.

See: Netflix/falcor#460

Added unit tests, and get integration test for these falsey value
cases.
  • Loading branch information
@sdesai
sdesai committed Aug 20, 2015
1 parent 7af36ac commit 248cc80
Show file tree
Hide file tree
Showing 4 changed files with 205 additions and 9 deletions.
14 changes: 7 additions & 7 deletions src/cache/optimizePathSets.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,20 @@ function optimizePathSet(cache, cacheRoot, pathSet,
depth, out, optimizedPath, maxRefFollow) {

// at missing, report optimized path.
if (!cache) {
if (cache === undefined) {
out[out.length] = catAndSlice(optimizedPath, pathSet, depth);
return;
}

// If the reference is the last item in the path then do not
// continue to search it.
if (cache.$type === $ref && depth === pathSet.length) {
// all other sentinels are short circuited.
// Or we found a primitive (which includes null)
if (cache === null || (cache.$type && cache.$type !== $ref) || (typeof cache !== 'object')) {
return;
}

// all other sentinels are short circuited.
// Or we found a primitive.
if (cache.$type && cache.$type !== $ref || typeof cache !== 'object') {
// If the reference is the last item in the path then do not
// continue to search it.
if (cache.$type === $ref && depth === pathSet.length) {
return;
}

Expand Down
61 changes: 60 additions & 1 deletion test/data/VideoRoutes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,66 @@ module.exports = function() {
}
}];
}
}
},
Falsey: function () {
return [{
route: 'videos.falsey.zero',
get: function(path) {
return Observable.return({
jsonGraph: {
videos: {
falsey: {
zero: $atom(0)
}
}
},
paths: [['videos', 'falsey', 'zero']]
});
}
}, {
route: 'videos.falsey.null',
get: function(path) {
return Observable.return({
jsonGraph: {
videos: {
falsey: {
'null': $atom(null)
}
}
},
paths: [['videos', 'falsey', 'null']]
});
}
}, {
route: 'videos.falsey.emptystring',
get: function(path) {
return Observable.return({
jsonGraph: {
videos: {
falsey: {
'emptystring': $atom('')
}
}
},
paths: [['videos', 'falsey', 'emptystring']]
});
}
}, {
route: 'videos.falsey.false',
get: function(path) {
return Observable.return({
jsonGraph: {
videos: {
falsey: {
'false': $atom(false)
}
}
},
paths: [['videos', 'falsey', 'false']]
});
}
}];
},
};
};

Expand Down
98 changes: 98 additions & 0 deletions test/unit/core/get.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ var chai = require('chai');
var expect = chai.expect;
var falcor = require('falcor');
var $ref = falcor.Model.ref;
var $atom = require('./../../../src/support/types').$atom;
var Observable = require('rx').Observable;
var sinon = require('sinon');

describe('Get', function() {

it('should execute a simple route matching.', function(done) {
var router = new R(Routes().Videos.Summary());
var obs = router.
Expand All @@ -24,6 +26,102 @@ describe('Get', function() {
});
});

it('should not return empty atoms for a null value', function(done) {
var router = new R(Routes().Videos.Falsey());
var onNext = sinon.spy();

router.get([['videos', 'falsey', 'null']]).
doAction(onNext).
doAction(noOp, noOp, function() {
expect(onNext.calledOnce).to.be.ok;
expect(onNext.getCall(0).args[0]).to.deep.equals({
jsonGraph: {
videos: {
falsey: {
'null': {
$type: $atom,
value: null
}
}
}
}
});
}).
subscribe(noOp, done, done);
});

it('should not return empty atoms for a zero value', function(done) {
var router = new R(Routes().Videos.Falsey());
var onNext = sinon.spy();

router.get([['videos', 'falsey', 'zero']]).
doAction(onNext).
doAction(noOp, noOp, function() {
expect(onNext.calledOnce).to.be.ok;
expect(onNext.getCall(0).args[0]).to.deep.equals({
jsonGraph: {
videos: {
falsey: {
zero: {
$type: $atom,
value: 0
}
}
}
}
});
}).
subscribe(noOp, done, done);
});

it('should not return empty atoms for an empty string value', function(done) {
var router = new R(Routes().Videos.Falsey());
var onNext = sinon.spy();

router.get([['videos', 'falsey', 'emptystring']]).
doAction(onNext).
doAction(noOp, noOp, function() {
expect(onNext.calledOnce).to.be.ok;
expect(onNext.getCall(0).args[0]).to.deep.equals({
jsonGraph: {
videos: {
falsey: {
emptystring: {
$type: $atom,
value: ''
}
}
}
}
});
}).
subscribe(noOp, done, done);
});

it('should not return empty atoms for a false value', function(done) {
var router = new R(Routes().Videos.Falsey());
var onNext = sinon.spy();

router.get([['videos', 'falsey', 'false']]).
doAction(onNext).
doAction(noOp, noOp, function() {
expect(onNext.calledOnce).to.be.ok;
expect(onNext.getCall(0).args[0]).to.deep.equals({
jsonGraph: {
videos: {
falsey: {
'false': {
$type: $atom,
value: false
}
}
}
}
});
}).
subscribe(noOp, done, done);
});

it('should validate that optimizedPathSets strips out already found data.', function(done) {
this.timeout(10000);
var serviceCalls = 0;
Expand Down
41 changes: 40 additions & 1 deletion test/unit/internal/optimizePathSets.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,33 @@ describe('optimizePathSets', function() {
expect(out).to.deep.equal(expected);
});

it('should short circuit on primitive string values', function() {
var cache = getCache();
var paths = [['videos', '6', 'summary']];

var out = optimizePathSets(cache, paths);
var expected = [];
expect(out).to.deep.equal(expected);
});

it('should short circuit on primitive number values', function() {
var cache = getCache();
var paths = [['videos', '7', 'summary']];

var out = optimizePathSets(cache, paths);
var expected = [];
expect(out).to.deep.equal(expected);
});

it('should short circuit on primitive boolean values', function() {
var cache = getCache();
var paths = [['videos', '8', 'summary']];

var out = optimizePathSets(cache, paths);
var expected = [];
expect(out).to.deep.equal(expected);
});

it('should throw.', function() {
var cache = getCache();
var paths = [['videosList', 'inner', 'summary']];
Expand All @@ -79,6 +106,7 @@ describe('optimizePathSets', function() {
}
expect(caught).to.equals(true);
});

});

function getCache() {
Expand All @@ -91,7 +119,18 @@ function getCache() {
inner: $ref('videosList[3].inner')
},
videos: {
5: $atom('title')
5: $atom('title'),

// Short circuit on primitives
6: $atom('a'),
7: $atom(1),
8: $atom(true),

// Falsey edge cases
9: $atom(''),
10: $atom(0),
11: $atom(false),
12: $atom(null)
}
};
}

0 comments on commit 248cc80

Please sign in to comment.