Monthly Archives: October 2014

Promises vs Callbacks - Code comparison

Promises vs Callbacks – Code comparison

I am not going to highlight pros of promises and cons of callbacks. There is plenty of good reading about this topic out there. I was recently writing simple Node module and decided to learn promises during its implementation. The result  can be interesting as comparison of Promises vs Callbacks approach applied to the same problems, because project contains

These are glued with Gulp build system to execute tests with all possible combinations. So for example Callbacks based test is executed also against Promises based module.

I named the project Jasstor. I am planning to use it for storage credentials into JSON file, hashing and verification of credentials. It stores user name, hashed password and user’s role in string format. Here is Github branch dedicated to this blog post, so that it’ll stay consistent. Please bear in mind, that I am learning Node development, ES6 features, Promises and Gulp on this project. So I could easily miss handy tricks or misused some features.

Project uses these main technologies:

I decided to have these constraints for Promises

  • Mocha will be excluded from promisification, so describe and it will be used with callbacks.
  • Jasstor‘s API will follow standard Node JS patterns
    • All functions are asynchronous with callback as last parameter
    • First parameter of callback is always error

When function signatures follow Node JS patters, it allows for promisification of modules. Such modules can be integrated into promise chain easily. But at the same time API isn’t tied to Promises at all. I like this approach because both camps (Promises or Callbacks fans) are happy.

Let’s take a look at code. I will explain and compare only most verbose use case and leave the rest for curios readers. You can find the code here.

Callback vs Promises – Tests comparison

Enough talking, let’s take a look at code. I’ll start with tests explanation as it promotes TDD thinking. Use case should test if existing password is overwritten when credentials for same user are stored. There are these phases in the test:

  1. Credentials file with initial password is created
  2. Read initial password
  3. Overwrite initial password with different one
  4. Read new password
  5. Verify that new password is different to initial one

Callbacks based test

var credentialsFile = 'testCredentials.txt';

var checkError = function(err, done) {
  if (err) {
    done(err);
  }
};

var readPassword = (credentialsFile, done, callback) => {
  fs.readFile(credentialsFile, (err, data) => {
    checkError(err, done);
    var jsonData = JSON.parse(data);
    callback(jsonData.user.password);
  });
};
describe('jasstor', () => {
  var jasstor = new Jasstor(credentialsFile);

  describe('when creadentials file already exist', () => {
    beforeEach(done => {
      fs.unlink(credentialsFile, () => {
        jasstor.saveCredentials('user', 'password', 'role', done);
      });
    });

    it('should overwrite existing password', done => {
      readPassword(credentialsFile, done, (originalPassword) => {
        jasstor.saveCredentials('user', 'password1', 'role', err => {
          checkError(err, done);
          readPassword(credentialsFile, done, (newPassword) => {
            newPassword.should.be.ok;
            originalPassword.should.be.ok;
            newPassword.should.not.equal(originalPassword);
            done();
          });
        });
      });
    });
  });
});

jasstor is testing object and jasstor.saveCredentials() is testing function. There is created helper function readPassword because password needs to be read twice during test. Pretty straight forward callbacks pyramid. I don’t like calling checkError at the beginning of each callback. Annoying Node pattern.

Promises based test

var fs = Bluebird.promisifyAll(require('fs'));
var credentialsFile = 'testCredentials.txt';

var readPassword = (credentialsFile, userName, done) => {
  return fs.readFileAsync(credentialsFile)
    .then(JSON.parse)
    .then(jsonData => {
      return jsonData[userName].password;
    }).catch(done);
};

var ignoreCallback = () => {};

describe('jasstor tested with promises', () => {
  var jasstor = Bluebird.promisifyAll(new Jasstor(credentialsFile));

  describe('when creadentials file already exist', () => {
    beforeEach(done => {
      fs.unlinkAsync(credentialsFile)
        .finally(() => {
          jasstor.saveCredentials('user', 'password', 'role', done);
        }).catch(ignoreCallback);
    });

    it('should overwrite existing password', done => {
      var originalPassword = readPassword(credentialsFile, 'user', done);
      var newPassword;
      jasstor.saveCredentialsAsync('user', 'password1', 'role')
        .then(() => {
          newPassword = readPassword(credentialsFile, 'user', done);
          newPassword.should.be.ok;
          originalPassword.should.be.ok;
          newPassword.should.not.equal(originalPassword);
          done();
        }).catch(done);
    });
  });
});

Important here is promisification of fs library (first line). It patches fs module to have additional methods with Async suffix. These methods return Promise and doesn’t take callback as parameter. This effectively converts existing API to promise based API. Same is done to testing object jasstor.

It was slight surprise to me that Promises actually doesn’t enable for less verbose code. Few facts are pretty obvious to me after this comparison:

  • Much more elegant error handling. As long as error callback has error as first parameter, you can just pass it to catch block as function reference.
  • Callbacks pyramid is flattened. This can improve readability. But readability is probably matter of maturity with certain approach.

Callback vs Promises – Node module comparison

Now I am going to compare code that was tested by tests above.

Callbacks based code

