You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@karaf.apache.org by "Christian Schneider (JIRA)" <ji...@apache.org> on 2011/07/08 16:12:17 UTC
[jira] [Created] (KARAF-717) Refactor Actions to get services
injected instead of fetching them
Refactor Actions to get services injected instead of fetching them
------------------------------------------------------------------
Key: KARAF-717
URL: https://issues.apache.org/jira/browse/KARAF-717
Project: Karaf
Issue Type: Improvement
Reporter: Christian Schneider
Assignee: Christian Schneider
Priority: Minor
Currently we use abstract classes like ObrCommandSupport and OsgiCommandSupport to handle the bundleContext and service references.
This makes the code harder to test and mixes technical and business code. By business I mean the stuff the class is meant to do by technical I mean other domains like osgi services.
So I propose to inject the services we need using blueprint so in the Action there is only the interface to the service and a setter. Additionally I would even skip the AbstractAction as it is not good to store the CommandSession in a class attribute as this makes the code non reentrant.
I have done a sample refactoring in http://svn.apache.org/viewvc?view=revision&revision=1144288
It shows how to change the ListCommand of the Obr to not need any abstract support classes to handle the RepositoryAdmin service.
This issue is a marker that we should do the refactoring and should be split into smaller tasks when we start the refactoring.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: [jira] [Created] (KARAF-717) Refactor Actions to get services
injected instead of fetching them
Posted by Christian Schneider <ch...@die-schneider.net>.
Hi Guillaume,
I did not intend to work on this in the moment. Still it is good that
you explained the issues below.
Basically I think it is ok that the command only appears when the
service is there. Typically the service belongs to the command anyway.
So for example
the instance command only uses the instance service from instance.core.
So tpyically they will be installed together anyway. An error message
about a missing
internal service the user probably would not even know existed probably
does not help that much anyway. So I think when we let blueprint stop
the command context when the service goes away this saves us from
handling these exceptions which makes the code simple.
Btw. With the new bundle:diag command in karaf 3 it is really simple for
the user to check for such situations like a missing service.
The case that a command it not available exists anyway as the commands
only exist as soon as the command bundle is started. So if the user
types fast at the start this may hit him anyway.
What do you think?
Christian
Am 14.05.2012 14:59, schrieb Guillaume Nodet:
> So the main problem is that when you inject a blueprint proxy into a bean,
> the bean does not know if the real service is available or not.
> If the reference is mandatory, the service that depends on that reference
> (the command in our case) won't be registered until the dependencies is
> satisfied. If the reference is optional, the service will be registered
> anyway, but in both cases, the bean that is injected has no way to know if
> the service is available or not and the calls will wait until the service
> is available or throw an exception when the timeout is elapsed.
> That means that either the commands won't be available, or the commands
> will block.
> If you look at the existing commands, for example the OBR commands, they
> will throw an exception if the service is not available without waiting for
> the timeout and the user will immediately know with a message printed in
> the console.
> I think that's a good behavior and I think we should keep it. It also
> seems that some commands don't behave that way (the admin commands for
> example) and I think they should rather be refactored to be able to fail
> fast the same way the OBR commands do.
>
> One way would be to make sure the timeout on the reference has a timeout of
> 0 (need to check if that means infinite or really zero) and catch the
> ServiceNotAvailable exception and rethrow a more meaningful exception to
> display to the user. Not sure if that will make things cleaner though.
> Another way is to use blueprint listeners so that the beans can know when
> the services come and go, but it complexity the code a bit too.
>
> Now, if you find a better and cleaner way to do that, I don't have any
> problems.
>
> On Mon, May 14, 2012 at 2:36 PM, Guillaume Nodet<gn...@gmail.com> wrote:
>
>> I'm don't think it's a good idea because of the way blueprint proxies
>> work. I'll to provide mire information later but I'm at the airport.
>> Please hold on until we can discuss that further.
>>
>> On Friday, July 8, 2011, Christian Schneider (JIRA) wrote:
>>
>>> Refactor Actions to get services injected instead of fetching them
>>> ------------------------------------------------------------------
>>>
>>> Key: KARAF-717
>>> URL: https://issues.apache.org/jira/browse/KARAF-717
>>> Project: Karaf
>>> Issue Type: Improvement
>>> Reporter: Christian Schneider
>>> Assignee: Christian Schneider
>>> Priority: Minor
>>>
>>>
>>> Currently we use abstract classes like ObrCommandSupport and
>>> OsgiCommandSupport to handle the bundleContext and service references.
>>> This makes the code harder to test and mixes technical and business code.
>>> By business I mean the stuff the class is meant to do by technical I mean
>>> other domains like osgi services.
>>>
>>> So I propose to inject the services we need using blueprint so in the
>>> Action there is only the interface to the service and a setter.
>>> Additionally I would even skip the AbstractAction as it is not good to
>>> store the CommandSession in a class attribute as this makes the code non
>>> reentrant.
>>>
>>> I have done a sample refactoring in
>>> http://svn.apache.org/viewvc?view=revision&revision=1144288
>>>
>>> It shows how to change the ListCommand of the Obr to not need any
>>> abstract support classes to handle the RepositoryAdmin service.
>>>
>>> This issue is a marker that we should do the refactoring and should be
>>> split into smaller tasks when we start the refactoring.
>>>
>>>
>>> --
>>> This message is automatically generated by JIRA.
>>> For more information on JIRA, see: http://www.atlassian.com/software/jira
>>>
>>>
>>>
>> --
>> ------------------------
>> Guillaume Nodet
>> ------------------------
>> Blog: http://gnodet.blogspot.com/
>> ------------------------
>> FuseSource, Integration everywhere
>> http://fusesource.com
>>
>
>
--
Christian Schneider
http://www.liquid-reality.de
Open Source Architect
Talend Application Integration Division http://www.talend.com
Re: [jira] [Created] (KARAF-717) Refactor Actions to get services
injected instead of fetching them
Posted by Guillaume Nodet <gn...@gmail.com>.
So the main problem is that when you inject a blueprint proxy into a bean,
the bean does not know if the real service is available or not.
If the reference is mandatory, the service that depends on that reference
(the command in our case) won't be registered until the dependencies is
satisfied. If the reference is optional, the service will be registered
anyway, but in both cases, the bean that is injected has no way to know if
the service is available or not and the calls will wait until the service
is available or throw an exception when the timeout is elapsed.
That means that either the commands won't be available, or the commands
will block.
If you look at the existing commands, for example the OBR commands, they
will throw an exception if the service is not available without waiting for
the timeout and the user will immediately know with a message printed in
the console.
I think that's a good behavior and I think we should keep it. It also
seems that some commands don't behave that way (the admin commands for
example) and I think they should rather be refactored to be able to fail
fast the same way the OBR commands do.
One way would be to make sure the timeout on the reference has a timeout of
0 (need to check if that means infinite or really zero) and catch the
ServiceNotAvailable exception and rethrow a more meaningful exception to
display to the user. Not sure if that will make things cleaner though.
Another way is to use blueprint listeners so that the beans can know when
the services come and go, but it complexity the code a bit too.
Now, if you find a better and cleaner way to do that, I don't have any
problems.
On Mon, May 14, 2012 at 2:36 PM, Guillaume Nodet <gn...@gmail.com> wrote:
> I'm don't think it's a good idea because of the way blueprint proxies
> work. I'll to provide mire information later but I'm at the airport.
> Please hold on until we can discuss that further.
>
> On Friday, July 8, 2011, Christian Schneider (JIRA) wrote:
>
>> Refactor Actions to get services injected instead of fetching them
>> ------------------------------------------------------------------
>>
>> Key: KARAF-717
>> URL: https://issues.apache.org/jira/browse/KARAF-717
>> Project: Karaf
>> Issue Type: Improvement
>> Reporter: Christian Schneider
>> Assignee: Christian Schneider
>> Priority: Minor
>>
>>
>> Currently we use abstract classes like ObrCommandSupport and
>> OsgiCommandSupport to handle the bundleContext and service references.
>> This makes the code harder to test and mixes technical and business code.
>> By business I mean the stuff the class is meant to do by technical I mean
>> other domains like osgi services.
>>
>> So I propose to inject the services we need using blueprint so in the
>> Action there is only the interface to the service and a setter.
>> Additionally I would even skip the AbstractAction as it is not good to
>> store the CommandSession in a class attribute as this makes the code non
>> reentrant.
>>
>> I have done a sample refactoring in
>> http://svn.apache.org/viewvc?view=revision&revision=1144288
>>
>> It shows how to change the ListCommand of the Obr to not need any
>> abstract support classes to handle the RepositoryAdmin service.
>>
>> This issue is a marker that we should do the refactoring and should be
>> split into smaller tasks when we start the refactoring.
>>
>>
>> --
>> This message is automatically generated by JIRA.
>> For more information on JIRA, see: http://www.atlassian.com/software/jira
>>
>>
>>
>
> --
> ------------------------
> Guillaume Nodet
> ------------------------
> Blog: http://gnodet.blogspot.com/
> ------------------------
> FuseSource, Integration everywhere
> http://fusesource.com
>
--
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com
Re: [jira] [Created] (KARAF-717) Refactor Actions to get services
injected instead of fetching them
Posted by Guillaume Nodet <gn...@gmail.com>.
I'm don't think it's a good idea because of the way blueprint proxies work.
I'll to provide mire information later but I'm at the airport. Please
hold on until we can discuss that further.
On Friday, July 8, 2011, Christian Schneider (JIRA) wrote:
> Refactor Actions to get services injected instead of fetching them
> ------------------------------------------------------------------
>
> Key: KARAF-717
> URL: https://issues.apache.org/jira/browse/KARAF-717
> Project: Karaf
> Issue Type: Improvement
> Reporter: Christian Schneider
> Assignee: Christian Schneider
> Priority: Minor
>
>
> Currently we use abstract classes like ObrCommandSupport and
> OsgiCommandSupport to handle the bundleContext and service references.
> This makes the code harder to test and mixes technical and business code.
> By business I mean the stuff the class is meant to do by technical I mean
> other domains like osgi services.
>
> So I propose to inject the services we need using blueprint so in the
> Action there is only the interface to the service and a setter.
> Additionally I would even skip the AbstractAction as it is not good to
> store the CommandSession in a class attribute as this makes the code non
> reentrant.
>
> I have done a sample refactoring in
> http://svn.apache.org/viewvc?view=revision&revision=1144288
>
> It shows how to change the ListCommand of the Obr to not need any abstract
> support classes to handle the RepositoryAdmin service.
>
> This issue is a marker that we should do the refactoring and should be
> split into smaller tasks when we start the refactoring.
>
>
> --
> This message is automatically generated by JIRA.
> For more information on JIRA, see: http://www.atlassian.com/software/jira
>
>
>
--
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com
[jira] [Created] (KARAF-717) Refactor Actions to get services
injected instead of fetching them
Posted by Guillaume Nodet <gn...@gmail.com>.
I'm don't think it's a good idea because of the way blueprint proxies work.
I'll to provide mire information later but I'm at the airport. Please
hold on until we can discuss that
On Friday, July 8, 2011, Christian Schneider (JIRA) wrote:
> Refactor Actions to get services injected instead of fetching them
> ------------------------------------------------------------------
>
> Key: KARAF-717
> URL: https://issues.apache.org/jira/browse/KARAF-717
> Project: Karaf
> Issue Type: Improvement
> Reporter: Christian Schneider
> Assignee: Christian Schneider
> Priority: Minor
>
>
> Currently we use abstract classes like ObrCommandSupport and
> OsgiCommandSupport to handle the bundleContext and service references.
> This makes the code harder to test and mixes technical and business code.
> By business I mean the stuff the class is meant to do by technical I mean
> other domains like osgi services.
>
> So I propose to inject the services we need using blueprint so in the
> Action there is only the interface to the service and a setter.
> Additionally I would even skip the AbstractAction as it is not good to
> store the CommandSession in a class attribute as this makes the code non
> reentrant.
>
> I have done a sample refactoring in
> http://svn.apache.org/viewvc?view=revision&revision=1144288
>
> It shows how to change the ListCommand of the Obr to not need any abstract
> support classes to handle the RepositoryAdmin service.
>
> This issue is a marker that we should do the refactoring and should be
> split into smaller tasks when we start the refactoring.
>
>
> --
> This message is automatically generated by JIRA.
> For more information on JIRA, see: http://www.atlassian.com/software/jira
>
>
>
--
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com
[jira] [Updated] (KARAF-717) Refactor Actions to get services
injected instead of fetching them
Posted by "Christian Schneider (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/KARAF-717?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Schneider updated KARAF-717:
--------------------------------------
Component/s: console
Fix Version/s: 3.1.0
> Refactor Actions to get services injected instead of fetching them
> ------------------------------------------------------------------
>
> Key: KARAF-717
> URL: https://issues.apache.org/jira/browse/KARAF-717
> Project: Karaf
> Issue Type: Improvement
> Components: console
> Reporter: Christian Schneider
> Assignee: Christian Schneider
> Priority: Minor
> Fix For: 3.1.0
>
>
> Currently we use abstract classes like ObrCommandSupport and OsgiCommandSupport to handle the bundleContext and service references.
> This makes the code harder to test and mixes technical and business code. By business I mean the stuff the class is meant to do by technical I mean other domains like osgi services.
> So I propose to inject the services we need using blueprint so in the Action there is only the interface to the service and a setter. Additionally I would even skip the AbstractAction as it is not good to store the CommandSession in a class attribute as this makes the code non reentrant.
> I have done a sample refactoring in http://svn.apache.org/viewvc?view=revision&revision=1144288
> It shows how to change the ListCommand of the Obr to not need any abstract support classes to handle the RepositoryAdmin service.
> This issue is a marker that we should do the refactoring and should be split into smaller tasks when we start the refactoring.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Assigned] (KARAF-717) Refactor Actions to get services
injected instead of fetching them
Posted by "Christian Schneider (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/KARAF-717?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christian Schneider reassigned KARAF-717:
-----------------------------------------
Assignee: (was: Christian Schneider)
> Refactor Actions to get services injected instead of fetching them
> ------------------------------------------------------------------
>
> Key: KARAF-717
> URL: https://issues.apache.org/jira/browse/KARAF-717
> Project: Karaf
> Issue Type: Improvement
> Components: karaf-shell
> Reporter: Christian Schneider
> Priority: Minor
> Fix For: 3.1.0
>
>
> Currently we use abstract classes like ObrCommandSupport and OsgiCommandSupport to handle the bundleContext and service references.
> This makes the code harder to test and mixes technical and business code. By business I mean the stuff the class is meant to do by technical I mean other domains like osgi services.
> So I propose to inject the services we need using blueprint so in the Action there is only the interface to the service and a setter. Additionally I would even skip the AbstractAction as it is not good to store the CommandSession in a class attribute as this makes the code non reentrant.
> I have done a sample refactoring in http://svn.apache.org/viewvc?view=revision&revision=1144288
> It shows how to change the ListCommand of the Obr to not need any abstract support classes to handle the RepositoryAdmin service.
> This issue is a marker that we should do the refactoring and should be split into smaller tasks when we start the refactoring.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira