Thursday, August 29, 2013

Configuring ssh for Jenkins for Git on Ubuntu

I ran into some problems configuring ssh for my jenkins box. It turns out that when installing jenkins on ubuntu via the package manager that jenkins is setup to run as its own user. So, Jenkins did not have access to my .ssh directory. Of course this made perfect sense once I stopped and thought about it.

Anyhow, I could not find a clear answer in one place so posted here. This assumes that you have an ssh key defined for your Git service of choice. I used the same key for myself and my Jenkins user. You can of course create your own ssh key for the Jenkins User.

1. Setup the Git settings for the Jenkins account - assume the identity of the Jenkins user and configure:

sudo su jenkins
jenkins@lubuntu:~$ git config --global user.email "some email address"
jenkins@lubuntu:~$ git config --global user.name "some name you associate with jenkins"
exit

2. cd to your user home directory and copy the .ssh key info into the Jenkins account.

cd /home//ssh
sudo cp id_rsa /var/lib/jenkins/.ssh/
sudo cp id_rsa.pub /var/lib/jenkins/.ssh/
sudo jenkins:nogroup chown /var/lib/jenkins/.ssh/id_rsa
sudo jenkins:nogroup chown /var/lib/jenkins/.ssh/id_rsa.pub

3. Assume the identity of Jenkins again, cd to the Jenkins directory, create a temporary directory, clone your repository from the Git repository, fix any issues this reveals then delete the temporary directory:

sudo su jenkins
cd /var/lib/jenkins
mkdir junk
cd junk
git clone git clone git@bitbucket.org:butbucket user name/user’s repository.git
cd ..
rm rf junk

4. Kick off your Jenkins build and verify that the Source Code Management plugin works.

Thursday, August 22, 2013

Multi-Part File Upload with Node JS and Express

Several examples exist on the internet that detail how one can use Node.js to handle a multi-part form upload. Well, the scenario I ran into was that I needed to expose the upload functionality in my RESTful service, but then needed to pass that upload along to another service. So I had to construct the request object via code. Typical request objects are not so hard to construct in Node.js but the multi-part form upload was tripping me up. Here’s what I eventually came up with.

This example uses Express. Express is a minimal web application framework. You can find it here: http://expressjs.com/. Express exposes the collection of files via the req.files object. Note that express is going to use the name assigned by request creator - so, part of the contract may require the name be a specific value, otherwise there is no way to glean what the user may have named the file - at least not that I have found so far..

We have to tell Express to use the body parser. Do some googling on the bodyParser, there are some properties for specifying the temp directory for the upload and other things as well. Adding this line will cause Express to grab and expose the files as part of the request:
app.use(express.bodyParser());

And here is the POST handler:

app.post('/me/avatar', function(req, res){
   logger.debug('handling POST /me/avatar');
   logger.debug('files :' + JSON.stringify(req.files));
     
  
   // ToDo: this is a problem. It looks like we have to force people to use 'upload' as the form name for the file.
   // Cannot find away around this after hours of searching. Probably something simple,
   // just can’t find it (node.js noob).
   if(!req.files.upload){
      res.statusCode = 400;
      res.send("File not found in request.");
      return;
   }

   var subServiceUrl = “http://some_service_i_need_to_upload_to/my/file”;
       
   // read the file from disk. It has already been uploaded to a temp directory
   var filePath = req.files.upload.path;
   logger.debug('reading file: ' + filePath);
   fs.readFile(filePath, function (err, fileData) {
      if (err){
         logger.debug("error loading file: " + filePath);
         logger.debug(err);
         res.statusCode = 500;
         res.send('');
         return;
      }
   
   
      // the options setup is the tricky part for the multi-part upload
      var options = {
        uri: subServiceUrl ,
        method: 'POST',
        headers: {
          'content-type': 'multipart/form-data'
        },
        multipart: [{
           'Content-Disposition': 'form-data; name="upload"; filename="' + req.files.upload.name + '"',
           'Content-Type': req.files.upload.type,
           body: fileData
        }]
      };
   
      request(options, function(error, response, body){
         logger.debug('statusCode : ' + response.statusCode);
     
         if((response.statusCode != 201) || (response.statusCode != 200)){
            sendError(body, res, response);
         }
         else{
           res.statusCode = response.statusCode;
           res.send(response.statusCode, body);
         }
      });
   });
});

Saturday, August 10, 2013

Are Static Methods an Anti-Pattern?

Is using the Static keyword on class members and methods an anti-pattern? I think so and hopefully this post will get you thinking about it. Primarily I see Static abused when programmers do not want to go through the work of instantiating a class. This is in circumstances in which instantiation is not strictly necessary. The problem with Static members is that they get in the way of approaches such as Inversion of Control and Test Driven development.

