The static injection was used as a hack to avoid doing the (much needed)
fixing of the fetcher pipeline. The chain of command used for implementing
oauth and caching is pretty awful, but refactoring it to a clean design
(probably a simple composed model) would have been very disruptive. Now that
we've branched that code it's a good time to fix this, but since it hasn't
been causing any major problems nobody's touched it.
On Tue, Dec 2, 2008 at 6:43 PM, Henning P. Schmiedehausen <
[EMAIL PROTECTED]> 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
>
