Most of your code is private, internal, proprietary, and will never be exposed to public. If that’s the case, you can relax – you can refactor all of your mistakes, including those that incur breaking API changes.
If you’re maintining public API, however, that’s not the case. If you’re maintaining public SPI (Service Provider Interfaces), then things get even worse.
The H2 Trigger SPI
In a recent Stack Overflow question about how to implement an H2 database trigger with jOOQ, I have encountered the org.h2.api.Trigger SPI again – a simple and easy-to-implement SPI that implements trigger semantics. Here’s how triggers work in the H2 database:
Use the trigger
CREATE TRIGGER my_trigger BEFORE UPDATE ON my_table FOR EACH ROW CALL "com.example.MyTrigger"
Implement the trigger
public class MyTrigger implements Trigger {
@Override
public void init(
Connection conn,
String schemaName,
String triggerName,
String tableName,
boolean before,
int type
)
throws SQLException {}
@Override
public void fire(
Connection conn,
Object[] oldRow,
Object[] newRow
)
throws SQLException {
// Using jOOQ inside of the trigger, of course
DSL.using(conn)
.insertInto(LOG, LOG.FIELD1, LOG.FIELD2, ..)
.values(newRow[0], newRow[1], ..)
.execute();
}
@Override
public void close() throws SQLException {}
@Override
public void remove() throws SQLException {}
}The whole H2 Trigger SPI is actually rather elegant, and usually you only need to implement the fire() method.
So, how is this SPI wrong?
It is wrong very subtly. Consider the init() method. It has a boolean flag to indicate whether the trigger should fire before or after the triggering event, i.e. the UPDATE. What if suddenly, H2 were to also support INSTEAD OF triggers? Ideally, this flag would then be replaced by an enum:
public enum TriggerTiming {
BEFORE,
AFTER,
INSTEAD_OF
}But we can’t simply introduce this new enum type because the init() method shouldn’t be changed incompatibly, breaking all implementing code! With Java 8, we could at least declare an overload like this:
default void init(
Connection conn,
String schemaName,
String triggerName,
String tableName,
TriggerTiming timing,
int type
)
throws SQLException {
// New feature isn't supported by default
if (timing == INSTEAD_OF)
throw new SQLFeatureNotSupportedException();
// Call through to old feature by default
init(conn, schemaName, triggerName,
tableName, timing == BEFORE, type);
}This would allow new implementations to handle INSTEAD_OF triggers while old implementations would still work. But it feels hairy, doesn’t it?
Now, imagine, we’d also support ENABLE / DISABLE clauses and we want to pass those values to the init() method. Or maybe, we want to handle FOR EACH ROW. There’s currently no way to do that with this SPI. So we’re going to get more and more of these overloads, which are very hard to implement. And effectively, this has happened already, as there is also org.h2.tools.TriggerAdapter, which is redundant with (but subtly different from) Trigger.
What would be a better approach, then?
The ideal approach for an SPI provider is to provide “argument objects”, like these:
public interface Trigger {
default void init(InitArguments args)
throws SQLException {}
default void fire(FireArguments args)
throws SQLException {}
default void close(CloseArguments args)
throws SQLException {}
default void remove(RemoveArguments args)
throws SQLException {}
final class InitArguments {
public Connection connection() { ... }
public String schemaName() { ... }
public String triggerName() { ... }
public String tableName() { ... }
/** use #timing() instead */
@Deprecated
public boolean before() { ... }
public TriggerTiming timing() { ... }
public int type() { ... }
}
final class FireArguments {
public Connection connection() { ... }
public Object[] oldRow() { ... }
public Object[] newRow() { ... }
}
// These currently don't have any properties
final class CloseArguments {}
final class RemoveArguments {}
}As you can see in the above example, Trigger.InitArguments has been successfully evolved with appropriate deprecation warnings. No client code was broken, and the new functionality is ready to be used, if needed. Also, close() and remove() are ready for future evolutions, even if we don’t need any arguments yet.
The overhead of this solution is at most one object allocation per method call, which shouldn’t hurt too much.
Another example: Hibernate’s UserType
Unfortunately, this mistake happens way too often. Another prominent example is Hibernate’s hard-to-implement org.hibernate.usertype.UserType SPI:
public interface UserType {
int[] sqlTypes();
Class returnedClass();
boolean equals(Object x, Object y);
int hashCode(Object x);
Object nullSafeGet(
ResultSet rs,
String[] names,
SessionImplementor session,
Object owner
) throws SQLException;
void nullSafeSet(
PreparedStatement st,
Object value,
int index,
SessionImplementor session
) throws SQLException;
Object deepCopy(Object value);
boolean isMutable();
Serializable disassemble(Object value);
Object assemble(
Serializable cached,
Object owner
);
Object replace(
Object original,
Object target,
Object owner
);
}The SPI looks rather difficult to implement. Probably, you can get something working rather quickly, but will you feel at ease? Will you think that you got it right? Some examples:
- Is there never a case where you need the
ownerreference also innullSafeSet()? - What if your JDBC driver doesn’t support fetching values by name from
ResultSet? - What if you need to use your user type in a
CallableStatementfor a stored procedure?
Another important aspect of such SPIs is the way implementors can provide values back to the framework. It is generally a bad idea to have non-void methods in SPIs as you will never be able to change the return type of a method again. Ideally, you should have argument types that accept “outcomes”. A lot of the above methods could be replaced by a single configuration() method like this:
public interface UserType {
default void configure(ConfigureArgs args) {}
final class ConfigureArgs {
public void sqlTypes(int[] types) { ... }
public void returnedClass(Class<?> clazz) { ... }
public void mutable(boolean mutable) { ... }
}
// ...
}Another example, a SAX ContentHandler
Have a look at this example here:
public interface ContentHandler {
void setDocumentLocator (Locator locator);
void startDocument ();
void endDocument();
void startPrefixMapping (String prefix, String uri);
void endPrefixMapping (String prefix);
void startElement (String uri, String localName,
String qName, Attributes atts);
void endElement (String uri, String localName,
String qName);
void characters (char ch[], int start, int length);
void ignorableWhitespace (char ch[], int start, int length);
void processingInstruction (String target, String data);
void skippedEntity (String name);
}Some examples for drawbacks of this SPI:
- What if you need the attributes of an element at the
endElement()event? You’ll have to remember them yourself. - What if you’d like to know the prefix mapping uri at the
endPrefixMapping()event? Or at any other event?
Clearly, SAX was optimised for speed, and it was optimised for speed at a time when the JIT and the GC were still weak. Nonetheless, implementing a SAX handler is not trivial. Parts of this is due to the SPI being hard to implement.
We don’t know the future
As API or SPI providers, we simply do not know the future. Right now, we may think that a given SPI is sufficient, but we’ll break it already in the next minor release. Or we don’t break it and tell our users that we cannot implement these new features.
With the above tricks, we can continue evolving our SPI without incurring any breaking changes:
- Always pass exactly one argument object to the methods.
- Always return
void. Let implementors interact with SPI state via the argument object. - Use Java 8’s
defaultmethods, or provide an “empty” default implementation.
| Reference: | Do Not Make This Mistake When Developing an SPI from our JCG partner Lukas Eder at the JAVA, SQL, AND JOOQ blog. |