First, lets take a look at an example that illustrates what I am talking about concerning using Static on class members. For the sake of discussion I will be using java, but the same is true in c#. Here is the main object of discussion, a Sentinel. This implementation uses an object with Static methods as a dependent object.


public class StaticSentinel {
    private AiState aiState = AiState.Scanning;
    private long posX;
    private long posY;
    private long posZ;

    public void live() {
        int threatLevel = 0;

        // unit testing here is problematic because of the dependency upon StaticMutantScanner. Since
        // StaticMutantScanner's implementation is static it cannot be overriden. Therefore, this.live() cannot
        // be unit tested in isolation of the dependency upon StaticMutantScanner. A unit test here is no longer
        // a unit test but is an integration test - testing both StaticSentinel and StaticMutantScanner components.
        Mutant mutant = StaticMutantScanner.detectMutant(this.posX, this.posY, this.posZ);

        if (mutant != null) {
            threatLevel = StaticMutantScanner.assessThreatLevel(mutant);
            // do something here based on the threat level.
            if (threatLevel > 0) {
                this.aiState = aiState.Hunting;
            }
        }

        // some other ai related activities, move, sleep, attack, guard, etc.
        doAi();
    }

    private void doAi() {
        // do something here.
    }

    public AiState getAiState() {
        return this.aiState;
    }
}
And here is the class with the Static methods. Our scanner, it looks for Mutants and assess their threat level.
public class StaticMutantScanner {
    public static Mutant detectMutant(long posx, long posy, long posz) {
        // create a mutant here via factory pattern and return it if nearby
        // instead of newing-up this mutant. newing-up is used here just for illustration.
        return new Mutant();
    }

    public static int assessThreatLevel(Mutant mutant) {
        // do something complicated here
        // and return threat level of the mutant
        return 0;
    }
}

What I’ve outlined above in the Sentinel class is not an uncommon thing to see in legacy code bases. In the case of the StaticMutantScanner - it is an object that does not require instantiation. So, the developer creates some kind of Static implementation. It makes sense, why bother instantiating it and all the irritating work associated with the task? This thought works - until we try to test it.

If we don’t care about good software quality and highly testable software, we don’t really need to care that we have a Static member. Some organizations out there still test by releasing or through manual and labor intensive processes. For the rest of us though, we want to know quickly and reliably - if and when we break our software by making inevitable changes. So we test. For us, Unit Testing is not an academic exercise.

Here is the test of the StaticSentintel:

    @Test
    public void liveTest(){
        StaticSentinel sentinel = new StaticSentinel();
        sentinel.live();
        Assert.assertTrue(sentinel.getAiState() == AiState.Scanning);
    }

