I don’t think It’s just me, but maybe I am the only person who gets easily frustrated working on Eclipse plugins. I almost feel like I’ve missed a secret meeting sometimes that ties together all of the loose ends of plugin development. One particular problem I ran into lately was with adding a project decorator. I thought it would be nice if Frame2 projects had some kind of icon that identified them. I added the extension to the plugin and set the enablement for IJavaProject. I thought this was appropriate, since Frame2 projects are Java projects. This setting resulted in every single file in the project being decorated. I thought this was a bit strange, so I added a lightweight label decorator class. The ILightweightLabelDecorator interface defines a method that implementors use to apply the decorator to objects. Sure enough, this method was called once for every file in the project. However, I could not filter these inputs since every time the method was called, the input parameter was the same!

Changing the enablement class to IProject yielded the desired behavior – the decorator method only gets called once for each project in the workspace. A little filtering to check if a project is a Java project and has the Frame2 nature, and presto! – project decorators.

Now, why did I see the behavior I did? Chances are it’s a bug, but it seems equally likely that it’s supposed to behave that way and I haven’t found the decoder ring that tells me that.

There’s been a bug in the Frame2 Eclipse plugin from day 1 that I believe I have finally squashed. The reading and writing of the frame2-config file happened outside of the Eclipse API, and when the file was updated using a wizard the resource got out of sync with the workspace. It hasn’t really been a big issue, but it’s annoying to always be asked by Eclipse if you’d like to reload the file since it has changed.

My original stab at fixing the problem was to change the file as it’s always been changed, but call refreshLocal() on the Eclipse IFile representation. The call never threw an exception, but it also never seemed to do anything. I finally refactored the model reading and writing code to use Eclipse APIs and platform objects. This worked fine until I got to the point where I needed to write the modified configuration to file. The existing API required an OutputStream, while the Eclipse API wanted an InputStream.

Luckily, Google turned up some suggestions, most notably using piped streams. My first attempt looked like this:

PipedInputStream in = new PipedInputStream();
PipedOutputStream out = new PipedOutputStream(in);
config.write(out);
modelFile.setContents(in, true, true, monitor);

Config is the Frame2 model object that knows how to write itself to an OutputStream, and modelFile is the IFile object representing the config file in Eclipse. Anybody who’s ever used piped streams in Java should be able to tell what happened when I ran the code – it hung. It always pays to read the JavaDoc in addition to copying somebody’s snippet of code from the web. What the JavaDoc says is this: “Attempting to use both objects from a single thread is not recommended as it may deadlock the thread.” In this case, I think may is a bit soft. It deadlocked every single time I ran it.

Based on another online snippet, I modified the code to this:

PipedInputStream in = new PipedInputStream();
final PipedOutputStream out = new PipedOutputStream(in);
new Thread(new Runnable() {
  public void run() {
    try {
      config.write(out);
    } catch (IOException e) {
      e.printStackTrace();
    }
  }
}).start();
modelFile.setContents(in, true, true, monitor);

This is the variant that I ran across most often online. It uses two threads to avoid deadlocking, but it still has a fatal flaw. The setContents() method always throws an IOException with this code. The IOException’s message is always “Pipe Broken”. I found a *lot* of entries online from people asking how best to ignore this exception. There were lots of helpful explanations as to the cause of the error, but I didn’t see a single solution. Luckily, the answer was easy to find in this case.

The “Pipe Broken” message is happening because the the thread doing the writing to the OutputStream has terminated and nothing more is being added to it. The solution is to simply close the OutputStream after writing the configuration to it. The OutputStream is correctly marked as being finished and the exception is not thrown. An examination of the read() method in PipedInputStream shows why the OutputStream must be closed:

if (closedByWriter) {
  /* closed by writer, return EOF */
  return -1;
}if ((writeSide != null) && (!writeSide.isAlive()) && (--trials < 0)) {
  throw new IOException("Pipe broken");
}

If the writing side of the pipe simply exits, writeSide.isAlive() returns false and the exception is thrown. If the OutputStream is closed, however, the reading side of the pipe sees it as EOF and terminates gracefully.

To sum up, the correct example for using piped streams in Java should look like this:

