Using time-dependent data

I stumbled across a piece of code the other day, allowing some versioned entity from our domain model to find out whether it's valid from a certain point in time. The corresponding test failed sporadically, so I went and looked into it.

The class in question uses Joda Time's DateTime class to represent timestamps:

public class SomeEntity {

  private DateTime validFrom = new DateTime();

  // ...

  public boolean isBeforeOrEqualNow() {
    return validFrom.isBeforeNow() || validFrom.isEqualNow();
  }

  public boolean isAfterOrEqual(DateTime timeStamp) {
    return validFrom.isEqual(timeStamp) || validFrom.isAfter(timeStamp);
  }

  // ...
}

So far, so faulty - although it might not seem obvious by now. Luckily, there is a test for at least the first of these two methods, which fails sporadically:

@Test
public class SomeEntityTest {

  public void testSomeEntityIsAlwaysBeforeOrEqualNow() {
    SomeEntity entity = new SomeEntity();
    assertThat(entity.isBeforeOrEqualNow()).isTrue();
  }

}

Why can this fail at all? Isn't it just checking if validFrom is before or equal to now? Kind of. Recall the implementation of isBeforeOrEqualNow:

  return validFrom.isBeforeNow() || validFrom.isEqualNow();

Time advances - even in absence of concurrent threads

It calls two methods on DateTime and combines the result. isBeforeNow and isEqualNow compare validFrom to the current system time. Both methods fetch the system time independently of each other. Let's peek into the source code of AbstractInstant, which is a superclass of DateTime:

public abstract class AbstractInstant implements ReadableInstant {

    // ... 

    public boolean isBeforeNow() {
        return isBefore(DateTimeUtils.currentTimeMillis());
    }

    // ...

    public boolean isEqualNow() {
        return isEqual(DateTimeUtils.currentTimeMillis());
    }

    // ...
}

Of course, the system time might advance between the call to isBeforeNow() and isEqualNow(). The above implementation of isBeforeOrEqualNow() assumes that both calls receive the same value for the system time.

Voilá, a race condition.

The behaviour depends on the timing of the two calls. If the system timer value gets updated in between, the test fails. Otherwise, the test succeeds. This can be seen as a special kind of race condition, where no two software threads are involved, but a piece of hardware advancing a counter, i. e. a clock.

Fixing the implementation

So how can the desired function be implemented correctly? Very simple: just obtain one timestamp for now and perform the desired operations against this single timestamp:

  public boolean isBeforeOrEqualNow() {
    DateTime now = new DateTime();
    return validFrom.isBefore(now) || validFrom.isEqual(now);
  }

Testing time-dependent behaviour

Using this implementation, the test from above will probably not fail any more. But how can we make sure it does not? The Joda Time guys actually implemented some support for time-dependent tests: DateTimeUtils allows us to set the current system time to a fixed value, or a fixed offset against the actual system time. It does this using an internal service provider interface to obtain the current system clock:

public abstract class AbstractInstant implements ReadableInstant {

    // ...

    public boolean isEqualNow() {
        return isEqual(DateTimeUtils.currentTimeMillis());
    }

    // ...

}

public class DateTimeUtils {

    private static final SystemMillisProvider SYSTEM_MILLIS_PROVIDER = new SystemMillisProvider();
    
    private static MillisProvider cMillisProvider = SYSTEM_MILLIS_PROVIDER;

    // ...

    public static final long currentTimeMillis() {
        return cMillisProvider.getMillis();
    }

    // ...

    public static final void setCurrentMillisSystem() throws SecurityException {
        checkPermission();
        cMillisProvider = SYSTEM_MILLIS_PROVIDER;
    }

    public static final void setCurrentMillisFixed(long fixedMillis) throws SecurityException {
        checkPermission();
        cMillisProvider = new FixedMillisProvider(fixedMillis);
    }

    public static final void setCurrentMillisOffset(long offsetMillis) throws SecurityException {
        checkPermission();
        if (offsetMillis == 0) {
            cMillisProvider = SYSTEM_MILLIS_PROVIDER;
        } else {
            cMillisProvider = new OffsetMillisProvider(offsetMillis);
        }
    }
}

Unfortunately, it does not allow us to simulate the behaviour of a system clock that delivers two different values for each subsequent peek at its value. There are three pre-defined implementations of MillisProvider, which cannot be added to easily:

public class DateTimeUtils {

  abstract static class MillisProvider {
    abstract long getMillis();
  }

  static class SystemMillisProvider extends MillisProvider {
    long getMillis() {
      return System.currentTimeMillis();
    }
  }

  static class FixedMillisProvider extends MillisProvider {
  
    private final long iMillis;
        
    FixedMillisProvider(long fixedMillis) {
      iMillis = fixedMillis;
    }
        
    long getMillis() {
      return iMillis;
    }
  }

  static class OffsetMillisProvider extends MillisProvider {
 
    private final long iMillis;
        
    OffsetMillisProvider(long offsetMillis) {
      iMillis = offsetMillis;
    }
        
    long getMillis() {
      return System.currentTimeMillis() + iMillis;
    }
  }
}

Need to configure a custom MillisProvider

For our case, it would be very helpful to be able to set a custom MillisProvider, like so:

public class ProgressingMillisProvider extends MillisProvider {

  private long millis = 0;

  public long getMillis() {
    return millis++;
  }
}


public class DateTimeUtils {

  // ...
    
  private static MillisProvider cMillisProvider = SYSTEM_MILLIS_PROVIDER;

  public void setMillisProvider(MillisProvider newProvider) {
    checkPermission();
    assertThat(newProvider).isNotNull();
    cMillisProvider = newProvider;
  }

  // ...
}
    

With the new method setMillisProvider it would be possible to write a (failing) test against the old implementation as follows:

@Test
public class SomeEntityTest {

  public void testSomeEntityIsAlwaysBeforeOrEqualNow() {
    SomeEntity entity = new SomeEntity();
    DateTimeUtils.setMillisProvider(new ProgressingMillisProvider());
    assertThat(entity.isBeforeOrEqualNow()).isTrue();
  }
}

This way, we could actually test that the corrected implementation from above actually works for a clock which returns progressing timestamp values. I heard the other day this was the typical behaviour of clocks... ;-)

Update: joda-time trunk contains necessary updates

Fortunately, the changes required to implement a test like described above is already in the joda-time trunk in svn, as pointed out by project owner Stephen Colebourne on the mailing list. Thank you guy :-)