2014-02-17 16:01 GMT+01:00 Christian Schneider <[email protected]>:
> I can very well imagine that this would work. I am not sure if it is a > good idea thought. > We would recreate parts of the dependency injection framework. > Well, which parts ? I don't understand. My first attempt was indeed creating a very small DI framework, but here, my goal is to delegate everything to the real DI. The namespace handler would simply be enhanced to discover the actions and the injection points and create blueprint metadata out of it. This would obviously require a bit of reflection on the classes, but nothing different from your proposal. > > In any case it would require that we implement a solution for each DI > framework out there. Additionally we would have to keep our extensions up > to date with the framework evolution. > While I can understand that my approach has its own fallacies I think it > would create less issues and less work even in the long term. > > What I could imagine as a kind of middle ground is to simply have a > special processing for the @Command annotation. > > For example in blueprint we could have: > > <bean class="MyAction"> > .... do some injections --- > </bean> > > We then let blueprint do its injections but also create the > BlueprintCommand for it and export the service. So this would be very > similar to the namespace but not requiring it. Not sure if this is possible > in blueprint though. The nice thing with this approach is that it might > also work with blueprint annotations. In any case I propose we let the > framework do its injections and only work with the fully injected Action. Ok, so I misunderstood as it seems we don't have the same goals. My point is to make typical things easy for the user and not having to write this boilerplate code. I.e. in 95% of the use cases, the user should not have to deal with blueprint xml or DS specific annotations. If the user wants to fully reuse its DI, he can do so, but our current model is quite verbose. We have such an example for blueprint: https://github.com/apache/karaf/blob/master/shell/console/src/main/resources/OSGI-INF/blueprint/karaf-console.xml#L56 and for DS: https://github.com/apache/karaf/blob/karaf-3.0.0/scr/command/src/main/java/org/apache/karaf/scr/command/DetailsCommandComponent.java So I'm not trying to change the above model, rather keep this model (Action / Command), but make things easier for the user to register commands. I first got rid of the completers and used annotations to wire them through OSGi services, now I want to get rid of exposing the command in the OSGi registry manually, even if we delegate to the underlying DI framework. IIUC, what you're proposing is a new simplified model so that users don't have to deal with commands but only with actions. While I don't have any problems with discussing a new model, I'd like to avoid misusing our current model (and breaking a few OSGi contracts). More importantly, your proposal does not help a single bit solving my problem. Though, if my goal can be achieved somehow, there's no real value left in your proposal, as it would only help in 1% of the cases that can't be covered by my proposal, and I think those will be the same that can't be covered by the blueprint xml handler now (i.e. exit and help commands). > > > Christian > > > On 17.02.2014 15:48, Guillaume Nodet wrote: > >> I think we could build upon those annotations and have them leveraged by >> the native dependency injection framework (be it blueprint or DS). >> >> I need to experiment a bit, but I don't see any reason why the blueprint >> namespace handler would not be able to scan those commands and do create >> blueprint metadata out of it. >> >> The blueprint xml would simply look like: >> >> <command-bundle xmlns="http://karaf.apache.org/xmlns/shell/v1.1.0"; >> scan="my.package" /> >> >> This would be equivalent to the current declaration of commands and >> completers (by simply adding the @Register and @Reference annotations). >> >> For DS, it should also be possible to have them translated into native SCR >> annotations through a maven plugin with an annotation scanner which would >> build the SCR descriptions. >> >> I think the overall goals should be the following: >> * make things as easy as possible for the user >> * make things as robust as possible, i.e. keep the OSGi registry >> contracts >> * for the typical use cases, the commands code should be self >> describing, >> i.e. no additional metadata needed >> >> I'll go ahead and experiment with the above extended blueprint namespace >> now. I'll report soon. >> >> 2014-02-17 14:35 GMT+01:00 Guillaume Nodet <[email protected]>: >> >> I have experimented this morning with a possible implementation of what I >>> outlined last week. >>> We have a very simple set of 4 annotations: >>> @Service @Reference @Init @Destroy >>> >>> @Service can be added on an action (already annotated with @Command) or a >>> completer implementation. >>> @Reference is used on fields that contain an OSGi service that needs to >>> be >>> retrieved from the OSGi registry before the object can be used. >>> @Init and @Destroy can be used to further control the lifecycle if >>> needed. >>> >>> A bundle extender will track bundles that have a Karaf-Inject header and >>> scan the classes annotated with @Service. For non-commands, it will >>> create >>> the instances, inject and register them. For commands, it will register >>> an >>> AbstractCommand object which will, when asked, create and inject the >>> action. >>> >>> The annotation don't have any parameters, they're simplistic on purpose, >>> as anything more complicated would mean rewriting a layer such as >>> blueprint, DS, etc... and I don't think we should go this way. >>> >>> Here's an example on how it could be used: >>> >>> https://github.com/gnodet/karaf/commit/9649a3bf8d5967d58c048b07619f12 >>> 18284d040b >>> >>> >>> 2014-02-14 20:31 GMT+01:00 Guillaume Nodet <[email protected]>: >>> >>> >>>> >>>> 2014-02-14 18:29 GMT+01:00 Christian Schneider <[email protected] >>>> >: >>>> >>>> The DSL will make it easier to write the command. So it is quite >>>> >>>>> convenient in the start. >>>>> The downside I see is that it will create quite strong coupling between >>>>> someone creating the service and the karaf impl. So it might be >>>>> difficult >>>>> when we do karaf 4 which might introduce a lot of changes in the >>>>> implementation. >>>>> >>>>> Mmh, I'm not sure how this make things more coupled really. The >>>> blueprint DSL is karaf specific anyway so that commands are now tied to >>>> the >>>> blueprint DSL. >>>> >>>> >>>> I think it would be better if we find a way that allows the implementer >>>>> of a command to simply implement an interface, use some annotations and >>>>> export the command as a service. Basically we are almost there >>>>> already. The >>>>> user code already only depends on the interface Action and some >>>>> annotations. >>>>> >>>>> I know we currently have the problem that e.g. in BlueprintCommand we >>>>> create a new instance of the Action. Perhaps we could avoid that. >>>>> >>>>> Well, it's not specific to blueprint, it's a separate decision. >>>> Commands are now stateless and recreated each time. This has a benefit >>>> which is that users don't have to deal with thread safety issues. We >>>> could >>>> change that, but that would definitely be a big incompatible change. >>>> >>>> >>>> How about this: >>>>> The user simply exports his Action object as a service. >>>>> >>>> >>>> We pick it up in the shell bundle and simply use it in a synchronized >>>>> fashion on command execution at a time. We can then prepare the command >>>>> (fill in the arguments and execute it) and execute it without creating >>>>> a >>>>> new instance. >>>>> >>>>> Synchronizing on commands would be a really bad idea. Some commands >>>> do >>>> run for quite a long time (log:tail, etc...). Re-preparing the same >>>> command is more difficult because you need to know the default values as >>>> they have been lost. >>>> >>>> Anyway, if you look at the DSL, declaring a command in the most simple >>>> use case is as simple as: >>>> command(MyAction.class) >>>> >>>> We could avoid that by adding annotations, but the commands usually need >>>> to be injected with a service. What could eventually be done, is to >>>> add a >>>> new set of annotations for auto-registration and service injection. If >>>> we >>>> do that, we should not rely on another extender such as CDI or DS, as >>>> the >>>> benefit would be lost, so it has to be pure karaf. >>>> I guess this would cover 90% of the commands if we were able to inject >>>> OSGI services by looking for a service matching the field type. >>>> >>>> @Command(...) >>>> @Register >>>> public class MyCommand extends OsgiCommandSupport { >>>> >>>> @Inject >>>> private JndiService jndiService; >>>> >>>> public Object doExecute() throws Exception { >>>> .... >>>> } >>>> >>>> } >>>> >>>> So with 2 new annotations, it should remove most of the blueprint stuff >>>> needed I think. >>>> >>>> I really don't think changing the model action/command model is a good >>>> idea. I'd much rather write a new one if we want to not instantiate new >>>> commands each time. >>>> >>>> >>>> Best regards >>>>> >>>>> Christian >>>>> >>>>> >>>>> On 14.02.2014 17:47, Guillaume Nodet wrote: >>>>> >>>>> See https://issues.apache.org/jira/browse/KARAF-2761 >>>>>> >>>>>> The idea is to simplify writing commands as much as possible. >>>>>> With the recent @Completer annotation, things are already much easier >>>>>> to >>>>>> deal with, but writing commands without blueprint is a real pain. >>>>>> >>>>>> I've committed a simple java DSL to help around that, so you have an >>>>>> example at >>>>>> https://git-wip-us.apache.org/repos/asf?p=karaf.git;a=blob; >>>>>> f=scr/command/src/main/java/org/apache/karaf/scr/command/ >>>>>> Commands.java;h=70533e5640e5ab0492b67df71a9a028a7895135e;hb= >>>>>> 40140e4f0c0745a41c1a4579f529f07e5819d785#l39 >>>>>> So far the SCR commands, it's a clear win. >>>>>> >>>>>> I'm also considering using it for blueprint based commands, but I >>>>>> haven't >>>>>> had any time to experiment yet. One limitation of blueprint is that >>>>>> there's no "abstract" definitions, so no way to make things common to >>>>>> all >>>>>> commands definitions in a given bundle. >>>>>> Let's look at a common use case: >>>>>> >>>>>> https://git-wip-us.apache.org/repos/asf?p=karaf.git;a=blob; >>>>>> f=jndi/command/src/main/resources/OSGI-INF/blueprint/ >>>>>> jndi-command.xml;h=d9b9f148ca2bc3be6e6dcd574840ed19576f11da;hb=HEAD >>>>>> >>>>>> The only real interesting things in the above xml is the class names >>>>>> of >>>>>> commands and completers. I think reusing the above DSL, the blueprint >>>>>> would look like >>>>>> >>>>>> <reference id="jndiService" interface="org.apache.karaf. >>>>>> jndi.JndiService" >>>>>> /> >>>>>> <bean class="org.apache.karaf.jndi.command.Commands" >>>>>> init-method="init" >>>>>> destroy-method="unregister"> >>>>>> <property name="context" ref="blueprintBundleConext" /> >>>>>> <property name="jndiService" ref="jndiService" /> >>>>>> </bean> >>>>>> >>>>>> and then, the java code: >>>>>> >>>>>> public class Commands extends org.apache.karaf.shell. >>>>>> commands.Commands >>>>>> { >>>>>> private JndiService jndiService; >>>>>> public void init() { >>>>>> completer(new ContextsCompleter(jndiService)); >>>>>> completer(new NamesCompleter(jndiService)); >>>>>> completer(new ServiceIdsCompleter(getContext())); >>>>>> command(AliasCommand.class); >>>>>> command(BindCommand.class); >>>>>> command(ContextsCommand.class); >>>>>> command(CreateCommand.class); >>>>>> command(DeleteCommand.class); >>>>>> command(NamesCommand.class); >>>>>> command(UnbindCommand.class); >>>>>> register(); >>>>>> } >>>>>> public CommandBuilder command(Class<? extends Action> >>>>>> actionClass) { >>>>>> return super.command(actionClass) >>>>>> .properties(jndiService); >>>>>> } >>>>>> public void setJndiService(JndiService jndiService) { >>>>>> this.jndiService = jndiService; >>>>>> } >>>>>> } >>>>>> >>>>>> I haven't tried the above code yet, so there may be slight typos, but >>>>>> that >>>>>> gives you an idea of what it would look like. >>>>>> Thoughts ? >>>>>> >>>>>> Guillaume >>>>>> >>>>>> >>>>>> -- >>>>> Christian Schneider >>>>> http://www.liquid-reality.de >>>>> >>>>> Open Source Architect >>>>> http://www.talend.com >>>>> >>>>> >>>>> > > -- > Christian Schneider > http://www.liquid-reality.de > > Open Source Architect > http://www.talend.com > >
