Thursday, March 15, 2012

Restlet Guice extension considered ... unnecessary

I wrote some code back in 2008 to help inject my Restlet resources using Guice and then blogged about it. In 2009, the Restlet team added it to their "incubator" as a potential Restlet extension. (Calling it a Guice extension is a bit of a misnomer, since part of it is independent of Guice and applies to any JSR-330-compliant DI framework.) They've talked about promoting it out of the incubator as early as release 2.2. Update: It's now part of Restlet 2.2 and I've updated code references to point to the 2.2 branch.

More recently, I argued (and Restlet's Jérôme Louvel seemed to accept the argument) that Restlet needs a uniform way to create a non-standard Finder for all the Restlet classes that have methods accepting a Class<? extends ServerResource> in place of a Restlet. This would make the use of the Guice extension invisible when wiring up the routing structure: Currently you have to convert the resource type to a Restlet explicity,
    router.attach("/path/to/resource", finderFactory.finder(MyResource.class));

and the extra support would let you write:
    router.attach("/path/to/resource", MyResource.class);

It's all very gratifying, except I realized suddenly this week that for
most people who just want to inject their 
ServerResource 
subclasses it's completely unnecessary. 
It's much simpler to define 
doInit()
to inject the resource's members, and then you can just use the second, simpler form above.
I wrote an abstract base class extending ServerResource that does just that:


SelfInjectingServerResource.java

Notice that SelfInjectingServerResource uses only the standard @Inject annotation and nothing specific to Guice. In order to make it actually work with Guice, you need to tell Guice that you want self-injecting server resources. The following class is a Guice Module that accomplishes this by requesting static injection of the SelfInjectingServerResource class and providing an implementation of its nested MembersInjector interface to perform the injection.

SelfInjectingServerResourceModule.java

Install this module when creating the top-level injector, make sure that the server resources you want injected inherit from SelfInjectingServerResource, and it just works. Here's a JUnit test class that shows a trivial example of the idea in practice:

SelfInjectingServerResourceModuleTest.java

We'd like this technique to co-exist with other techniques that manage resource injection differently (e.g., the current Guice extension), without worrying about whether it's OK to extend SelfInjectingServerResource, so this code uses an atomic boolean to prevent multiple injections. (AtomicBoolean is probably overkill, but it can't hurt to be safe.)

[Deleted obsolete discussion of how to use prior to adoption as part of Restlet.]

I said that the old Guice extension was unnecessary for most users, but there are a few things the old code does that the new code doesn't do:
  1. It does constructor injection, letting you have final fields with injected values.
  2. It lets you create a Finder for a resource interface, decoupling the target of the routing from the resource implementation.
  3. It lets you use qualifiers when looking up resource types, further decoupling the target of an attachment from the implementation.
  4. It doesn't require inheritance from a common abstract base class.
  5. You don't have to remember to call super.doInit() when you override doInit
As it happens, I'm taking advantage of #2 in my current work. (Note that there's no way to avoid the explicit finder call in this case, even with added Restlet support, because the Restlet signatures expect a ServerResource subclass.) So I'm stuck with my mistake for the foreseeable future. But no one else need be, now.

Update (2013 July 9)

I got rid of my dependency on #2 above, and I am now using the new approach exclusively, and I strongly recommend others do the same.

Further update (2014 Sep 1)

The Restlet Guice extension package docs discuss how to use each of three approaches, so you aren't forced to use the approach that I prefer.

19 comments:

Dave said...

The Guice team themselves state that requestStaticInjection() is a crutch rather than desirable: http://code.google.com/p/google-guice/wiki/AvoidStaticState

I've only made limited use of it so far and I'm no expert on the subject, but the incubator project feels much cleaner to me.

Tembrel said...

The use of requestStaticInjection is tucked away in the abstract base class SelfInjectingServerResource and the related Guice module. Users of these classes don't have to deal with static injection, they just extend the base class, add @Inject to fields and methods (but not constructors!), install the module, and they're done.

The incubator project, in contrast, asks the user to deal with a new type, FinderFactory, and muddies the wiring code:

router.attach("/path",
finderFactory.finder(MyServerResource.class));

instead of just

router.attach("/path", MyServerResource.class);

You're free to use the code in the incubator project if it works for you. I just don't want to have to support it any more, now that I've found what I think is a better alternative.

neo said...

I'm a bit confused by the static nature the new code. Using your test class, could I create a second module like

static class TestModule2 extends AbstractModule {
@Provides @Named(HELLO_KEY) String helloMessage() {
return "another message";
}
protected void configure() {}
}

and then use this module for an "Hello2Resource.class"? In the incubator project, I would do it like:

first = new RestletGuice.Module(new TestModule());
second = new RestletGuice.Module(new TestModule2());
...
router.attach("/hello", first.finder(HelloResource.class));
router.attach("/hello2", first.finder(Hello2Resource.class));

But I can't see any way to do it with the new code.

Tembrel said...

The RestletGuice.Module stuff was intended for use as a singleton in quick-and-dirty applications. The preferred usage is to inject FinderFactory where needed and only mention RestletGuice.Module (or RestletGuice.createInjector) in the injector setup code at the top level.

The SelfInjectingServerResource stuff only works in settings where a single injector will be used to inject all resource instances. I have yet to come across the need to use multiple injectors to inject the same class differently in different applications, but if I did I'd factor out the differences and pass the variant initialization information via the Context.

neo said...

I am using Restlet in a servlet container, so I have a single org.restlet.Application class like the following:

public class MyService extends Application {
..private final FinderFactory ff;

..public MyService() {
....ff = new RestletGuice.Module(new MyModule());
..}

..@Override
..public Restlet createInboundRoot() {
....Router router = new Router(getContext());
....router.attach("/myresource", ff.finder(MyResource.class));
....return router;
..}
}
(used .. for indentation...)

Is that wrong? In my opinion, this class is my top level. I don't understand why you say "inject FinderFactory where needed".

Tembrel said...

That's fine, but in a larger setting you'd have lots of Guice modules and you'd want to share a single FinderFactory instance. In that case you'd create your Injector at the Restlet Component level, e.g.,

Injector inj = RestletGuice.createInjector(...your modules...);
FinderFactory ff = inj.getInstance(FinderFactory.class);
host.attach("/app1", new Application1(..., ff));
host.attach("/app2", new Application2(..., ff));

Better, use Guice to create your Component and its Applications:

https://pastebin.com/RrncW8Rf

But SelfInjectingServerResource is even simpler:

Module selfInjection = new SelfInjectingServerResourceModule();
Injector inj = Guice.createInjector(selfInjection, ... others ...);
inj.getInstance(MyComponent.class).start();

In which case there is no need for FinderFactory. Just add @Inject to your resource fields and methods and extend SelfInjectingServerResource rather than ServerResource in those classes.

neo said...

Thanks for your code snippets, now I understand it :)

