OpenHAB Broadlink Binding situation report

After #dadlife, #newjob and #otherstuff got in the way for a while last year, I got back into my role as maintainer of the OpenHAB Broadlink device binding. My first priority was to create a binding JAR that would actually work with the newly-published OpenHAB version 2.5. As the Broadlink binding is still not part of the official OpenHAB binding repository, it doesn't "automagically" get the necessary changes applied when the upstream APIs change. Luckily, it wasn't too much work.

My priorities for 2020 for this binding remain unchanged; get it to a high-quality state, make it pass the (extremely strict) linter guidelines for OpenHAB code, and get it merged into the official codebase. For me to consider it high-quality, there are still the following tasks to do:

• Get a solid chunk of it covered by unit tests to prevent regressions; and
• Redesign the device-discovery and identification areas of the code, to make adding new devices easier

Unit Tests

Prior to OpenHAB 2.5, bindings/addons that wished to define tests had to create an entire second project that shadowed the production code and could only be run via a strange incantation to Maven which did horrible OSGi things to run integration-style tests. Essentially, OpenHAB addons were not unit-testable by conventional means. Which given most addons are developed by unpaid volunteers, naturally meant that hardly any addons had tests.

Fortunately, one of the major changes in the 2.5 architecture has been a move towards more Java-idiomatic unit testing. Finally, classic JUnit-style unit testing with Mockito mocking will be available for fast, reliable testing within the binding. I'll be shooting for at least 60% test coverage before I'll consider submitting a PR to OpenHAB.

Discovery redesign

I've been told that new versions of some popular Broadlink devices will be arriving in 2020. In anticipation of that, I want to make it much easier to add a new device. At the moment it requires defining a new subclass of BroadlinkBaseThingHandler (which is par-for-the-course for OpenHAB, being a pretty standard Java app), but also adding "magic numbers" in a number of places to assist in looking-up and identifying devices during "discovery" and also when they boot up. I want to consolidate this such that everything needed to support a device is located within one .java file - i.e. adding support for a new device will require exactly two changes in Git:

• The new .java file containing all the required information to support the new device; and
• Adding a reference to this class somewhere to "pick it up".

I see no technical reason why this can't happen, and consider it only fair if maintenance of the binding will (at least partly) be a burden on the core OpenHAB team. So again, I won't be submitting this binding to become official until that work is complete.

Thanks for all the kind words from users/testers of this binding - it's very rewarding to hear people using it with great success!

What's in a name?

You've probably seen the old joke.
There are only 2 hard problems in Computer Science:

1. Naming things
2. Cache invalidation
3. Off-by-one errors

But like every good joke there's a substantial core of truth in it. And there's a good reason why Naming Things is top of that list. It's hard. And with most geeks being complexity-addicts, we like to ladle additional difficulties on top of the naming problem; for example:

• Compiler restrictions - Java insists on one-class-per-file, with a match between filename and class name
• Linter rules - Requiring interfaces to start with I or implementations with Impl
• Usability problems - if all your files are called index.js (because they live in different directories), navigating your text editor's tabs becomes a challenge
• Readability problems - Spending too long with AbstractInjectedDAOFactoryDecorator and AbstractInjectedDAOFactoryDecoratorBuilder-type names will wear out your sanity even faster than your keyboard

But the thing is, it's worth it. Especially in the increasingly-decomposed, microservicey, lambda-esque landscape, being able to latch onto a variable and/or class because its name is well-chosen and descriptive, can be the difference between a laser-like open file by name, make change, test, commit, DONE and a flailing find in all files, open a dozen of them, trace execution path, make speculative change, get it wrong, repeat until somehow fixed loop.

Better yet, as your team grows in experience, your shared vocabulary grows with it. Those well-named classes become a shorthand; you can use them as examples or references and everyone knows what you're on about - much like the Gang of Four Design Patterns are/were all about. I say "were" because of the 23 original GoF patterns, I would argue only a handful are in common usage today and have achieved the universality of understanding the authors were hoping for: Singleton, Iterator, Observer (although probably referred to as publisher-subscriber), Adapter/Proxy (often used interchangeably) and Factory Method.

I note that the design patterns that have truly "taken off" are without exception, the "simple" ones. I mean, look at Adapter (diagrams from the Black Wasp site also linked above):

... and now compare with Abstract Factory:

All those classes. All of them need names. And in Java at least, each one living in its own file. Possibly in different directories too. Is it any wonder that people have shied away from these complex patterns?

Serviio 2.0 on Raspbian "Buster"

After a recent SD card corruption (a whole-house power outage while the Raspberry Pi Model 3B must have been writing to the SD card), I have been forced to rebuild the little guy. It's been a good opportunity to get the latest-and-greatest, even if it means sometimes the various instructional guides are slightly out-of-date. Here's what I did to get the Serviio Media Streamer, version 2.0, to work on Raspbian Buster [Lite](*):

Install dependencies

Coming from the "lite" install, we'll need a JDK, plus the various encode/decode tools Serviio uses to do the heavy lifting:

$ sudo su
# apt-get update
# apt-get install ffmpeg x264 dcraw
# apt-get install --no-install-recommends openjdk-11-jdk

Download and unpack Serviio 2.0

This will result in serviio being located in /opt/serviio-2.0:

# wget http://download.serviio.org/releases/serviio-2.0-linux.tar.gz
# tar -xvzf serviio-2.0-linux.tar.gz -C /opt

Set up the Serviio service in systemd

Create a serviio user, and give them ownership of the install directory:

# useradd -U -s /sbin/nologin serviio
# chown -R serviio:serviio /opt/serviio-2.0

Create the a file serviio.service with the following contents:

[Unit]
Description=Serviio media Server
After=syslog.target network.target

[Service]
User=serviio
ExecStart=/opt/serviio-2.0/bin/serviio.sh
ExecStop=/opt/serviio-2.0/bin/serviio.sh -stop

[Install]
WantedBy=multi-user.target

Copy it into position, enable the service, and reboot:

# cp serviio.service /etc/systemd/system
# systemctl enable serviio.service
# reboot

Verify, Configure, Enjoy

After reboot, check things are happy:

$ sudo systemctl status serviio.service 
● serviio.service - Serviio media Server
   Loaded: loaded (/etc/systemd/system/serviio.service; enabled;)
   Active: active (running) since Sat 2019-08-31 16:54:48 AEST; 7min ago
   ...

It's also very handy to watch the logs while using Serviio:

$ tail -f /opt/serviio-2.0/logs/serviio.log

This is also a good time to set up the filesystem mount of the video media directory from my NAS, which the NAS user naspi has been given read-only access to:

 
  $ sudo apt-get install cifs-utils
  $ sudo vim /etc/fstab

(add line:)

//mynas/video /mnt/NAS cifs username=naspi,password=naspi,auto

Next, use a client app (I enjoy the Serviidroid app for Android devices) to locate and configure the instance, remembering that paths to media directories are always from the Pi's point of view, e.g. /mnt/NAS/movies if using the above example mount.

* This guide is very much based on the linuxconfig.org guide for Serviio 1.9, with updates as needed.

Green Millhouse - The Broadlink Binding part 3