Unit Testing is where Static methods and members make things go wrong. If I try to Unit Test Sentinel.live() in its current state, I cannot do so without exercising the code that lives in StaticMutantScanner.detectMutant() and StaticMutantScanner.accessThreat(). That is, I test live() by settings the a Sentinel’s coordinates and based on whether or not a Mutant is nearby get some kind of AiState property value to Assert against. So this means I have to guess or know in advance the values that would make StaticMutantScanner behave the way we need it to to cover bother branches of the “ if (threatLevel > 0) {“ statement. Some of you at this point are saying, ‘yes, that is Unit Testing.’

No it isn’t. It is an Integration Testing of the Sentinel and MutantScanner objects and how they work in tandem. In a Unit Test - a test of Sentinel.live() should be done by not actually descending into the the logic exposed by sub-components; thereby ‘testing the smallest unit of code’, a.k.a., Unit Test. Integration Testing is perfectly valid and should be done along side Unit Testing, but this is probably the job a QA person, not a Dev. Regardless of the ‘who’ both Integration and Unit tests should exist and serve distinctly different purposes.

In this scenario, Inversion of Control (IoC), also known as Dependency Injection (DI), becomes extremely important and is one reason why DI has emerged as a best practice. In the case of Unit Testing, DI lets us inject an implementation of a dependent child object to explicitly cause a state to exist in the child component that will cause one branch of code to behave in an expected manner.

For this discussion I implement DI this through an overloaded constructor and without some kind of IoC container. I have created a new Sentinel object named TestableSentinel. This object has an overloaded constructor and now acts more like a true composite object with an instantiated MutantScanner member. Note that the calls to the MutantScanner Interface are now through an instantiated object, not a class with Static members.

public class TestableSentinel {
    private MutantScanner mutantScanner;
    private AiState aiState = AiState.Scanning;
    private long posX;
    private long posY;
    private long posZ;

    /**
     * Overloaded constructor. Ultimately for use with Inversion of Control/Dependency Injection.
     * We can inject an implementation of MutantScanner at run time to properly Unit Test
     * TestableSentinel
     *
     * @param mutantScanner This is an implementation of an interface.
     */
    public TestableSentinel(MutantScanner mutantScanner){
        this.mutantScanner = mutantScanner;
    }

    public void live() {
        int threatLevel = 0;
        // The problem with the dependency upon mutantScanner's implementations are solved here by passing in
        // a new implementation during testing that returns the expected result for each test. This ensures that
        // TestableSentinel.live() is being tested in isolation of the mutantScanner object.
        Mutant mutant = this.mutantScanner.detectMutant(this.posX, this.posY, this.posZ);

        if (mutant != null) {
            threatLevel = this.mutantScanner.assessThreatLevel(mutant);
            // do something here based on the threat level.
            if (threatLevel > 0) {
                this.aiState = aiState.Hunting;
            }
        }
        // some other ai related activities, move, sleep, attack, guard, etc.
        doAi();
    }

    private void doAi() {
        // do something here.
    }

    public AiState getAiState() {
        return this.aiState;
    }

Here is the MutantScanner Interface:

public interface MutantScanner {
    Mutant detectMutant(long posx, long posy, long posz);
    int assessThreatLevel(Mutant mutant);
}

Note that we don’t know what the implementation of MutantScanner does. We just know that if it returns a Mutant and returns a threatLevel > 0 that the Sentinel will start Hunting. This is all we need to know and should know for the purpose of Unit Testing TestableSentinel.live().

The tests are a bit more interesting. I’ve deliberately not supplied an implementation of MutantScanner in the main branch of the source. This is to illustrate that one of the purposes behind the DI in the overloaded constructor of TestableSentinel is that we can supply an implementation specific to our test’s needs.

Here is the first test:

    @Test
    public void liveTest_ExpectThreatLevelHunting(){
        int threatLevel = 1;
        MutantScanner mutantScanner = new TestMutantScanner(threatLevel);
        TestableSentinel sentinel = new TestableSentinel(mutantScanner);
        sentinel.live();
        Assert.assertTrue(sentinel.getAiState() == AiState.Hunting);
    }

Here we set the threatLevel as a parameter in the TestMutantScanner’s constructor. TestMutantScanner is an implemetnation of MutantScanner created solely for the purpose of Unit Testing the Sentinel’s live() method. In this implementation, assessThreatLevel() simply returns the value supplied to the constructor when the TestMutantScanner is instantiated. When the test is run, the branch of code is executed that sets the sentinel’s AiState to Hunting and the test verifies this.

public class TestMutantScanner implements MutantScanner {
    int threatLevel;

    /**
     * For the purpose of discussion here and the associated test,
     * this implementation simply returns the threatLevel that is set in the constructor.
     * This is to give a point in which I can control the branch of code to unit test in
     * TestableSentinel via this implementation.
     * @param threatLevel
     */
    public TestMutantScanner(int threatLevel){
        this.threatLevel = threatLevel;
    }
    public Mutant detectMutant(long posx, long posy, long posz){
        return new Mutant();
    }

    public int assessThreatLevel(Mutant mutant){
        return threatLevel;
    }
}

We test the other branch, the branch that sets the Sentinel’s AiMode to Scanning, just by changing the parameter passed to TestMutantScanner to 0. This gets complete coverage of both branches of code associated with the if (threatLevel > 0) line of code. See the second test:

    @Test
    public void liveTest_ExpectThreatLevelScanning(){
        int threatLevel = 0;
        MutantScanner mutantScanner = new TestMutantScanner(threatLevel);
        TestableSentinel sentinel = new TestableSentinel(mutantScanner);
        sentinel.live();
        Assert.assertTrue(sentinel.getAiState() == AiState.Scanning);
    }

That’s how we get great Unit Test coverage. Unit Testing takes work, but it reduces delivered bugs. It also reveals flaws in design as in the case of Static methods. Some tools and frameworks exist to make this easier - MOQ http://code.google.com/p/moq/in the .Net site of things and Mockito http://code.google.com/p/mockito/ on the java side of things are two examples. The topic of mocking gets pretty big. Essentially, I created a manual mock when I implemented MutantScanner for the purpose of testing.

To reiterate - the problem with Static members lie in inability to test them. We can’t change the implementation. This creates untestable software when the Static members are in a class that is used by another. None of the testing strategies outlined above work for static methods.

Other strategies besides DI/IoC exist for dealing with Static methods, such as wrapper classes. But if we can, we should refactor and eliminate these legacy artifacts. We certainly should not inject Static methods into new code bases without compelling reasons to do so. Using Test Driven Development will reveal the pain associated with Static methods and the need to avoid them.