Although it might be a little simpler and doesn't pollute the Application classes with FinderFactory stuff, I still like the incubator one more, particularly because of having constructor injection and not inheriting from the self injection base class. It also feels less like a hack to me (compared to creating an injector and not using it directly but later on indirectly through the static field).

Tembrel said...

Leaving aside SelfInjectingServerResource, I'd distinguish between two kinds of uses of RestletGuice: (1) Using new RestletGuice.Module(...) to avoid creating an explicit Injector; and (2) Using RestletGuice.createInjector() or the equivalent RestletGuice.Module rgm = new RestletGuice.Module(); Guice.createInjector(rgm, ...) and either passing rgm (as a FinderFactory) somehow down to wherever it's needed or just using @Inject FinderFactory where needed.

I think (1) is only appropriate for quick-and-dirty examples, where you aren't really using Guice seriously. (2) is the way to go if you want construction injection or binding to resource-type-plus-qualifier. And once you're using (2), it's just easier to @Inject FinderFactory than it is to pass it down a chain of constructors.

Regarding constructor injection: It's great to able to mark things final, but remember that there's a new resource instance for each request, and unless you're using the NIO connector, access to that resource is confined to a single thread. So final doesn't buy you much in resources.

Another thing that makes me less interested in constructor injection: There are some Restlet calls that must be made later than the constructor to work properly, like configuring services for Applications. I'm not sure if that's true of ServerResources, but it's still fair to say that efforts to move *all* initialization to constructors in Restlet are doomed to fail.

