add sync API

[oliverbrooks]

Working sync API. Let me know what you think.

[orangemug]

@oliverbrooks at first look there is too much duplication between the sync and async API. The only bit that actually needs to be async are the file operations.

There has also been some loss of functionality for example errors.NoSuchTemplate no longer seems to be thrown in sync

The sync operations are tested only the end-to-end functionality, not the individual operations, which I think would have caught the error above.

I think there are too many changes here and we should take a lighter step by step approach. Lets split this into a few steps

1. PR to inline all things which don't actually need to be promises at the moment, there are a few of these about the codebase
   • This will also increase performance because all promises are async even if they are not actually async in the function handler
2. Lets decide on a strategy to make these promises 'sync' once we work out how many we have to factor out
   • At first looks there are only a couple which are actually needed. We could write a wrapper to 'sync' the promise chain for example. With that approach would probably only be a few lines of code change.

[oliverbrooks]

The only async parts are to do with fs calls. These were:

• lib/parser/index.js parse function
• require operators (which call the above parse function)

I've refactored the "require" operator to no longer do the async bit in the operator but as the parse function is the core of the require recursion I'm not sure how to refactor it out without a re-write of all the token parsing logic, hence the duplication.

The issue in sharing code between sync and async is building the output and error stack in the context of iteration. The async flow builds an array of promises adding custom error handling where appropriate to add to the error stack. Sync on the other hand uses custom try/catch statements to build the errors.

Refactor idea 1: Abstract token parsing

One way to remove the duplication might be to abstract the regex parsing and token creation so that whether async or sync a set of tokens is returned.

• The async flow could use Bluebird.resolve to turn the tokens into promises and then Bluebird.reflect to return a value whether it succeeded or failed. The resulting promise reflections could be used to manually build the output and error stack.
• The sync flow could run each token building the output and catching errors in try/catch.

Refactor idea 2: Build sql then handle tokens

As the only async part is the file loading the async/sync flows could walk the SQL building a single large SQL file. It could then use common, synchronous tokenising logic.

The SQL building would need to respect the param parsing effectively doing string replacements as it requires files into the master.

Any thoughts on ways to do this?

[orangemug]

One way to remove the duplication might be to abstract the regex parsing and token creation so that whether async or sync a set of tokens is returned.

I think we almost have this here https://github.com/orangemug/sql-stamp/blob/master/lib/parser/index.js#L175-L184

I think we maybe just need to flatten our approach the real issue is with require being nested if we can flatten this I think the problem goes away

Maybe if this

if(!this.templates.hasOwnProperty(filepath)) {
  return Bluebird.props({
    template: this.recurse(filepath)
  });
} else {
  return Bluebird.props({
    template: this.templates[filepath]
  });
}

Became

if(!this.templates.hasOwnProperty(filepath)) {
  this.addForParsing(filepath)
}
return {
  template: this.templates[filepath]
};

Note addForParsing would need to store the parse location for error reporting, but that seems ok.

Then our tasks would be a flat list which should make the problem a lot simpler