Skip to content

Commit

Permalink
feat(pathError hook): add pathError hook for errors identified while
Browse files Browse the repository at this point in the history
merging jsonGraphs

if a route returns a jsonGraph that has a node inside of it that is of
`$type: 'error'`, then during the merge process the `pathError` hook
will be called.

Just add a `hooks.pathError` function to the options argument to
`Router` ctor:

```js
var router = new Router(routes, {
  hooks: {
    pathError: function (pathValue) { }
  }
});
```
  • Loading branch information
benlesh committed Jan 20, 2017
1 parent 3049aa5 commit 816eccc
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/Router.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var Router = function(routes, options) {
this._rst = parseTree(routes);
this._matcher = matcher(this._rst);
this._debug = opts.debug;
this._pathErrorHook = (opts.hooks && opts.hooks.pathError) || noOp;
this._errorHook = (opts.hooks && opts.hooks.error) || noOp;
this.maxRefFollow = opts.maxRefFollow || MAX_REF_FOLLOW;
this.maxPaths = opts.maxPaths || MAX_PATHS;
Expand Down
10 changes: 9 additions & 1 deletion src/cache/jsongMerge.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
var iterateKeySet = require('falcor-path-utils').iterateKeySet;
var types = require('./../support/types');
var $ref = types.$ref;
var $error = types.$error;
var clone = require('./../support/clone');
var cloneArray = require('./../support/cloneArray');
var catAndSlice = require('./../support/catAndSlice');

/**
* merges jsong into a seed
*/
module.exports = function jsongMerge(cache, jsongEnv) {
module.exports = function jsongMerge(cache, jsongEnv, routerInstance) {
var paths = jsongEnv.paths;
var j = jsongEnv.jsonGraph;
var references = [];
var values = [];

paths.forEach(function(p) {
merge({
router: routerInstance,
cacheRoot: cache,
messageRoot: j,
references: references,
Expand Down Expand Up @@ -49,6 +51,12 @@ function merge(config, cache, message, depth, path, fromParent, fromKey) {
if (message === null || typeOfMessage !== 'object' || message.$type) {
fromParent[fromKey] = clone(message);

// If we notice an error while merging, we'll fire the error hook
// for logging purposes.
if (message && message.$type === $error) {
config.router._pathErrorHook({ path: path, value: message });
}

// NOTE: If we have found a reference at our cloning position
// and we have resolved our path then add the reference to
// the unfulfilledRefernces.
Expand Down
2 changes: 1 addition & 1 deletion src/router/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ module.exports = function routerGet(paths) {
mCGRI(out.jsonGraph, [{
jsonGraph: jsonGraphFragment.jsonGraph,
paths: unhandledPaths
}]);
}], router);
return out;
}).
defaultIfEmpty(out);
Expand Down
2 changes: 1 addition & 1 deletion src/router/set.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ module.exports = function routerSet(jsonGraph) {
mCGRI(out.jsonGraph, [{
jsonGraph: unhandledJsonGraphEnv.jsonGraph,
paths: unhandledPaths
}]);
}], router);
return out;
}).
defaultIfEmpty(out);
Expand Down
3 changes: 2 additions & 1 deletion src/run/call/runCallAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ function runCallAction(matchAndPath, routerInstance, callPath, args,

});