PipedInputStream in = new PipedInputStream();
PipedOutputStream out = new PipedOutputStream(in);
new Thread(new Runnable() {
  public void run() {
    try {
      doSomethingWithOut(out);
      out.close(); // Critical!
    } catch (IOException e) {
      e.printStackTrace(); // Do more than just this, OK?
    }
  }
}).start();
doSomethingWithIn(in);

This seems to happen to me no matter what project I’m on. I try to implement a simple (or not so simple) feature and end up exposing at least one bug that’s been hiding around for quite some time.

As an illustration, I was adding a feature to Frame2 today to allow some parameters to be passed between events within a mapping – without making developers use the HttpSession. After a couple of false starts, I had a pretty good idea of how to make the changes and starting ripping out code. I came up with several unit tests, and modifying code to make the tests pass. Pretty standard Test Driven Development…until I wrote the test that made sure event chain validation was working.

Before I explain the bug, I’ll give a high level description of how Frame2 works when submitting form data:

1. Form is filled out by user and submitted.
2. Frame2 looks up the appropriate event from the form action.
3. An instance of the appropriate event is created and populated by the introspector.
4. Based on the mapping for the event, Frame2 will then validate the event. This may involve the Commons Validator if it is enabled and/or overriding the Event.validate() method.
5. If validation fails, the mapping must specify a location for the framework to forward back to. This is usually the same page where the data was entered.
6. If validation passes, the framework then iterates through the list of event handlers, calling the handle() method. If handle() returns null, the framework continues to the next handler. A non-null result usually indicates an error, and the framework will forward to the location named by the result.
7. Once all of the handlers have been invoked, the framework will default to the view specified in the mapping. Usually this is just a JSP, but Frame2 supports forwarding to events – called “chaining”.
8. For the chained event, Frame2 is supposed to start back at step 2 and continue processing until it gets to the end of the chain.

Notice how I say “supposed to” in step 9? The framework was actually only going back to step 6, bypassing validation for all events except the first in the chain. Subtle, yes, and unlikely to be noticed by most users – but it’s still a bug. The bug has now been fixed so that the framework goes back to step 2 like it’s supposed to.

In case you’re wondering what use event chaining is, here’s an example:

1. User enters a blog entry and submits the form.
2. Frame2 validates the form data and begins invoking handlers.
3. The blog entry handler persists the data, perhaps to a database.
4. The default view for the event mapping tells the framework to forward to the “View All Blog Entries” event.
5. The framework creates the appropriate object and begins invoking the handlers in the “View All” mapping.
6. The “View All” handler populates all of the blog entries into the “View All” event.
7. The default view for the “View All” mapping is a JSP page, which renders the blog entries using JSTL.

Chaining allows the “View All” event to remain its own entity that can be invoked separately, while allowing the same functionality to be used in other places where appropriate.

By way of comparison, the feature I added allows the above sequence to be reworked like this:

1. User enters a blog entry and submits the form.
2. Frame2 validates the form data and begins invoking handlers.
3. The blog entry handler persists the data, perhaps to a database.
4. The blog entry handler sets an attribute with the new entry id into the context.
5. The default view for the event mapping tells the framework to forward to the “View Specific Blog Entry” event.
6. The framework creates the appropriate object and uses the context value(s) with the introspector to set some values in the event.
7. The framework then begins invoking the handlers in the “View Specific” mapping.
8. The “View Specific” handler looks up the entry with the specified id and sets its data into the event.
9. The default view for the “View Specific” mapping is a JSP page, which renders the blog entry using JSTL.

Frame2 is almost ready to be released as a new version. I may still make some changes, but it’s definitely in Release Candidate stage now. There are only a few things that I may add before testing the heck out of it:

• Upgrade unit tests to JUnit 4. There are a lot of tests, especially the ones that mock a servlet container, that do a lot of repetitive work in setUp() . JUnit 4 has finally brought the ability to annotate methods to be run before and after all tests in the class with @BeforeClass and @AfterClass, respectively. Individual test setup activities can still be performed by annotating a method with @Before.
• Fix outstanding bugs. There’s only one right now, and it’s SOAP specific, so I may or may not fix it. It may take me longer to figure out how to reproduce the problem with unit tests than it’s worth.
• Add outstanding feature requests. Again, there’s only one, but it’s (in my opinion) a biggie – the ability to add parameters to the response when handling an event.
• Doc updates. The web services doc needs to be written, and I’m sure all of the other doc needs a good examination.

