The files “*component.xml” MUST be contained in the folder named “OSGI-INF”
Ahh ok, from http://wiki.osgi.org/wiki/Declarative_Services it appeared that the path can be arbitrary. I also found no further specification about “Declarative Services” in http://www.osgi.org/download/r5/osgi.core-5.0.0.pdf
Where is this specified? Sad that OSGi doesn’t allow more flexibility here, but no deal breaker, as long the the filename can be freely choosen.
That’s right… Maybe in the new version. The Compendium Specification 6.0.0 will be published soon. The Declarative Services specification is an extension. All of them are described in a seperate document. Have a look here: http://www.osgi.org/Download/File?url=/download/r5/osgi.cmpn-5.0.0.pdf
I just made a quick look to the specification… It seems that OSGI-INF is the default location, but not a must. I am sorry, that’s also new to me. I will check that and report to you. Maybe we can revert the change. That was my fault. Sorry for that.
No problem. Would be great if you could report if it works as intended with b015ab ie. one commit before the switch to OSGI-INF.
Another question, you write “<scr:component…” whereas the Wiki http://wiki.osgi.org/wiki/Declarative_Services states simply “<component”. Is there any difference between those two?
I missed the namespace declaration. To be 100% in line with DS we should use the following:
<?xml version="1.0" encoding="UTF-8"?><scr:component xmlns:scr=“http://www.osgi.org/xmlns/scr/v1.2.0” enabled=“true” immediate=“true” name=“org.jivesoftware.smack.initializer.extensions.ExtensionsInitializer” activate=“initialize”>
</scr:component>
What do you think about using a more human readable String for the name attribute? E.g. “Smack Extensions” or “Smack Extensions Component”
No objections, that change should be ok.
Allright, will incooperate the change as soon as you confirm that we can keep the component defintions in org.jivesoftware.smackx/ instead of OSIG-INF/.
Bad news… The changes are not working. I have corrected most of the problems, but things start to become ugly. The problem is that at multiple locations in the core Class.forName is used which forwards the call to the classloader of the core bundle that does not see the additional packages. Have a close look to my diff-file to check which changes were necessary to get things forward.
A small summary:
(1) Replace the component-xml by those found in the diff. The namespace is important for SCR.
(2) There cannot be two methods which the same name, but which an input output structure not corresponding to the SCR spec. Therefore, I had to introduce the class MultiException to get rid of the return type List and remove the initialize(ClassLoader) method.
(3) The component-xml must not be located in the OSGI-INF folder. You can revert the last change.
A blocker is now, that the correct classloader must be forwarded to SmackConfiguration. From my point of view, we have only two options to solve this issue:
(1) Create a separate class that performs the config file parsing which can be instantiated with a classloader used for class loading.
(2) Use the fragment approach.
We have to replace all “Class.ForName” calls!
How to proceed?
.diff.zip (3450 Bytes)
(1) Replace the component-xml by those found in the diff. The namespace is important for SCR.
done
(2) There cannot be two methods which the same name, but which an input output structure not corresponding to the SCR spec. Therefore, I had to introduce the class MultiException to get rid of the return type List and remove the initialize(ClassLoader) method.
I don’t understand “There cannot be two methods with the same name”. But from OSGi 5.0 CMPN 112.5.8 I see that the ‘List’ return type is not valid for the activate method. My fix was simply creating a new ‘activate’ method with ‘void’ as return type. And since this method is also named ‘activate’, we can omit the attribute in the component XML configuration.
A blocker is now, that the correct classloader must be forwarded to SmackConfiguration.
My solution is to use ‘this.getClass().getClassLoader()’ in the activate method. Since ‘this’ is a class out of the components bundle, e.g. o.j.s.initializer.experimental.ExperimentalInitialzier, I would expect this class loader to be able to see and load the required files and classes. Or am I wrong? If so, what is the correct classloader? And how to obtain it?
Have a look at my current OSGi work at https://github.com/Flowdalic/Smack/tree/osgi
A blocker is now, that the correct classloader must be forwarded to SmackConfiguration. From my point of view, we have only two options to solve this issue:
(1) Create a separate class that performs the config file parsing which can be instantiated with a classloader used for class loading.
After reading this section again: Are you talking about the config file parsing that is done by smack-core? Does smack-cores initialization by processing of org.jivesoftware.smack/smack-config.xml fail in OSGi? If so, this should be easily fixable by also adding a Declarative Service for smack-core.
I checked your changes… The introduction of an activate method should work. The classloaders are the correct ones. I will test your changes tomorrow in order to validate that everything works.
Well, let’s code speak 
UrlInitializer (line: 83)
SmackConfiguration.processConfigFile(is, exceptions);
The classloader is able to get an InputStream and the method call of “processConfigFile” ends finally in the method “loadSmackClass”, where the class is loaded “initClass = Class.forName(className);”. Here, the call is processed from the core bundle classloader which fails. We need at this point the classloader from e.g. the extension bundle.
Here, the call is processed from the core bundle classloader which fails.
The core bundle should be able to load it’s classes, which are the defined as startupClasses in smack-config.xml. All other classes are defined as optionalStartupClasses, of which most are not from the core bundle and therefore should fail when the core bundle tries to load them. But in the OSGi world that job is done by the Declarative Service we just created.
So I see no problem here. Or am I missing something?
Sorry, I didn’t get it… DS calls the activate method of e.g, “ExtensionsInitializer” and this call leads to “getConfigUrl” where ExtensionsInitializer returns “classpath:org.jivesoftware.smackx/extensions.xml”. The inputstream is obtained and then leads to the call “SmackConfiguration.processConfigFile(is, exceptions);”. Here, the classloader is lost… “extensions.xml” contains e.g. “org.jivesoftware.smackx.disco.ServiceDiscoveryManager”. This class can and will never be loaded because where are in the core scope.
Ahh now I got you. https://github.com/Flowdalic/Smack/commit/42eb408e124a45bde7a0fcf75821b4f8a2d652 2f should take care of that.
I think, we have got it, but please fix the component files (see my diff):
(1) src -> scr (xml namespace prefix)
(2) activate=“activate” (declare the activation method)
We should also provide a “deactivate”-method that removes the Provider from the ProviderManager.
02.diff.zip (599 Bytes)
(2) activate=“activate” (declare the activation method)
Is this required? From the spec it appeared that ‘activate’ is the default value for the activtate attribute? I would like to pefer “convention over configuration” and therefore avoid stating this if we use the default value.
We should also provide a “deactivate”-method that removes the Provider from the ProviderManager.
I thought about this too. What happens atm if you unload/deactivate a Smack API extension bundle (experimental, extensions, legacy)? Does the deactivation fail/hang, e.g. because the ProviderManager still holds references to classes from the bundle?
Yes, you are right, you can omit the “active” declaration.
I thought about this too. What happens atm if you unload/deactivate a Smack API extension bundle (experimental, extensions, legacy)? Does the deactivation fail/hang, e.g. because the ProviderManager still holds references to classes from the bundle?
That’s a Java problem, you cannot destroy a reference that is hold by another object, that’s why OSGi recommends interaction via services, but (like in our case) the creation of a technology independent solution often requires an interaction on object level. If we uninstall an extension, everything will still work, because the provider instances registered in the ProviderManager still exist. If we install the bundle again, the active methods registers new instances. In the deactivate method we must do all the cleanup necessary to create a state as if the bundle was never installed. The main task is to remove the registered instances hold by the ProviderManager. All other instances created must be notified to shutdown.
If we install the bundle again, the active methods registers new instances. In the deactivate method we must do all the cleanup necessary to create a state as if the bundle was never installed.
I wonder if this is really required. If a new version from e.g smack-extension is installed, then the old provider instance is overwritten with the new one.
All other instances created must be notified to shutdown.
Provider instances? What should they do if they are notified about their bundle being shut down?
II’d like to defer any further related code changes after the 4.0 release. Do you consider the missing deactivation method a blocker? Does Smack work reasonably well in an OSGi environment as of 13b522?
Do you consider the missing deactivation method a blocker?
No. The shutdown of the extension, experimental and legagcy bundle should not be part of a proper use case.
Does Smack work reasonably well in an OSGi environment as of 13b522?
I will check that tomorrow (not so much time) or on Thursday and will give you feedback here.