Removing the services as discussed here is an api change.
I think the risks involved are very low since the services hasn't been
documented and since the instances returned are accessed using static
methods on the QueryUtil class. AFAIK, all known clients use the
methods on the QueryUtil class. No public java API is affected by the
change.
The merits or removing the services should be clear by this email
conversation and the bugzilla.
On Tue, Mar 23, 2010 at 9:01 AM, Thomas
Hallgren <thomas@xxxxxxx> wrote:
Hi Jason,
The problem with the ql bundle not being present has been resolved [1]
(all functionality has been moved into the metadata bundle) so there
are no longer any static references to services that are not declared
in the same bundle. In fact, the need for the parser and factory
services is gone so perhaps the next step would be to remove them
altogether.
We ran into a problem here, and I would
second this being
a problem, but for slightly different reasons. I don't know if it's
being pedantic, but having service lookups in static initializers seems
to be contrary to the service paradigm (where code should gracefully
handle services not being available, or coming and going). For
instance, if the service is not available, or a non compatible service
is bound and an exception occurs (which happens if the ql bundle is not
provided), the class is hosed, it can't be loaded again (even if you
fixed the problem and now have a service available). You have to
restart the JVM (although maybe there is some way to make the JVM
forget about the exception in the initialization of a class.. but
still).
-Jason
On Tue, Mar 23, 2010 at 8:41 AM, Thomas
Hallgren <thomas@xxxxxxx>
wrote:
Hi
Matt,
p2 is an OSGi based framework so when running test on it, you need to
run them as "JUnit Plug-in tests". That has always been the case as far
as I know. A lot of p2 relies on services being started etc.
Regards,
Thomas Hallgren
On 03/23/2010 04:07 PM, Matt Hughes wrote:
Now
that many of the *Query classes have moved into static methods on
QueryUtil, there is a problem with unit testability.
QueryUtil has a static initializers that use ExpressionUtil.
ExpressionUtil has static initializers that *depend on running in an
OSGi environment* (not unit test):
public final class ExpressionUtil {
private static final LDAPFilterParser ldapFilterParser = new
LDAPFilterParser(ExpressionFactory.INSTANCE);
public static final IExpression TRUE_EXPRESSION =
getFactory().constant(Boolean.TRUE);
public static final IExpression FALSE_EXPRESSION =
getFactory().constant(Boolean.FALSE);
private ExpressionUtil() {
//We don't want to ppl to instantiate this class
}
/**
* Returns the global _expression_ factory
* @return The global _expression_ factory.
*/
public static IExpressionFactory getFactory() {
return MetadataActivator.getExpressionFactory();
}
(As soon as one even references ExpressionUtil, its static
initializers are called which in turn invoke #getFactory which rely on
MetadataActivator to have been activated by OSGi context.)
Therefore, if any class uses QueryUtil in its implementation (which
many must do now since the *Query classes have gone away), those
classes cannot be unit tested. One could try and get around this by
setting MetadataActivator.instance, but that would only get you so
far. Metadata#getExpressionFactory would still fail and cannot be
stubbed out so far as I can tell.