Hi Guillaume,
Thanks for updating the APIs. I have the following comments -
1. SubsystemAdmin.getSubsystem(id). I wonder if we can provide a
method like getSubsystem(String scope). subsystemScope can be
calculated using subsystem symbolic name + "_" + subsystem version,
and it can be used to uniquely identify subsystems instead of (or in
addition to) a long id. Whenever a subsystem gets updated, we would
need some way to tell which of the existing sybsystem it is updating.
To me, it is easy to tell that by the subsystem scope which can be
simply calculated from the bundle location. Another thought is to use
bundle location, but I don't think that is guaranteed to be unique,
when people are using install(location, inputstream) to install the
subsystem.
2. SubsystemAdmin.getSubsystems() returns Collection<Subsystem>. I
wonder if it is more convenient to return Map<String, Subsystem> where
the key is the subsystem scope.
3. Subsystem.install(location). I think this should be install(String
location, BundleContext bc) instead, because we would need the bundle
context to get the CompositeAdmin service from OSGi service registry,
assuming we would use that to install the composite. We would also
need bundle context to install constituents.
4. similar as No. 3, Subsyste.install(location, content) should be
install(String location, InputStream content, BundleContext bc)
5. Subsystem.getState(). I wonder if we should reinvent here. Is
there any reason why we don't make the return type of int, and just
delegate this to the compositeBundle.getState(). Also, what if the
composite bundle and constituents have different states. Does this
method only work when the states are consistent among these bundles?
6. Subsystem.start() or stop(), I think it should throw either
BundleException or SubsystemException.
7. The return type of Subsystem.getHeaders, do we want to use
Dictionary<String, String> or Dictionary to be consistent with
Bundle.getHeaders()? when we implement this method, we can just
delegate to compositeBundle.getHeaders.
8. I'd like the add the following methods to Subsystem.java:
+ void uninstall() throws SubsystemException;
+
+ String getScope();
+
+ void update() throws SubsystemException;
+
+ void update(InputStream content) throws SubsystemException;
I think the uninstall(), update() are needed, because the
SubsystemAdmin.uninstall/update can just delegate the work to the
actual subsystem. In fact, the SubsustemAdmin cannot really do the
work, unless we make the composite bundle available out of the
Subsystem.
I am also attaching a patch for stuff described above. I'd love to
help on implementing these APIs if that is ok with you.
Thanks
Lin
On Mon, Mar 29, 2010 at 6:34 PM, Guillaume Nodet <[email protected]> wrote:
> FWIW, I've slightly updated the API with some feedback here and mostly two
> other changes:
> * moved some management methods from Subsystem to SubsystemAdmin (update
> and uninstall)
> * make SubsystemConstants a non instantiable class instead of an interface
>
> On Fri, Mar 26, 2010 at 16:27, Guillaume Nodet <[email protected]> wrote:
>
>> Sorry, forgot to give a pointer:
>>
>> http://svn.apache.org/repos/asf/incubator/aries/trunk/subsystem/subsystem-api/src/main/java/org/apache/aries/subsystem/
>>
>>
>> On Fri, Mar 26, 2010 at 15:27, Guillaume Nodet <[email protected]> wrote:
>>
>>> I've just checked in a user-oriented API for subsystems (which I hope can
>>> be a superset of applications).
>>> This is not the SPI part, but simply what the user needs to manipulate
>>> subsystems at runtime.
>>> The design has been chosen to be much more familiar to the OSGi API for
>>> bundles.
>>>
>>> I'm planning to continue on this area when time permits, but I welcome
>>> everyone to have a look and give feedback.
>>>
>>> On Thu, Mar 18, 2010 at 18:16, Jarek Gawor <[email protected]> wrote:
>>>
>>>> Hi all,
>>>>
>>>> I have a few questions / comments on the Aries Application API.
>>>> Specifically, about AriesApplicationManager and
>>>> AriesApplicationContextManager API.
>>>>
>>>> The application-management module provides a default implementation
>>>> for the AriesApplicationManager API while the application-runtime
>>>> module provides an example implementation of the
>>>> AriesApplicationContextManager API. It is expected (as indicated by
>>>> JavaDoc) that different server runtimes will provide their own
>>>> implementations of the AriesApplicationContextManager API.
>>>>
>>>> So it seems to me like the AriesApplicationContextManager is more of a
>>>> SPI - something that user shouldn't/wouldn't normally use. If so, the
>>>> user would only interact with the AriesApplicationManager API to
>>>> perform application operations. If that's true then the
>>>> AriesApplicationManager is missing methods for getting all
>>>> AriesApplicationContexts or finding one by name. And at the same time
>>>> the AriesApplicationContextManager would probably need to be
>>>> completely reworked. For example to be named
>>>> AriesApplicationManagerProvider and have corresponding
>>>> install/uninstall/getContexts/findContext operations.
>>>> Or maybe we should just get rid off the AriesApplicationContextManager
>>>> completely and let folks implement the AriesApplicationManager API
>>>> directly?
>>>>
>>>> Btw, by 'user' I meant some code that uses these API to perform some
>>>> application operations. For example, in my case, I would like to have
>>>> some osgi shell commands that use these API to
>>>> install/uninstall/start/stop applications.
>>>>
>>>> Jarek
>>>>
>>>
>>>
>>>
>>> --
>>> Cheers,
>>> Guillaume Nodet
>>> ------------------------
>>> Blog: http://gnodet.blogspot.com/
>>> ------------------------
>>> Open Source SOA
>>> http://fusesource.com
>>>
>>>
>>>
>>
>>
>> --
>> Cheers,
>> Guillaume Nodet
>> ------------------------
>> Blog: http://gnodet.blogspot.com/
>> ------------------------
>> Open Source SOA
>> http://fusesource.com
>>
>>
>>
>
>
> --
> Cheers,
> Guillaume Nodet
> ------------------------
> Blog: http://gnodet.blogspot.com/
> ------------------------
> Open Source SOA
> http://fusesource.com
>
Index:
subsystem-api/src/main/java/org/apache/aries/subsystem/SubsystemAdmin.java
===================================================================
--- subsystem-api/src/main/java/org/apache/aries/subsystem/SubsystemAdmin.java
(revision 929230)
+++ subsystem-api/src/main/java/org/apache/aries/subsystem/SubsystemAdmin.java
(working copy)
@@ -15,7 +15,10 @@
import java.io.InputStream;
import java.util.Collection;
+import java.util.Map;
+import org.osgi.framework.BundleContext;
+
/**
* Subsystems administration interface.
*
@@ -36,11 +39,13 @@
*/
Subsystem getSubsystem(long id);
+ Subsystem getSubsystem(String subsystemScope);
+
/**
* Retrieve all subsystems managed by this service
* @return
*/
- Collection<Subsystem> getSubsystems();
+ Map<String, Subsystem> getSubsystems();
/**
* Install a new subsystem.
@@ -48,7 +53,7 @@
* @param location
* @return
*/
- Subsystem install(String location) throws SubsystemException;
+ Subsystem install(String location, BundleContext bc) throws
SubsystemException;
/**
* Install a new subsystem.
@@ -57,7 +62,7 @@
* @param content
* @return
*/
- Subsystem install(String location, InputStream content) throws
SubsystemException;
+ Subsystem install(String location, InputStream content, BundleContext bc)
throws SubsystemException;
/**
* Update the given subsystem.
Index: subsystem-api/src/main/java/org/apache/aries/subsystem/Subsystem.java
===================================================================
--- subsystem-api/src/main/java/org/apache/aries/subsystem/Subsystem.java
(revision 929230)
+++ subsystem-api/src/main/java/org/apache/aries/subsystem/Subsystem.java
(working copy)
@@ -13,10 +13,13 @@
*/
package org.apache.aries.subsystem;
+import java.io.InputStream;
import java.util.Collection;
+import java.util.Dictionary;
import java.util.Map;
import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleException;
import org.osgi.framework.Version;
/**
@@ -40,17 +43,17 @@
*
* @return
*/
- State getState();
+ int getState();
/**
* Start the subsystem (i.e. start all its constituent bundles according
to their start level).
*/
- void start();
+ void start() throws BundleException;
/**
* Stop the subsystem (i.e. stop all its constituent bundles).
*/
- void stop();
+ void stop() throws BundleException;
/**
* The identifier of the subsystem. Must be unique in the framework.
@@ -87,14 +90,14 @@
*
* @return
*/
- Map<String, String> getHeaders();
+ Dictionary<String, String> getHeaders();
/**
* Return the subsystem headers
*
* @return
*/
- Map<String, String> getHeaders(String locale);
+ Dictionary<String, String> getHeaders(String locale);
/**
* Retrieve the constituent bundles of this subsystem.
@@ -103,4 +106,12 @@
*/
Collection<Bundle> getConstituents();
+ void uninstall() throws SubsystemException;
+
+ String getScope();
+
+ void update() throws SubsystemException;
+
+ void update(InputStream content) throws SubsystemException;
+
}