Open GoogleCodeExporter opened 9 years ago
Since we don't have conditional bindings in Guice this is quite hard to
accomplish without a major feature
addition to Guice core.
I think annotating with @Singleton or a @provides method is an acceptable
compromise.
The javadoc is out of date, I will fix it. Thanks for pointing it out.
Original comment by dha...@gmail.com
on 14 Feb 2009 at 12:28
Dhanji, keep in mind that a lot of the time we cannot touch the servlet class
to add
@Singleton. There absolutely must be a way to get this working outside of the
servlet/filter class and ideally it should require the minimum amount of work
possible.
If you insist on going down this road then I need to ask, why can't we define
the
servlet scope directly in ServletModule? Why do we need to bind the servlet in
ServletModule and its scope in a separate Module?
Thirdly, what is the @Provides annotation you referred to? I don't see it in
the Javadoc.
Original comment by gili.tza...@gmail.com
on 14 Feb 2009 at 12:43
Yea, it's for that case that I'm suggesting using a @Provides method. This is a
very low overhead way of creating
and binding sealed classes:
http://code.google.com/docreader/#p=google-guice&s=google-guice&t=Changes20
You certainly can define the scope in the ServletModule, if you like--you just
need an additional bind() rule near
the filter() rule, that's all. A separate module is not necessary.
Original comment by dha...@gmail.com
on 14 Feb 2009 at 1:16
I think both of you have a point. On one hand, the "common case" should be
straightforward, on the other
hand it would be very tricky to implement (conflicts with multiple
ServletModules).
I wonder if this can't be solved using the SPI. Find all the web artifacts and
generate a module that adds the
missing bindings. Ideally this would ship with Guice Servlet because it would
depend on its internals (the
bindings it generates). You'd still have to install an extra module, but maybe
it eases the pain?
Another option would be to ship different ServletModules; one for single module
usage, and one for multi-
module usage. The single module one would bind automagically, the multimodule
one would need the SPI
utility. But, all in all this also complicates the API and could be confusing.
The real answer is that we have yet to discover a good pattern that keeps the
"multiple modules" flexibility,
but also retains central control over the bindings that get generated (avoiding
duplicates). I also noticed this
while working on Warp Persist 2.0. The trunk currently uses the SPI idea for
this utility module (potentially
needed for UnitOfWork.TRANSACTION in a non-web environment):
http://code.google.com/p/warp-persist/source/browse/trunk/warp-
persist/src/com/wideplay/warp/persist/PersistenceServiceExtrasModule.java
Original comment by robbie.v...@gmail.com
on 14 Feb 2009 at 1:18
I don't see any problem with explicitly binding the scope (or annotating the
class directly) and with multiple
ServletModules.
We are talking about 1 extra line of code for sealed classes and or an
annotation for the common case. Do we
really need to be doing all this to avoid that? And to lose the explicit
documentation that these are singletons..
Why do you need an extra module? ServletModule is like any other Guice module
and supports anything they do.
Original comment by dha...@gmail.com
on 14 Feb 2009 at 1:34
Dhanji,
I think the API would be clearer if you could bind the scope directly off the
servlet() or filter() method call. For example:
filter("/*").through(PersistenceFilter.class).in(Scopes.SINGLETON)
I only mention this because the rest of Guice works this way so I feel it would
be
more consistent. Alternatively, discuss this explicitly in the Javadoc so users
know
to use bind() -- I personally never thought of it.
Robbie,
I must be missing something. Doesn't Guice throw an error if the user binds the
same
type multiple times (even across different modules)? I say we let the user bind
the
servlet/filter once and if he tries binding them elsewhere we throw an error. I
recall Guice already throwing this sort of error for other classes which is why
I
believe this is already implemented.
Original comment by gili.tza...@gmail.com
on 14 Feb 2009 at 1:40
Dhanji,
As you well know, good APIs "don't make the user do any work you can do on
his/her
behalf". If we can do this then I really think we should. The complexity belongs
under the hood, not outside it :)
Original comment by gili.tza...@gmail.com
on 14 Feb 2009 at 1:44
The extra module idea was just that you could have one module type that could
automatically install the
missing bindings, and one module type that wouldn't. But like I said I also
don't like this option, I'm just
writing down some of my reasoning.
Other than that, I agree that this is not really a problem; it's just that it
seems like something we'll be doing
over and over. Be it the binding, or the @Singleton on the Servlet/Filter.
Now that I think about it, the best option would probably be to integrate it
into the DSL somehow.
For example, the default behaviour would be to bind it on the spot:
// no Key overloads
serve("/my/*").with(MyServlet.class);
filter("/*").through(MyFilter.class);
// Key, class overloads
If people want to customize, they can use:
serve("/my/*").withBinding(MyServlet.class);
filter("/*").throughBinding(MyFilter.class);
Original comment by robbie.v...@gmail.com
on 14 Feb 2009 at 1:51
@gili
I have changed the javadoc accordingly, this morning. Note that our public doc
has already had this info for
some time now:
http://code.google.com/p/google-guice/wiki/Servlets
Jesse and I put a lot of thought into this and I believe our API is very
self-consistent, consider the case when
you bind an interface to a key:
bind(A.class).in(Singleton.class);
bind(Interface.class).to(A.class); //implicitly a singleton
This is how you should look at the filter()/bind() combination =)
I think good APIs are also consistent and concise. In the 80% case you will be
able to annotate the class
directly, which is a clean and self-documenting solution. In the 20% case, the
explicit bind() is simple and
effective.
Original comment by dha...@gmail.com
on 14 Feb 2009 at 1:59
We have a hard requirement for singletons. We may want to investigate
specifying servlet instances - I've opened
issue 361 to track that.
Original comment by limpbizkit
on 26 Apr 2009 at 9:11
Original issue reported on code.google.com by
gili.tza...@gmail.com
on 13 Feb 2009 at 6:19