AfflictionNetworks/depot
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixed a problem with binding transformed values in updatePartial.

Browse Source 
When we go to bind values in an update statement, we either have:

- field names and a pojo from which to extract them, in which case
  transformation happens naturally when we extract the current value from the
  pojo

- value expressions (like 1, or "bob") which we were previously just binding
  directly to the statement, which was wrong because they need to first be
  transformed in the case where the field in question has an @Transform
  annotation; now we get the field marshaller for the field in question and
  pass the raw value to it, so that it can do the necessary transformations in
  writeToStatement()

- other expressions (like LiteralExp("true") or or IntervalExp or anything else
  where the database is involved in computing the final value to be assigned to
  the column).

This last case still holds the potential for badness with regard to transformed
fields. We can't magically transform the value to be stored into a field if the
database is computing the ultimate value. Say, for a contrived example, that
you had a record where you stored an int column as a String for shits and
giggles:

public class MyRecord {
    @Transform(IntToStringTransformer.class)
    public int someValue;
}

someValue will be a string column in the database, owing to the
IntToStringTransformer converting the int to a String.

Now you come along and think to yourself, "Hey, I want to store the current
minute in my  'int' field." and you write something like:

updatePartial(MyRecord.class, MyRecord.SOME_VALUE,
    DateFuncs.minute(Exps.literal(new Timestamp(System.currentTimeMillis()))));

Well, that's going to end up trying to stuff an int-valued expression (computed
by the database) into a String field, and the shit will hit the fan.

I can't really think of a non-contrived situation where this is likely to bite
us in the ass, but I don't especially like grass covered pits like this lying
around, waiting for someone to unsuspectingly step into them.

I could fail if you try to update a transformed field with anything other than
a ValueExp (or the current value from a pojo), but that would prevent you from
doing something potentially useful and safe like copying one field to another,
which both use the same transformation.
This commit is contained in:
Michael Bayne
2011-02-05 00:22:53 +00:00
parent 9aba944105
commit b8fec6cab5
2 changed files with 63 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/com/samskivert/depot/impl/BuildVisitor.java
+50 -42
View File
@@ -467,7 +467,8 @@ public abstract class BuildVisitor implements FragmentVisitor<Void>
_builder.append(" = ");
if (pojo != null) {
bindField(pClass, fields[ii], pojo);
} else if (values[ii] instanceof ValueExp<?>) {
bindFieldValue(pClass, fields[ii], (ValueExp<?>)values[ii]);
} else {
values[ii].accept(this);
}
@@ -747,18 +748,61 @@ public abstract class BuildVisitor implements FragmentVisitor<Void>
return null;
}
protected Void bindValue (Object object)
protected Void bindValue (final Object value)
{
_bindables.add(newBindable(object));
_bindables.add(new Bindable() {
public void doBind (Connection conn, PreparedStatement stmt, int argIx)
throws Exception {
// TODO: how can we abstract this fieldless marshalling
if (value instanceof ByteEnum) {
// byte enums require special conversion
stmt.setByte(argIx, ((ByteEnum)value).toByte());
} else if (value instanceof Enum<?>) {
// enums are converted to strings
stmt.setString(argIx, ((Enum<?>)value).name());
} else if (value instanceof int[]) {
// int arrays require conversion to byte arrays
int[] data = (int[])value;
ByteBuffer bbuf = ByteBuffer.allocate(data.length * 4);
bbuf.asIntBuffer().put(data);
stmt.setObject(argIx, bbuf.array());
} else {
stmt.setObject(argIx, value);
}
}
});
_builder.append("?");
return null;
}
protected Void bindField (
Class<? extends PersistentRecord> pClass, ColumnExp<?> field, Object pojo)
Class<? extends PersistentRecord> pClass, ColumnExp<?> field, final Object pojo)
{
final DepotMarshaller<?> marshaller = _types.getMarshaller(pClass);
_bindables.add(newBindable(marshaller, field, pojo));
final FieldMarshaller<?> fmarsh =
_types.getMarshaller(pClass).getFieldMarshaller(field.name);
_bindables.add(new Bindable() {
public void doBind (Connection conn, PreparedStatement stmt, int argIx)
throws Exception {
fmarsh.getAndWriteToStatement(stmt, argIx, pojo);
}
});
_builder.append("?");
return null;
}
protected <T> Void bindFieldValue (
Class<? extends PersistentRecord> pClass, ColumnExp<?> field, final ValueExp<T> value)
{
// we know that the Ts match in FieldMarshaller<T> and ValueExp<T>, but it's hard
// to convince the type system of that
final @SuppressWarnings("unchecked") FieldMarshaller<T> fmarsh =
(FieldMarshaller<T>)_types.getMarshaller(pClass).getFieldMarshaller(field.name);
_bindables.add(new Bindable() {
public void doBind (Connection conn, PreparedStatement stmt, int argIx)
throws Exception {
fmarsh.writeToStatement(stmt, argIx, value.getValue());
}
});
_builder.append("?");
return null;
}
@@ -936,42 +980,6 @@ public abstract class BuildVisitor implements FragmentVisitor<Void>
void doBind (Connection conn, PreparedStatement stmt, int argIx) throws Exception;
}
protected static Bindable newBindable (
final DepotMarshaller<?> marshaller, final ColumnExp<?> field, final Object pojo)
{
return new Bindable() {
public void doBind (Connection conn, PreparedStatement stmt, int argIx)
throws Exception {
marshaller.getFieldMarshaller(field.name).getAndWriteToStatement(stmt, argIx, pojo);
}
};
}
protected static Bindable newBindable (final Object value)
{
return new Bindable() {
public void doBind (Connection conn, PreparedStatement stmt, int argIx)
throws Exception {
// TODO: how can we abstract this fieldless marshalling
if (value instanceof ByteEnum) {
// byte enums require special conversion
stmt.setByte(argIx, ((ByteEnum)value).toByte());
} else if (value instanceof Enum<?>) {
// enums are converted to strings
stmt.setString(argIx, ((Enum<?>)value).name());
} else if (value instanceof int[]) {
// int arrays require conversion to byte arrays
int[] data = (int[])value;
ByteBuffer bbuf = ByteBuffer.allocate(data.length * 4);
bbuf.asIntBuffer().put(data);
stmt.setObject(argIx, bbuf.array());
} else {
stmt.setObject(argIx, value);
}
}
};
}
protected DepotTypes _types;
/** For each SQL parameter ? we add an {@link Comparable} to bind to this list. */
src/test/java/com/samskivert/depot/TransformTest.java
+13
View File
@@ -240,6 +240,19 @@ public class TransformTest extends TestBase
delete(in);
}
@Test public void testUpdatePartial ()
{
TransformRecord in = createAndInsert(new String[] { "one", "two", "three" });
EnumSet<ExtraOrdinal> nbobs = EnumSet.of(ExtraOrdinal.ONE, ExtraOrdinal.THREE);
_repo.updatePartial(TransformRecord.getKey(in.recordId), TransformRecord.BOBS, nbobs);
TransformRecord in2 = _repo.load(TransformRecord.getKey(in.recordId));
assertEquals(nbobs, in2.bobs);
delete(in);
}
protected TransformRecord createAndInsert (String[] strings)
{
TransformRecord in = new TransformRecord();