Although I understand where you are going with the remainder of the
message, using the ImplementedBy was brought in in Shindig to make it
easier (with Guice 1.0) for implementors to provide their own model
and spi implementations without having to duplicate the Module code.
Now we could break all the modules out into areas of functionality and
eliminate all the ImplementedBy annotations.
I think the need for ImplementedBy disappears in Guice2.
Ian
On 3 Dec 2008, at 13:43, Henning P. Schmiedehausen wrote:
As I run a tighter ship with our integration than the original Shindig
code base does, I am working on some changes with the way Guice is
used (In a nutshell: @ImplementedBy was incepted by the same mindset
that brought us Spring Autowiring and has to go). All of my changes
are pretty small and isolated and so far, all test cases check out.
Until now. When I run "mvn clean test" on the gadgets module, I get
Results :
Failed tests:
testStrictPragmaJunk(org.apache.shindig.gadgets.http.HttpResponseTest)
testFixedDate(org.apache.shindig.gadgets.http.HttpResponseTest)
Tests run: 561, Failures: 2, Errors: 0, Skipped: 0
The scary part is, that when I run
mvn -Dtest=HttpResponseTest test
I get
Running org.apache.shindig.gadgets.http.HttpResponseTest
Tests run: 27, Failures: 0, Errors: 0, Skipped: 0, Time elapsed:
0.235 sec
Results :
Tests run: 27, Failures: 0, Errors: 0, Skipped: 0
It took me a while to find out, that the reason for this is the fact
that HttpResponse contains two fields
@Inject @Named("shindig.cache.http.negativeCacheTtl")
private static long negativeCacheTtl = DEFAULT_NEGATIVE_CACHE_TTL;
@Inject @Named("shindig.cache.http.defaultTtl")
private static long defaultTtl = DEFAULT_TTL;
which are injected when the whole test suite (due to
requestStaticInjection(HttpResponse.class) in DefaultGuiceModule) is
run (probably by some other test case at some point in the same VM).
with the values from shindig.properties. Which in turn differ from the
default constants in HttpResponse.
When running just the single test case, Guice is never set up and the
injection never happens.
=> Different behaviour. Bad.
I honestly don't understand the point behind the logic with
HttpResponse and HttpResponseBuilder (I do understand what it is
*trying* to achieve, but if you have Guice handy, there should be a
factory creating these objects which in turn injects these values on
the c'tor. And as these fields are only assigned once, they definitely
should be final.
Would you accept a patch cleaning up that logic so that it leverages
Guice to do what it is supposed to do?
Ciao
Henning
--
Henning P. Schmiedehausen - Palo Alto, California, U.S.A.
[EMAIL PROTECTED] "We're Germans and we use Unix.
[EMAIL PROTECTED] That's a combination of two demographic
groups
known to have no sense of humour
whatsoever."
-- Hanno Mueller,
de.comp.os.unix.programming
