Bug 349189: Added support for returning icons that are the closest possible match to the requested size.v20111121-2050

3 files changed, 71 insertions, 72 deletions

diff --git a/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/DataParser.java b/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/DataParser.java
index e8d20a8e1..295085bbc 100644
--- a/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/DataParser.java
+++ b/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/DataParser.java
@@ -304,6 +304,7 @@ public class DataParser {
 		String _refID;
 		ObjectClassDefinitionImpl _ocd;
 		Vector<AttributeDefinitionImpl> _ad_vector = new Vector<AttributeDefinitionImpl>(7);
+		List<Icon> icons = new ArrayList<Icon>(5);
 
 		public OcdHandler(ContentHandler handler) {
 			super(handler);
@@ -350,23 +351,17 @@ public class DataParser {
 			} else if (name.equalsIgnoreCase(ICON)) {
 				IconHandler iconHandler = new IconHandler(this);
 				iconHandler.init(name, atts);
-				if (iconHandler._isParsedDataValid) {
-					// Because XML schema allows at most one icon for
-					// one OCD, if more than one icons are read from 
-					// MetaData, then only the final icon will be kept.
-					_ocd.setIcon(iconHandler._icon);
-				}
+				if (iconHandler._isParsedDataValid)
+					icons.add(iconHandler._icon);
 			} else {
 				logger.log(LogService.LOG_WARNING, NLS.bind(MetaTypeMsg.UNEXPECTED_ELEMENT, name));
 			}
 		}
 
 		protected void finished() {
-
 			logger.log(LogService.LOG_DEBUG, "Here is OcdHandler():finished()"); //$NON-NLS-1$
 			if (!_isParsedDataValid)
 				return;
-
 			if (_ad_vector.size() == 0) {
 				// Schema defines at least one AD is required.
 				_isParsedDataValid = false;
@@ -379,7 +374,7 @@ public class DataParser {
 				AttributeDefinitionImpl ad = adKey.nextElement();
 				_ocd.addAttributeDefinition(ad, ad._isRequired);
 			}
-
+			_ocd.setIcons(icons);
 			_parent_OCDs_hashtable.put(_refID, _ocd);
 		}
 	}
diff --git a/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/Icon.java b/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/Icon.java
index 591914e94..868275ec2 100644
--- a/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/Icon.java
+++ b/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/Icon.java
@@ -17,33 +17,21 @@ import org.osgi.framework.Bundle;
  */
 class Icon implements Cloneable {
 
-	private String _fileName;
-	private int _size;
-	private Bundle _bundle;
+	private final String _fileName;
+	private final Integer _size;
+	private final Bundle _bundle;
 
 	/**
 	 * Constructor of class Icon.
 	 */
-	public Icon(String fileName, int size, Bundle bundle) {
+	public Icon(String fileName, Integer size, Bundle bundle) {
 
 		this._fileName = fileName;
 		this._size = size;
 		this._bundle = bundle;
 	}
 
-	/**
-	 * Constructor of class Icon.
-	 */
-	public Icon(String fileName, Bundle bundle) {
-
-		// Integer.MIN_VALUE signifies size was not specified
-		this(fileName, Integer.MIN_VALUE, bundle);
-	}
-
-	/*
-	 * 
-	 */
-	public synchronized Object clone() {
+	public Object clone() {
 		return new Icon(this._fileName, this._size, this._bundle);
 	}
 
@@ -59,7 +47,7 @@ class Icon implements Cloneable {
 	 * 
 	 * @return size or Integer.MIN_VALUE if no size was specified
 	 */
-	int getIconSize() {
+	Integer getIconSize() {
 		return _size;
 	}
 
diff --git a/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/ObjectClassDefinitionImpl.java b/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/ObjectClassDefinitionImpl.java
index 6cc887983..82d13f8d4 100644
--- a/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/ObjectClassDefinitionImpl.java
+++ b/bundles/org.eclipse.equinox.metatype/src/org/eclipse/equinox/metatype/impl/ObjectClassDefinitionImpl.java
@@ -10,33 +10,37 @@
  *******************************************************************************/
 package org.eclipse.equinox.metatype.impl;
 
-import org.eclipse.equinox.metatype.EquinoxAttributeDefinition;
-import org.eclipse.equinox.metatype.EquinoxObjectClassDefinition;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
 import java.util.*;
+import org.eclipse.equinox.metatype.EquinoxAttributeDefinition;
+import org.eclipse.equinox.metatype.EquinoxObjectClassDefinition;
 import org.osgi.framework.Bundle;
 
 /**
  * Implementation of ObjectClassDefinition
  */
 public class ObjectClassDefinitionImpl extends LocalizationElement implements EquinoxObjectClassDefinition, Cloneable {
-
 	public static final char LOCALE_SEP = '_';
 
-	String _name;
-	String _id;
-	String _description;
-
-	int _type;
-	Vector<AttributeDefinitionImpl> _required = new Vector<AttributeDefinitionImpl>(7);
-	Vector<AttributeDefinitionImpl> _optional = new Vector<AttributeDefinitionImpl>(7);
-	Icon _icon;
-
+	private static final Comparator<Icon> iconComparator = new Comparator<Icon>() {
+		public int compare(Icon icon1, Icon icon2) {
+			return icon1.getIconSize().compareTo(icon2.getIconSize());
+		}
+	};
+
+	private final String _name;
+	private final String _id;
+	private final String _description;
+	private final int _type;
+	private final Vector<AttributeDefinitionImpl> _required = new Vector<AttributeDefinitionImpl>(7);
+	private final Vector<AttributeDefinitionImpl> _optional = new Vector<AttributeDefinitionImpl>(7);
 	private final ExtendableHelper helper;
 
+	// @GuardedBy("this")
+	private List<Icon> icons;
+
 	/*
 	 * Constructor of class ObjectClassDefinitionImpl.
 	 */
@@ -70,9 +74,8 @@ public class ObjectClassDefinitionImpl extends LocalizationElement implements Eq
 			AttributeDefinitionImpl ad = _optional.elementAt(i);
 			ocd.addAttributeDefinition((AttributeDefinitionImpl) ad.clone(), false);
 		}
-		if (_icon != null) {
-			ocd.setIcon((Icon) _icon.clone());
-		}
+		if (icons != null)
+			ocd.setIcons(new ArrayList<Icon>(icons));
 		return ocd;
 	}
 
@@ -85,13 +88,6 @@ public class ObjectClassDefinitionImpl extends LocalizationElement implements Eq
 		return getLocalized(_name);
 	}
 
-	/**
-	 * Method to set the name of ObjectClassDefinition.
-	 */
-	void setName(String name) {
-		this._name = name;
-	}
-
 	/*
 	 * (non-Javadoc)
 	 * 
@@ -111,13 +107,6 @@ public class ObjectClassDefinitionImpl extends LocalizationElement implements Eq
 	}
 
 	/*
-	 * Method to set the description of ObjectClassDefinition.
-	 */
-	void setDescription(String description) {
-		this._description = description;
-	}
-
-	/*
 	 * (non-Javadoc)
 	 * 
 	 * @see org.osgi.service.metatype.ObjectClassDefinition#getAttributeDefinitions(int)
@@ -169,29 +158,56 @@ public class ObjectClassDefinitionImpl extends LocalizationElement implements Eq
 	 * 
 	 * @see org.osgi.service.metatype.ObjectClassDefinition#getIcon(int)
 	 */
-	public InputStream getIcon(int sizeHint) throws IOException {
+	public synchronized InputStream getIcon(int sizeHint) throws IOException {
 		// The parameter simply represents a requested size. This method should never return null if an
 		// icon exists.
-		// TODO This method may change further depending on the outcome of certain ongoing CPEG discussions.
-		// It is thought that users should be able to specify the same icon multiple times but of different
-		// sizes. This would require a change to the XML schema. This method would then return the icon with
-		// a size closest to the requested size.
-		if ((_icon == null)) {
-			return null;
-		}
-		Bundle b = _icon.getIconBundle();
-		URL[] urls = FragmentUtils.findEntries(b, getLocalized(_icon.getIconName()));
+		// Temporary icon to hold the requested size for use in binary search comparator.
+		Icon icon = new Icon(null, sizeHint, null);
+		@SuppressWarnings("hiding")
+		// Use a local reference to the icon list to be sure we don't suddenly start using a new one.
+		List<Icon> icons = this.icons;
+		int index = Collections.binarySearch(icons, icon, iconComparator);
+		if (index < 0) {
+			// If the index is less than zero, there wasn't an exact match.
+			// Compute the insertion point. This will be the index of the first icon whose 
+			// size was greater than the requested size, or the list's length if there were none.
+			int insertionPoint = -(index + 1);
+			Icon lessThan = insertionPoint == 0 ? null : icons.get(insertionPoint - 1);
+			Icon greaterThan = insertionPoint == icons.size() ? null : icons.get(insertionPoint);
+			if (lessThan == null)
+				// There were no icons whose size was smaller than the requested size.
+				icon = greaterThan;
+			else if (greaterThan == null)
+				// There were no icons whose size was greater than the requested size.
+				icon = lessThan;
+			else {
+				// There was at least one icon with a smaller size and at least one with
+				// a greater size than the requested size. Compute the average to see which one to choose.
+				int average = (greaterThan.getIconSize() + lessThan.getIconSize()) / 2;
+				if (sizeHint < average)
+					// The smaller icon is closer to the requested size.
+					icon = lessThan;
+				else
+					// The larger icon is closer to the requested size.
+					icon = greaterThan;
+			}
+		} else
+			// The index was greater than or equal to zero, indicating the index of an exact match.
+			icon = icons.get(index);
+		Bundle b = icon.getIconBundle();
+		URL[] urls = FragmentUtils.findEntries(b, getLocalized(icon.getIconName()));
 		if (urls != null && urls.length > 0) {
 			return urls[0].openStream();
 		}
 		return null;
 	}
 
-	/**
-	 * Method to set the icon of ObjectClassDefinition.
-	 */
-	void setIcon(Icon icon) {
-		this._icon = icon;
+	synchronized void setIcons(List<Icon> icons) {
+		// Prepare the list of icons for binary searches as in getIcon(int).
+		Collections.sort(icons, iconComparator);
+		// Make the list unmodifiable for safe binary searches without copying.
+		// We assume the caller makes no modifications to the list.
+		this.icons = Collections.unmodifiableList(icons);
 	}
 
 	/**