Bug 510641 - service registry: Rewrite ServiceProperties

The class is now based on CaseInsensitiveDictionaryMap (instead of
Headers). Once created, the service properties are internally
represented as a Map.

Change-Id: I3041237d73e36a533c6d73985d75fb680ad129d7
Signed-off-by: BJ Hargrave <hargrave@us.ibm.com>

4 files changed, 98 insertions, 119 deletions

diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/FilterImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/FilterImpl.java
index 34ce4e049..0cc706ddb 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/FilterImpl.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/FilterImpl.java
@@ -161,9 +161,9 @@ public class FilterImpl implements Filter /* since Framework 1.1 */ {
 	 */
 	public boolean match(ServiceReference<?> reference) {
 		if (reference instanceof ServiceReferenceImpl) {
-			return matchCase(((ServiceReferenceImpl<?>) reference).getRegistration().getProperties());
+			return matches(((ServiceReferenceImpl<?>) reference).getRegistration().getProperties());
 		}
-		return matchCase(new ServiceReferenceDictionary(reference));
+		return matches(new ServiceReferenceMap(reference));
 	}
 
 	/**
@@ -1754,19 +1754,20 @@ public class FilterImpl implements Filter /* since Framework 1.1 */ {
 	}
 
 	/**
-	 * This Dictionary is used for key lookup from a ServiceReference during
-	 * filter evaluation. This Dictionary implementation only supports the get
+	 * This Map is used for key lookup from a ServiceReference during
+	 * filter evaluation. This Map implementation only supports the get
 	 * operation using a String key as no other operations are used by the
 	 * Filter implementation.
 	 * 
 	 */
-	private static class ServiceReferenceDictionary extends Dictionary<String, Object> {
+	static private final class ServiceReferenceMap extends AbstractMap<String, Object> implements Map<String, Object> {
 		private final ServiceReference<?> reference;
 
-		ServiceReferenceDictionary(ServiceReference<?> reference) {
+		ServiceReferenceMap(ServiceReference<?> reference) {
 			this.reference = reference;
 		}
 
+		@Override
 		public Object get(Object key) {
 			if (reference == null) {
 				return null;
@@ -1774,27 +1775,8 @@ public class FilterImpl implements Filter /* since Framework 1.1 */ {
 			return reference.getProperty((String) key);
 		}
 
-		public boolean isEmpty() {
-			throw new UnsupportedOperationException();
-		}
-
-		public Enumeration<String> keys() {
-			throw new UnsupportedOperationException();
-		}
-
-		public Enumeration<Object> elements() {
-			throw new UnsupportedOperationException();
-		}
-
-		public Object put(String key, Object value) {
-			throw new UnsupportedOperationException();
-		}
-
-		public Object remove(Object key) {
-			throw new UnsupportedOperationException();
-		}
-
-		public int size() {
+		@Override
+		public Set<Entry<String, Object>> entrySet() {
 			throw new UnsupportedOperationException();
 		}
 	}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ModifiedServiceEvent.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ModifiedServiceEvent.java
index 195b979fa..ca05b1e97 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ModifiedServiceEvent.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ModifiedServiceEvent.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2012 IBM Corporation and others.
+ * Copyright (c) 2009, 2017 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -11,6 +11,7 @@
 
 package org.eclipse.osgi.internal.serviceregistry;
 
+import java.util.Map;
 import org.osgi.framework.*;
 
 /**
@@ -21,7 +22,7 @@ class ModifiedServiceEvent extends ServiceEvent {
 	private static final long serialVersionUID = -5373850978543026102L;
 	private final ServiceEvent modified;
 	private final ServiceEvent modifiedEndMatch;
-	private final ServiceProperties previousProperties;
+	private final Map<String, Object> previousProperties;
 
 	/**
 	 * Create a ServiceEvent containing the service properties prior to modification.
@@ -29,7 +30,7 @@ class ModifiedServiceEvent extends ServiceEvent {
 	 * @param reference Reference to service with modified properties.
 	 * @param previousProperties Service properties prior to modification.
 	 */
-	ModifiedServiceEvent(ServiceReference<?> reference, ServiceProperties previousProperties) {
+	ModifiedServiceEvent(ServiceReference<?> reference, Map<String, Object> previousProperties) {
 		super(ServiceEvent.MODIFIED, reference);
 		this.modified = new ServiceEvent(ServiceEvent.MODIFIED, reference);
 		this.modifiedEndMatch = new ServiceEvent(ServiceEvent.MODIFIED_ENDMATCH, reference);
@@ -63,9 +64,9 @@ class ModifiedServiceEvent extends ServiceEvent {
 	 * @return True is the filter matches the previous service properties.
 	 */
 	boolean matchPreviousProperties(Filter filter) {
-		/* We use matchCase here since ServiceProperties already 
+		/* We use matches here since ServiceProperties already 
 		 * does case insensitive lookup.
 		 */
-		return filter.matchCase(previousProperties);
+		return filter.matches(previousProperties);
 	}
 }
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java
index bc80b1bfd..97d235090 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceProperties.java
@@ -13,102 +13,91 @@ package org.eclipse.osgi.internal.serviceregistry;
 
 import java.lang.reflect.Array;
 import java.util.*;
-import org.eclipse.osgi.framework.util.Headers;
+import org.eclipse.osgi.framework.util.CaseInsensitiveDictionaryMap;
+import org.eclipse.osgi.internal.messages.Msg;
+import org.eclipse.osgi.util.NLS;
 import org.osgi.framework.Constants;
 
 /**
- * Hashtable for service properties.
+ * Service properties.
  * 
  * Supports case-insensitive key lookup.
  */
-class ServiceProperties extends Headers<String, Object> {
+class ServiceProperties extends CaseInsensitiveDictionaryMap<String, Object> {
 	/**
-	 * Create a properties object for the service.
+	 * Create a properties object from a Dictionary.
 	 *
 	 * @param props The properties for this service.
+	 * @throws IllegalArgumentException If a case-variants of a key are
+	 * in the props parameter.
 	 */
-	private ServiceProperties(int size, Dictionary<String, ?> props) {
-		super(size);
+	ServiceProperties(Dictionary<String, ?> props) {
+		this(props, 0);
+	}
 
+	/**
+	 * Create a properties object from a Dictionary.
+	 *
+	 * @param props The properties for this service.
+	 * @param extra Extra capacity in the map.
+	 * @throws IllegalArgumentException If a case-variants of a key are
+	 * in the props parameter.
+	 */
+	ServiceProperties(Dictionary<String, ?> props, int extra) {
+		super(initialCapacity((props == null) ? extra : props.size() + extra));
 		if (props == null) {
 			return;
 		}
 		synchronized (props) {
 			Enumeration<?> keysEnum = props.keys();
-
 			while (keysEnum.hasMoreElements()) {
 				Object key = keysEnum.nextElement();
-
 				if (key instanceof String) {
 					String header = (String) key;
-
-					setProperty(header, props.get(header));
+					if (putIfAbsent(header, cloneValue(props.get(header))) != null) {
+						throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, key));
+					}
 				}
 			}
 		}
 	}
 
 	/**
-	 * Create a properties object for the service.
+	 * Create a properties object from a Map.
 	 *
 	 * @param props The properties for this service.
+	 * @throws IllegalArgumentException If a case-variants of a key are
+	 * in the props parameter.
 	 */
-	ServiceProperties(Dictionary<String, ?> props) {
-		this((props == null) ? 2 : props.size() + 2, props);
-	}
-
-	/**
-	 * Get a clone of the value of a service's property.
-	 *
-	 * @param key header name.
-	 * @return Clone of the value of the property or <code>null</code> if there is
-	 * no property by that name.
-	 */
-	Object getProperty(String key) {
-		return cloneValue(get(key));
-	}
-
-	/**
-	 * Get the list of key names for the service's properties.
-	 *
-	 * @return The list of property key names.
-	 */
-	synchronized String[] getPropertyKeys() {
-		int size = size();
-
-		String[] keynames = new String[size];
-
-		Enumeration<String> keysEnum = keys();
-
-		for (int i = 0; i < size; i++) {
-			keynames[i] = keysEnum.nextElement();
+	ServiceProperties(Map<String, ?> props) {
+		super(initialCapacity((props == null) ? 0 : props.size()));
+		if (props == null) {
+			return;
+		}
+		synchronized (props) {
+			for (Entry<?, ?> e : props.entrySet()) {
+				Object key = e.getKey();
+				if (key instanceof String) {
+					String header = (String) key;
+					if (putIfAbsent(header, cloneValue(e.getValue())) != null) {
+						throw new IllegalArgumentException(NLS.bind(Msg.HEADER_DUPLICATE_KEY_EXCEPTION, key));
+					}
+				}
+			}
 		}
-
-		return keynames;
-	}
-
-	/**
-	 * Put a clone of the property value into this property object.
-	 *
-	 * @param key Name of property.
-	 * @param value Value of property.
-	 * @return previous property value.
-	 */
-	synchronized Object setProperty(String key, Object value) {
-		return set(key, cloneValue(value));
 	}
 
 	/**
 	 * Attempt to clone the value if necessary and possible.
 	 *
-	 * For some strange reason, you can test to see of an Object is
+	 * For some strange reason, you can test to see if an Object is
 	 * Cloneable but you can't call the clone method since it is
 	 * protected on Object!
 	 *
 	 * @param value object to be cloned.
 	 * @return cloned object or original object if we didn't clone it.
 	 */
-	private static Object cloneValue(Object value) {
+	static Object cloneValue(Object value) {
 		if (value == null)
 			return null;
 		if (value instanceof String) /* shortcut String */
@@ -129,33 +118,28 @@ class ServiceProperties extends Headers<String, Object> {
 			System.arraycopy(value, 0, clonedArray, 0, len);
 			return clonedArray;
 		}
-		// must use reflection because Object clone method is protected!!
-		try {
-			return clazz.getMethod("clone", (Class<?>[]) null).invoke(value, (Object[]) null); //$NON-NLS-1$
-		} catch (Exception e) {
-			/* clone is not a public method on value's class */
-		} catch (Error e) {
-			/* JCL does not support reflection; try some well known types */
-			if (value instanceof Vector<?>)
-				return ((Vector<?>) value).clone();
-			if (value instanceof Hashtable<?, ?>)
-				return ((Hashtable<?, ?>) value).clone();
+
+		if (value instanceof Cloneable) {
+			// must use reflection because Object clone method is protected!!
+			try {
+				return clazz.getMethod("clone", (Class<?>[]) null).invoke(value, (Object[]) null); //$NON-NLS-1$
+			} catch (Exception e) {
+				/* clone is not a public method on value's class */
+			}
 		}
 		return value;
 	}
 
-	public synchronized String toString() {
-		String keys[] = getPropertyKeys();
-
-		int size = keys.length;
+	@Override
+	public String toString() {
+		Set<String> keys = keySet();
 
-		StringBuilder sb = new StringBuilder(20 * size);
+		StringBuilder sb = new StringBuilder(20 * keys.size());
 
 		sb.append('{');
 
 		int n = 0;
-		for (int i = 0; i < size; i++) {
-			String key = keys[i];
+		for (String key : keys) {
 			if (!key.equals(Constants.OBJECTCLASS)) {
 				if (n > 0)
 					sb.append(", "); //$NON-NLS-1$
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
index f419cb33d..6ad73da28 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
@@ -62,7 +62,7 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 
 	/** properties for this registration. */
 	/* @GuardedBy("registrationLock") */
-	private ServiceProperties properties;
+	private Map<String, Object> properties;
 
 	/** service id. */
 	private final long serviceid;
@@ -151,7 +151,7 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 	 */
 	public void setProperties(Dictionary<String, ?> props) {
 		final ServiceReferenceImpl<S> ref;
-		final ServiceProperties previousProperties;
+		final Map<String, Object> previousProperties;
 		synchronized (registry) {
 			synchronized (registrationLock) {
 				if (state != REGISTERED) { /* in the process of unregisterING */
@@ -290,6 +290,19 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 	}
 
 	/**
+	 * Count of service properties set by framework for each
+	 * service registration.
+	 * <ul>
+	 * <li>Constants.OBJECTCLASS</li>
+	 * <li>Constants.SERVICE_ID</li>
+	 * <li>Constants.SERVICE_BUNDLEID</li>
+	 * <li>Constants.SERVICE_SCOPE</li>
+	 * </ul>
+	 * @see #createProperties(Dictionary)
+	 */
+	private static final int FRAMEWORK_SET_SERVICE_PROPERTIES_COUNT = 4;
+
+	/**
 	 * Construct a properties object from the dictionary for this
 	 * ServiceRegistration.
 	 *
@@ -297,13 +310,13 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 	 * @return A Properties object for this ServiceRegistration.
 	 */
 	/* @GuardedBy("registrationLock") */
-	private ServiceProperties createProperties(Dictionary<String, ?> p) {
+	private Map<String, Object> createProperties(Dictionary<String, ?> p) {
 		assert Thread.holdsLock(registrationLock);
-		ServiceProperties props = new ServiceProperties(p);
+		ServiceProperties props = new ServiceProperties(p, FRAMEWORK_SET_SERVICE_PROPERTIES_COUNT);
 
-		props.set(Constants.OBJECTCLASS, clazzes, true);
-		props.set(Constants.SERVICE_ID, Long.valueOf(serviceid), true);
-		props.set(Constants.SERVICE_BUNDLEID, Long.valueOf(bundle.getBundleId()), true);
+		props.put(Constants.OBJECTCLASS, clazzes);
+		props.put(Constants.SERVICE_ID, Long.valueOf(serviceid));
+		props.put(Constants.SERVICE_BUNDLEID, Long.valueOf(bundle.getBundleId()));
 		final String scope;
 		if (service instanceof ServiceFactory) {
 			if (service instanceof PrototypeServiceFactory) {
@@ -314,10 +327,9 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 		} else {
 			scope = Constants.SCOPE_SINGLETON;
 		}
-		props.set(Constants.SERVICE_SCOPE, scope, true);
-		props.setReadOnly();
+		props.put(Constants.SERVICE_SCOPE, scope);
 
-		Object ranking = props.getProperty(Constants.SERVICE_RANKING);
+		Object ranking = props.get(Constants.SERVICE_RANKING);
 		if (ranking instanceof Integer) {
 			serviceranking = ((Integer) ranking).intValue();
 		} else {
@@ -327,14 +339,14 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 			}
 		}
 
-		return props;
+		return props.asUnmodifiableMap();
 	}
 
 	/**
 	 * Return the properties object. This is for framework internal use only.
 	 * @return The service registration's properties.
 	 */
-	public ServiceProperties getProperties() {
+	public Map<String, Object> getProperties() {
 		synchronized (registrationLock) {
 			return properties;
 		}
@@ -354,7 +366,7 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 	 */
 	Object getProperty(String key) {
 		synchronized (registrationLock) {
-			return properties.getProperty(key);
+			return ServiceProperties.cloneValue(properties.get(key));
 		}
 	}
 
@@ -370,7 +382,7 @@ public class ServiceRegistrationImpl<S> implements ServiceRegistration<S>, Compa
 	 */
 	String[] getPropertyKeys() {
 		synchronized (registrationLock) {
-			return properties.getPropertyKeys();
+			return properties.keySet().toArray(new String[0]);
 		}
 	}