I think we should probably move this to the dev list and out of the
review.
For those interested, the conversation started here:
https://reviews.apache.org/r/2362/
Jesse, I agree something should be done to make dealing with the container
config easier. I just don't think it's going to be possible for us to
make this generic enough so that a ContainerConfig would be able to drive
this setting (security token key) and many other changes as well. To me,
that sounds like... "config driven implementation" in my mind. I don't
know if that's practical in shindig.
If we did something like that, we would need a hardened interface and
extension pattern for being able to control the types of returned
values... So the ContainerConfig would return a byte[]?
Having the interpretation of how the config value is something I'd rather
leave up to the particular implementation.
For instance, in Stanton's review, they default key provider is container
specific... But I really don't see why an encryption key would really
need to be container specific. If the default impl is set up that way we
want to cope, but the impl we have in mind is container agnostic, and will
have other mechanisms to revoke and manage a key. The provider pattern
allows us to be configured if we want to, or ignore the container config.
Though you're right... it's far to awkward to deal with the container
config, and the change listening is prone to concurrency issues and other
problems.
I'd like to propose that we introduce some abstract classes for
implementing future Provider classes that do the work of listening to a
arbitrary key(impl specific) and providing some pre-baked impl to deal
with changes and further notify consumers of the provider if the value
changes.
This will make it much easier to roll your own implementation of features.
My proposed heirarchy is:
IOnChangedListener<type> (interface used by a provider to handle changes
in the value being provided)
IProvider<type>
AbstractProvider (for impls not based on a ContainerConfig)
AbastractConfiguredProvider (for impls that listen to a container
config setting)
What do you think?
(BTW, this discussion has been excellent and has really helped me
understand a lot more about shindig... I'm very grateful you're
participating in it and hopefully others can chime in as well)