var hashPassword = (password, callback) => {
  bcrypt.genSalt(10, (err, salt) => bcrypt.hash(password, salt, callback));
};

var readJsonFile = (storageFile, callback) => {
  fs.exists(storageFile, (result) => {
    if (result === false) {
      callback(null, {});
    } else {
      fs.readFile(storageFile, (err, data) => {
        var jsonData = JSON.parse(data);
        callback(err, jsonData);
      });
    }
  });
};

module.exports = class Jasstor {
  constructor(storageFile) {
    this.storageFile = storageFile;
  }

  saveCredentials(user, password, role, callback) {
    readJsonFile(this.storageFile, (err, jsonData) => {
      hashPassword(password, (err, hash) => {
        jsonData[user] = {
          password: hash,
          role: role
        };
        var jsonDataString = JSON.stringify(jsonData);

        fs.writeFile(this.storageFile, jsonDataString, callback);
      });
    });
  }
};

Here we have ES6 class Jasstor with constructor and method that saves credentials into JSON file. There are two helper methods hashPassword and readJsonFile to help with repetitive tasks across the Jasstor class. We can see callback pyramid again. It is slightly simplified by helper functions.

Promises based code

var fs = Bluebird.promisifyAll(require('fs'));
var bcrypt = Bluebird.promisifyAll(require('bcrypt'));

var hashPassword = password => {
  return new Promise(resolve => {
    bcrypt.genSaltAsync(10)
      .then(salt => {
        return bcrypt.hashAsync(password, salt);
      }).then(resolve);
  });
};

var readJsonFile = storageFile => {
  return new Promise((resolve, reject) => {
    fs.exists(storageFile, result => {
      if (result === true) {
        fs.readFileAsync(storageFile)
          .then(JSON.parse)
          .then(resolve)
          .catch(reject);
      } else {
        resolve({});
      }
    });
  });
};

module.exports = class Jasstor {
  constructor(storageFile) {
    this.storageFile = storageFile;
  }

  saveCredentials(user, password, role, callback) {
    readJsonFile(this.storageFile).then(jsonData => {
      hashPassword(password).then(hash => {
        jsonData[user] = {
          password: hash,
          role: role
        };
        return jsonData;
      }).then(JSON.stringify)
        .then(jsonDataString => {
          fs.writeFile(this.storageFile, jsonDataString, callback);
        }).catch(callback);
    }).catch(callback);
  }
};

Same implementation packed into Promises is more verbose (hopefully I missed some tricks that could made it shorter). I like again simplified error handling. You maybe spot  that fs.exists isn’t promisified. If you take a look at its API, callback doesn’t have error as first parameter. I suspect, this is why fs.existsAsync doesn’t work correctly. Not sure if this is limitation of Bluebird promises library I am using or Promises A+ specification.

Conclusion

Promises are very nice approach that could totally change style of your programming. But I have to admit that I am not 100% sold to it yet. It took me some time to wrap my head around the concept. Promises also seem to me slightly more verbose than callbacks. When you have functions with one parameter and return value, you can nicely chain them with just passing function references into Promise chain. But mostly you don’t have such comfortable APIs and you end up doing “flattened callbacks pyramid”.

I would suggest to try Promises on your project (or small library) and make own opinion. Examples aren’t enough challenging to push the Promises into its limits.

Watch file changes and propagate errors with Gulp

Gulp fever infected me. Streaming model is very interesting and modern. After initial excitement, I started to experience first pitfalls. This is understandable for such young project. I am going to describe my problem with watching file changes and propagating errors.

Error in Gulp by default breaks the pipe, terminates the build/test and whole Gulp process with some error code. This is fine for CI process. But breaking the pipe stops file watch task also. This is big problem when developer wants to watch file changes and re-run particular tasks (e.g. tests).  You have to start watch task again after error occurs. This makes default watch task in Gulp pretty much useless. It is known and not the only problem of Gulp file watcher.

Fortunately Gulp 4 version if going to fix this. But I needed to come up with solution now. Google search points you to gulp-plumber. Idea behind it is to use gulp-plumber at the beginning of each pipe.

var plumber = require('gulp-plumber');
var coffee = require('gulp-coffee');

gulp.src('./src/*.ext')
    .pipe(plumber())
    .pipe(coffee())
    .pipe(gulp.dest('./dist'));

It prevents unpiping on error and forces the build process to continue regardless of error. Nice. Looks like problem with watch task solved.

I applied this approach on my pet project. It is simple node module that should store encrypted passwords into JSON file. But domain is not important for this blog post. When I checked in build process with gulp-plumber, I started to get false positives by drone.io CI server [EDIT: Build link doesn’t exist anymore].  Drone.io is using process error propagation, where each process returns error code. Non-zero value indicates error and zero means that process finished without error. gulp-plumber forces gulp process to continue and just writes errors to the console. Result is always zero error code from Gulp process.

So my goal is to use gulp-plumber to be able to continuously watch file changes and have fast feedback loop but also force Gulp process exit with non zero result when some error occurs.

First I declared variable to gather if error occurred.

var errorOccured = false;

Created handler for error recording.

var errorHandler = function () {
  console.log('Error occured... ');
  errorOccured = true;
};

