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.
public void testPreparedInsert() {
String sql = new SqlGenerator().createPreparedInsert(TABLE, COLUMNS);
assertEquals("insert into t (a,b) values (?,?)", sql);
}
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:
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);
}
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.
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;
}
}
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:
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.
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.)
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.
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:
public List<T> findMatches(Criteria criteria) {
return new Persister<T>(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<Customer> 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:
package application;
import java.util.*;
import persistence.*;
import sql.*;
import domain.*;
class MockCustomerAccess extends CustomerAccess {
private Persistable customer;
public List<Customer> findMatches(Criteria criteria) {
List<Customer> results = new ArrayList<Customer>();
if (criteria.sqlString().indexOf("'n%'") >= 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:
public void testBrowseCustomers() {
final String id1 = "id1";
final String name1 = "name1";
final String matchingCustomerPattern = "'n%'";
application.setCustomerAccess(new CustomerAccess() {
private Persistable customer;
public List<Customer> findMatches(Criteria criteria) {
List<Customer> results = new ArrayList<Customer>();
if (criteria.sqlString().indexOf(matchingCustomerPattern) >= 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<Customer> 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 SqlGeneratorEnjoy. 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.
code (Note that I did an optimize imports against the entire source base, so there are a few more changes than you might expect.)
February 2004 March 2004 May 2004 September 2004 October 2004 January 2005 February 2005 September 2005 October 2005 November 2005 December 2005 January 2006 February 2006 March 2006 June 2006 August 2006 January 2007 February 2007 March 2007 April 2007 September 2007 October 2007 November 2007 December 2007 January 2008