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

Feature Request: First Class API Stubs #318

Open
davemo opened this issue Sep 3, 2014 · 14 comments
Open

Feature Request: First Class API Stubs #318

davemo opened this issue Sep 3, 2014 · 14 comments

Comments

@davemo
Copy link
Member

davemo commented Sep 3, 2014

I wanted to create this to capture the thoughts while they are fresh in my mind after context on a client project where this would have saved some time:

I think we should formalize a config/stubs directory which has the following properties:

  1. is watched for changes to files within, triggering a restart of express on change
  2. captures syntax errors in watched stub files and catches the exception to avoid killing express
  3. (optional) loosely formalize a blessed JSON stub format (simplest thing is module.exports = {} for now), we don't need to get too complicated here
  4. (optional) auto require stubs based on filename such that they are available within config/server.{js,coffee} either as a local in the scope or on the node global object

The first two would be mostly quality of life improvements to the way I use the existing API stubbing feature, the latter two are things that would be really cool if we could make them not suck in implementation ;)

If anyone is willing to pair with me on this, I'd like to start with the first two which are captured in #268.

@jasonkarns
Copy link
Member

I am in favor of this and I recall an old feature request being opened to
do something similar. (I believe it was closed with the advice of simply
using require from config/server.js)

Should we consider placing the stubs under /spec? I realize that would
complicate the glob patterns for including spec files and helpers. But
stubs used for testing make more sense (to me) under /spec than /config.

On Wed, Sep 3, 2014 at 9:31 AM, David Mosher notifications@github.com
wrote:

I wanted to create this to capture the thoughts while they are fresh in my
mind after context on a client project where this would have saved some
time:

I think we should formalize a config/stubs directory which has the
following properties:

  1. is watched for changes to files within, triggering a restart of
    express on change
  2. captures syntax errors in watched stub files and catches the
    exception to avoid killing express
  3. (optional) loosely formalize a blessed JSON stub format (simplest
    thing is module.exports = {} for now), we don't need to get too
    complicated here
  4. (optional) auto require stubs based on filename such that they are
    available within config/server.{js,coffee} either as a local in the
    scope or on the node global object

The first two would be mostly quality of life improvements to the way I
use the existing API stubbing feature, the latter two are things that would
be really cool if we could make them not suck in implementation ;)

If anyone is willing to pair with me on this, I'd like to start with the
first two which are captured in #268
#268.


Reply to this email directly or view it on GitHub
#318.

@searls
Copy link
Member

searls commented Sep 3, 2014

Since these stubs would be mostly/entirely used for the dev server and not for tests, I don't think it makes sense for them to be stored under spec. Since our testing facility only covers unit tests, I don't want to encourage people to start using fixtures in external files.

@searls
Copy link
Member

searls commented Sep 3, 2014

As I re-read Dave's ticket, it sounds like this is a way to auto-break up the server file into n smaller server files, and not necessarily an approach for stubbings at a particular route.

But I'm pretty sure I'm just not clear. Dave could you put together a gist or small repo of files/folders of how you'd like to use this feature and then explain what they'd do (e.g. "config/server/stubs/foo/bar/baz.json adds a GET /foo/bar/baz route to express and returns baz.json")?

@jasonkarns
Copy link
Member

@searls good call on the fixtures

@davemo
Copy link
Member Author

davemo commented Sep 4, 2014

