hrm..
Due to the ordered magic of ImmutableSet we do get things injected in the
correct order:
As you can see below we inject the random ordered multibindings first,
followed by the comma split paths of "shindig.features.default".
@Provides
@Singleton
@Named("org.apache.shindig.features")
protected Set<String>
defaultFeatures(@Named("shindig.features.default")String features,
@Named("org.apache.shindig.features-extended")Set<String> extended) {
return
ImmutableSet.<String>builder().addAll(extended).add(StringUtils.split(features,
',')).build();
}
So I believe this is safe to apply as-is. Still it seems like there should
be a better way to do this. The use cases I can see that we might need to
address:
* Override default features (works by insuring that we add user features
last).
* Removing features from the default. Trickier... Currently you have to
copy features.txt and remove the ones you don't want. Might make sense to
have a blacklist of features you don't want to support.
There are a bunch of things we inject that are order-dependent (basically
Lists) that would be nice to support in a plugin/contributor style. The
only easy way to do this is to inject available features as a Set/Map and
then apply some filter and/or ordering.
Consider Rewriters for example. We might define an ordered 'phase' enum
that each rewriter runs in and use a default comparator for that. However
we can add specific rules that can insure that Rewriter B runs after
Rewriter A by using a custom compare() method.
More info on Set vs List here.
http://publicobject.com/2008/05/why-no-multibindings-for-lists-or.html
On Thu, May 13, 2010 at 11:25 AM, John Hjelmstad <[email protected]> wrote:
> This actually isn't backward-compatible. Set<String> is unordered, whereas
> the previous ordered array makes it possible to override existing feature
> declarations. For instance, we have a custom rpc/feature.xml override of
> the
> Shindig default.
>
> --j
>
> On Thu, May 13, 2010 at 6:58 AM, Paul Lindner <[email protected]> wrote:
>
> > Here's the next step towards allowing a shindig plugin architecture.
> This
> > one allows external modules to add their javascript features to the
> > FeatureRegistry.
> >
> > It's backwards compatible with the way we currently inject feature
> > directories (via shindig.features.default) so I'm going to commit this
> soon
> > unless there are objections.
> >
> > On Thu, May 13, 2010 at 6:53 AM, <[email protected]> wrote:
> >
> > > Reviewers: shindig.remailer_gmail.com,
> > >
> > >
> > >
> > > Please review this at http://codereview.appspot.com/1197041/show
> > >
> > > Affected files:
> > > M
> > >
> >
> java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java
> > > M
> > >
> >
> java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java
> > > M
> > >
> >
> java/gadgets/src/test/java/org/apache/shindig/gadgets/features/FeatureRegistryTest.java
> > > M
> > >
> >
> java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/tags/TemplateBasedTagHandlerTest.java
> > >
> > >
> > >
> >
>