Apologies to people who were interested in my Broadlink binding for OpenHAB 2.0 ... I migrated my code to the 2.4.0 family and things apparently stopped working. I was short on spare time so assumed something major had changed in the OpenHAB binding API and didn't have time to investigate. Turns out it was actually my own stupid fault, refactoring to clean up the code, I'd managed to cut out the "happy path" that actually updated OpenHAB with values from the device <facepalm />.

Anyway, back into it now with a couple of extra things rectified as well:

2.4.0-BETA-2.jar

This version also keeps track of the ScheduledFuture that is used to periodically poll the Broadlink device, and correctly calls cancel() on it if the Broadlink binding is getting disposed. This should put an end to those
...the handler was already disposed
errors flooding the logs.

2.4.0-BETA-3.jar

This version finally addresses the "reconnect" issues; i.e. when a device unexpectedly falls off the network, re-authentication with it would, more often than not, fail. The fix was to reset most of the state associated with the device once we lose contact with it. In particular, the packet counter, deviceId and deviceKey variables that we obtained from the previous authentication response.

As a result of this, my A1 Environmental Sensor seems to be working in all scenarios. Next up is long-term robustness testing while sorting out device discovery, making sure my RM3 Mini works, and checking the multiple heterogeneous devices will all play nicely together.

2.4.0-BETA-4.jar

This version reflects my testing with my RM3 Mini IR blaster. This device seems more sensitive to certain parameters in the authentication procedure - and it turned out that resetting the packet counter (as introduced in BETA-3) is not needed and can actually cause re-authentication to fail for this device. So there's actually less code in this release, but more functionality - which is always nice.
Device discovery is also working now too. From the OpenHAB Paper UI, go to
Configuration -> Things -> '+' -> Broadlink Binding and wait 10 seconds for the network broadcast to complete. If you have several Broadlink devices active on your network it may take a couple of scans to pick them all up.

2.4.0-BETA-5.jar

Small refactors for code cleanliness, e.g. grouping all networking functions together in NetworkUtils.java. Also added specific polling code for the SP3 smart switch device; prior to now this was using the same polling code as an SP2 device, but based on comments on this post from Jorg, it has its own function now, with extra diagnostic logging to hopefully pinpoint what is going on. I suspect that it doesn't reply in the same way as an SP2, so this is an "investigative" release; expect BETA-6 to follow with (hopefully) fixed support for the SP3. I may have to resort to buying one myself if we can't sort it out over the internet ;-)

2.4.0-BETA-6.jar

Fixed misidentification of an SP3 switch as an SP2 (appears to have been a typo in the original JAR). After investigating Jorg's incorrect polling issue, I took a look over at the python-broadlink module code for inspiration, and lo and behold, there was more going on there. This module seems to have had a lot of love (42 contributors!) and seems to have addressed all the quirks of these devices, so I had no hesitations in implementing the changes; namely that testing payload[4] == 1 is not sufficient. The Python code also checks against 3 and 0xFD. Looks like the LS bit of that byte is the one that matters, but if this works, that's fine!

2.4.0-BETA-7.jar

Added a ton more weird-and-wonderful variants of the RM2, courtesy of the aforementioned Python library. Broadlink seem to be constantly updating their device names (RM2, RM2 Pro Plus, RM2 Pro Plus R1, RM2 Pro Plus 2, etc etc), giving a new identification code to each one. I can't fathom a method to their coding scheme, so for the moment, we just have to play catch-up. In the case where we can't identify what seems to be a Broadlink device, I'm now logging (at ERROR level) the identification code we found, so that people out there can feature-request the support for their device.

2.4.0-BETA-8.jar

With thanks to excellent Github contributor FreddyFox, querying SP2/SP3 switch state should now be working. The old code attempted to decode the state of the switch from the encrypted payload, when of course it needs to be decrypted first. Thanks again FreddyFox!

2.4.0-BETA-9.jar

The main feature of this version is support for Dynamic IP Addresses.
There is a new switch available under the advanced Thing properties (open the SHOW MORE area in PaperUI) - it’s marked “Static IP” and it defaults to ON. If you are on a network where your Broadlink device might periodically be issued a different IP address, move it to the OFF position.
Now, if we lose communications with this Thing, rather than mark it OFFLINE, we’ll go into a mini-Discovery mode, and attempt to find the device on the network again, using its MAC address (which never changes). If we find the device, we update its current IP address in its Thing config, and everything continues without a hiccup. If we fail to find it, it goes OFFLINE as normal.
I should give credit to the LIFX binding which uses a very similar scheme - by chance I happened to come across it and realised it would be great for Broadlink devices too. As an extra bonus, it prompted me to redesign the entire discovery system and make it asynchronous; as a result it is now MUCH more reliable at finding devices (particularly if you have multiple Broadlink devices on your network) without having to scan repeatedly.

2.4.0-BETA-10.jar

This version fixes a few small issues:

• RM3 devices can now have a polling frequency specified (this was an omission from the thing-types.xml configuration file)
• Thing logging extracted to its own class and improved: Device status ONLINE/OFFLINE/undetermined shown in logs as ^/v/?
• Each ThingHandler's network socket is now explicitly closed when we lose contact with the device. This seems to help subsequent reconnections.

2.4.0-BETA-11.jar

This version attempts to improve general reliability. The Broadlink network protocol always acknowledges every command sent to a device, so logically, the sendPacket() and receivePacket() functions have been coalesced to sendAndReceivePacket(). This in turn allowed for a simple retry mechanism to be implemented. If we time out waiting for a response from the device, we immediately retry, sending the packet again. Together with some improved logging, this should hopefully be enough to fix (or at least understand) devices prematurely being marked "offline" on unreliable networks.

Here's the latest JAR anyway, put it into your addons directory and let me know in the comments how it goes for you: https://dl.bintray.com/themillhousegroup/generic/org.openhab.binding.broadlink-2.4.0-BETA-11.jar

Green Millhouse - Fixing the OpenHAB BroadLink Binding (part 2)

Development of the Broadlink OpenHAB binding has been going very well, but with just one scenario (and arguably the most-common one) not working reliably. With the OpenHAB server up-and-running and the Broadlink binding installed and configured for a particular disconnected device, turn the device on.

The copious logging I've added to the binding told me that this was resulting in an error with code -7 when the device was first detected, and I was a little stumped as to how to proceed. As many before me have done, I started entering random semi-related strings into Google to see what might turn up. And I hit gold. The author of the Python Broadlink library did some great work documenting the Broadlink protocol, and while perusing it a particular sentence jumped out at me:

You must obtain an authorisation key from the device before you can communicate.

Of course! When the binding first boots it calls authenticate for each device it knows about, and it does the same when a device setting is changed. But it does not when, after finding a newly-booted device, tries to get its status!

I added an authenticated boolean in the base Handler class, only setting it when authenticate() succeeds and clearing it whenever we lose contact with the device. Any attempt to communicate with the device first checks the boolean and does the authenticate() dance if necessary. And it works like a charm. We're very nearly there now - I just want to DRY up some of the payload-handling methods which seem to have been copy-pasted, add a 'presence' channel that mirrors the internal state of the binding, and remove the heavy dependence on IP addresses, and I think it's good to go.

Green Millhouse - Fixing the OpenHAB BroadLink Binding (part 1)