@searls, what you describe in breaking up the config/server.js is part of the larger and more difficult task in 3, and 4 above; here's an idea of what that might look like in code-ish form:

  • user adds the file path config/stubs/users/get.js and config/stubs/users/post.js which contains something like module.exports = { id: 1, email: 'foo@bar.com };
  • lineman detects that and auto wires up GET /api/users and POST /api/users to return respective results
  • the mechanics of convention is up for debate as is whether it's worth going this far

In a simpler scenario, all I really want at this point is:

  • user adds config/stubs/user.js
  • lineman adds a watch glob for config/stubs such that when the file changes express restarts
  • AND exceptions in require('path/to/a/stub.js);` are caught and handled in the console output to notify users that something in their stub is syntactically incorrect.

@searls
Copy link
Member

searls commented Sep 4, 2014

So in your simpler example what does stubs/user.js export? A function that takes an express app and does [THINGS] to it? Or something more constrained?

On Thu, Sep 4, 2014 at 12:32 PM, David Mosher notifications@github.com
wrote:

@searls, what you describe in breaking up the config/server.js is part of the larger and more difficult task in 3, and 4 above; here's an idea of what that might look like in code-ish form:

  • user adds the file path config/stubs/users/get.js and config/stubs/users/post.js which contains something like module.exports = { id: 1, email: 'foo@bar.com };
  • lineman detects that and auto wires up GET /api/users and POST /api/users to return respective results
  • the mechanics of convention is up for debate as is whether it's worth going this far
    In a simpler scenario, all I really want at this point is:
  • user adds config/stubs/user.js
  • lineman adds a watch glob for config/stubs such that when the file changes express restarts

* AND exceptions in require('path/to/a/stub.js);` are caught and handled in the console output to notify users that something in their stub is syntactically incorrect.

Reply to this email directly or view it on GitHub:
#318 (comment)

@davemo
Copy link
Member Author

davemo commented Sep 4, 2014

stubs/user.js is the same in the simpler example, i just manually manage routing in config/server.js as per normal.

@searls
Copy link
Member

searls commented Sep 4, 2014

I'm still not getting it, then.

@searls
Copy link
Member

searls commented Sep 4, 2014

Could you post an example listing of stubs/user.js and config/server?

@davemo
Copy link
Member Author

davemo commented Sep 4, 2014

config/server

var users = require('./stubs/users.js');

module.exports = {
  drawRoutes: function(app) {
    app.get('/api/users', function(req, res) { res.json(users); });
  }
};

stubs/users.js

module.exports = {
  users: [
    { id:1, name: 'Dave' },
    { id:2, name: 'Justin' }
  ]
}

@searls
Copy link
Member

searls commented Sep 4, 2014

oh, well in that case I think what you're asking for is much simpler and less opinionated than what I thought you were asking for.

I think the default behavior absolutely should explode on errors, though, to the same extent a compilation failure does. If one particular API starts barfing and we don't tell people, they won't know where to look. And there's no good reason for them to start exploding. (If all you're saying is you don't want the process to blow up, I agree)

Error handling generally should probably give feedback in the browser with a lineman error page and not just in the terminal.

@jasonkarns
Copy link
Member

Here's a thing that's somewhat related. worth a look: http://webpro.github.io/dyson/

@davemo
Copy link
Member Author

davemo commented Nov 7, 2014

I'm going to post some of the api stubs we're using in a current project for reference.

There are some interesting usage patterns I've discovered working through these;

  • I comment/uncomment to turn on/off stubs as features move from stubbed to integrating with the real backend.
  • I leave the commented ones and stubfiles in place because it's a nice way to be able to look at the shape of the data that describes the API; we use the stubs to dictate how the backend will look so they are still useful to have around after finishing and integrating the feature end to end.
  • The most annoying part of working in this way is having to restart lineman when a stubbed file changes; changes to anything within drawRoutes seems to be detected with the watchr setup internally in Lineman, but anything that is required within the file that changes is not monitored.
  • Syntax errors in require'd files are swallowed :(
  • We frequently create small functions that use underscore to update the stub data structures in memory to act as a temporary "database" to prove the UPDATE or SAVE portion of a feature works during demo.
var _     = require('underscore');

// var drillingData = require('./stubs/execution/drilling_data.js');
// var upToBatData = require('./stubs/uptobat/uptobat_data.js');
// var upToBatGridsData = require('./stubs/uptobat/uptobat_grids_data.js');
// var upToBatFilterData = require('./stubs/uptobat/uptobat_filter_data.js');
// var projectIssuesOptionsData = require('./stubs/issues/project_issues_options_data.js');
// var drillingExecutionGridWidgetOptions = require('./stubs/execution/drilling_grid_widget_options.js');
// var includesCopyButtonJson = require('./stubs/uptobat/includes_copy_button_configuration.js');
var parameters             = require('./stubs/admin/parameters.js');
var notifications          = require('./stubs/admin/notifications.js');
var notificationRecipients = require("./stubs/admin/notification_recipients.js");
var notificationTriggers   = require("./stubs/admin/notification_triggers.js");
var uptobatCounts          = require('./stubs/uptobat/uptobat_counts.js');

module.exports = {
  drawRoutes: function(app) {
    // ADMIN SCREEN STUBS

    // uptobat counts

    app.get('/api/uptobats/counts', function (req, res) {
        console.log("\n *** LINEMAN STUB: GET /api/uptobat/counts");
        _.each(uptobatCounts, function(v, k){
          uptobatCounts[k] = v + 1;
        });
        res.json(uptobatCounts);
    });

    // parameters
    app.get('/api/admin/parameters/:buid', function(req, res) {
        console.log("\n *** LINEMAN STUB: GET /api/admin/parameters/" + req.params.buid);
        res.json(parameters);
    });

    app.post('/api/admin/parameters/:buid', function (req, res) {
        console.log("\n *** LINEMAN STUB: POST /api/admin/parameters/" + req.params.buid);
        _.extend(parameters, req.body.data);
        res.json(parameters);
    });

    var logEvents = [];
    app.post('/api/log', function(req, res) {
        logEvents.push(req.body);
        console.log("\n *** LOG EVENT LINEMAN STUB ***\n", req.body);
        res.json({status: "ok"});
    });

    app.get('/api/log', function(req, res) {
        res.json(logEvents);
    });

    // notifications
    app.post('/api/admin/notifications/:buid', function (req, res) {
        console.log("\n *** LINEMAN STUB: POST /api/admin/notifications/" + req.params.buid);
        res.json(notifications);
    });

    app.get('/api/admin/notifications/:nid', function (req, res) {
        console.log("\n *** LINEMAN STUB: GET /api/notifications//" + req.params.nid);
        var notification = getNotificationById(req.params.nid);
        res.json(notification);
    });

    app.get('/api/admin/notifications/:nid/recipients', function (req, res) {
        console.log("\n *** LINEMAN STUB: GET /api/notifications/" + req.params.nid + "/recipients");
        res.json(notificationRecipients);
    });

    app.get('/api/admin/notifications/:nid/triggers', function (req, res) {
        console.log("\n *** LINEMAN STUB: GET /api/notifications/" + req.params.nid + "/triggers");
        res.json(notificationTriggers);
    });

    app.put('/api/admin/notifications/:nid/recipients/update', function (req, res) {
        console.log("\n *** LINEMAN STUB: ---> NOTIFICATION RECIPIENT SAVED", req.body.form.nid);
        res.json(req.body.form);
    });

    app.post('/api/admin.notifications/:nid', function(req, res) {
        console.log("\n *** LINEMAN STUB: ---> EMAIL TEMPLATE SAVED");
        var notification = getNotificationById(req.params.nid);
        notification.subject = req.body.subject;
        notification.name = req.body.name;
        notification.body = req.body.body;
        notification.triggers = req.body.triggers;
        notification.recipients = req.body.recipients;
        res.json(notification);
    });

    function getNotificationById(id) {
       for (var key in notifications.data) {
            var notification = notifications.data[key];
            if (notification.id == id) {
                return notification;
            }
       }
    }
    // app.put('/api/data/uptobat/:statenamespace/:uptobatId/:buid', function(req, res){
    //   console.log('\n *** UPDATE up to bat row ***');
    //   console.log('\n *** LINEMAN STUB: PUT /api/data/uptobat/:statenamespace/:uptobatId/:buid ***');
    //   var objToUpdate = _.find(upToBatGridsData, function(obj){
    //     return obj.id == req.body.models[0].id;
    //   });

    //   _.extend(objToUpdate, req.body.models[0]);
    //   res.json(objToUpdate);
    // });

    // app.post('/api/data/uptobat/:statenamespace/:uptobatId/:buid', function(req, res){
    //   console.log('\n *** LINEMAN STUB: POST /api/data/uptobat/' + req.params.statenamespace + '/' + req.params.uptobatId + '/' + req.params.buid + ' *** \n');
    //   res.json(upToBatGridsData);
    // });

    // app.put('/api/data/execution.drilling/:buid', function(req, res) {
    //   console.log('\n *** LINEMAN STUB: PUT /api/data/execution.drilling/' + req.params.buid + ' *** \n');
    //   res.json(req.body);
    // });

    // app.get('/api/display/widget/options/execution.drilling/drilling-grid/:buid', function (req, res) {
    //   console.log('\n *** LINEMAN STUB: GET /api/display/widget/options/execution.drilling/drilling-grid/' + req.params.buId + ' *** \n');
    //   res.json(drillingExecutionGridWidgetOptions);
    // });

    // app.post('/api/data/execution.drilling/:buid', function(req, res){
    //   console.log('\n *** LINEMAN STUB: POST /api/data/execution.drilling/' + req.params.buid + ' *** \n');
    //   res.json(drillingData);
    // });

    // app.get('/api/session', function(req, res) {
    //   console.log('\n *** LINEMAN STUB: GET /api/session *** \n');
    //   res.json(session);
    // });

    // app.post('/api/user/profile', function(req, res) {
    //   _.extend(session.user.profile, req.body);
    //   console.log('\n *** LINEMAN STUB: POST /api/userprofile: persisting session.user.profile in memory *** \n');
    //   console.log(session.user.profile);
    //   res.send(200);
    // });
  }
};

@BurningDog
Copy link

The most annoying part of working in this way is having to restart lineman when a stubbed file changes; changes to anything within drawRoutes seems to be detected with the watchr setup internally in Lineman, but anything that is required within the file that changes is not monitored.

Agreed. I'm not sure how to configure lineman to watch an additional folder for changes, but I edited node_modules/lineman/tasks/server.coffee and changed the following lines from

  resetRoutesOnServerConfigChange = (app) ->
    watchr grunt.file.expand('config/server.*'), (err, watcher) ->

to

  resetRoutesOnServerConfigChange = (app) ->
    watchr grunt.file.expand('config/server.*', 'config/stubs/**/*'), (err, watcher) ->

This allows for file changes without restarting lineman, but for some reason livereload doesn't trigger. I'd love to know how to put this change into config/application.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants