Database TDD Part 26: Prepared Statements

Prepared statement support is essential in any RDBMS persistence layer. The issue is, how do I build this support safely? Even if it didn’t break the whole notion of encapsulating all this JDBC code, I wouldn’t want to entrust handing out PreparedStatement objects to clients.

As a general strategy, I can reverse the technique I use for retrieving information. Queries return rows in the form of a list of maps. Each map is a row, with column names and values being the key-value pairs in the map. The same strategy can work for inserting data using a prepared statement: clients will populate a list of map-rows. JDBC code can then manage the entire process from start to finish.

I’ll start in the simplest place, SQL generation.

SqlGeneratorTest

public void testPreparedInsert() {
   String sql = new SqlGenerator().createPreparedInsert(TABLE, COLUMNS);
   assertEquals("insert into t (a,b) values (?,?)", sql);
}

SqlGenerator

public String createPreparedInsert(String table, Column[] columns) {
   return String.format("insert into %s (%s) values (%s)", table,
         createColumnList(columns), createPlaceholderList(columns));
}

private Object createPlaceholderList(Column[] columns) {
   final Transformer questions = new Transformer() {
      public String transform(Object ignored) {
         return "?";
      }
   };
   return StringUtil.commaDelimit(columns, questions);
}

Nothing earth shattering there. These methods are getting easier to write as I add more functionality.

The new JDBC code:

JdbcAccessTest

public void testExecutePreparedStatement() {
  List<Map<String,Object>> rows = new ArrayList<Map<String,Object>>();
  Map<String,Object> row1 = new HashMap<String,Object>();
  row1.put(COLUMN_NAME, VALUE1);
  Map<String,Object> row2 = new HashMap<String,Object>();
  row2.put(COLUMN_NAME, VALUE2);

  rows.add(row1);
  rows.add(row2);

  String sql = String.format("insert into %s values(?)", TABLE);
  access.executeAll(sql, COLUMNS, rows);

  assertResults(access.executeQuery(createSelectSql(), COLUMNS));
}

private void assertResults(List<Map<String, Object>> rows) {
  assertEquals(2, rows.size());
  assertContains(rows, VALUE1);
  assertContains(rows, VALUE2);
}

JdbcAccess

private PreparedStatement preparedStatement;
...
public void executeAll(String sql, Column[] columns,
      List<Map<String, Object>> rows) {
   try {
      createPreparedStatement(sql);

      for (Map<String, Object> row : rows) {
         int i = 0;
         for (Column column : columns)
            preparedStatement.setObject(++i, row.get(column.getName()));
         preparedStatement.execute();
      }

      connection.close();
   }
   catch (SQLException e) {
      throw new JdbcException(sql, e);
   }
}

OK, so here’s the problem: I’m introducing a construct that is solely for performance needs. Yet I now force the client of JdbcAccess to populate a list of map-rows in order to use a PreparedStatement. Yuk. Granted, database performance concerns are far more significant than in-memory performance. But it seems unfortunate to add performance overhead at the same time I’m trying to address it. I’ll take a look at this in either the next or a future installment.

Now back to the bane of this suite, the stub/mock friendly PersisterTest. I finally got fed up with the difficulty of understanding tests in this class. I spent about 15 minutes incrementally refactoring it so that the mock JdbcAccess definitions appear within each test method (for the most part). This makes it clear which behavior of JdbcAccess is getting mocked and which is not; I think it makes the tests much easier to follow.

PersisterTest

package persistence;

import java.util.*;

import junit.framework.*;
import persistence.types.*;
import sql.*;

public class PersisterTest extends TestCase {
   private static final String TABLE = "x";

   private static final Persistable RETURN_OBJECT1 = new Persistable() {
      public Object get(String key) {
         return null;
      }
   };
   private static final Persistable RETURN_OBJECT2 = new Persistable() {
      public Object get(String key) {
         return null;
      }
   };

   private static final String BAD_KEY = "not found";
   private static final String COLUMN1 = "a";
   private static final String COLUMN2 = "b";
   private static final String ROW1_VALUE1 = "a1";
   private static final String ROW1_VALUE1_PATTERN = "a%";
   private static final String ROW1_VALUE2 = "a2";
   private static final String ROW2_VALUE1 = "b1";
   private static final String ROW2_VALUE2 = "b2";
   private static final Column[] COLUMNS = { new StringColumn(COLUMN1),
         new StringColumn(COLUMN2) };

   private String lastSql;
   private PersistableMetadata<Persistable> metadata;

   private Persister<Persistable> persister;
   private Persistable persistable;

   protected void setUp() {
      metadata = new PersistableMetadata<Persistable>() {
         public String getTable() {
            return TABLE;
         }

         public String getKeyColumn() {
            return COLUMNS[0].getName();
         }

         public Column[] getColumns() {
            return COLUMNS;
         }

         public Persistable create(Map<String, Object> row) {
            if (row.get(COLUMN1).equals(ROW1_VALUE1)
                  && row.get(COLUMN2).equals(ROW1_VALUE2))
               return RETURN_OBJECT1;
            if (row.get(COLUMN1).equals(ROW2_VALUE1)
                  && row.get(COLUMN2).equals(ROW2_VALUE2))
               return RETURN_OBJECT2;
            return null;
         }
      };

      persistable = new Persistable() {
         public Object get(String key) {
            if (key.equals(COLUMN1))
               return ROW1_VALUE1;
            if (key.equals(COLUMN2))
               return ROW1_VALUE2;
            return null;
         }
      };
   }

   public void testSave() {
      createPersister(new JdbcAccess() {
         public void execute(String sql) {
            lastSql = sql;
         }
      });

      persister.save(persistable);
      assertLastSql(String.format("insert into %s (%s,%s) values ('%s','%s')",
            TABLE, COLUMN1, COLUMN2, ROW1_VALUE1, ROW1_VALUE2));
   }

   public void testSaveAll() {
      createPersister(new JdbcAccess() {
         public void executeAll(String sql, Column[] columns,
               List<Map<String, Object>> rows) {
            lastSql = sql;
            Assert.assertEquals(COLUMNS, columns);
            Assert.assertEquals(1, rows.size());
            Map<String, Object> row = rows.get(0);
            Assert.assertEquals(ROW1_VALUE1, row.get(COLUMN1));
         }
      });

      Collection<Persistable> collection = new ArrayList<Persistable>();
      collection.add(persistable);
      persister.saveAll(collection);
      assertEquals(String.format("insert into %s (%s,%s) values (?,?)", TABLE,
            COLUMN1, COLUMN2), lastSql);
   }

   public void testFindBy() {
      createPersister(new JdbcAccess() {
         public Map<String, Object> executeQueryExpectingOneRow(String sql,
               Column[] columns) {
            Assert.assertEquals(COLUMNS, columns);
            lastSql = sql;
            return createRow1();
         }
      });

      final String key = ROW1_VALUE1;
      assertEquals(RETURN_OBJECT1, persister.find(key));
      assertLastSql(String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, key));
   }

   public void testFindNotFound() {
      createPersister(new JdbcAccess() {
         public Map<String, Object> executeQueryExpectingOneRow(String sql,
               Column[] columns) {
            lastSql = sql;
            return null;
         }
      });
      assertNull(persister.find(BAD_KEY));
      assertLastSql(String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, BAD_KEY));
   }

   public void testGetAll() {
      createMockedExecuteQueryPersister();
      assertQueryResults(persister.getAll());
      assertEquals(String.format("select %s,%s from %s", COLUMN1, COLUMN2,
            TABLE), lastSql);
   }

   public void testFindMatches() {
      createMockedExecuteQueryPersister();
      assertQueryResults(persister.findMatches(COLUMNS[0], ROW1_VALUE1_PATTERN));
      assertLastSql(String.format("select %s,%s from %s where %s like '%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, ROW1_VALUE1_PATTERN));
   }

   public void testFindWithCriteria() {
      createMockedExecuteQueryPersister();
      Criteria criteria = new EqualsCriteria(COLUMNS[0], ROW1_VALUE1);
      assertQueryResults(persister.find(criteria));
      assertLastSql(String.format("select %s,%s from %s where %s='%s'",
            COLUMN1, COLUMN2, TABLE, COLUMN1, ROW1_VALUE1));
   }

   private void createMockedExecuteQueryPersister() {
      createPersister(new JdbcAccess() {
         public List<Map<String, Object>> executeQuery(String sql,
               Column[] columns) {
            lastSql = sql;
            List<Map<String, Object>> results = new ArrayList<Map<String, Object>>();
            results.add(createRow1());
            results.add(createRow2());
            return results;
         }
      });
   }

   private void createPersister(JdbcAccess accessMock) {
      persister = new Persister<Persistable>(metadata, accessMock);
   }

   private void assertLastSql(String sql) {
      assertEquals(sql, lastSql);
   }

   private void assertQueryResults(List<Persistable> results) {
      assertEquals(2, results.size());
      assertTrue(results.contains(RETURN_OBJECT1));
      assertTrue(results.contains(RETURN_OBJECT2));
   }

   protected Map<String, Object> createRow1() {
      return createRow(ROW1_VALUE1, ROW1_VALUE2);
   }

   protected Map<String, Object> createRow2() {
      return createRow(ROW2_VALUE1, ROW2_VALUE2);
   }

   protected Map<String, Object> createRow(String value1, String value2) {
      Map<String, Object> row = new HashMap<String, Object>();
      row.put(COLUMN1, value1);
      row.put(COLUMN2, value2);
      return row;
   }
}

Persister

public void saveAll(Collection<T> collection) {
   String sql = new SqlGenerator().createPreparedInsert(metadata.getTable(),
         metadata.getColumns());
   access.executeAll(sql, metadata.getColumns(), createInsertRows(collection));
}

private List<Map<String, Object>> createInsertRows(Collection<T> collection) {
   List<Map<String, Object>> rows = new ArrayList<Map<String, Object>>();
   for (T persistable : collection)
      rows.add(createInsertRow(persistable));
   return rows;
}

private Map<String, Object> createInsertRow(T persistable) {
   Map<String, Object> row = new HashMap<String, Object>();
   for (Column column : metadata.getColumns()) {
      Object value = persistable.get(column.getName());
      row.put(column.getName(), value);
   }
   return row;
}

After creating the saveAll code in Persister, I did a little refactoring on the save method:

Persister

public void save(T persistable) {
   String sql = new SqlGenerator().createInsert(metadata.getTable(),
         metadata.getColumns(), extractValues(persistable, metadata
               .getColumns()));
   access.execute(sql);
}

private Object[] extractValues(T persistable, Column[] columns) {
   Object[] values = new Object[columns.length];
   for (int i = 0; i < columns.length; i++)
      values[i] = persistable.get(columns[i].getName());
   return values;
}

It looks like there are some good similarities between the two save methods that I want to try to reconcile in the near future.

To test all this code out I wrote the following (live) database test.

CustomerAccessTest

package domain;

import java.util.*;

import persistence.*;

import junit.framework.*;

public class CustomerAccessTest extends TestCase {
   private CustomerAccess access;

   protected void setUp() {
      access = new CustomerAccess();
      JdbcAccess jdbc = new JdbcAccess();
      jdbc.execute("truncate table " + access.getTable());
   }

   public void testPersist() {
      final String name = "a";
      final String id = "1";
      final int amount = 100;

      Customer customer = new Customer(id, name);
      customer.charge(amount);

      access.save(customer);
      Customer retrievedCustomer = access.find(id);
      assertEquals(id, retrievedCustomer.getId());
      assertEquals(name, retrievedCustomer.getName());
      assertEquals(amount, retrievedCustomer.getBalance());
   }

   public void testPersistLots() {
      final int count = 10;
      Collection<Customer> customers = new ArrayList<Customer>();
      for (int i = 0; i < count; i++) {
         String id = "" + i;
         Customer customer = new Customer(id, "a");
         customer.charge(i);
         customers.add(customer);
      }

      access.saveAll(customers);

      for (int i = 0; i < count; i++) {
         String id = "" + i;
         Customer retrievedCustomer = access.find(id);
         assertEquals(i, retrievedCustomer.getBalance());
      }
   }
}

In doing so I recognized that the customer table needed to get cleared out with each execution, so I added the setUp method. Here’s the implementation in the DataAccess superclass. (If you’re looking at older code, note that I recognized and corrected a deficiency with my declaration of the parameterized type.)

DataAccess

abstract public class DataAccess<T extends Persistable> implements
      PersistableMetadata<T> {
   ...
   public void saveAll(Collection<T> collection) {
      new Persister<T>(this).saveAll(collection);
   }
   ...
}

I note that I’ve just added another “live” persistence test to CustomerAccess. This will start to increase the amount of time to execute my complete suite of tests. Still, I’m at a very comfortable ~5 seconds. I think the next time I feel compelled to add such a live “confidence” test I’ll revisit what I want to do about this potential execution time bloat. Maybe it’s not a concern–I’m not writing these tests for every possible DataAccess subclass. I think there are a few missing tests that I might add, but I don’t know that they’ll severely increase test execution time.

I’m still acting non-traditionally, by the way, in working this backward. Inside-out, some might call it. This is partly because the need for PreparedStatement support is artificial (I was too lazy to dream up and work something down from the application level). It’s also because sometimes it’s the easiest way for me to approach solving the problem.

Database TDD Part 25: Modifying the Application Mock

Before launching into the latest changes, first I’d like say that puppies and sleep don’t mix.

The Persister class now supports a generalized find that takes a Criteria object as an argument. Backing things up, the code in DataAccess becomes:

DataAccess

public List findMatches(Criteria criteria) {
   return new Persister(this).find(criteria);
}

Hmm, I don’t recall writing a test for this code. That’s something I’ll have to go back and do. Remind me please. It’ll get tested in context, but I’m not willing to let that sit for long.

Code within Application becomes:

public List findMatchingCustomers(String namePattern) {
   Column name = customerAccess.getColumn(Customer.NAME);
   return customerAccess.findMatches(new LikeCriteria(name, namePattern));
}

There are two significant results of this change. As mentioned, I wanted to back responsibility for coming up with query criteria to the client code, i.e. the application itself in this case. That closes the persistence layer off to all new inevitable query criteria. But this decision results in the fact that the application layer must now import from the persistence package. The persistence package is one big honking set of code. Maybe it’s time for the package to be broken out. The obvious point of separation, based on this new change, is to pull Criteria and perhaps SQL generation classes out into their own package.

Also, note that the client is now responsible for extracting a Column object based on a name. This is because the criteria objects require complete column information in order to be able to construct the appropriate SQL. One alternative would be to pass the column name to the criteria object, and let the criteria object obtain the appropriate column, but I don’t like the dependency that introduces. Another alternative would be to resurrect the column information when the SQL is rendered, in the Persister class. For now I’ll stick with what I have, and fix it later if it looks to be unacceptable duplication in client code.

Now, the other problem with changing Application implementation details is that my mock no longer works! Ugh. Bad trap–this is why I suggested that you should keep mocks as simple as possible. The problem is that mocks must inherently mirror some level of implementation detail. I simplified my MockCustomerAccess class considerably:

MockCustomerAccess

package application;

import java.util.*;

import persistence.*;
import sql.*;
import domain.*;

class MockCustomerAccess extends CustomerAccess {
   private Persistable customer;

   public List findMatches(Criteria criteria) {
      List results = new ArrayList();
      if (criteria.sqlString().indexOf("'n%'") &gt;= 0)
         results.add((Customer)customer);
      return results;
   }

   public void save(Persistable persistable) {
      customer = persistable;
   }
}

It’s now pretty hard-coded to the test that uses it. That’s the way I want it. In fact, I’d just as soon the mock appear in the test itself, that way the two remain tightly bound:

MockCustomerAccess

public void testBrowseCustomers() {
   final String id1 = "id1";
   final String name1 = "name1";
   final String matchingCustomerPattern = "'n%'";

   application.setCustomerAccess(new CustomerAccess() {
      private Persistable customer;

      public List findMatches(Criteria criteria) {
         List results = new ArrayList();
         if (criteria.sqlString().indexOf(matchingCustomerPattern) &gt;= 0)
            results.add((Customer)customer);
         return results;
      }

      public void save(Persistable persistable) {
         customer = persistable;
      }
   });

   Customer customer1 = new Customer(id1, name1);
   application.add(customer1);
   assertTrue(application.findMatchingCustomers("'a%'").isEmpty());
   List customers = application
         .findMatchingCustomers(matchingCustomerPattern);
   assertEquals(1, customers.size());
   Customer retrievedCustomer = customers.get(0);
   assertEquals(id1, retrievedCustomer.getId());
   assertEquals(name1, retrievedCustomer.getName());
}

The test is now a bit long, but I’m ok with that. A second test related to customer access would no doubt expose duplication to be refactored.

The dogs kept me up all night, so I’m looking to wrap this up. Here’s the new package structure:

persistence:
   DataAccess
   JdbcAccess
   JdbcException
   Persistable
   Persister

persistence.types:
   AbstractColumn
   Column
   IntegerColumn
   StringColumn

sql:
   AndCriteria
   BinaryOperator
   Criteria
   EqualsCriteria
   LikeCriteria
   LogicalCriteria
   OrCriteria
   SqlGenerator

Enjoy. I’m pretty happy with the way this is shaping up so far. I’m sure there are things you can find to shake a stick at, too. That’s inevitable, and that’s also why this might be a far more interesting exercise for a pair. I figure a couple of solid programmers should be able to get to the point that I’ve gotten to here within a day, at most two days.

For future challenges in the blog series, I intend on taking a look at prepared statements, and then maybe some real ugliness such as join statements and subselects.

 

Original Comments:

I would almost certainly make the classes in the sql package And, Binary, Criteria, Like, Logical, Or, and Sql.

factoring duplication, a technique Fred showed me leaves the class names long, and uses a static import on the SqlGenerator to import and, or, like, criteria, etc. methods.

===
I forgot to sign a post, so -probably- the previous message is –JeffBay, unless its been haxx0red.

===
I’m not following the second sentence. Is that a good thing or not, it seems to contradict what you suggest in the first sentence.

I’ve used static import in tests for the class under test, and I think it’s great. I’ll take a look at the SqlGenerator class and see what that ends up looking like.

Right now SqlGenerator is just that, it’s all functional methods. It has no state, that’d be the only reason I’d resist renaming it to Sql. But I still might.

Atom