Mutation.WriteBuilder.build() does not check state #2257
Description
It is possible to "build" a Cloud Spanner mutation with invalid internal state, specifically with multiple values bound for the same column name. Using the mutation later on (e.g. in DatabaseClient#write) will result in a SpannerException. Duplicate bindings should be detected during mutation creation, rather than letting the invalid state stay hidden. Effective Java items 2 and 34 give detailed discussion of why invariants should be checked in build rather than while the object is used.
Minimal Example
// This succeeds
Mutation m = Mutation.newInsertBuilder().set("A").to("val1").set("A").to("val2").build();
// This fails with "INVALID_ARGUMENT: Multiple values for column..."
spannerClient.write(singletonList(m));
Practical Example
The issue of re-binding came up when testing a thin wrapper over the mutation builder. The goal was to abstract away the binding of some complicated key columns separately from the rest of the query. I was surprised to find that I could bind multiple keys to the same mutation. It looked something like this:
WriteBuilder bindKeys(WriteBuilder wb, MyObject obj) {
return wb.set(Key1).to(obj.getA()).set(Key2).to(obj.getB());
}
WriteBuilder wb = Mutation.newInsertBuilder();
wb = bindKeys(wb, obj1);
wb = bindKeys(wb, obj2);
// I expected an error, but no exception was thrown:
Mutation m = wb.build();
// Later on, here is where the exception is thrown:
spannerClient.write(< list containing mutation m >);