var invsRefsAndValues = mCGRI(jsongCache, callOutput);
var invsRefsAndValues =
mCGRI(jsongCache, callOutput, routerInstance);
invsRefsAndValues.references.forEach(function(ref) {
refs[++refLen] = ref;
});
Expand Down
6 changes: 4 additions & 2 deletions src/run/mergeCacheAndGatherRefsAndInvalidations.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ module.exports = mergeCacheAndGatherRefsAndInvalidations;
* @param {Object} cache
* @param {Array} jsongOrPVs
*/
function mergeCacheAndGatherRefsAndInvalidations(cache, jsongOrPVs) {
function mergeCacheAndGatherRefsAndInvalidations(
cache, jsongOrPVs, routerInstance
) {
var references = [];
var len = -1;
var invalidations = [];
Expand All @@ -39,7 +41,7 @@ function mergeCacheAndGatherRefsAndInvalidations(cache, jsongOrPVs) {
}

else if (isJSONG(jsongOrPV)) {
refsAndValues = jsongMerge(cache, jsongOrPV);
refsAndValues = jsongMerge(cache, jsongOrPV, routerInstance);
}

// Last option are path values.
Expand Down
3 changes: 2 additions & 1 deletion src/run/recurseMatchAndExecute.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ function _recurseMatchAndExecute(
value = [value];
}

var invsRefsAndValues = mCGRI(jsongCache, value);
var invsRefsAndValues =
mCGRI(jsongCache, value, routerInstance);
var invalidations = invsRefsAndValues.invalidations;
var unhandled = invsRefsAndValues.unhandledPaths;
var messages = invsRefsAndValues.messages;
Expand Down
2 changes: 1 addition & 1 deletion src/run/set/runSetAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function runSetAction(routerInstance, jsongMessage, matchAndPath, jsongCache) {
paths: [match.requested]
};
arg = {};
jsongMerge(arg, subJsonGraphEnv);
jsongMerge(arg, subJsonGraphEnv, routerInstance);
}
try {
out = match.action.call(routerInstance, arg);
Expand Down
62 changes: 62 additions & 0 deletions test/unit/core/get.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,68 @@ describe('Get', function() {
subscribe(noOp, done, done);
});

it('should fire the error hook if the graph node in it', function (done) {
var callCount = 0;
var callContext = null;
var callArgs = null;
var router = new R([{
route: 'videos[{integers:ids}].title',
get: function (alias) {
return Observable.of({
jsonGraph: {
videos: {
1: {
title: { $type: 'error', value: { message: 'bad luck for you' } }
}
}
}
});
}
}],
{
hooks: {
pathError: function () {
callCount++;
callArgs = Array.prototype.slice.call(arguments, 0);
callContext = this;
}
}
});

router.get([['videos',1,'title']])
.do(
function (jsonGraph) {
expect(jsonGraph).to.deep.equal({
jsonGraph: {
'videos': {
1: {
'title': {
$type: 'error', value: {
message: 'bad luck for you'
}
}
}
}
}
})
},
noOp,
function () {
expect(callCount).to.equal(1);
expect(callArgs).to.deep.equal([{
path: ['videos', 1, 'title'],
value: { $type: 'error', value: { message: 'bad luck for you' } }
}]);
expect(callContext).to.equal(router);
}
)
.subscribe(
noOp,
done,
done
)
});

it('should fire the error hook passed in via options.hooks', function (done) {
var callCount = 0;
var callContext = null;
Expand Down
41 changes: 41 additions & 0 deletions test/unit/internal/jsongMerge.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,47 @@ describe('JSONG - Merge', function() {
}]
});
});

it('should fire the router error hook if $type:"error" is found while walking', function () {
var jsong = {
jsonGraph: {
this: {
is: {
deep: {
yo: { $type: 'error', value: { message: 'lol error' } }
}
}
}
},
paths: [['this', 'is', 'deep', 'yo']]
};
var callArgs = null;
var callCount = 0;
var mockRouter = {
_pathErrorHook: function () {
callCount++;
callArgs = Array.prototype.slice.call(arguments);
}
}
var cache = {};
var out = jsongMerge(cache, jsong, mockRouter);
expect(out).to.deep.equal({
values: [{
path: ['this', 'is', 'deep', 'yo'],
value: { $type: 'error', value: { message: 'lol error' } }
}],
references: []
});
expect(callArgs).to.deep.equals([
{
path: ['this', 'is', 'deep', 'yo'],
value : {
$type: 'error', value: { message: 'lol error'}
}
}
]);
expect(callCount).to.equals(1);
});
});

function mergeTest(jsong) {
Expand Down

0 comments on commit 816eccc

Please sign in to comment.