Use gulp-plumber together with error handler for each Gulp pipe.

var transpilePipe = lazypipe()
  .pipe(plumber, {
    errorHandler: errorHandler
  })
  .pipe(jshint)
  .pipe(jshint.reporter, stylish)
  .pipe(jshint.reporter, 'fail')
  .pipe(traceur);

//Compiles ES6 into ES5
gulp.task('build', function () {
  return gulp.src(paths.scripts)
    .pipe(plumber({
      errorHandler: errorHandler
    }))
    .pipe(transpilePipe())
    .pipe(gulp.dest('dist'));
});

//Transpile to ES5 and runs mocha test
gulp.task('test', ['build'], function (cb) {
  gulp.src([paths.dist])
    .pipe(plumber({
      errorHandler: errorHandler
    }))
    .pipe(istanbul())
    .on('finish', function () {
      gulp.src(paths.tests)
        .pipe(plumber({
          errorHandler: errorHandler
        }))
        .pipe(transpilePipe())
        .pipe(gulp.dest('tmp'))
        .pipe(mocha())
        .pipe(istanbul.writeReports())
        .on('end', cb);
    });
});

This replaces gulp-plumber default error handler. It allows to record any error. (Example uses Lazypipe module. It can declare reusable pipe chunks. Lazypipe isn’t integrated with gulp-plumber, so it is needed also in sub-pipe.)

Next we need error checking Gulp task. It exits process with non-zero error code to indicate error state to Gulp process environment.

gulp.task('checkError', ['test'], function () {
  if (errorOccured) {
    console.log('Error occured, exitting build process... ');
    process.exit(1);
  }
});

Finally we call error checking task at the end of main Gulp task (right before submitting test coverage to coveralls.io in this case).

gulp.task('default', ['test', 'checkError', 'coveralls']);

Watch task is pretty standard, but doesn’t stop on error now.

gulp.task('watch', function () {
  var filesToWatch = paths.tests.concat(paths.scripts);
  gulp.watch(filesToWatch, ['test']);
});

And that’s it. Drone.io CI server properly highlights errors. I can also continuously watch file changes and automatically re-run tests. I agree that solution is little bit verbose, but I can live with that until Gulp 4 will be out.

Source code for this blog post can be found on Github.

Avoid unwanted component scanning of Spring Configuration

I came through interesting problem on Stack Overflow. Brett Ryan had problem that Spring Security configuration was initialized twice. When I was looking into his code I spot the problem. Let me show show the code.

He has pretty standard Spring application (not using Spring Boot). Uses more modern Java servlet Configuration based on Spring’s AbstractAnnotationConfigDispatcherServletInitializer.

import org.springframework.web.servlet.support.AbstractAnnotationConfigDispatcherServletInitializer;

public class AppInitializer extends
		AbstractAnnotationConfigDispatcherServletInitializer {


    @Override
    protected Class<?>[] getRootConfigClasses() {
        return new Class[]{SecurityConfig.class};
    }

    @Override
    protected Class<?>[] getServletConfigClasses() {
        return new Class[]{WebConfig.class};
    }

    @Override
    protected String[] getServletMappings() {
        return new String[]{"/"};
    }

}

As you can see, there are two configuration classes:

  • SecurityConfig – holds Spring Security configuration
  • WebConfig – main Spring’s IoC container configuration
package net.lkrnac.blog.dontscanconfigurations;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.config.annotation.web.servlet.configuration.EnableWebMvcSecurity;

@Configuration
@EnableWebMvcSecurity
public class SecurityConfig extends WebSecurityConfigurerAdapter {

    @Autowired
    public void configureGlobal(AuthenticationManagerBuilder auth) throws Exception {
        System.out.println("Spring Security init...");
        auth
                .inMemoryAuthentication()
                .withUser("user").password("password").roles("USER");
    }

}
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;

@Configuration
@EnableWebMvc
@ComponentScan(basePackages = "net.lkrnac.blog.dontscanconfigurations")
public class WebConfig extends WebMvcConfigurerAdapter {

}

Pay attention to the component scanning in WebConfig. It is scanning package where all three classes are located. When you run this on servlet container, text “Spring Security init…” is written to console twice. It mean mean SecurityConfig configuration is loaded twice. It was loaded

  1. During initialization of servlet container in method AppInitializer.getRootConfigClasses()
  2. By component scan in class WebConfig

Why? I found this explanation in Spring’s documentation:

Remember that @Configuration classes are meta-annotated with @Component, so they are candidates for component-scanning!

So this is feature of Spring and therefore we want to avoid component scanning of Spring @Configuration used by Servlet configuration. Brett Ryan independently found this problem and showed his solution in mentioned Stack Overflow question:

@ComponentScan(
    basePackages = "com.acme.app",
    excludeFilters = {
        @Filter(
            type = ASSIGNABLE_TYPE,
            value = { WebConfig.class, SecurityConfig.class }
        )
    })

I don’t like this solution. Annotation is too verbose for me. Also some developer can create new @Configuration class and forget to include it into this filter. I would rather specify special package that would be excluded from Spring’s component scanning.

I created sample project on Github so that you can play with it.