Community
Participate
Working Groups
See https://www.osgi.org/bugzilla/show_bug.cgi?id=90 Equinox DS is the OSGi reference implementation for DS. Bugs in the OSGi 4.2 Compliance Tests for DS demonstrate that Equinox DS impl does not return a read only Dictionary from ComponentContext.getProperties. According to 112.12.3.5, the ComponentContext.getProperties() method must return a read-only Dictionary.
Created attachment 176890 [details] proposed patch
Shouldn't it return the same Dictionary on successive calls rather than a new one each time? That is, getProperties() == getProperties().
(In reply to comment #2) > Shouldn't it return the same Dictionary on successive calls rather than a new > one each time? You are right. I will provide update of the patch
Created attachment 176894 [details] proposed patch This patch avoids creating a new read only instance of the properties unless necessary.
More comments on the patch. ReadOnlyDictionary is also a Map because it incorrectly extends Hashtable (which is also a Map in Java 5). It must only extend Dictionary. This removes the method which return collections (which are not in Dictionary). ReadOnlyDictionary (once made to extend Dictionary) must then delegate the non-mutation methods to the composed Dictionary. It is improper to use inheritance of Hashtable. See item 16, Effective Java 2nd Ed. Also, the mutation methods (put, remove, ...) must throw UnsupportedOperationException rather than silently doing nothing.
Created attachment 176982 [details] optimized patch
(In reply to comment #5) > More comments on the patch. > > ReadOnlyDictionary is also a Map because it incorrectly extends Hashtable > (which is also a Map in Java 5). It must only extend Dictionary. This removes > the method which return collections (which are not in Dictionary). ReadOnlyDictionary should implement Map because ComponentContext.getProperties() is used as Map parameter when calling the activate/deactivate/modify method of the component. In the last patch ReadOnlyDictionary implements the Map interface. > Also, the mutation methods (put, remove, ...) must throw > UnsupportedOperationException rather than silently doing nothing. Agree. It is already done.
Latest patch looks good.
Fixed in HEAD