infiniteset said...

I'm using both of your approaches. One downside of this version is that without constructor injection it's harder for others to re-use my classes unless they're using DI as well.

I'm grateful you've put in the work, it's very useful.

Tembrel said...

There's nothing stopping you from using method and/or field injection (via SelfInjectingServerResource) *and* providing an alternative constructor (not injected!) for those who are willing to write custom Finders, say.

https://gist.github.com/Tembrel/8540328

I adjusted SelfInjectingServerResource so that it will function in non-DI settings:

https://github.com/restlet/restlet-framework-java/blob/master/incubator/org.restlet.ext.guice/src/org/restlet/ext/guice/SelfInjectingServerResource.java

Mike Mitterer said...

Isn't this https://github.com/robharper/restlet-guice a much cleaner approach or what do you think is the disadvantage?

Tembrel said...

Different goals: Rob Harper wants to mimic guice-servlet, but for Restlet, and that's fine. I, on the other hand, have been seeking to minimize the Guice footprint in my code ever since the days when I went too far in the other direction, using Guice for everything. (Maybe I've overcompensated.)

I want to inject my ServerResource instances but I also want to keep using the "native" Restlet routing machinery, not hand it over to Guice and force users to learn a new (albeit tiny) DSL. My preferred solution does this in a minimalist way that might not satisfy power Guice users who see Guice as the "engine" behind everything.

Mike Mitterer said...

OK, thx for your answer!

Tony Piazza said...

I'm attempting to use this extension for a relatively simple application and am having some difficulty with the initial setup. I have multiple ServerResource subclasses that extend SelfInjectingServerResource and have a single field marked with @Inject. I modified the application class to extend ResourceInjectingApplication, modified the createInboundRoot method to use the newRouter method instead of using the Router class constructor and marked one application method with @Provides to return an instance of the implementation class needed for injection. Lastly, in the main method of the app class I call Guice.createInjector passing in a new instance of SelfInjectingServerResourceModule. When I launch the app and attempt to execute a route, a ConfigurationException is thrown (at com.google.inject.internal.InjectorImpl.getMembersInjector(InjectorImpl.java:952) with the following message:

Could not find a suitable constructor in com.myco.Foo. Classes must have either one (and only one) constructor annotated with @Inject or a zero-argument constructor that is not private.

I'm obviously missing something here. Please let me know if you can provide any insight.

Thanks in advance for your help!

-Tony

Tembrel said...

The @Provides annotation can only go on methods of a Guice Module -- if you put it on a method of your Restlet Application, it will be ignored, and there might not be a binding for the type it was supposed to provide.

That message ("Could not find suitable constructor...") is Guice trying to construct com.myco.Foo and not finding a constructor to do so. Is com.myco.Foo the type of the @Inject-ed field on one or more of your resources? Is it the return value of the @Provides? That would explain the problem.

How comfortable are you with Guice? If not very, you'll want to read up on it more before trying to use it with Restlet. The SelfInjectingServerResource stuff is a way to get injection into your ServerResources, but it assumes you already have some sort of existing Guice usage that you want to fit it into.

Tony Piazza said...

Answering my own question here.... What I failed to do was create a subclass of SelfInjectingServerResourceModule with appropriate methods marked with @Provides. I had mistakenly put those methods in the Application class and expected that Guice would find them there. Anyways, thanks for your work on this extension!

-Tony

Tembrel said...

Great! There's no need to subclass SelfInjectingServerResourceModule, though. You can (and probably should) call Guice.createInjector(...) with a bunch of separate Modules, including SelfInjectingServerResourceModule, of course, but also your own AbstractModule extensions. That way the provision of your Foo instances is kept separate from the resource-injection concerns -- they only appear together in the createInjector call.

Unknown said...

Hi there, I'm currently using different approaches with different apps but noticed 1 little problem when using SelfInjectingServerResourceModule and it's SelfInjectingServerResource. Everything works great except for method interception for the resources ( i use that together with Shiro). Anyone else also having problems getting methods intercepted in SelfInjectingServerResources ?

Tembrel said...

SelfInjectingServerResource doesn't use constructor injection, so Guice can't do interception.

Use one of the other techniques in the "Guice" extension if you need constructor injection.