Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discussion]: Strategies for modular, testable routes #183

Open
eddieajau opened this issue Jun 4, 2016 · 0 comments
Open

[Discussion]: Strategies for modular, testable routes #183

eddieajau opened this issue Jun 4, 2016 · 0 comments

Comments

@eddieajau
Copy link

eddieajau commented Jun 4, 2016

Apologies in advance because I'm struggling to articulate the difficulties I'm facing.

First, our team sees the Falcor router a a really good fit for us to access our data model because, like Netflix, it's split up into many Microservices - it's going to solve a lot of problems we are currently having. However, what's challenging for us is that our application is built up of what could be considered many individual Netflix's (blogs, user management, employee leave, rostering, inventory, sales, etc and so on). So in building our model.json router we want to build up the routes in a modular way, grouping them by our microservices.

For now, we have no alternative but to build a monolithic router with all the routes for the entire application. We will have dozens of routes connecting to different microservices so it's not practical to have all the routes in one huge file. So I'm breaking each Route into individual files accompanied by the respective unit/integration tests for each route.

The problem I'm having is because of the recommended way to include arbitrary request information (the id of the calling user, for example). I initial though, ok, I can just encapsulate by routes in a class.

export class ArticlesLengthRoute implements GetRoute {
  constructor(service) {
    this.service = service;
  }

  route: 'articles.length',

  get: (pathSet) => {
    return this.service.getArticleCount()
      .then((result) => {
        return {
          path: ['articles', 'length'],
          value: result
        }
      });
  }
}

When creating the router, the routes array would look something like:

let routes = [
   new ArticleLengthRoute(articlesService)
]

Ok, that's very neat and encapsulated and I can easier write unit tests for it, but it's not going to work because this in get is set to the router, not my instance of ArticlesLengthRoute (so this.service is not going to work).

Ok, reluctantly I consider plan B which is to use an object factory and I can do something like this:

export function articlesLengthRoute(service) {
  return {
    route: 'articles.length',

    get: (pathSet) => {
      return service.getArticleCount()
        .then((result) => {
          return {
            path: ['articles', 'length'],
            value: result
          }
        });
    }
  }
}

and I can build up my routes array like:

let routes = [
   articlesLengthRoute(articlesService)
]

Ok, I can live with that, but now we need to find only the articles for the user calling the router. Per the documentation I employ the extends Router.createClass(routes) strategy, adding a getter to my custom router class that will expose a support method getUserId in the get method.

export function articlesLengthRoute(service) {
  return {
    route: 'articles.length',

    get: (pathSet) => {
      return service.getArticleCount(this.getUserId())
        .then((result) => {
          return {
            path: ['articles', 'length'],
            value: result
          }
        });
    }
  }
}

I can write a unit test for this along the lines of:

describe('Articles length route', () => {
  it('should work', () => {
    let serviceStub = { getArticleCount: () => Promise.resolve(2) };
    let routerStub = { getUserId: () => { return 101; } );
    let route = articlesLengthRoute(serviceStub);

    return route.get.call(routerStub, ['articles', 'length'])
      .then((result) => {
        assert.equal(result.path, ['articles', 'length'], 'should return the correct path');
        assert.equal(result.value, 2, 'should return the response from the service call');
      });
  });
});

Now, I can get that all to work but I'm not overly happy with it because:

  1. I lose the encapsulation of dependancies I had with my first option (but I can live with an object factory).
  2. The code using the object factory has this this.getUserId() appearing out of nowhere so I really need to put documentation in every route about what this actually refers to.
  3. My unit test has to explicitly manipulate this in the route instance otherwise it won't work.

To make matters worse, we write our source in TypeScript and it's taking a lot of effort to coax the compiler not to throw errors on code that otherwise runs perfectly well in vanilla JavaScript land.

That said, I really do understand why this is the recommended way, but this comes at the expense of a learning curve pertaining to the context in which the route is being invoked. The code is less readable, you loose any design-time type checking help from your IDE (or the TypeScript compiler), and the testing is a little more complicated.

So I guess my question is is there a better way to model the instance of the route without manipulating this?

Shooting from the hip, what if the route's method took the router as an argument.

export function articlesLengthRoute(service) {
  return {
    route: 'articles.length',

    get: (pathSet, router) => {
      return service.getArticleCount(router.getUserId())
        .then((result) => {
          return {
            path: ['articles', 'length'],
            value: result
          }
        });
    }
  }
}

Ok now I can inject a router stub into the method invocation rather than apply magic to this, and in TypeScript I can add a type to my router which gives me type hinting for getUserId. This would be fully backward compatible - existing code can still work on the premise that this is the router, or new code could optionally use the instance of the router from the argument list.

Thoughts?

@eddieajau eddieajau changed the title [Discussion]: Strategies for modular routes that are easier to test [Discussion]: Strategies for modular, testable routes Jun 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant