diff --git a/src/java/com/samskivert/jdbc/depot/Key.java b/src/java/com/samskivert/jdbc/depot/Key.java index 66d07b8..a5ad529 100644 --- a/src/java/com/samskivert/jdbc/depot/Key.java +++ b/src/java/com/samskivert/jdbc/depot/Key.java @@ -21,6 +21,7 @@ package com.samskivert.jdbc.depot; import java.io.Serializable; +import java.lang.reflect.Field; import java.sql.PreparedStatement; import java.sql.SQLException; import java.util.ArrayList; @@ -28,7 +29,9 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import com.samskivert.jdbc.depot.annotation.Id; import com.samskivert.jdbc.depot.clause.Where; +import com.samskivert.util.StringUtil; /** * A special form of {@link Where} clause that uniquely specifices a single database row and @@ -68,31 +71,46 @@ public class Key extends Where /** * Constructs a new multi-column {@code Key} with the given values. - * - * TODO: There is no reason to store both fields and values here (and doing so probably more - * than doubles the space the key consumes in the cache). The primary key fields are known - * by the DepotMarshaller; we should simply check tha the given indices match those, and - * store the values in the order defined by the DepotMarshaller. The sanity check would be - * welcome in any case. The only problem with this is that we don't currently have access to - * a {@link PersistenceContext}. It may be that we should make this class internal to - * {@link DepotRepository} and only create it ourselves. This would require *lots* more - * convenience methods in {@link DepotRepository} though. */ public Key (Class pClass, String[] fields, Comparable[] values) { // TODO: make Where an interface so we don't have to do this ugly super call super(null); - + _pClass = pClass; + if (fields.length != values.length) { throw new IllegalArgumentException("Field and Value arrays must be of equal length."); } - _pClass = pClass; - _map = new HashMap(); + + // build a local map of field name -> field value + Map map = new HashMap(); for (int i = 0; i < fields.length; i ++) { - _map.put(fields[i], values[i]); + map.put(fields[i], values[i]); + } + + // then introspect on the persistent record and iterate over its actual fields + _fields = new ArrayList(fields.length); + _values = new ArrayList(fields.length); + for (Field field : _pClass.getFields()) { + // look for @Id fields + if (field.getAnnotation(Id.class) != null) { + String fName = field.getName(); + // make sure we were provided with a value for this primary key field + if (map.containsKey(fName) == false) { + throw new IllegalArgumentException("Missing value for key field: " + fName); + } + _fields.add(fName); + _values.add(map.get(fName)); + map.remove(fName); + } + } + // finally make sure we were not given any fields that are not in fact primary key fields + if (map.size() > 0) { + throw new IllegalArgumentException( + "Non-key columns given: " + StringUtil.join(map.keySet().toArray(), ", ")); } } - + // from QueryClause public Collection> getClassSet () { @@ -106,15 +124,11 @@ public class Key extends Where public void appendClause (ConstructedQuery query, StringBuilder builder) { builder.append(" where "); - boolean first = true; - for (Map.Entry entry : _map.entrySet()) { - if (first) { - first = false; - } else { + for (int ii = 0; ii < _fields.size(); ii ++) { + if (ii > 0) { builder.append(" and "); } - builder.append(entry.getKey()); - builder.append(entry.getValue() == null ? " is null " : " = ? "); + builder.append(_fields.get(ii)).append(_values.get(ii) == null ? " is null " : " = ? "); } } @@ -122,9 +136,9 @@ public class Key extends Where public int bindArguments (PreparedStatement pstmt, int argIdx) throws SQLException { - for (Map.Entry entry : _map.entrySet()) { - if (entry.getValue() != null) { - pstmt.setObject(argIdx ++, entry.getValue()); + for (int ii = 0; ii < _fields.size(); ii ++) { + if (_values.get(ii) != null) { + pstmt.setObject(argIdx ++, _values.get(ii)); } } return argIdx; @@ -139,7 +153,7 @@ public class Key extends Where // from CacheKey public Serializable getCacheKey () { - return _map; + return _values; } // from CacheInvalidator @@ -151,7 +165,7 @@ public class Key extends Where @Override public int hashCode () { - return _pClass.hashCode() + 31 * _map.hashCode(); + return _pClass.hashCode() + 31 * _values.hashCode(); } @Override @@ -164,20 +178,22 @@ public class Key extends Where return false; } Key other = (Key) obj; - return (_pClass == other._pClass && _map.equals(other._map)); + return _pClass == other._pClass && _values.equals(other._values); } @Override public String toString () { StringBuilder builder = new StringBuilder("[Key pClass=" + _pClass.getName()); - for (Map.Entry entry : _map.entrySet()) { - builder.append(", ").append(entry.getKey()).append("=").append(entry.getValue()); + for (int ii = 0; ii < _fields.size(); ii ++) { + builder.append(", ").append(_fields.get(ii)).append("=").append(_values.get(ii)); } builder.append("]"); return builder.toString(); } - + + protected Class _pClass; - protected HashMap _map; + protected ArrayList _fields; + protected ArrayList _values; }