I’ll be testing the latest build with a web application that I’ve been working on for years that I’ve come to think of as an unofficial reference implementation of Frame2. I haven’t decided yet if the web app should become part of the project on Sourceforge or not. We’ll see.

Once the testing is done and I’ve released a new version of Frame2, I plan on working on the Eclipse plugin. The poor plugin has pretty much been ignored for several years now, and it needs some attention.

I’ve been mucking around in the codebase, cleaning up pieces of the API that annoy me. Some bright person thought that it was a good idea to have a method that returns an Iterator to access a collection. I’ve been cleaning up all of these methods, changing them so that they return an actual collection – occasionally made unmodifiable through the Collections class.

Naturally, this has led to unit test failures. Almost all of the failures have been easy to fix, but once in a while one pops up that makes me scratch my head. Here’s an example:

public void testGetIfEmpty() {
assertTrue(this.errors.isEmpty());
assertNull(this.errors.get(FOO));
assertEquals(0, this.errors.get().length);
assertEquals(0, this.errors.get(FOO).length);
}

In this instance, errors is a class that aggregates individual Error objects. The get(String) method returns an array of Error objects that have the specified key, while get() retrieves all Errors. Pretty simple, right? After my changes, the test fails on the assertNull() statement, which isn’t surprising.

The failing statement read assertNull(this.errors.iterator(FOO)) before I removed the iterator() method, which tells us a bit about how the API was originally coded. The iterator(String) returned null if there were no matching entries. However, as the unit test clearly shows, get(String) returns an empty array when it doesn’t find any matching Errors. Quite the difference, and potentially confusing – the developer has to remember which method may return null.

The API has now been changed to return an empty collection instead of null when no matching errors are found. Now, a user of the API simply has to call the method and loop over the result (zero times if it’s empty) without having to make null checks.

Whoever wrote this unit test obviously knew that one method returned null while the other returned an empty array for the same input, since the test passed. I only wish that person would have looked at those lines and noticed the inconsistency, instead of leaving it for me to find.

I’ve mentioned before that I’m using FindBugs to help make sure that the Frame2 code is up to snuff. The Eclipse compiler can catch a lot of things, bug FindBugs has some neat tricks up its sleeve to find things that Eclipse can’t. FindBugs uses static analysis of the code to search for patterns that indicate possible issues. Being the Type A person that I am, I always turn on all of the possible matches and whittle the list down. Quite often, however, FindBugs will find a false positive or two. Up until now, I’ve either tried to rework the code to avoid the warning or disabled the category altogether. I’ve never liked the approach of turning off a whole category just to avoid seeing a warning or two, so I spent the morning muttering to myself how useful it would be if FindBugs allowed me to ignore certain warnings like I can do with @SuppressWarnings.

Then I went and RTFM.

FindBugs has a filtering system that makes @SuppressWarnings look amateur. Here’s an example: in the SoapRequestProcessor, FindBugs marked a warning that an Exception was being caught when no Exception was being thrown. After looking at the code and verifying that the try block in question throws several different exceptions, I created a filter entry. The entry looks like this:

<Match>
<Class name="org.megatome.frame2.front.SoapRequestProcessor"/>
<Method name="getEvents"/>
<Bug pattern="REC_CATCH_EXCEPTION"/>
</Match>

This tells FindBugs to match a specific bug type in a specified method in a desired class. This entry can be used in both an inclusion or exclusion filter – I use it to exclude that warning from the results.

Here’s a more complex example:

<Match>
<Class name="~.*introspector\.Bean\d+" />
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/>
</Match>

There are some test classes that don’t exactly follow good rules of programming when it comes to dealing with mutability. Since they are test classes, I don’t really care to fix them. Instead I set up the filter to match all classes in an introspector package named Bean1, Bean2, etc. Simple as can be!

If you aren’t using FindBugs, you should be. Go check it out.