Code cleanup, documentation, formatting

6 files changed, 131 insertions, 87 deletions

diff --git a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/.options b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/.options
index 3c8f01bae..3df9205ff 100644
--- a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/.options
+++ b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/.options
@@ -15,8 +15,6 @@ org.eclipse.ecf.osgi.services.distribution/debug/methods/entering=true
 # Trace when methods are exited
 org.eclipse.ecf.osgi.services.distribution/debug/methods/exiting=true
 
-org.eclipse.ecf.osgi.services.distribution/debug/findhook=true
-
 org.eclipse.ecf.osgi.services.distribution/debug/eventhook=true
 
 org.eclipse.ecf.osgi.services.distribution/debug/discoveredservicetracker=true
diff --git a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/META-INF/MANIFEST.MF b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/META-INF/MANIFEST.MF
index 4655c4805..1e3cbb27f 100644
--- a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/META-INF/MANIFEST.MF
+++ b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/META-INF/MANIFEST.MF
@@ -19,6 +19,7 @@ Import-Package: org.eclipse.ecf.core,
  org.osgi.framework.hooks.service,
  org.osgi.service.discovery;version="1.0.0",
  org.osgi.service.distribution;version="1.0.0",
+ org.osgi.service.log;version="1.3.0",
  org.osgi.util.tracker
 Bundle-ActivationPolicy: lazy
 Bundle-Localization: plugin
diff --git a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/AbstractEventHookImpl.java b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/AbstractEventHookImpl.java
index 19afe19c4..0a8af7fa5 100644
--- a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/AbstractEventHookImpl.java
+++ b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/AbstractEventHookImpl.java
@@ -10,6 +10,8 @@
 package org.eclipse.ecf.internal.osgi.services.distribution;
 
 import java.util.*;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.ecf.core.util.Trace;
 import org.eclipse.ecf.osgi.services.distribution.ECFServiceConstants;
 import org.eclipse.ecf.remoteservice.IRemoteServiceRegistration;
