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"
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
owner reference also in nullSafeSet()?
- 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
CallableStatement for 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
default methods, or provide an “empty” default implementation.
Did you enjoy this read? You might also enjoy:
Like this:
Like Loading...
Great post Lukas. I agree with most of this except for your use of the word “Always”. Using the word “Always” as a developer is “Always” dangerous :). Instead maybe consider (given the SPI topic) using RFC 2119 keywords https://www.ietf.org/rfc/rfc2119.txt .. I dare you to rewrite the most with SHALL , MUST NOT and other friends.
On a serious note though I have made this mistake myself mainly due to concern of object creation performance ( http://site.jatl.googlecode.com/hg/apidocs/com/googlecode/jatl/Indenter.html ).
I do think there are exceptions to returning values instead of void and I will try to explain.
My company has been doing more and more reactive programming and void is great in terms of Non Blocking API because returning a value obviously can cause blocking or at least often implies it.
However on the other hand if you do want to enforce single thread and/or blocking behavior or maybe some declarative transformation (ie monad ie convert one observable to another observable) it is actually a pain to enforce that with arguments being used for return values especially in a asynchronous platform (you see this often with the pre 3.0 servlet api.. people will try to use the blocking api in a non blocking way).
One way I mitigate the SPI changing is providing Abstract classes that are strongly encouraged to use over implementing the interface that come with the SPI (I guess in Java 8 you could go multiple interfaces and defender method but I feel like that is sort of an abuse). Many other libraries do this as well.
Your RECOMMENDED replacement of “Always” MAY be a good idea! ;)
I’m curious about how a “void-only”, “return-argument-value” SPI causes issues with your programming style, could you show an example? I mean, we’re talking about SPI after all. No one calls the SPI except the library’s deepest internals, which don’t have to be pretty.
(note the abstract class suggestion prior to Java 8 was also mentioned in the article)
There are 4 issues that come to bind with doing output arguments (aka HttpServlet interface style).
1. It requires more work for the SPI spec author as they have to be very very clear on the order of operations and whether or not the interface is being accessed in a synchronous/asynchronous manner. You can see this in the reactive-streams api (https://github.com/reactive-streams/reactive-streams-jvm)
2. Because your return objects are essentially mutable and can be accessed long after the SPI method has been called great care has to be taken by the calling library to ensure proper use. An example again is the Servlet API where getOutputStream() cannot be called more than once. These need to be documented and enforced. Also to do this correctly to handle async clients is actually rather hard (queues etc) as opposed to just handling a returned Future.
3. Because of #1 and #2 a burden is now put on the implementer of the extension as additional doc needs to be read along with much greater difficulty of unit testing (proper mocking of HttpServletRequest and HttpServletResponse are actually fairly complicated).
4. Ironically (given FRP.. reactive streams) it is not functional. Functional code ie methods/functions that take an input and produce an output are easier to understand and easier to test.
Now to give you can example since you asked… lets say I have an SPI where I really really do not want the implementer to spawn threads or async stuff. I want them to just transform some data. It is massive overkill to provide an argument object just to save me on not changing the interface later.
An example of this is transforming Observable chain which is very FP style and is actually an immutable object.. well actually its a monad which is the FP way to handle abstractions like this SPI problem we are talking about. Anway here is an example:
public Observable transform(Observable input);
That is I’m calling you just to transform the configuration (ie declarative programming). I don’t want you to actually execute it.
Another great example of this is the Service Loader which is the SPI of SPIs :)
http://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html
It has a blocking like API on purpose for simplicity.
But yeah the SPI approach your presenting buys the most flexibility and I generally prefer it but one has to argue… why not just add more interfaces and do some instanceof checking?
1. I do hope the SPI spec author spends a bit more time on the SPI spec than on some random internals. It’s time worth spending, so there’s no excuse here :)
2. Again, the calling library is the one that better be sound. There’s no excuse for writing a crap library :) Perhaps,
getOutputStream()was not well-designed, or there was an important optimisation in times when the JVM wasn’t as strong as today yet.3. You can cover this with end-to-end tests easily. The SPI changes the overall behaviour of the library. An SPI implementation has some testable effect on some other API.
4. I’m not sure if this is needed. Not everything needs functional / reactive / async / insert-your-modelling-hype-here. But I may just be misunderstanding your point.
I think you’re abusing on the term SPI, just because some implementation can be injected into some algorithm by implementing interfaces. What you’re talking about would be called the strategy pattern in OO terms, or just plain functional programming in FP terms. I don’t think an
Observablequalifies as an SPI, but perhaps, this isn’t such a black/white discussion as the article seems to have triggered anyway :)Why would you want to have more than a single interface / contract in the examples exposed in the article? Those are real SPIs (not functional interfaces), which have a contract that is hard to express in more simple terms. It wouldn’t make sense to split contract parts into single interfaces… I mean for what greater goal?
I think there is some sort of disconnect. I’m a terrible communicator so I’m sure its my fault.
As far as abusing the SPI term I’m not sure I can abuse an already highly nebulous term. When I use SPI I’m talking about an API that will do the calling (ie framework style.. ie will call you).
Oracles sort of definition is here: http://docs.oracle.com/javase/tutorial/sound/SPI-intro.html
What ever OO pattterns are picked (ie strategy) IMO are left to the library. I would like to point out that services should have a separation of behavior and state as that is general what service oriented is…. which is very functional. Passing in objects with behavior (ie mutable writers) is ironically not very service oriented.. but then again service is another completely fucking vacuous term just like SPI.
Because your changing the library. Lets say you provide a synchronous library or lets say its push.. then later on you want to provide a pull model or async model but have the same library. Its pretty much a bad idea NOT to make those separate interfaces.. some even make them separate libraries (ie STAX and SAX to be apropo to your examples).
My problem (and we are talking trivial) is that you said “Always” to something I don’t believe is always the case. There is nothing wrong with SPI’s that have methods that return values. There are in fact plenty of them that are successful. However I don’t think I’m going to convince you as I just don’t have the time to make a better case :)
(I’ve edited your comment to add
<blockquote>. Hope you don’t mind)I guess there are indeed only two real problems in software. Concurrency, naming things, and off-by-one errors. In any case, I guess we can both agree that my article will certainly not apply to the contract imposed by a type like
Observable, whatever we will classifyObservableas (SPI, function, strategy, etc…). I hope we don’t need to discuss the definition of contract, though ;)But you expose good points here. There is a category of types that encapsulate behaviour across a non-trivial contract (which I called SPI). In that hard-to-name category, API evolution is extremely difficult simply because of the fact that you will break implementations (whereas in “classic” APIs, you mostly break call-sites, which is easier to avoid).
We could, perhaps, agree on
Observablebeing a type in said category that is sufficiently simple for it not to need to adhere to the rules suggested in this article. Perhaps there’s an category (SPI) threshold for the category and the rules to become applicable.Whew :)
Fair enough :) I always say always, which is never an accurate quantifier for what I really want to say.
Hi
I am having a little problem to undestand (and visualize myself) the code you suggested for: “implementor provides values back to the framework” case based on Hibernate.
Is it like: a framework will at one point call
configuremethod on UserType implementation passing framework-implemented ConfigureArgs instance , and then expecting an implementor to call specific methods(likesqlTypes) on passed in ConfigureArgs, so that way an implementor can somehow provide back information (like handled sql types ) back to the framework ?Am i right ?
Yes, your understanding of the example is correct.
How is the framework or implementer supposed to implement the *Arguments object when they are declared final within the interface enclosing it? Perhaps an example to how this strategy was employed within jOOQ?
The fact that those argument classes are nested is just a stylistic detail for this blog. They could be top level classes. But even if they weren’t, there could be package-private constructors and the framework implementor could then instanciate the arguments classes.
Only the framework would implement these. The user would never do it.
In jOOQ, the argument types are actually interfaces and the implementation is hidden – but that too would have been too much code for the post. Then again, compared to this explanation, it might have been a better option ;-)