views:

388

answers:

7

Thank you all for your help. A number of you posted (as I should have expected) answers indicating my whole approach was wrong, or that low-level code should never have to know whether or not it is running in a container. I would tend to agree. However, I'm dealing with a complex legacy application and do not have the option of doing a major refactoring for the current problem.

Let me step back and ask the question the motivated my original question.

I have a legacy application running under JBoss, and have made some modifications to lower-level code. I have created a unit test for my modification. In order to run the test, I need to connect to a database.

The legacy code gets the data source this way:

(jndiName is a defined string)

Context ctx = new InitialContext();
DataSource dataSource = (DataSource) ctx.lookup(jndiName);

My problem is that when I run this code under unit test, the Context has no data sources defined. My solution to this was to try to see if I'm running under the application server and, if not, create the test DataSource and return it. If I am running under the app server, then I use the code above.

So, my real question is: What is the correct way to do this? Is there some approved way the unit test can set up the context to return the appropriate data source so that the code under test doesn't need to be aware of where it's running?


For Context: MY ORIGINAL QUESTION:

I have some Java code that needs to know whether or not it is running under JBoss. Is there a canonical way for code to tell whether it is running in a container?

My first approach was developed through experimention and consists of getting the initial context and testing that it can look up certain values.

private boolean isRunningUnderJBoss(Context ctx) {
        boolean runningUnderJBoss = false;
        try {
            // The following invokes a naming exception when not running under
            // JBoss.
            ctx.getNameInNamespace();

            // The URL packages must contain the string "jboss".
            String urlPackages = (String) ctx.lookup("java.naming.factory.url.pkgs");
            if ((urlPackages != null) && (urlPackages.toUpperCase().contains("JBOSS"))) {
                runningUnderJBoss = true;
            }
        } catch (Exception e) {
            // If we get there, we are not under JBoss
            runningUnderJBoss = false;
        }
        return runningUnderJBoss;
    }

Context ctx = new InitialContext();
if (isRunningUnderJboss(ctx)
{
.........

Now, this seems to work, but it feels like a hack. What is the "correct" way to do this? Ideally, I'd like a way that would work with a variety of application servers, not just JBoss.

+1  A: 

Perhaps something like this ( ugly but it may work )

 private void isRunningOn( String thatServerName ) { 

     String uniqueClassName = getSpecialClassNameFor( thatServerName );
     try { 
         Class.forName( uniqueClassName );
     } catch ( ClassNotFoudException cnfe ) { 
         return false;
     }
     return true;
 }

The getSpecialClassNameFor method would return a class that is unique for each Application Server ( and may return new class names when more apps servers are added )

Then you use it like:

  if( isRunningOn("JBoss")) {
         createJBossStrategy....etcetc
  }
OscarRyz
A: 

A clean way to do this would be to have lifecycle listeners configured in web.xml. These can set global flags if you want. For example, you could define a ServletContextListener in your web.xml and in the contextInitialized method, set a global flag that you're running inside a container. If the global flag is not set, then you are not running inside a container.

Eddie
Since when are global flags clean?
Tom Hawtin - tackline
Global flags are not always evil. Too much global state usually indicates a bad design, but a single global flag is reasonable for this kind of information. If you have a better idea, suggest it yourself.
Eddie
+1  A: 

There are a couple of ways to tackle this problem. One is to pass a Context object to the class when it is under unit test. If you can't change the method signature, refactor the creation of the inital context to a protected method and test a subclass that returns the mocked context object by overriding the method. That can at least put the class under test so you can refactor to better alternatives from there.

The next option is to make database connections a factory that can tell if it is in a container or not, and do the appropriate thing in each case.

One thing to think about is - once you have this database connection out of the container, what are you going to do with it? It is easier, but it isn't quite a unit test if you have to carry the whole data access layer.

For further help in this direction of moving legacy code under unit test, I suggest you look at Michael Feather's Working Effectively with Legacy Code.

Yishai
+5  A: 

The whole concept is back to front. Lower level code should not be doing this sort of testing. If you need a different implementation pass it down at a relevant point.

Tom Hawtin - tackline
Perhaps he's trying to determinate if he needs a different implementation in first place.
OscarRyz
Then he needs to place that higher up.
Tom Hawtin - tackline
+3  A: 

The whole approach feels wrong headed to me. If your app needs to know which container it's running in you're doing something wrong.

When I use Spring I can move from Tomcat to WebLogic and back without changing anything. I'm sure that with proper configuration I could do the same trick with JBOSS as well. That's the goal I'd shoot for.

duffymo
Many people made helpful suggestions, but this answer describes the approach I ended up taking.I ripped out any logic in the code under test that was trying to see if it was in the test environment. I then dug through the documentation and figured out how to create my own initial context and data source so that the code under test just got a different data source when running under JUnit.Thank you to all that answered.
Mark
Mark - would you mind posting that configuration? It would be helpful to others that find themselves in a similar situation.
Rich Kroll
+4  A: 

Some combination of Dependency Injection (whether through Spring, config files, or program arguments) and the Factory Pattern would usually work best.

As an example I pass an argument to my Ant scripts that setup config files depending if the ear or war is going into a development, testing, or production environment.

Rob Spieldenner
+1 for DI, this approach would allow you to easily pass mock objects in
matt b
Agreed. If you want to keep it simple, you can just use a Strategy Pattern to get the datasource at runtime. A config file would determine which strategy to use based on environment, much like the suggested option in the answer. You will need a dev datasource of course, but that is pretty simple to write.
Robin
+1  A: 
Context ctx = new InitialContext();
DataSource dataSource = (DataSource) ctx.lookup(jndiName);

Who constructs the InitialContext? Its construction must be outside the code that you are trying to test, or otherwise you won't be able to mock the context.

Since you said that you are working on a legacy application, first refactor the code so that you can easily dependency inject the context or data source to the class. Then you can more easily write tests for that class.

You can transition the legacy code by having two constructors, as in the below code, until you have refactored the code that constructs the class. This way you can more easily test Foo and you can keep the code that uses Foo unchanged. Then you can slowly refactor the code, so that the old constructor is completely removed and all dependencies are dependency injected.

public class Foo {
  private final DataSource dataSource;
  public Foo() { // production code calls this - no changes needed to callers
    Context ctx = new InitialContext();
    this.dataSource = (DataSource) ctx.lookup(jndiName);
  }
  public Foo(DataSource dataSource) { // test code calls this
    this.dataSource = dataSource;
  }
  // methods that use dataSource
}

But before you start doing that refactoring, you should have some integration tests to cover your back. Otherwise you can't know whether even the simple refactorings, such as moving the DataSource lookup to the constructor, break something. Then when the code gets better, more testable, you can write unit tests. (By definition, if a test touches the file system, network or database, it is not a unit test - it is an integration test.)

The benefit of unit tests is that they run fast - hundreds or thousands per second - and are very focused to testing just one behaviour at a time. That makes it possible run then often (if you hesitate running all unit tests after changing one line, they run too slowly) so that you get quick feedback. And because they are very focused, you will know just by looking at the name of the failing test that exactly where in the production code the bug is.

The benefit of integration tests is that they make sure that all parts are plugged together correctly. That is also important, but you can not run them very often because things like touching the database make them very slow. But you should still run them at least once a day on your continuous integration server.

Esko Luontola