@@ -64,33 +66,32 @@ public abstract class AbstractEventHookImpl implements EventHook {
 	void handleRegisteredServiceEvent(ServiceReference serviceReference,
 			Collection contexts) {
 		// This checks to see if the serviceReference has any remote interfaces
-		// declared via
-		// osgi.remote.interfaces
+		// declared via osgi.remote.interfaces property
 		Object osgiRemotes = serviceReference
 				.getProperty(ECFServiceConstants.OSGI_REMOTE_INTERFACES);
 		// If osgi.remote.interfaces required property is non-null then we
-		// handle further, if null then ignore
+		// handle further, if null then ignore the service registration event
 		if (osgiRemotes != null) {
 			// The osgiRemotes should be of type String [] according to
 			// RFC119...if it's not String [] we ignore
 			String[] remoteInterfacesArr = (String[]) ((osgiRemotes instanceof String[]) ? osgiRemotes
 					: null);
 			if (remoteInterfacesArr == null) {
-				trace("handleRegisteredServiceEvent",
+				logError("handleRegisteredServiceEvent",
 						"remoteInterfaces not of String [] type as required by RFC 119");
 				return;
 			}
 			trace("handleRegisteredServiceEvent", "serviceReference="
-					+ serviceReference + " has remoteInterfaces="
+					+ serviceReference + ",remoteInterfaces="
 					+ Arrays.asList(remoteInterfacesArr));
 			// We compare the osgi.remote.interfaces with those exposed by the
-			// service reference and
-			// make sure that expose some common interfaces
+			// service reference and make sure that expose some common
+			// interfaces
 			String[] remoteInterfaces = (remoteInterfacesArr != null) ? getInterfacesForServiceReference(
 					remoteInterfacesArr, serviceReference)
 					: null;
 			if (remoteInterfaces == null) {
-				trace("handleRegisteredServiceEvent",
+				logError("handleRegisteredServiceEvent",
 						"No exposed remoteInterfaces found for serviceReference="
 								+ serviceReference);
 				return;
@@ -104,7 +105,7 @@ public abstract class AbstractEventHookImpl implements EventHook {
 			String[] remoteConfigurationType = null;
 			if (osgiRemoteConfigurationType != null) {
 				if (!(osgiRemoteConfigurationType instanceof String[])) {
-					trace("handleRegisteredServiceEvent",
+					logError("handleRegisteredServiceEvent",
 							"osgi.remote.configuration.type is not String[] as required by RFC 119");
 					return;
 				}
@@ -135,21 +136,27 @@ public abstract class AbstractEventHookImpl implements EventHook {
 	}
 
 	protected void fireRemoteServiceUnregistered(ServiceReference reference) {
-
+		IRemoteServiceRegistration[] registrations = null;
 		synchronized (srvRefToRemoteSrvRegistration) {
 			distributionProvider.removeExposedService(reference);
 			List l = (List) srvRefToRemoteSrvRegistration.remove(reference);
 			if (l != null) {
-				for (Iterator i = l.iterator(); i.hasNext();) {
-					IRemoteServiceRegistration reg = (IRemoteServiceRegistration) i
-							.next();
-					trace("fireRemoteServiceUnregistered", "sr=" + reference
-							+ "; reg=" + reg);
-					reg.unregister();
-				}
+				registrations = (IRemoteServiceRegistration[]) l
+						.toArray(new IRemoteServiceRegistration[] {});
 				l.clear();
 			}
 		}
+		if (registrations != null) {
+			for (int i = 0; i < registrations.length; i++) {
+				try {
+					registrations[i].unregister();
+				} catch (Exception e) {
+					logError("fireRemoteServiceUnregistered",
+							"Exception unregistering remote registration="
+									+ registrations[i], e);
+				}
+			}
+		}
 	}
 
 	protected void fireRemoteServicePublished(
@@ -167,21 +174,27 @@ public abstract class AbstractEventHookImpl implements EventHook {
 	}
 
 	protected void fireRemoteServiceUnpublished(ServiceReference reference) {
+		ServiceRegistration[] registrations = null;
 		synchronized (srvRefToServicePublicationRegistration) {
 			List l = (List) srvRefToServicePublicationRegistration
 					.remove(reference);
 			if (l != null) {
-				for (Iterator i = l.iterator(); i.hasNext();) {
-					ServiceRegistration reg = (ServiceRegistration) i.next();
-					if (reg != null) {
-						trace("fireRemoteServiceUnpublished", "sr=" + reference
-								+ "; reg=" + reg);
-						reg.unregister();
-					}
-				}
+				registrations = (ServiceRegistration[]) l
+						.toArray(new ServiceRegistration[] {});
 				l.clear();
 			}
 		}
+		if (registrations != null) {
+			for (int i = 0; i < registrations.length; i++) {
+				try {
+					registrations[i].unregister();
+				} catch (Exception e) {
+					logError("fireRemoteServiceUnpublished",
+							"Exception unregistering service publication registrations="
+									+ registrations[i], e);
+				}
+			}
+		}
 	}
 
 	private String[] getInterfacesForServiceReference(
@@ -207,6 +220,25 @@ public abstract class AbstractEventHookImpl implements EventHook {
 				.getClass(), methodName, message);
 	}
 
+	protected void traceException(String methodName, String message, Throwable t) {
+		Trace.catching(Activator.PLUGIN_ID, DebugOptions.EXCEPTIONS_CATCHING,
+				this.getClass(), methodName + ":" + message, t);
+	}
+
+	protected void logError(String methodName, String message, Throwable t) {
+		Activator.getDefault().log(
+				new Status(IStatus.ERROR, Activator.PLUGIN_ID, IStatus.ERROR,
+						methodName + ":" + message, t));
+		traceException(methodName, message, t);
+	}
+
+	protected void logError(String methodName, String message) {
+		Activator.getDefault().log(
+				new Status(IStatus.ERROR, Activator.PLUGIN_ID, IStatus.ERROR,
+						methodName + ":" + message, null));
+		traceException(methodName, message, null);
+	}
+
 	protected void handleUnregisteringServiceEvent(
 			ServiceReference serviceReference, Collection contexts) {
 		fireRemoteServiceUnregistered(serviceReference);
diff --git a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/Activator.java b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/Activator.java
index 89c47247f..2de87369a 100644
--- a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/Activator.java
+++ b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/Activator.java
@@ -11,12 +11,16 @@ package org.eclipse.ecf.internal.osgi.services.distribution;
 
 import java.util.Dictionary;
 import java.util.Hashtable;
+import org.eclipse.core.runtime.IStatus;
 import org.eclipse.ecf.core.IContainerManager;
+import org.eclipse.ecf.core.util.LogHelper;
+import org.eclipse.ecf.core.util.SystemLogService;
 import org.eclipse.ecf.osgi.services.distribution.ECFServiceConstants;
 import org.osgi.framework.*;
 import org.osgi.framework.hooks.service.EventHook;
 import org.osgi.service.discovery.DiscoveredServiceTracker;
 import org.osgi.service.distribution.DistributionProvider;
+import org.osgi.service.log.LogService;
 import org.osgi.util.tracker.ServiceTracker;
 
 public class Activator implements BundleActivator {
@@ -34,6 +38,9 @@ public class Activator implements BundleActivator {
 	private ServiceRegistration distributionProviderRegistration;
 	private ServiceRegistration discoveredServiceTrackerRegistration;
 
+	private ServiceTracker logServiceTracker = null;
+	private LogService logService = null;
+
 	public static Activator getDefault() {
 		return plugin;
 	}
@@ -42,6 +49,38 @@ public class Activator implements BundleActivator {
 		return context;
 	}
 
+	protected LogService getLogService() {
+		if (logServiceTracker == null) {
+			logServiceTracker = new ServiceTracker(this.context,
+					LogService.class.getName(), null);
+			logServiceTracker.open();
+		}
+		logService = (LogService) logServiceTracker.getService();
+		if (logService == null)
+			logService = new SystemLogService(PLUGIN_ID);
+		return logService;
+	}
+
+	public void log(IStatus status) {
+		if (logService == null)
+			logService = getLogService();
+		if (logService != null)
+			logService.log(null, LogHelper.getLogCode(status), LogHelper
+					.getLogMessage(status), status.getException());
+	}
+
+	public void log(ServiceReference sr, IStatus status) {
+		log(sr, LogHelper.getLogCode(status), LogHelper.getLogMessage(status),
+				status.getException());
+	}
+
+	public void log(ServiceReference sr, int level, String message, Throwable t) {
+		if (logService == null)
+			logService = getLogService();
+		if (logService != null)
+			logService.log(sr, level, message, t);
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * 
@@ -136,7 +175,15 @@ public class Activator implements BundleActivator {
 			containerManagerTracker.close();
 			containerManagerTracker = null;
 		}
-		this.distributionProvider = null;
+		if (logServiceTracker != null) {
+			logServiceTracker.close();
+			logServiceTracker = null;
+			logService = null;
+		}
+		if (distributionProvider != null) {
+			distributionProvider.dispose();
+			distributionProvider = null;
+		}
 		this.context = null;
 		plugin = null;
 	}
diff --git a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DebugOptions.java b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DebugOptions.java
index 302314f50..ae9dbd604 100644
--- a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DebugOptions.java
+++ b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DebugOptions.java
@@ -25,8 +25,6 @@ public interface DebugOptions {
 
 	public static final String EVENTHOOKDEBUG = DEBUG + "/eventhook";
 
-	public static final String FINDHOOKDEBUG = DEBUG + "/findhook";
-
 	public static final String DISCOVEREDSERVICETRACKER = DEBUG
 			+ "/discoveredservicetracker";
 
diff --git a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DistributionProviderImpl.java b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DistributionProviderImpl.java
index 243b0bdd3..41956a5bc 100644
--- a/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DistributionProviderImpl.java
+++ b/compendium/bundles/org.eclipse.ecf.osgi.services.distribution/src/org/eclipse/ecf/internal/osgi/services/distribution/DistributionProviderImpl.java
@@ -13,82 +13,45 @@ import java.util.*;
 import org.eclipse.ecf.core.ContainerTypeDescription;
 import org.eclipse.ecf.core.IContainerFactory;
 import org.eclipse.ecf.remoteservice.IRemoteServiceContainerAdapter;
-import org.osgi.framework.Constants;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.distribution.DistributionProvider;
 
 public class DistributionProviderImpl implements DistributionProvider {
 
 	public static Object VENDOR_NAME = "Eclipse Foundation";
-	public static Object PRODUCT_NAME = "Eclipse ECF";
-	public static Object PRODUCT_VERSION = "1.0.0";
+	public static Object PRODUCT_NAME = "Eclipse Communication Framework (ECF)";
+	public static Object PRODUCT_VERSION = "3.0";
 
-	Map exposedServices = Collections.synchronizedMap(new HashMap());
-	Map remoteServices = Collections.synchronizedMap(new HashMap());
+	List exposedServices = Collections.synchronizedList(new ArrayList());
+	List remoteServices = Collections.synchronizedList(new ArrayList());
 
-	Long getServiceId(ServiceReference sr) {
-		return (Long) sr.getProperty(Constants.SERVICE_ID);
-	}
-
-	ServiceReference addExposedService(ServiceReference sr) {
+	boolean addExposedService(ServiceReference sr) {
 		if (sr == null)
-			return null;
-		return (ServiceReference) exposedServices.put(getServiceId(sr), sr);
+			return false;
+		return exposedServices.add(sr);
 	}
 
-	ServiceReference addRemoteService(ServiceReference sr) {
+	boolean addRemoteService(ServiceReference sr) {
 		if (sr == null)
-			return null;
-		return (ServiceReference) remoteServices.put(getServiceId(sr), sr);
-	}
-
-	ServiceReference removeExposedService(Long sid) {
-		if (sid == null)
-			return null;
-		return (ServiceReference) exposedServices.remove(sid);
-	}
-
-	ServiceReference removeExposedService(ServiceReference sr) {
-		return removeExposedService(getServiceId(sr));
-	}
-
-	ServiceReference removeRemoteService(Long sid) {
-		if (sid == null)
-			return null;
-		return (ServiceReference) remoteServices.remove(sid);
-	}
-
-	ServiceReference removeRemoteService(ServiceReference sr) {
-		return removeRemoteService(getServiceId(sr));
-	}
-
-	boolean containsExposedService(Long sid) {
-		if (sid == null)
 			return false;
-		return exposedServices.containsKey(sid);
+		return remoteServices.add(sr);
 	}
 
-	boolean containsRemoteService(Long sid) {
-		if (sid == null)
+	boolean removeExposedService(ServiceReference sr) {
+		if (sr == null)
 			return false;
-		return remoteServices.containsKey(sid);
-	}
-
-	ServiceReference getExposedService(Long sid) {
-		if (sid == null)
-			return null;
-		return (ServiceReference) exposedServices.get(sid);
+		return exposedServices.remove(sr);
 	}
 
-	ServiceReference getRemoteService(Long sid) {
-		if (sid == null)
-			return null;
-		return (ServiceReference) remoteServices.get(sid);
+	boolean removeRemoteService(ServiceReference sr) {
+		if (sr == null)
+			return false;
+		return remoteServices.remove(sr);
 	}
 
 	public ServiceReference[] getExposedServices() {
-		return (ServiceReference[]) exposedServices.entrySet().toArray(
-				new ServiceReference[] {});
+		return (ServiceReference[]) exposedServices
+				.toArray(new ServiceReference[] {});
 	}
 
 	public Map getPublicationProperties(ServiceReference sr) {
@@ -121,14 +84,19 @@ public class DistributionProviderImpl implements DistributionProvider {
 		return result;
 	}
 
+	public void dispose() {
+		exposedServices.clear();
+		remoteServices.clear();
+	}
+
 	// XXX word on the street is that this is being removed from spec
 	public ServiceReference[] getPublishedServices() {
 		return null;
 	}
 
 	public ServiceReference[] getRemoteServices() {
-		return (ServiceReference[]) remoteServices.entrySet().toArray(
-				new ServiceReference[] {});
+		return (ServiceReference[]) remoteServices
+				.toArray(new ServiceReference[] {});
 	}
 
 	public Collection /* String */getSupportedIntents() {