Multiple feature enum support

[Richard Steele]

It looks like multiple feature enums are supported out of the box using the EnumBasedFeatureProvider().addFeatureEnum() method.  This would allow us to maintain (at least) two sets of feature switches, ones that are specific to the application and others that are shared across multiple applications across the enterprise.

However, I do have a couple of questions about the practical side of how this would work:

1. It looks like only the enum name is used in the state repositories, at least as far as I can tell from what the generated properties files.  The implication is if I have FEATURE_ONE in two enums that they'll be treated as the same feature.  Is this correct?  I'm not sure if this a Good Thing or a Bad Thing--it could, for example, allow us to migrate from application-specific switches to enterprise ones, but it could also be the source of great confusion if we're not careful.

2. It looks like TogglzRule only allows for enabling or disabling the features in a specific enum, correct?  Would I need to use multiple rules for the different enums?

Rich

[Christian Kaltepoth]

Hey Richard,

yeah, that's correct. If you use EnumBasedFeatureProvider directly,
you will be able to use multiple feature enums.

Regarding your questions:

1. It's correct, that internally the feature is represented by a
simple string. And it is also correct that you may run into problems
if both enums contain the same features name. If you want to work
around that, you could think about extending EnumBasedFeatureProvider
or create a custom FeatureProvider implementation. This shouldn't be
too hard. You could for example add a prefix identifying the enum to
the feature name in this case. Togglz has been developed in a way that
allows to plug in such custom functionality.

2. I'm not sure if two rules in one class would work as expected. But
it should be simple to adapt the rule to support multiple enums. The
class is really simple.

I hope this helps

Christian

--
Christian Kaltepoth
Blog: http://chkal.blogspot.com/
Twitter: http://twitter.com/chkal

[Richard Steele]

For #1 it looks like if I either extend EnumBasedFeatureProvider or implemented a new kind of EnumBasedFeatureProvider by implementing FeatureProvider it would have to return some kind of FeatureProxy from getFeatures(), instead of the Features directly, and that proxy would decorate the name() from each enum value with the package name of the enum class.  This seem right to you?

By the way, we're introducing Togglz into several applications and are impressed with its flexibility.  Thanks for putting this out there.

[Christian Kaltepoth]

Hey Richard,

I'm very sorry for the delayed response. I'm very busy and loosing control over my inbox. ;)

Yeah, basically you are correct. You could use a custom FeatureProvider (extending EnumBasedFeatureProvider makes sense here) that would return decorated Feature instances which modify name() to return a fully qualified name. The problem here is, that you will also have to use these decorated Feature instances when checking the feature state.

This:

MyFeatures.FEATURE_ONE.isActive()

is basically the same as:

FeatureContext.getFeatureManager().isActive( MyFeatures.FEATURE_ONE )

So in this case you would use an undecorated feature enum which will lead to the unqualified feature name.

But I guess you can fix this by defining your feature enum like this:

public enum MyFeatures implements Feature {

    FEATURE_ONE,
    FEATURE_TWO;
   
    public boolean isActive() {
        return FeatureContext.getFeatureManager().isActive( new QualifiedFeatureDecorator(this) );
    }
   
}

Assuming your feature wrapper class is called QualifiedFeatureDecorator.

I hope this helps. I'm not 100% sure this will work. But it is worth a try.

Unfortunately the name() method of enums is final. Therefore you cannot simply overwrite it in the enum declaration. That would have made everything much simpler. :)

Christian

Blog: http://blog.kaltepoth.de/

Twitter: http://twitter.com/chkal

GitHub: https://github.com/chkal

[Richard Steele]

Thanks for the confirmation.  At first I thought I wasn't going to worry too much about it but then I got a backlash from some in the community who were concerned about name collisions.

Anyway, I was hoping to be able to get all fancy by adding support for @RepositoryName and @RepositoryPrefix annotations but can't seem to connect the pieces: in order to decorate with the information from those annotations I need to look up the metadata for the feature but it won't be found if I haven't decorated it yet.  I don't think there's much I can do about that with the current design, but I'm holding out hope you'll see something I'm missing.

[Richard Steele]

Unfortunately I'm having problems decorating the enum features as suggested: I'm consistently getting errors like the following:

     [java] Caused by: java.lang.NullPointerException

     [java]     at org.togglz.core.metadata.enums.EnumFeatureMetaData.<init>(EnumFeatureMetaData.java:44)

     [java]     at org.togglz.core.manager.EnumBasedFeatureProvider.getMetaData(EnumBasedFeatureProvider.java:46)

     [java]     at org.togglz.core.manager.DefaultFeatureManager.getMetaData(DefaultFeatureManager.java:57)

     [java]     at org.togglz.core.manager.DefaultFeatureManager.getFeatureState(DefaultFeatureManager.java:103)

This is happening in areas where I'm finding and iterating over the set of features for diagnostic purposes--a home-grown console embedded into our Swing application. The summary of what's causing this is that EnumFeatureMetaData calls FeatureAnnotations.getAnnotations(), which assumes that the Feature.name() is going to be the name of a field on the class.  Since it's not (feature.name() returns a fully qualified name like com.example.MyAppFeatures.FOO) getAnnotations() returns null and we get the NPE above.

Thoughts?

[Richard Steele]

I finally got some more time to look at this more deeply and figured I just have to create my own implementation of FeatureMetaData.  I chose to extend the existing EnumMetaData and used delegation to forward to the underlying feature; e.g.,

private final EnumFeatureMetaData impl;

QualifiedEnumMetaData(QualifiedFeatureDecorator feature) {

this.impl = new EnumFeatureMetaData(feature.getFeatureImpl());

}

@Override

public String getLabel() {

return impl.getLabel();

}

So far so good.

It looks like EnumMetaData is only used by EnumBasedFeatureProvider, though the fact that it's public makes me a bit nervous since future changes could construct and use it directly.  Any chances this might happen?

[Christian Kaltepoth]

Hey Richard,

sorry for my very delayed response. As Togglz is just an spare time project, I don't always find time to answer all the mails regarding it. :(

I think you are on the right road. I wasn't aware that the decoration of the Feature would cause problems in EnumFeatureMetaData. I think what you are currently doing should work fine. EnumBasedFeatureProvider is an SPI class. So you should be able to extend it without problems. I see now reason for it to change.

But perhaps it is even simpler. What about changing your implementation of FeatureProvider.getMetaData() to something like this:

    @Override

    public FeatureMetaData getMetaData(Feature feature) {

        Feature f = getFeatureByName(feature.name());

        if(f instanceof QualifiedFeatureDecorator) {

            Feature featureEnum = ((QualifiedFeatureDecorator) f).getFeatureImpl();

            return new EnumFeatureMetaData(featureEnum);

        }

        return new EnumFeatureMetaData(f);

    }

This way you won't have to subcass EnumFeatureMetaData. You just make sure that a real enum is passed into the constructor. Perhaps even a bit simpler?

Christian

[Steele, Richard]

<blush>

Yes, that's a simpler approach, thanks.

</blush>