You can follow along at Github, but my rebuilding of the Broadlink OpenHAB binding is nearing completion.

I've been building and testing locally with my A1 Air Quality Sensor, and since fixing some shared-state issues in the network layer, haven't yet experienced any of the reliability problems that plagued the original binding.

For reasons that aren't clear (because I'm working from a decompiled JAR file), the original binding was set up like this in the base Thing handler (which all Broadlink Things inherit from):

public class BroadlinkBaseThingHandler extends BaseThingHandler {
   private static DatagramSocket socket = null;
   static boolean commandRunning = false;
   ...
}

As soon as I saw those static members, alarms started ringing in my head - especially when combined with an inheritance model, you've got a definite "fragile base class" problem at compile-time, and untold misery at runtime when multiple subclasses start accessing the socket like it's their exclusive property!

An attempt to mitigate the race-conditions which must have abounded, the commandRunning boolean only complicated matters:

    public boolean sendDatagram(byte message[])
    {
        try
        {
            if(socket == null || socket.isClosed())
            {
                socket = new DatagramSocket();
                socket.setBroadcast(true);
            }
            InetAddress host = InetAddress.getByName(thingConfig.getIpAddress());
            int port = thingConfig.getPort();
            DatagramPacket sendPacket = new DatagramPacket(message, message.length, new InetSocketAddress(host, port));
            commandRunning = true;
            socket.send(sendPacket);
        }
        catch(IOException e)
        {
            logger.error("IO error for device '{}' during UDP command sending: {}", getThing().getUID(), e.getMessage());
            commandRunning = false;
            return false;
        }
        return true;
    }

    public byte[] receiveDatagram()
    {
        try {
            socket.setReuseAddress(true);
            socket.setSoTimeout(5000);
        } catch (SocketException se) {
            commandRunning = false;
            socket.close();
            return null;
        }

        if(!commandRunning) {
            logger.error("No command running - device '{}' should not be receiving at this time!", getThing().getUID());
            return null;
        }

        try
        {
            if(socket != null)
            {
                byte response[] = new byte[1024];
                DatagramPacket receivePacket = new DatagramPacket(response, response.length);
                socket.receive(receivePacket);
                response = receivePacket.getData();
                commandRunning = false;
                socket.close();
                return response;
            }
        }
        catch (SocketTimeoutException ste) {
            if(logger.isDebugEnabled()) {
                logger.debug("No further response received for device '{}'", getThing().getUID());
            }
        }

        catch(Exception e)
        {
            logger.error("IO Exception: '{}", e.getMessage());
        }

        commandRunning = false;
        return null;
    }

So we got a pseudo-semaphore that is trying to detect getting into a bad state (because of shared-state), but itself is shared-state, thereby experiencing the same unreliability.

Here's what the new code looks like:

public class BroadlinkBaseThingHandler extends BaseThingHandler {
    private DatagramSocket socket = null;
    ...

    public boolean sendDatagram(byte message[], String purpose) {
        try {
            logTrace("Sending " + purpose);
            if (socket == null || socket.isClosed()) {
                socket = new DatagramSocket();
                socket.setBroadcast(true);
                socket.setReuseAddress(true);
                socket.setSoTimeout(5000);
            }
            InetAddress host = InetAddress.getByName(thingConfig.getIpAddress());
            int port = thingConfig.getPort();
            DatagramPacket sendPacket = new DatagramPacket(message, message.length, new InetSocketAddress(host, port));
            socket.send(sendPacket);
        } catch (IOException e) {
            logger.error("IO error for device '{}' during UDP command sending: {}", getThing().getUID(), e.getMessage());
            return false;
        }
        logTrace("Sending " + purpose + " complete");
        return true;
    }

