This simple file monitor class notifies listeners when a watched file is modified:
public class FileMonitor {
private Set listeners =
new HashSet();
private int duration;
private File file;
private long lastModified;
public FileMonitor(String filename, int duration) {
this.file = new File(filename);
lastModified = file.lastModified();
this.duration = duration;
}
public void start() {
boolean isDaemon = true;
long initialDelay = duration;
new Timer(isDaemon).schedule(
new FileMonitorTask(), initialDelay, duration);
}
public void addListener(FileChangeListener listener) {
listeners.add(listener);
}
class FileMonitorTask extends TimerTask {
@Override
public void run() {
if (file.lastModified() > lastModified) {
lastModified = file.lastModified();
for (FileChangeListener listener: listeners) {
listener.modified();
}
}
}
}
}
A FileMonitor schedules a TimerTask that just checks the modified date from time to time, and compares it to the last modified date. The code above seems like a typical and reasonable implementation. FileMonitorTask could be an anonymous inner class, of course. I spent more time than I would have preferred test-driving this solution, since I made the mistake of test-driving it as a single unit. These tests had to deal with the vagaries of threading due to the Timer scheduling.
import java.io.*;
import java.util.*;
import org.junit.*;
import static org.junit.Assert.*;
public class FileMonitorTest {
private static final String FILENAME = "FileMonitorTest.properties";
private static final int MONITOR_DURATION = 1;
private static final int TIMEOUT = 50;
private FileMonitor monitor;
private File file;
private List fileChangeNotifications;
private Thread waitThread;
@Before
public void initializeListenerCounter() {
fileChangeNotifications = new ArrayList();
}
@Before
public void createFileAndMonitor() throws IOException {
FileUtils.writeFile(FILENAME, "property=originalValue");
file = new File(FILENAME);
monitor = new FileMonitor(FILENAME, MONITOR_DURATION);
}
@After
public void deleteFile() {
FileUtils.deleteIfExists(FILENAME);
}
@Test
public void shouldNotifyWhenFileModified() throws Exception {
monitor.addListener(createCountingFileChangeListener());
waitForFileChangeNotifications(1);
monitor.start();
alterFileToChangeLastModified();
joinWaitThread();
assertEquals(1, numberOfFileChangeNotifications());
}
@Test
public void shouldSupportMultipleListeners() throws Exception {
monitor.addListener(createCountingFileChangeListener());
monitor.addListener(createCountingFileChangeListener());
waitForFileChangeNotifications(2);
monitor.start();
alterFileToChangeLastModified();
joinWaitThread();
assertEquals(2, numberOfFileChangeNotifications());
assertAllListenersDistinctlyNotified();
}
@Test
public void shouldOnlyReportOnceForSingleModification()
throws InterruptedException {
// slow--must wait on timeout. Better way?
monitor.addListener(createCountingFileChangeListener());
waitForFileChangeNotifications(2);
monitor.start();
alterFileToChangeLastModified();
joinWaitThread();
assertEquals(1, numberOfFileChangeNotifications());
}
@Test
public void shouldReportMultipleTimesForMultipleModification()
throws InterruptedException {
monitor.addListener(createCountingFileChangeListener());
waitForFileChangeNotifications(1);
monitor.start();
alterFileToChangeLastModified();
joinWaitThread();
waitForFileChangeNotifications(2);
alterFileToChangeLastModified();
joinWaitThread();
assertEquals(2, numberOfFileChangeNotifications());
}
private FileChangeListener createCountingFileChangeListener() {
return new FileChangeListener() {
@Override
public void modified() {
fileChangeNotifications.add(this);
}
};
}
private int numberOfFileChangeNotifications() {
return fileChangeNotifications.size();
}
private void waitForFileChangeNotifications(final int expected) {
waitThread = new Thread(new Runnable() {
@Override
public void run() {
while (numberOfFileChangeNotifications() != expected) {
sleep(1);
}
}
});
waitThread.start();
}
private void sleep(int ms) {
try {
Thread.sleep(ms);
} catch (InterruptedException exception) {
fail(exception.getMessage());
}
}
private void alterFileToChangeLastModified() {
file.setLastModified(file.lastModified() + 1000);
}
private void joinWaitThread() throws InterruptedException {
waitThread.join(TIMEOUT);
}
private void assertAllListenersDistinctlyNotified() {
Set notifications =
new HashSet();
notifications.addAll(fileChangeNotifications);
assertEquals(fileChangeNotifications.size(),
notifications.size());
}
}
Wow, that’s a bit longer than I would have hoped. Yes, there are better ways to test the threading. And there may be better threading mechanisms to implement the monitor, although I think the Timer is pretty simple. I don’t do enough threading, so I usually code a “familiar” solution (i.e. something I already know well) initially, and then refactor into a more expressive and concise solution. I’m pretty sure this test is flawed. But rather than invest in fixing it, I reconsidered my design (by virtue of wanting an easier way to test it).
The SUT is really two classes, the monitor and the task. Each one can easily be tested separately, in isolation, as a unit. Sure, I want to see the threading in action, but perhaps that’s better relegated to an integration test (which might also double as an acceptance test). I decided to change the solution to accommodate more focused and more robust tests. The file monitor tests are pretty much what was there before (with some small additional refactorings), except that there’s now no concern over threading–I just test the run method:
import java.io.*;
import org.junit.*;
import static org.mockito.Mockito.*;
import org.mockito.*;
public class FileMonitor_TaskTest {
private static final String FILENAME = "FileMonitorTest.properties";
private FileMonitor.Task task = new FileMonitor.Task(FILENAME);
@Mock
private FileChangeListener listener;
@Before
public void initializeMockito() {
MockitoAnnotations.initMocks(this);
}
@BeforeClass
public static void createFile() throws IOException {
FileUtils.writeFile(FILENAME, "property=originalValue");
}
@AfterClass
public static void deleteFile() {
FileUtils.deleteIfExists(FILENAME);
}
@Before
public void createTask() {
task = new FileMonitor.Task(FILENAME);
}
@Test
public void shouldNotNotifyWhenFileNotModified() {
task.addListener(listener);
task.run();
verify(listener, never()).modified();
}
@Test
public void shouldNotifyWhenFileModified() throws Exception {
task.addListener(listener);
changeFileLastModifiedTime();
task.run();
verify(listener, times(1)).modified();
}
@Test
public void shouldSupportMultipleListeners() throws Exception {
task.addListener(listener);
FileChangeListener secondListener = mock(FileChangeListener.class);
task.addListener(secondListener);
changeFileLastModifiedTime();
task.run();
verify(listener, times(1)).modified();
verify(secondListener, times(1)).modified();
}
@Test
public void shouldOnlyReportOnceForSingleModification()
throws InterruptedException {
task.addListener(listener);
task.run();
changeFileLastModifiedTime();
task.run();
task.run();
verify(listener, times(1)).modified();
}
@Test
public void shouldReportMultipleTimesForMultipleModification()
throws InterruptedException {
task.addListener(listener);
task.run();
changeFileLastModifiedTime();
task.run();
changeFileLastModifiedTime();
task.run();
verify(listener, times(2)).modified();
}
private void changeFileLastModifiedTime() {
File file = new File(FILENAME);
file.setLastModified(file.lastModified() + 1000);
}
}
I introduced Mockito to provide a simple verifying stub for the listener. I suppose I should also stub out interactions with File, but I’ll incur the speed penalty for now. Next, I needed to test the remaining FileMonitor code. To do this involved proving that it creates and schedules a task appropriately using a timer, and that it handles listeners appropriately.
import xxx.CollectionUtil;
import java.util.Timer;
import org.junit.*;
import org.mockito.*;
import static org.mockito.Mockito.*;
public class FileMonitorTest {
@Mock
private Timer timer;
@Before
public void initializeMockito() {
MockitoAnnotations.initMocks(this);
}
@Test
public void shouldScheduleTimerWhenStarted() {
String filename = "filename";
long duration = 10;
FileMonitor monitor = new FileMonitor(filename, duration) {
@Override
protected Timer createTimer() {
return timer;
}
};
monitor.start();
verify(timer).schedule(any(FileMonitor.Task.class),
eq(duration), eq(duration));
}
@Test
public void shouldDelegateListenersToTask() {
FileChangeListener listener = mock(FileChangeListener.class);
FileMonitor monitor = new FileMonitor("", 0);
monitor.addListener(listener);
CollectionUtil.assertContains(
monitor.getTask().getListeners(), listener);
}
}
The design of the production code had to change based on my interest in better tests. A number of small, incremental refactoring steps led me to this solution:
import java.io.*;
import java.util.*;
public class FileMonitor {
private final long duration;
private Task task;
public FileMonitor(String filename, long durationInMs) {
task = new Task(filename);
this.duration = durationInMs;
}
public void start() {
long initialDelay = duration;
createTimer().schedule(task, initialDelay, duration);
}
protected Timer createTimer() {
final boolean isDaemon = true;
return new Timer(isDaemon);
}
Task getTask() {
return task;
}
public void addListener(FileChangeListener listener) {
task.addListener(listener);
}
static class Task extends TimerTask {
private final File file;
private long lastModified;
private Set listeners =
new HashSet();
Task(String filename) {
this.file = new File(filename);
lastModified = file.lastModified();
}
public void addListener(FileChangeListener listener) {
listeners.add(listener);
}
Set<FileChangeListener> getListeners() {
return listeners;
}
@Override
public void run() {
if (file.lastModified() > lastModified) {
lastModified = file.lastModified();
for (FileChangeListener listener: listeners) {
listener.modified();
}
}
}
}
}
Well, most interestingly, I no longer have any tests that must deal with additional threads. I test that the monitor delegates appropriately, and I test that the task works properly when run. I still want the integration tests, but I think they can be relegated to a higher, acceptance level test for the story:
reload properties at runtime
Otherwise, at this point, I have a high level of confidence in my code. I know the Timer class works, and I know my two classes work, and I’ve also demonstrated to myself that they all work together. I’m ready to move on. Some additional relevant points:
-
Threading is tough to get right, and it can be even tougher to test it.
-
My final solution is a few more lines of code due to supporting my interest in more focused testing. I’m ok with the additional code because of the flexibility having the tests gives me, and more often than not, having them allows me to easily eliminate duplication elsewhere.
-
It’s hard to be overdo the SRP. Far more classes than not weigh in the wrong direction of too many responsibilities.
-
The original solution, probably fine otherwise, is what you might expect from someone uninterested in TDD. It works (I think) but it represents rigid code, code that is more costly to change.
