10

I have a Jersey resource that access the database. Basically it opens a database connection in the initialization of the resource. Performs queries on the resource's methods.

I have observed that when I do not use @Singleton, the database is being open at each request. And we know opening a connection is really expensive right?

So my question is, should I specify that the resource be singleton or is it really better to keep it at per request especially when the resource is connecting to the database?

My resource code looks like this:

//Use @Singleton here or not?
@Path(/myservice/)
public class MyResource {

    private ResponseGenerator responser;
    private Log logger = LogFactory.getLog(MyResource.class);

    public MyResource() {
        responser = new ResponseGenerator();
    }

    @GET
    @Path("/clients")
    public String getClients() {

        logger.info("GETTING LIST OF CLIENTS");

        return responser.returnClients();
    }

    ...
    // some more methods
    ...

}

And I connect to the database using a code similar to this:

public class ResponseGenerator {
    private Connection conn;
    private PreparedStatement prepStmt;
    private ResultSet rs;

    public ResponseGenerator(){
        Class.forName("org.h2.Driver");
        conn = DriverManager.getConnection("jdbc:h2:testdb");
    }

    public String returnClients(){
        String result;
        try{
           prepStmt = conn.prepareStatement("SELECT * FROM hosts");

           rs = prepStmt.executeQuery();

           ...
           //do some processing here
           ...
        } catch (SQLException se){
            logger.warn("Some message");
        } finally {
            rs.close();
            prepStmt.close();
            // should I also close the connection here (in every method) if I stick to per request
            // and add getting of connection at the start of every method
            // conn.close();
        }

        return result
    }

    ...
    // some more methods
    ...

}

Some comments on best practices for the code will also be helpful.

dexter
  • 1,869
  • 2
  • 14
  • 13

2 Answers2

0

Rather than thinking about making the resource a singleton, focus more on managing backend, service type objects like your ResponseGenerator class as singletons, which obviously shouldn't be instantiated every request.

Making the resource a singleton as well is one way of managing ResponseGenerator as a singleton, but it's not the only or necessarily the best way, see Access external objects in Jersey Resource class and How to wire in a collaborator into a Jersey resource? for ways to inject this into non-singleton resources.

Note that your ResponseGenerator class would need work before it would function as a singleton, whether injected into a per-request resource or instantiated in a singleton resource. It's not thread safe, and you would open a single connection on startup and reuse it across requests, which won't work, you should use a connection pool to do the heavy lifting of efficiently + safely reusing connections across requests.

Some comments on best practices for the code will also be helpful.

You'll get better responses on http://codereview.stackexchange.com, but:

  • ResponseGenerator is a poor name for a class (just about everything in a web application is a response generator).

  • don't use String as the return type of your service and object, use proper typed objects (eg it sounds like you're returning a java.util.List of something).

  • Don't swallow your SQLException, bubble it up to allow Jersey to generate a 5xx series response code in your resource.

  • Use final member variables.

  • Your log object should be static.

Community
  • 1
  • 1
Adrian Baker
  • 9,297
  • 1
  • 26
  • 22
-4

You best option is to use a framework like Spring with Jersey which I outlined in a similar post. The only difference is that instead of injecting a service bean you would inject a pooled DataSource and this can easily be configured using c3p0.

Example applicationContext.xml, notice the "scope" is set to prototype which is equivalent to a singleton in Spring parlance.

<bean id="pooledDataSource" scope="prototype" class="com.mchange.v2.c3p0.ComboPooledDataSource" destroy-method="close">
    <property name="jdbcUrl" value="${jpa.url}" />
    <property name="user" value="${jpa.username}" />
    <property name="password" value="${jpa.password}" />
    <property name="initialPoolSize" value="1" />
    <property name="minPoolSize" value="1" />
    <property name="maxPoolSize" value="3" />
    <property name="idleConnectionTestPeriod" value="500" />
    <property name="acquireIncrement" value="1" />
    <property name="maxStatements" value="50" />
    <property name="numHelperThreads" value="1" />
</bean>

In your MyResource.java you would simply add the following and Spring would inject it appropriately.

private DataSource pooledDataSource;
public void setPooledDataSource(DataSource pooledDataSource) {
    this.pooledDataSource = pooledDataSource;
}

Then you could change your ResponseGenerator to accept the DataSource and use this to query the database.

Community
  • 1
  • 1
Alex Winston
  • 651
  • 6
  • 10