    public byte[] receiveDatagram(String purpose) {
        logTrace("Receiving " + purpose);

        try {
            if (socket == null) {
                logError("receiveDatagram " + purpose + " for socket was unexpectedly null");
            } else {
                byte response[] = new byte[1024];
                DatagramPacket receivePacket = new DatagramPacket(response, response.length);
                socket.receive(receivePacket);
                response = receivePacket.getData();
//                socket.close();
                logTrace("Receiving " + purpose + " complete (OK)");
                return response;
            }
        } catch (SocketTimeoutException ste) {
            logDebug("No further " + purpose + " response received for device");
        } catch (Exception e) {
            logger.error("While {} - IO Exception: '{}'", purpose, e.getMessage());
        }

        return null;
    }    

A lot less controversial I'd say. The key changes:

• Each subclass instance (i.e. Thing) gets its own socket
• No need to track commandRunning - an instance owns its socket outright
• The socket gets configured just once, instead of being reconfigured between Tx- and Rx-time
• Improved diagnostic logging that always outputs the ThingID, and the purpose of the call

The next phase is now stress-testing the binding with a second heterogeneous device (sadly I don't have another A1, that would be great for further tests), my RM3 Mini IR-blaster. I'll be trying adding and removing the devices at various times, seeing if I can trip the binding up. The final step will be making sure the Thing discovery process (which is the main reason to upgrade to OpenHAB 2, and is brilliant) is as good as it can be. After that, I'll be tidying up the code to meet the OpenHAB guidelines and hopefully getting this thing into the official release!

Vultr Jenkins Slave GO!

I was alerted to the existence of VULTR on Twitter - high-performance compute nodes at reasonable prices sounded like a winner for Jenkins build boxes. After the incredible flaming-hoop-jumping required to get OpenShift Jenkins slaves running (and able to complete builds without dying) it was a real pleasure to have the simplicity of root-access to a Debian (8.x/Jessie) box and far-higher limits on RAM.

I selected a "20Gb SSD / 1024Mb" instance located in "Silicon Valley" for my slave. Being on the opposite side of the US to my OpenShift boxes feels like a small, but important factor in preventing total catastrophe in the event of a datacenter outage.

Setup Steps

(All these steps should be performed as root):

User and access

Create a jenkins user:

addgroup jenkins
adduser jenkins --ingroup jenkins

Now grab the id_rsa.pub from your Jenkins master's .ssh directory and put it into /home/jenkins/.ssh/authorized_keys. In the Jenkins UI, set up a new set of credentials corresponding to this, using "use a file from the Jenkins master .ssh". (which by the way, on OpenShift will be located at /var/lib/openshift/{userid}/app-root/data/.ssh/jenkins_id_rsa).
I like to keep things organised, so I made a vultr.com "domain" container and then created the credentials inside.

Install Java

echo "deb http://ppa.launchpad.net/webupd8team/java/ubuntu xenial main" | tee /etc/apt/sources.list.d/webupd8team-java.list
echo "deb-src http://ppa.launchpad.net/webupd8team/java/ubuntu xenial main" | tee -a /etc/apt/sources.list.d/webupd8team-java.list
apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys EEA14886
apt-get update
apt-get install oracle-java8-installer

Install SBT

apt-get install apt-transport-https
echo "deb https://dl.bintray.com/sbt/debian /" | tee -a /etc/apt/sources.list.d/sbt.list
apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 642AC823
apt-get update
apt-get install sbt

More useful bits

Git

apt-get install git

NodeJS

curl -sL https://deb.nodesource.com/setup_4.x | bash -
apt-get install nodejs

This machine is quite dramatically faster and has twice the RAM of my usual OpenShift nodes, making it extra-important to have those differences defined per-node instead of hard-coded into a job. One thing I was surprised to have to define was a memory limit for the JVM (via SBT's -mem argument) as I was getting "There is insufficient memory for the Java Runtime Environment to continue" errors when letting it choose its own upper limit. For posterity, here are the environment variables I have configured for my Vultr slave:

Strongly-Typed Time. Part 1: Rationale

I've quite recently become involved in an after-hours project that has a strong temporal component to it. Basically every interaction with the system will need to be labelled with a time, and they will constantly need to be compared and converted. Add to this the fact that the first beta customers are located on opposite sides of the Pacific, and that events can occur in a further 3 European countries, and a way to safely and unambiguously represent the time of something happening in a time and a place seems paramount.

While Joda Time has undoubtedly made date/calendar/timezone manipulation a happier task for the JVM developer, I'm looking for something stronger. I can pass around a DateTime all day long (no pun intended) but until I inspect its TimeZone I can't be sure where it originated from, or if it is in fact the canonical UTC.

As a result, there is nothing at compile time to stop me doing something like:

  def displayAllEventsBefore(allEvents:Seq[Event], threshold:DateTime) = {
 
    // allEvents have been normalized to UTC. But there's no way of knowing this:
    allEvents.filter(_.isBefore(threshold)).display    
  } 

  // ... much later, miles away
  val myTime = new DateTime() // Happens to be in "Europe/Paris"

  displayAllEventsBefore(events, myTime)

Which will work just fine most of the time, except when there's been an event in the last hour, when we won't see it. Or is it the other way around? Tricky, isn't it?

There's nothing in the type system to prevent these kinds of runtime problems. It comes down to developer diligence in naming/commenting/testing all the things - literally, every thing that uses a representation of time - to ensure correctness.

But hang on, aren't compilers really, really good at ensuring correctness?

Why Clojure is fascinating me

So I've hopped aboard the Clojure boat, as it's the preferred implementation language for "new stuff" at work.

And I'm liking it. A lot. Possibly because of the way we're using it (microservices), but probably just intrinsically, it is a language that seems to fit in the head very nicely. Not encumbered by special cases, exceptions, implicit magic and overloads. (Don't worry, I still enjoy Scala, but it's a very different Kettle[Either[Throwable, Map[String, Fish]]]).

The succinctness and elegance of Clojure is also thrown into sharp relief by the other thing I seem to be spending a lot of time on at work - grinding through a multiple-hundred-thousand-line instant-legacy untested Java codebase. This thing might have been considered state-of-the-art ten years ago when it was all about 3-tiered systems putting messages on busses - iff it had been implemented nicely, but it wasn't. As a result, it's a monolithic proliferation of POJO-manipulation, with control flow by exceptions, mutable state throughout, and impossible to test in isolation.

It can take hours to find code that actually "does something", but you have to follow the path(s) all the way down from "the top" just in case there's a bug or "hidden feature" somewhere on the way through the myriad layers with methods that look like this (anonymised somewhat):

  public List getAllFoo(Integer primaryId, Short secondaryId, String detail, Locale locale,
      String timeZone, String category) {

    if (category != null) {
      Map foosMap = ParameterConstants.foosMap;
      if (foosMap != null) {
        category = (foosMap.get(category.toUpperCase()) != null) ? foosMap.get(category.toUpperCase()) : category;
      }
    }
    List values = new ArrayList();
    FooValue searchValue = new FooValue();
    List fooValues = null;
    searchValue.setPrimaryID(primaryId);
    searchValue.setSecondaryId(secondaryId);
    searchValue.setCategory(category);

    try {
      LOGGER.info(CommonAPILoggingConstants.INF_JOBTYPE_GETALL_VALIDATION_COMPLETED);
      fooValues = fooDAO.getFoos(searchValue, detail);
    } catch (FooValidationException e) {
      handleException(e.getErrorId(), e);
    } catch (Exception e) {
      throw new InternalAPIException(UNKNOWN_CODE, e);

    }
    if (FULL.equalsIgnoreCase(detail)) {
      for (FooValue fooValue : fooValues) {
        Bar bar = null;
        try {
          if (StringUtils.isNotBlank(fooValue.getBarID())) {
            bar = barDAO.getBarByBarId(fooValue.getBarID());
            fooValue.setBarName(bar.getBarName());
            fooValue.setBarShortName(bar.getShortName());

            LOGGER.debug(CommonAPILoggingConstants.DBG_JOBTYPE_GETALL_FETCH_BAR_BY_ID,
                                bar.getBarName(),fooValue.getBarID());
          }
        } catch (Exception e) {
          throw new InternalAPIException(UNKNOWN_CODE, e);
        }

        try {
          if (null != bar) {
            if (StringUtils.isNotBlank(bar.getBrandID())) {
              fooValue.setBazID(bar.getBazID());
                            Baz baz = bazDAO.getBazByBazId(fooValue.getBazID());
              LOGGER.debug(CommonAPILoggingConstants.DBG_JOBTYPE_GETALL_FETCH_BAZ,
                                    baz.getName(),fooValue.getBazID());
              fooValue.setBazName(baz.getName());
            }
          }
        } catch (Exception e) {
          throw new InternalAPIException(UNKNOWN_CODE, e);
        }

        FooValue value = filterFooDetails(fooValue);
        values.add(value);
      }
    } else if (BASIC.equalsIgnoreCase(detail)) {

      for (FooValue fooValue : fooValues) {
        FooValue value = new FooValue();
        value.setFooID(fooValue.getFooID());
        value.setJobName(fooValue.getJobName());
        value.setContentTypeName(fooValue.getContentTypeName());
        value.setCategory(fooValue.getCategory());
        value.setIsOneToMany(fooValue.getIsOneToMany());
        values.add(value);
      }
    } else {
      throw new CommonAPIException(INVALID_DETAIL_PARAM,"Detail parameter value invalid");
    }
    return values;
  }

This is everywhere. The lines that get me most annoyed are things like this:

            fooValue.setBarName(bar.getBarName());
            fooValue.setBarShortName(bar.getShortName());

These x.setFoo(y.getFoo()) stanzas can go on for tens of lines. I haven't come across a name for them, so I'll call them POJO Shuffles. They suck the will-to-live out of anyone who has to navigate them as they frequently contain misalignments, micro-adjustments and hard-coding e.g.:

            fooValue.setBarName(bar.getBazName());
            fooValue.setBarShortName("Shortname: " + bar.getShortName());
            fooValue.setBarLongName(bar.getShortName().toUpperCase());

Did you notice:

• We're actually getting bazName from bar - almost certainly an autocomplete fail, but perhaps not?
• The "short name" of fooValue will actually be longer than in the source object. Is that important to something?
• There's a potential NullPointerException when we innocently try and set the "long name" of the fooValue

Then I read this gem of a paragraph from Rich Hickey, which is merely an introduction to the usage of defrecord in the official Clojure documentation, and yet reads like poetry when you've just come from code like the above:

It ends up that classes in most OO programs fall into two distinct categories: those classes that are artifacts of the implementation/programming domain, e.g. String or collection classes, or Clojure's reference types; and classes that represent application domain information, e.g. Employee, PurchaseOrder etc. It has always been an unfortunate characteristic of using classes for application domain information that it resulted in information being hidden behind class-specific micro-languages, e.g. even the seemingly harmless employee.getName() is a custom interface to data. Putting information in such classes is a problem, much like having every book being written in a different language would be a problem. You can no longer take a generic approach to information processing. This results in an explosion of needless specificity, and a dearth of reuse.

Rich Hickey

Scala by Stealth part 2: Scala-powered tests

Testing from Scala

Now for the fun part - we get to write some Scala!

Now if may turn out that this ends up being the end of the Scala road at your shop, due to restrictive policies about production code. That's a shame, and it could take a very long time to change. I know of one large company where "Java for production code, Scala for tests" has been the standard now for several years. Sure it's not perfect, but it's better than nothing, and developers who haven't yet caught the Scala bug can learn it in their day job.

The tests you write may eventually be the only unit tests for this code, so I would strive for complete coverage rather than merely a copy of the "legacy" Java-based tests. For the purposes of measuring this coverage I can highly recommend the jacoco4sbt plugin which is simple to get going, well-documented and produces excellent output that makes sense in Scala terms (some other Java-based coverage tools seem to struggle with some of the constructs the Scala compiler emits).

In addition to (possibly) getting introduced to Scala and also learning the basics of writing a specs2 test, you might even discover that your code under test is a little tricky to test from this new perspective. This is a good thing, and if it encourages a bit of mild refactoring (while keeping both Java- and Scala-based unit tests passing of course) then so much the better.

Once you've got some solid, measurable coverage from the Scala side (I like to shoot for 90+% line coverage), it's time to commit those changes again, and push them to your CI build. If you haven't already, install the JaCoCo Plugin for Jenkins so you can get pretty coverage graphs on the project page, and even automatically fail the build if coverage drops below your nominated threshold(s).

Switching your Jenkins build project to SBT

Speaking of which, you'll be wanting to adjust your Jenkins job (or equivalent) to push your new, somewhat-Scala-ish artifact to your Nexus (or equivalent). Firstly, for safety, I would actually be duplicating the existing job and disabling it rather than getting all gung-ho with something that can potentially be a very carefully-configured, nay curated Jenkins project configuration.

Luckily this should be pretty straightforward if you employ the Jenkins SBT Plugin - set the Actions to something like clean jacoco:cover publish to get the optimal blend of cleanliness, test-coverage visualisation, speed, and build traceability.

If for any reason you can't use the plugin, I'd recommend using your CI tool's Run script functionality, and including a dead-simple shell script in a suitable place in your repository; e.g.:

#!/bin/bash
echo "Running `which sbt`"
sbt -Dsbt.log.noformat=true -Dbuild.version=$BUILD_NUMBER clean jacoco:cover publish

Once you've got everything sorted out and artifacts uploading, you'll notice that your Nexus now has a new set of artifacts alongside your old Java ones, with a _2.10 (or whatever Scala version you're running) suffix. Scala in your corporate repo! Progress!

Scala By Stealth, Part 1: SBTifying your Mavenized Build

I was faced with updating and extending some old Java code of mine recently, and it seemed like much more of a chore than it used to. The code in question does a lot of collection manipulation, and I was looking at the Java code (which was, if I say so myself, not too bad - clean, thoroughly-tested and using nice libraries like Google Guava where at all possible) thinking "ugh - that would be a couple of lines in Scala and way more readable at the same time".

At this point I realised it would be a perfect candidate for a step-by-step guide for converting a simple Maveni[sz]ed Java library project (e.g. resulting in a JAR file artifact) to an SBT-based, Scala library.

Shortly after that I realised this could be a terrific way for a traditional "Java shop" where everything up until now has been delivered as JARs (and/or WARs) into a private Nexus to get its feet wet with Scala without having to go with a risky "big-bang" approach. An iterative migration, if you will. So let's get started!

A tiny bit of background first though - I'm not going to bother anonymising the library I'll be migrating, because I will almost certainly forget to do so somewhere in the example snippets I'll be including. So I'll say it here: the library is called brickhunter, and it's the "engine" behind the web-scraping LEGO search engine you can use at brickhunter.net. The site itself is a Java/Spring MVC/JQuery webapp that I launched in late 2012, and was the last significant bit of Java I ever wrote. It includes brickhunter.jar as a standard Maven dependency, pulling it from my private Maven repo hosted by CloudBees.

Step 0 (A Precondition): A Cared-For Maven Java Project

You need to be doing this migration for a library that has redeeming qualities, and not one that suffers from neglect, lack of test coverage, or a non-standard building process. Generally, using Maven will have made the latter difficult, but if, somehow, weird stuff is still going on, fix that. And make sure your tests are in order - comprehensive, relevant and not disabled!

Step 1: An SBTified Java Project

• Create a new directory alongside the "legacy" project directory with a suitable name. For me, the obvious one was brickhunter-scala.
• Now recursively copy everything under src from legacy to new. Hopefully that gets everything of importance; if not, see Step 0 and decide what should be done.
• While a number of people have written helpers to automate the creation of a build.sbt from a pom.xml, unless you have a truly enormous number of dependencies, you're probably better-off just writing it yourself. For one thing, it's the obvious entry point to the enormous world of SBT, and there's plenty to learn;
• In a typical Maven shop you may have quite a stack of parent POMs bringing in various dependencies - I found the quickest way to get all of them into SBT style was by invoking mvn dependency:tree which for my project, gave me:

  [INFO] +- org.jsoup:jsoup:jar:1.6.1:compile
  [INFO] +- commons-lang:commons-lang:jar:2.6:compile
  [INFO] +- com.google.guava:guava:jar:11.0.1:compile
  [INFO] |  \- com.google.code.findbugs:jsr305:jar:1.3.9:compile
  [INFO] +- log4j:log4j:jar:1.2.16:compile
  [INFO] +- org.slf4j:slf4j-api:jar:1.6.4:compile
  [INFO] +- org.slf4j:slf4j-log4j12:jar:1.6.4:compile
  [INFO] +- com.themillhousegroup:argon:jar:1.1-SNAPSHOT:compile
  [INFO] +- org.testng:testng:jar:6.3.1:test
  [INFO] |  +- junit:junit:jar:3.8.1:test
  [INFO] |  +- org.beanshell:bsh:jar:2.0b4:test
  [INFO] |  +- com.beust:jcommander:jar:1.12:test
  [INFO] |  \- org.yaml:snakeyaml:jar:1.6:test
  [INFO] +- org.mockito:mockito-all:jar:1.9.0:test
  [INFO] \- org.hamcrest:hamcrest-all:jar:1.1:test
  

• Anything transitive (i.e. indented once or more) can be omitted as SBT will work that out for us just as Maven did.
• The eagle-eyed might notice an in-house dependency (argon) which clearly isn't going to be found in the usual public repos - it will need its own resolver entry in build.sbt.
• Here's how mine looked at this point:

name := "brickhunter-scala"

organization := "com.themillhousegroup"

version := "0.1"

scalaVersion := "2.10.3"

credentials += Credentials(Path.userHome / ".ivy2" / ".credentials")

resolvers += "tmg-private-repo" at "https://repository-themillhousegroup.forge.cloudbees.com/private/"

libraryDependencies ++= Seq(
  "org.jsoup"             % "jsoup"           % "1.6.1",
  "commons-lang"          % "commons-lang"    % "2.6",
  "com.google.guava"      % "guava"           % "11.0.1",
  "log4j"                 % "log4j"           % "1.2.16",
  "org.testng"            % "testng"          % "6.3.1"         % "test",
  "org.mockito"           % "mockito-all"     % "1.9.0"         % "test",
  "com.themillhousegroup" % "argon"           % "1.1-SNAPSHOT"  % "test"
)

• At this point, firing up SBT and giving it a compile command should be successful. If so, pat yourself on the back, and commit all pertinent files in source control. This is a good milestone!

Step 2: A Tested SBTified Java Project

• Compiling is all very well but you can't really be sure your SBT-ification has been a success until all the tests are passing, just like they did in Maven. They did all pass in Maven, didn't they?
• Here's where I hit my first snag, as my Java tests were written using the TestNG framework, which SBT has no idea how to invoke. And thus, the brickhunter-scala project gets its first plugin, the sbt-testng-interface.
• But now when running sbt test, instead of "0 Tests Found", I get a big stack trace - the plugin is expecting to find a src/test/resources/testng.yaml and I don't have one, because Maven "just knows" how to run a load of TestNG-annotated tests it finds in src/test/java, and I've never needed to define what's in the default test suite.
• The fix is to create the simplest possible testng.yaml that will pick up all the tests:

  name: BrickhunterSuite
  threadCount: 4
   
  tests:
    - name: All
      packages:
      - com.themillhousegroup.brickhunter
  

• And now we should have the same number of tests running as under Maven, and all passing. Commit all the changes!

Next time: Publishing the new artifact to your private repository.

Future, Meet Past

Just because you're using a nice, shiny new language with all the latest bells-and-whistles doesn't make you immune from having to deal with problems that are almost as old as computers themselves. The case in point; watching a given directory, waiting for a new file to appear in it, and then doing something.

I was presented with solving this problem in Idiomatic Scala™ and was rather surprised to find very little built into the standard library to help.
Turning to Scala's grand-daddy was also rather astonishing - the "new, improved, async" NIO facilities are still, well, a bit clunky when you consider that there's not even an event-driven (i.e. register a callback) way to watch a directory.
So I set about implementing a directory watcher that works the way I'd like it to - namely, instantly returning a Future that will be completed only when the file I'm after has arrived (which of course I specify with a matching function). Here's my usage pattern:

val myDirWatcher = new DirectoryFileCreationWatcher(watchedDir)

myDirWatcher.awaitFile( _.endsWith("blah.txt") ).map { theNewDirectoryState =>
// Do something with the dir now that you know that *blah.txt is in it
}

And here's how I implemented it:

import java.nio.file._
import java.nio.file.StandardWatchEventKinds._
import scala.concurrent.ExecutionContext.Implicits.global
import scala.collection.JavaConversions._
 
class DirectoryFileCreationWatcher(directoryToWatch:Path) {

  val watcher = FileSystems.getDefault.newWatchService
 
  // Work properly on Mac:
  // http://stackoverflow.com/questions/9588737/is-java-7-watchservice-slow-for-anyone-else
  val kinds:Array[Kind[_]] = Seq(StandardWatchEventKinds.ENTRY_CREATE).toArray
  directoryToWatch.register(
    watcher, 
    kinds,     
    SensitivityWatchEventModifier.HIGH.asInstanceOf[WatchEvent.Modifier]
  )
 
  /**
   *
   * @param pathMatcher a function that returns true if this is the file we're looking for
   * @return a Future holding a Path that represents the directory in its "new" state
   */
  def awaitFile( pathMatcher: Path => Boolean):Future[Path] = Future[Path] {
    var foundMatch = false
    while (!foundMatch) {
      val watchKey = watcher.take // Blocks
      val events = watchKey.pollEvents
      foundMatch = events.exists { event =>
        val wep = event.asInstanceOf[WatchEvent[Path]]
        pathMatcher(wep.context)
      }
      watchKey.reset
    }
    directoryToWatch
  }
}

An annoying problem which I encountered during testing is that the JVM on MacOS does not implement the WatchService efficiently (i.e. by hooking into filesystem notifications), instead using the naïve polling approach. This will hopefully get rectified In Due Course™. As a result, I had to put quite lengthy sleeps into my test code (2000ms seemed to do it) after adding a file to a watched directory. That ugly bit of parameter-munging in the call to directoryToWatch.register() is configuring the "sensitive" version of the watcher, without which you'll need to wait 5+ seconds to be notified of changes. Ouch.

The really nice thing about using Futures in Scala is that once you've got one, you can just chain them up with map and friends, getting all of that asynchronous goodness with minimal boilerplate. Fun times.

Gist with unit tests is here.

Spring Roo Experiment, part 2

Making it perty

Roo does a great job of making a functional CRUD website, and that's great for an admin page. But public-facing websites, especially ones developed in 2011, need to look funky.

Enter Bootstrap, from the good people at Twitter. I'm not a Tweeter, but I have to extend maximum love and respect to these guys for turning out something to make things look so good, so easily.

Including the Bootstrap CSS in your webapp is like having the most web-2.0, rounded-cornered, missing-vowelled, fixie-riding front-end guy pairing with you, except he's actually completely finished the work, it works on all browsers and you don't have to listen to him talk about his iPhone all day :-)

Seriously though, Bootstrap does exactly what it says on the tin, with beautifully-clean, semantic markup, a wonderfully easy-to-use grid system and a contemporary, stylishly-minimal look.

I downloaded the Bootstrap CSS file and added it to my Spring Roo project by following these instructions - so now I can flip between the standard green Roo theme for my admin pages, and the super-funky Bootstrap theme for public pages. Lovely.

Spring Roo Experiment, part 1

Kicking the tyres

Wow - been getting a bit "10,000 foot view" recently. Back to good ol' Java. In particular, Spring Roo.

I had a bit of an idea for a basic database-backed web app, but the thought of all that XML-fiddling, boilerplate Java rubbish just to get something going was turning me right off. I'll admit, I seriously contemplated trying Ruby on Rails to get started ... then I remembered a colleague mentioning Roo as the Java answer to Rails. Java, Spring, Spring MVC and Maven best practices all driven from a console app?! Had to be worth a try!

I ran through the "pizza" sample, doing the setup for my app at the same time. Apart from a hiccup with my local Nexus instance (I always forget to add new repositories to the public Group!), it was super-smooth.

What struck me was how I was able to concentrate on my domain objects (aka entities in Roo-speak) and how they interacted. In four lines, I can make a typical tested, persisted POJO with a full CRUD web GUI!:

roo> entity --class ~.DescribedThing --testAutomatically
roo> field string --fieldName name --notNull
roo> field string --fieldName description --notNull
roo> web mvc all --package ~.web

It's really fun to "meta-program" like this. Less boilerplate, fewer typos (thanks Tab auto-complete!) and more designing.

I'm going to continue developing Roo-style for as far as it will take me and see how it goes.

Die Complexity! Die!

I've mentioned it before but I feel the need to do so again. What is it with developers and complexity?

You know when in a film or cartoon, a character opens an innocent-looking door and all manner of horror and noise belches out, so they quickly slam the door and the noise stops? I got another blast of that kinda thing recently - innocent-looking website, foul twisted complex horror beneath. Why does this happen?

My current theory boils down to a three-legged milking-stool of failures:

1. Developers move on and can't/don't/won't pass on all their knowledge. Replacement developers have to read between the lines (or worse, read bad and/or out-of-date documentation) and are doomed to repeat the past, slightly worse each iteration. Rinse and repeat


2. Developers love doing new stuff. At least, the good ones do. They love to cover a pristine whiteboard with boxes and arrows. Who doesn't love the purity of a codebase that consists only of interfaces? No implementation means no ugly real-life workaround warts! The trouble is, we can't all be developing new frameworks all the time


3. Lastly, and most controversially, The Agile Process, or rather, the blind adherence to some aspects of it, can be blamed. I'm talking Sprints here. It seems like in the effort to shoe-horn a large piece of work into a too-short sprint cycle, the lethal one-two punch of doing a half-a[rs]sed job and chalking up a load more technical debt to actually do it the right way* has become an acceptable outcome. It isn't

The solution? BAN Complexity in your development team. Stamp on it the instant it makes a first tentative push out of the soil. Make it a priority amongst the team members to simplify any new feature to the utmost. Exercise and extend the Boy-Scout Rule to make each and every code commit an incremental improvement in straightforwardness.

How will this help address the milking-stool of coding horror?

1. Simpler code groks faster so new devs don't need all the handover baggage
2. Developers can still show their design skills, but at de-baroquifying designs. Surely that's a whole lot harder than over-engineering?
3. And finally, if you can't change your sprint durations (and if not, why not?) then lower complexity should go hand-in-hand with higher velocity. There must be no Tech Debt. Ever

(*) In some mythical far-off time when the schedule does allow for longer-term work to be completed...

Tidier Varargs, part 3

Tidying up the loose ends

Some of you might have been a little incredulous at my earlier statement that no existing library offers methods to neatly deal with possibly-null varargs.

I stand by my position - it's the possibly-null aspect that is the kicker. Google Guava offers methods that almost get over the line:

My varargs method	Google's almost-the-same method
Iterable<T> VarargIterator.forArgs (T... optionalArgs)	ArrayList<E> Lists.newArrayList (E... elements)
Iterable<T> VarargIterator.forCompulsoryAndOptionalArgs(T t, T... ts)	List<E> Lists.asList (E first, E[] rest)

BUT they explode when fed nulls - making them pointless for this particular wart-removing exercise!

So the only thing left now is to clean up the implementation a little, because I think it's failing two of the rules of clean code:

1. Runs all the tests
2. Contains no duplications
3. Expresses the intent of the programmers
4. Minimizes the number of classes and methods

Here's the entire class as it stands (imports removed):

public class VarargIterator {
    /**
     * @return an {@link Iterable} of a suitable type. 
     * Null-safe; passing a null varargs will simply return
     * an "empty" {@code Iterable}.
     */
    public static final <T> Iterable<T> forArgs(T... optionalArgs) {
        if (optionalArgs == null) {
            return Collections.emptyList();
        } else {
            return Arrays.asList(optionalArgs);
        }
    }

    /**
     * @return an {@link Iterable} of a suitable type.
     * Null-safe; passing a null {@code optionalArgs} will simply return
     * a single-element {@code Iterable}.
     * @throws IllegalArgumentException if {@code compulsoryArg} is null
     */
    public static final <T> Iterable<T> forCompulsoryAndOptionalArgs(
        final T compulsoryArg, final T... optionalArgs) {

        Validate.notNull(compulsoryArg,
            "the first argument to this method is compulsory");
        if (optionalArgs == null) {
            return Collections.singletonList(compulsoryArg);
        } else {
            List<T> iterable = new ArrayList<T>(optionalArgs.length + 1);
            iterable.add(compulsoryArg);
            iterable.addAll(Arrays.asList(>optionalArgs));
            return iterable;
        }
    }

}

The bits that I don't like are the repetition of the optionalArgs null check in both methods, and the code in the else case of the second method. It's working at a "different level" to the other code in the method, losing the intent, and there's too much of it - it's made the whole method too long.

Of course I have a full suite of unit tests so I can be confident that I'm not breaking anything when I do this refactoring work. I use Cobertura in its Maven and Eclipse plugin forms to ensure I'm achieving 100% coverage from these tests.

The first step is easy - extract out a well-named method for the else case:

    /**
     * @return an {@link Iterable} of a suitable type.
     * Null-safe; passing a null {@code optionalArgs} will simply return
     * a single-element {@code Iterable}.
     * @throws IllegalArgumentException if {@code compulsoryArg} is null
     */
    public static final <T> Iterable<T> forCompulsoryAndOptionalArgs(
        T compulsoryArg, T... optionalArgs) {

        Validate.notNull(compulsoryArg,
            "the first argument to this method is compulsory");
        if (optionalArgs == null) {
            return Collections.singletonList(compulsoryArg);
        } else {
            return createListFrom(compulsoryArg, optionalArgs);
        }
    }

    private static <T> Iterable<T> createListFrom(
        T compulsoryArg, T... optionalArgs) {

        final List<T> list = new ArrayList<T>(optionalArgs.length + 1);
        list.add(compulsoryArg);
        list.addAll(Arrays.asList(optionalArgs));
        return list;
    }
    

I dithered a bit about the null check. Doing some kind of is-null/is-not-null closurey-interfacey thing seemed like pretty major overkill, so in the end I just extracted out the logic into a method. As a bonus, I realised I could also check for a zero-length (as opposed to null) array and save some cycles in that case. One last tweak was to drop the explicit else branches - because the methods are now so short there seems little point. So here's the final version - enjoy!

public class VarargIterator {
    /**
     * @return an {@link Iterable} of a suitable type. 
     * Null-safe; passing a null varargs will simply return
     * an "empty" {@code Iterable}.
     */
    public static final <T> Iterable<T> forArgs(T... optionalArgs) {
        if (isEmpty(optionalArgs)) {
            return Collections.emptyList();
        } 
        return Arrays.asList(optionalArgs);
    }

    /**
     * @return an {@link Iterable} of a suitable type.
     * Null-safe; passing a null {@code optionalArgs} will simply return
     * a single-element {@code Iterable}.
     * @throws IllegalArgumentException if {@code compulsoryArg} is null
     */
    public static final <T> Iterable<T> forCompulsoryAndOptionalArgs(
        T compulsoryArg, T... optionalArgs) {

        Validate.notNull(compulsoryArg,
            "the first argument to this method is compulsory");
        if (isEmpty(optionalArgs)) {
            return Collections.singletonList(compulsoryArg);
        } 
        return createListFrom(compulsoryArg, optionalArgs);
    }

    private static boolean isEmpty(Object... optionalArgs) {
        return (optionalArgs == null) || (optionalArgs.length == 0);
    }

    private static <T> Iterable<T> createListFrom(
        T compulsoryArg, T... optionalArgs) {

        final List<T> list = new ArrayList<T>(optionalArgs.length + 1);
        list.add(compulsoryArg);
        list.addAll(Arrays.asList(optionalArgs));
        return list;
    }
}

Tidier Varargs, part 2

Last time we looked at how we could tidy up code that deals with a single, possibly-null varargs parameter. But of course there are lots of cases where the business rule is an "at-least-one" scenario.

An example of a real API that operates like this is the mighty Mockito's thenReturn method, which looks like this:

OngoingStubbing<T> thenReturn(T value, T... values)

Which is typically used to define mock behaviour like this:

Mockito.when(
    mockCountdownAnnouncer.getCurrent()).thenReturn(
        "Three", "Two", "One", "Liftoff");

The nice thing about this is the "fluent" way you can add or remove parameters from the list, and it just keeps working - as long as you've specified at-least-one return value.

This kind of API is really pleasant to use, but it'd be doubly great to have clean code within methods like this. The VarargIterator needs some extension!

    
    /**
     * @return an {@link Iterable} of a suitable type.
     * Null-safe; passing a null {@code optionalArgs} will simply return
     * a single-element {@code Iterable}.
     * @throws IllegalArgumentException if {@code compulsoryArg} is null
     */
    public static final <T> Iterable<T> forCompulsoryAndOptionalArgs(
        final T compulsoryArg, final T... optionalArgs) {

        Validate.notNull(compulsoryArg,
            "the first argument to this method is compulsory");
        if (optionalArgs == null) {
            return Collections.singletonList(compulsoryArg);
        } else {
            List<T> iterable = new ArrayList<T>(optionalArgs.length + 1);
            iterable.add(compulsoryArg);
            iterable.addAll(Arrays.asList(optionalArgs));
            return iterable;
        }
    }

As you can see, we encapsulate null-checking and null-safety in this method, allowing users to simply write (with a static import in this example):

    public OngoingStubbing<T> thenReturn(T value, T... values) {

        for (T retValue :forCompulsoryAndOptionalArgs(value, values)) {
            // Do Mockito magic with each retValue
        }
    }

Mmmm. Squeaky-clean!

Tidier Varargs, part 1

One of the less-appreciated of the many massive improvements made to Java in version 1.5 was varargs, aka those little dots that are syntactic sugar sprinkled onto a strongly-typed array:

    public static void printAllOfThese(String... things) {
        for (String thing : things) {
            System.out.println(thing);
        }
    }

It goes hand-in-glove with the enhanced for loop. Very tidy. Except what happens when I inadvertently do this:?

   printAllOfThese(null);

You probably correctly guessed: NullPointerException at line 1 of printAllOfThese - yep - sadly, the enhanced for loop is not null-safe. What we really want is for a null argument to be silently ignored. So our previously oh-so-tidy, oh-so-readable varargs method ends up needing a null-check wart:

    public static void printAllOfThese(String... things) {
        if (things != null) {   
            for (String thing : things) {
                System.out.println(thing);
            }
        }
    }

Ew. That's ugly, and in turn requires more test cases. There has to be a better way, but amazingly, I couldn't find any solution in the usual common libraries. So, somewhat reluctantly (because if premature optimisation is the root of all evil, then wheel-reinvention is surely sudo), I present the VarargIterator helper class:

public class VarargIterator {

    /**
     * @return an {@link Iterable} of a suitable type. 
     * Null-safe; passing a null varargs will simply return
     * an "empty" {@code Iterable}.
     */
    public static final <T> Iterable<T> forArgs(T... optionalArgs) {
        if (optionalArgs == null) {
            return Collections.emptyList();
        } else {
            return Arrays.asList(optionalArgs);
        }
    }
}

This allows us to rewrite our printAllOfThese method very neatly:

    public static void printAllOfThese(String... things) {
        for (String thing : VarargIterator.forArgs(things)) {
            System.out.println(thing);
        }
    }

Next time: Elegantly dealing with compulsory and optional elements.

The Bore That Is Core

(and how less gives you more)

Every single Java codebase I have ever come across has had a project called core or common or somesuch. And almost-universally, dwelling within this library, under the banner of "useful functionality used by other projects" lurk classes like:

• StringUtils for String manipulation
• FileHelper for dealing with files
• ValidationUtils for argument-checking
• DateHelper for tweaking Dates

I know, I know. But I've seen this macro-level antipattern in even the most experienced of development houses.

If you have a "core" library featuring any of the above stuff, exorcise that code now. Right now. Your core library is a liability. You are maintaining redundant code that almost-certainly has bugs you haven't found yet.

In the spirit of simplification, may I humbly recommend:

• Apache Commons Lang gives you StringUtils, DateUtils and loads more besides.


• Google's Guava (formerly Google Collections) gives you fabulous, fully-genericised collection-manipulation, plus some handy-dandy utilities like Preconditions to take care of your argument-checking requirements

These libraries will slim down your "core" to stuff that is genuinely domain/site/whatever-specific.

Grab them, learn them, use them, love them :-)

Quartz Jobs in Spring 3.0

Elegant Simplicity

The Spring Framework's Quartz integration is a lovely example of a framework doing all the heavy lifting, allowing the developer to concentrate on the actual task.

Thanks to the MethodInvokingJobDetailFactoryBean, any Spring bean can be invoked periodically, with all of Spring's dependency wiring happening exactly as you'd expect. This makes unit testing an absolute breeze - you just test the expected functionality, ignoring the external timing concerns. Even integration testing can be accomplished quickly by substituting a faster schedule; for example, if our "runtime" bean definition looks like this:

<bean name="thingyServiceBean" class="com.millhouse.service.FooService"/>

<bean name="thingyJob" class=
"org.springframework.scheduling.quartz.MethodInvokingJobDetailFactoryBean">
  <property name="targetObject" ref="thingyServiceBean"/>
  <property name="targetMethod" value="doThingy"/>
  <property name="concurrent" value="false"/>
</bean> 
 
<bean name="thingyTrigger" class=
  "org.springframework.scheduling.quartz.SimpleTriggerBean">
  <property name="jobDetail" ref="thingyJob"/>
  <property name="repeatInterval" value="60000"/> <!-- 60 sec in millis -->
  <property name="repeatCount" value="-1"/> <!-- -1 means forever -->  
</bean>

<bean class="org.springframework.scheduling.quartz.SchedulerFactoryBean">
  <property name="triggers">
    <list>
      <ref bean="thingyTrigger"/>
    </list>
  </property>
</bean>

we obviously don't want to have our thingyServiceBean invoked every sixty seconds forever in integration tests, so we supply a different application context for testing:

<import resource="classpath:jobConfig.xml"/>
  
<-- Overridden definition for the trigger: -->
<bean name="thingyTrigger" class=
  "org.springframework.scheduling.quartz.SimpleTriggerBean">
  <property name="jobDetail" ref="thingyJob"/>
  <property name="repeatInterval" value="0"/> 
  <property name="repeatCount" value="1"/> <!-- run once for testing -->  
</bean>

If you're using Maven, placing the "runtime" jobConfig.xml in src/main/resources and the above file in src/test/resources Just Works - and you've proven your Spring wireup is correct. Lovely.