Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 322649

Summary:

[DS] Dictionary returned from ComponentContext.getProperties is not readonly

Product:

[Eclipse Project] Equinox

Reporter:

BJ Hargrave <hargrave>

Component:

Compendium

Assignee:

Stoyan Boshev <s.boshev>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

contact

Version:

3.6

Target Milestone:

3.7 M2

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
proposed patch	none
proposed patch	none
optimized patch	none

Description BJ Hargrave

2010-08-13 09:04:39 EDT

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.

Comment 1 Stoyan Boshev

2010-08-18 09:39:39 EDT

Created attachment 176890 [details]
proposed patch

Comment 2 BJ Hargrave

2010-08-18 09:59:29 EDT

Shouldn't it return the same Dictionary on successive calls rather than a new one each time?

That is, getProperties() == getProperties().

Comment 3 Stoyan Boshev

2010-08-18 10:23:21 EDT

(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

Comment 4 Stoyan Boshev

2010-08-18 10:25:27 EDT

Created attachment 176894 [details]
proposed patch

This patch avoids creating a new read only instance of the properties unless necessary.

Comment 5 BJ Hargrave

2010-08-18 11:06:58 EDT

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.

Comment 6 Stoyan Boshev

2010-08-19 07:44:00 EDT

Created attachment 176982 [details]
optimized patch

Comment 7 Stoyan Boshev

2010-08-19 07:50:52 EDT

(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.

Comment 8 BJ Hargrave

2010-08-19 08:10:56 EDT

Latest patch looks good.

Comment 9 Stoyan Boshev

2010-08-30 09:25:51 EDT

Fixed in HEAD