Bug 467153 - Remove last remnants of per type config providers.

Also adds a tighter check for ownsConfiguration based on class name
of the provider that created the config.

Change-Id: If197246af0906cb5af92819171ee97dc7cd30bee

bundles/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/AbstractLaunchConfigProvider.java

@@ -14,30 +14,34 @@ import org.eclipse.remote.core.IRemoteConnection;
  */
 public abstract class AbstractLaunchConfigProvider implements ILaunchConfigurationProvider {
 
-	private static final String ORIGINAL_NAME = Activator.PLUGIN_ID + ".originalName"; //$NON-NLS-1$
+	private static final String ATTR_ORIGINAL_NAME = Activator.PLUGIN_ID + ".originalName"; //$NON-NLS-1$
+	private static final String ATTR_PROVIDER_CLASS = Activator.PLUGIN_ID + ".providerClass"; //$NON-NLS-1$
 
 	protected ILaunchConfiguration createLaunchConfiguration(ILaunchDescriptor descriptor, IRemoteConnection target) throws CoreException {
 		ILaunchManager launchManager = DebugPlugin.getDefault().getLaunchManager();
 		String name = launchManager.generateLaunchConfigurationName(descriptor.getName());
 		ILaunchConfigurationWorkingCopy workingCopy = getLaunchConfigurationType(descriptor, target).newInstance(null, name);
 
-		populateLaunchConfiguration(descriptor, workingCopy);
+		populateLaunchConfiguration(descriptor, target, workingCopy);
 
 		return workingCopy.doSave();
 	}
 	
-	protected void populateLaunchConfiguration(ILaunchDescriptor descriptor, ILaunchConfigurationWorkingCopy workingCopy)
+	protected void populateLaunchConfiguration(ILaunchDescriptor descriptor, IRemoteConnection target,
-			throws CoreException {
+			ILaunchConfigurationWorkingCopy workingCopy) throws CoreException {
 		// Leave our breadcrumb
-		workingCopy.setAttribute(ORIGINAL_NAME, workingCopy.getName());
+		workingCopy.setAttribute(ATTR_ORIGINAL_NAME, workingCopy.getName());
+		workingCopy.setAttribute(ATTR_PROVIDER_CLASS, getClass().getName());
 	}
 
 	@Override
 	public boolean ownsLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException {
-		// We created it if it has the same name we created it with.
+		// Check for our class name but also that the config name
+		// matches what we originally set it to.
 		// This covers the case when the config was duplicated.
 		// We can own only one, the original one.
-		return configuration.getAttribute(ORIGINAL_NAME, "").equals(configuration.getName()); //$NON-NLS-1$
+		return configuration.getAttribute(ATTR_PROVIDER_CLASS, "").equals(getClass().getName()) //$NON-NLS-1$
+			&& configuration.getAttribute(ATTR_ORIGINAL_NAME, "").equals(configuration.getName()); //$NON-NLS-1$
 	}
 
 }

bundles/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/DefaultLaunchConfigProvider.java

@@ -32,17 +32,13 @@ public class DefaultLaunchConfigProvider implements ILaunchConfigurationProvider
 
 	@Override
 	public boolean ownsLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException {
-		// If I get here, I own it
+		// return false so that the config is added as a launch object
-		return true;
+		return false;
-	}
-
-	@Override
-	public Object launchConfigurationAdded(ILaunchConfiguration configuration) throws CoreException {
-		return configuration;
 	}
 
 	@Override
 	public boolean launchConfigurationRemoved(ILaunchConfiguration configuration) throws CoreException {
+		// catch any left over configs
 		return true;
 	}
 

bundles/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/ILaunchConfigurationProvider.java

@@ -68,17 +68,6 @@ public interface ILaunchConfigurationProvider {
 	 */
 	boolean ownsLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException;
 
-	/**
-	 * A launch configuration has been added.
-	 * Return the launch object associated with this configuration and the launch bar manager
-	 * will ensure the descriptor is created for it.
-	 * 
-	 * @param configuration
-	 * @return whether this provider owns this launch configuration
-	 * @throws CoreException
-	 */
-	Object launchConfigurationAdded(ILaunchConfiguration configuration) throws CoreException;
-
 	/**
 	 * A launch configuration has been removed. 
 	 * This notification can be used to purge internal cache for example. 

bundles/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/PerTargetLaunchConfigProvider.java

@@ -0,0 +1,78 @@
+package org.eclipse.launchbar.core;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.debug.core.ILaunchConfiguration;
+import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
+import org.eclipse.remote.core.IRemoteConnection;
+
+public abstract class PerTargetLaunchConfigProvider extends AbstractLaunchConfigProvider {
+
+	public static final String ATTR_CONNECTION_TYPE = "connectionType"; //$NON-NLS-1$
+	public static final String ATTR_CONNECTION_NAME = "connectionName"; //$NON-NLS-1$
+
+	private final Map<ILaunchDescriptor, Map<IRemoteConnection, ILaunchConfiguration>> configMap = new HashMap<>();
+
+	@Override
+	public ILaunchConfiguration getLaunchConfiguration(ILaunchDescriptor descriptor, IRemoteConnection target)
+			throws CoreException {
+		Map<IRemoteConnection, ILaunchConfiguration> targetMap = configMap.get(descriptor);
+		if (targetMap != null) {
+			ILaunchConfiguration config = targetMap.get(target);
+			if (config != null) {
+				return config;
+			}
+		}
+
+		ILaunchConfiguration config = createLaunchConfiguration(descriptor, target);
+		if (targetMap == null) {
+			targetMap = new HashMap<>();
+			configMap.put(descriptor, targetMap);
+		}
+		targetMap.put(target, config);
+		return config;
+	}
+
+	@Override
+	protected void populateLaunchConfiguration(ILaunchDescriptor descriptor, IRemoteConnection target,
+			ILaunchConfigurationWorkingCopy workingCopy) throws CoreException {
+		super.populateLaunchConfiguration(descriptor, target, workingCopy);
+		workingCopy.setAttribute(ATTR_CONNECTION_TYPE, target.getConnectionType().getId());
+		workingCopy.setAttribute(ATTR_CONNECTION_NAME, target.getName());
+	}
+
+	@Override
+	public boolean launchConfigurationRemoved(ILaunchConfiguration configuration) throws CoreException {
+		for (Entry<ILaunchDescriptor, Map<IRemoteConnection, ILaunchConfiguration>> descEntry : configMap.entrySet()) {
+			for (Entry<IRemoteConnection, ILaunchConfiguration> targetEntry : descEntry.getValue().entrySet()) {
+				if (targetEntry.getValue().equals(configuration)) {
+					descEntry.getValue().remove(targetEntry.getKey());
+					if (descEntry.getValue().isEmpty()) {
+						configMap.remove(descEntry.getKey());
+					}
+					return true;
+				}
+			}
+		}
+		return false;
+	}
+
+	@Override
+	public void launchDescriptorRemoved(ILaunchDescriptor descriptor) throws CoreException {
+		configMap.remove(descriptor);
+	}
+
+	@Override
+	public void launchTargetRemoved(IRemoteConnection target) throws CoreException {
+		for (Entry<ILaunchDescriptor, Map<IRemoteConnection, ILaunchConfiguration>> descEntry : configMap.entrySet()) {
+			descEntry.getValue().remove(target);
+			if (descEntry.getValue().isEmpty()) {
+				configMap.remove(descEntry.getKey());
+			}
+		}
+	}
+
+}

bundles/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/PerTypeLaunchConfigProvider.java

@@ -1,115 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2015 QNX Software Systems 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
- * http://www.eclipse.org/legal/epl-v10.html
- *
- * Contributors:
- *     QNX Software Systems - Initial API and implementation
- *******************************************************************************/
-package org.eclipse.launchbar.core;
-
-import java.util.HashMap;
-import java.util.Map;
-
-import org.eclipse.core.runtime.CoreException;
-import org.eclipse.debug.core.DebugPlugin;
-import org.eclipse.debug.core.ILaunchConfiguration;
-import org.eclipse.debug.core.ILaunchConfigurationType;
-import org.eclipse.remote.core.IRemoteConnection;
-
-/**
- * Common launch configuration provider for cases where it provides for a single
- * connection type and a single launch configuration type.
- */
-public abstract class PerTypeLaunchConfigProvider extends AbstractLaunchConfigProvider {
-
-	// Map from launch object to launch configuration
-	private Map<Object, ILaunchConfiguration> configMap = new HashMap<>();
-
-	protected abstract String getRemoteConnectionTypeId();
-
-	protected abstract String getLaunchConfigurationTypeId();
-
-	protected abstract Object getLaunchObject(ILaunchDescriptor descriptor);
-	
-	protected abstract Object getLaunchObject(ILaunchConfiguration configuration) throws CoreException;
-
-	protected ILaunchConfigurationType getLaunchConfigurationType() {
-		return DebugPlugin.getDefault().getLaunchManager().getLaunchConfigurationType(getLaunchConfigurationTypeId());
-	}
-
-	@Override
-	public boolean supports(ILaunchDescriptor descriptor, IRemoteConnection target) throws CoreException {
-		// If target is null, assume we support it.
-		return target == null || target.getConnectionType().getId().equals(getRemoteConnectionTypeId());
-	}
-
-	@Override
-	public ILaunchConfigurationType getLaunchConfigurationType(ILaunchDescriptor descriptor, IRemoteConnection target) throws CoreException {
-		if (supports(descriptor, target)) {
-			return getLaunchConfigurationType();
-		}
-		return null;
-	}
-
-	@Override
-	public ILaunchConfiguration getLaunchConfiguration(ILaunchDescriptor descriptor, IRemoteConnection target) throws CoreException {
-		if (supports(descriptor, target)) {
-			Object launchObject = getLaunchObject(descriptor);
-			ILaunchConfiguration config = configMap.get(launchObject);
-			if (config == null) {
-				config = createLaunchConfiguration(descriptor, target);
-				configMap.put(launchObject, config);
-			}
-			return config;
-		} else { 
-			return null;
-		}
-	}
-
-	@Override
-	public boolean ownsLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException {
-		if (!super.ownsLaunchConfiguration(configuration)) {
-			return false;
-		}
-		
-		// Must be of our type
-		return configuration.getType().equals(getLaunchConfigurationType());
-	}
-
-	@Override
-	public Object launchConfigurationAdded(ILaunchConfiguration configuration) throws CoreException {
-		if (ownsLaunchConfiguration(configuration)) {
-			Object launchObject = getLaunchObject(configuration);
-			configMap.put(launchObject, configuration);
-			return launchObject;
-		}
-		return null;
-	}
-
-	@Override
-	public boolean launchConfigurationRemoved(ILaunchConfiguration configuration) throws CoreException {
-		if (ownsLaunchConfiguration(configuration)) {
-			Object launchObject = getLaunchObject(configuration);
-			configMap.remove(launchObject);
-			return true;
-		}
-		return false;
-	}
-
-	@Override
-	public void launchDescriptorRemoved(ILaunchDescriptor descriptor) throws CoreException {
-		Object launchObject = getLaunchObject(descriptor);
-		if (launchObject != null) {
-			configMap.remove(launchObject);
-		}
-	}
-
-	@Override
-	public void launchTargetRemoved(IRemoteConnection target) throws CoreException {
-		// nothing to do per target
-	}
-
-}

bundles/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/ProjectPerTargetLaunchConfigProvider.java

@@ -13,43 +13,20 @@ package org.eclipse.launchbar.core;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.CoreException;
-import org.eclipse.debug.core.ILaunchConfiguration;
 import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
-import org.eclipse.launchbar.core.internal.Activator;
 import org.eclipse.remote.core.IRemoteConnection;
 
-public abstract class ProjectPerTypeLaunchConfigProvider extends PerTypeLaunchConfigProvider {
+public abstract class ProjectPerTargetLaunchConfigProvider extends PerTargetLaunchConfigProvider {
-
-	private static final String PROJECT_CONFIG = Activator.PLUGIN_ID + ".projectConfig"; //$NON-NLS-1$
-
-	@Override
-	protected Object getLaunchObject(ILaunchDescriptor descriptor) {
-		return descriptor.getAdapter(IProject.class);
-	}
-
-	@Override
-	protected Object getLaunchObject(ILaunchConfiguration configuration) throws CoreException {
-		for (IResource resource : configuration.getMappedResources()) {
-			if (resource instanceof IProject) {
-				return (IProject) resource;
-			}
-		}
-		return null;
-	}
 
 	@Override
 	public boolean supports(ILaunchDescriptor descriptor, IRemoteConnection target) throws CoreException {
-		if (!super.supports(descriptor, target)) {
-			return false;
-		}
-
 		return descriptor.getAdapter(IProject.class) != null;
 	}
 
 	@Override
-	protected void populateLaunchConfiguration(ILaunchDescriptor descriptor, ILaunchConfigurationWorkingCopy workingCopy)
+	protected void populateLaunchConfiguration(ILaunchDescriptor descriptor, IRemoteConnection target, 
-			throws CoreException {
+			ILaunchConfigurationWorkingCopy workingCopy) throws CoreException {
-		super.populateLaunchConfiguration(descriptor, workingCopy);
+		super.populateLaunchConfiguration(descriptor, target, workingCopy);
 
 		// Add our project to the mapped resources
 		IProject project = descriptor.getAdapter(IProject.class);
@@ -62,16 +39,6 @@ public abstract class ProjectPerTypeLaunchConfigProvider extends PerTypeLaunchCo
 			newResources[mappedResources.length] = project;
 			workingCopy.setMappedResources(newResources);
 		}
-		workingCopy.setAttribute(PROJECT_CONFIG, true);
-	}
-
-	@Override
-	public boolean ownsLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException {
-		if (!super.ownsLaunchConfiguration(configuration)) {
-			return false;
-		}
-
-		return configuration.getAttribute(PROJECT_CONFIG, false);
 	}
 
 }

bundles/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/internal/LaunchBarManager.java

@@ -745,14 +745,7 @@ public class LaunchBarManager implements ILaunchBarManager, ILaunchConfiguration
 		for (LaunchDescriptorTypeInfo descTypeInfo : orderedDescriptorTypes) {
 			for (LaunchConfigProviderInfo providerInfo : configProviders.get(descTypeInfo.getId())) {
 				try {
-					Object launchObject = providerInfo.getProvider().launchConfigurationAdded(configuration);
+					if (providerInfo.getProvider().ownsLaunchConfiguration(configuration)) {
-					if (launchObject != null) {
-						ILaunchDescriptor descriptor = objectDescriptorMap.get(launchObject);
-						if (descriptor != null) {
-							setActiveLaunchDescriptor(descriptor);
-						} else {
-							launchObjectAdded(configuration);
-						}
 						return;
 					}
 				} catch (Throwable e) {
@@ -760,6 +753,8 @@ public class LaunchBarManager implements ILaunchBarManager, ILaunchConfiguration
 				}
 			}
 		}
+		// No one clamed it, add it as a launch object
+		launchObjectAdded(configuration);
 	}
 
 	@Override

tests/org.eclipse.launchbar.core.tests/src/org/eclipse/launchbar/core/internal/LaunchBarManager2Test.java

@@ -56,7 +56,7 @@ import org.eclipse.launchbar.core.ILaunchConfigurationProvider;
 import org.eclipse.launchbar.core.ILaunchDescriptor;
 import org.eclipse.launchbar.core.ILaunchDescriptorType;
 import org.eclipse.launchbar.core.ProjectLaunchDescriptor;
-import org.eclipse.launchbar.core.ProjectPerTypeLaunchConfigProvider;
+import org.eclipse.launchbar.core.ProjectPerTargetLaunchConfigProvider;
 import org.eclipse.launchbar.core.internal.LaunchBarManager.Listener;
 import org.eclipse.remote.core.IRemoteConnection;
 import org.eclipse.remote.core.IRemoteConnectionType;
@@ -136,7 +136,6 @@ public class LaunchBarManager2Test {
 		doReturn(config).when(provider).getLaunchConfiguration(descriptor, target);
 		doReturn(true).when(provider).supports(descriptor, target);
 		doReturn(true).when(provider).ownsLaunchConfiguration(config);
-		doReturn(launchObj).when(provider).launchConfigurationAdded(config);
 		return provider;
 	}
 
@@ -312,13 +311,6 @@ public class LaunchBarManager2Test {
 		assertNull(manager.getActiveLaunchMode());
 	}
 
-	@Test
-	public void testAddConfigProviderBad() throws CoreException {
-		doThrow(new NullPointerException()).when(provider).launchConfigurationAdded(any(ILaunchConfiguration.class));
-		manager.launchConfigurationAdded(launchConfig);
-		verify(provider).launchConfigurationAdded(any(ILaunchConfiguration.class));
-	}
-
 	@Test
 	public void testAddDescriptorTypeBad() throws CoreException {
 		doThrow(new NullPointerException()).when(descriptorType).ownsLaunchObject(any());
@@ -403,6 +395,7 @@ public class LaunchBarManager2Test {
 
 	@Test
 	public void testGetLaunchDescriptors() throws CoreException {
+		manager.launchObjectAdded(launchObject);
 		manager.launchConfigurationAdded(launchConfig);
 		ILaunchDescriptor[] launchDescriptors = manager.getLaunchDescriptors();
 		assertEquals(1, launchDescriptors.length);
@@ -447,6 +440,7 @@ public class LaunchBarManager2Test {
 
 	@Test
 	public void testLaunchConfigurationAdded() throws CoreException {
+		manager.launchObjectAdded(launchObject);
 		manager.launchConfigurationAdded(launchConfig);
 		ILaunchConfiguration lc2 = mockLC("lc2", launchConfigType);
 		manager.launchConfigurationAdded(lc2);
@@ -564,36 +558,17 @@ public class LaunchBarManager2Test {
 		mockLCAttribute(launchConfig, ORIGINAL_NAME, aaa.getName());
 		mockLCAttribute(launchConfig, PROJECT_CONFIG, true);
 		assertEquals(0, manager.getLaunchDescriptors().length);
-		provider = new ProjectPerTypeLaunchConfigProvider() {
+		provider = new ProjectPerTargetLaunchConfigProvider() {
 			@Override
-			protected String getRemoteConnectionTypeId() {
+			public ILaunchConfigurationType getLaunchConfigurationType(ILaunchDescriptor descriptor,
-				return localTargetTypeId;
+					IRemoteConnection target) throws CoreException {
-			}
+				return launchConfigType;
-
-			@Override
-			protected String getLaunchConfigurationTypeId() {
-				return launchConfigType.getIdentifier();
 			}
 
 			@Override
 			public boolean ownsLaunchConfiguration(ILaunchConfiguration configuration) throws CoreException {
 				return configuration == launchConfig;
 			}
-			
-			@Override
-			protected Object getLaunchObject(ILaunchConfiguration configuration) throws CoreException {
-				if (configuration == launchConfig)
-					return aaa;
-				return null;
-			}
-			
-			@Override
-			protected Object getLaunchObject(ILaunchDescriptor d) {
-				if (descriptor == d) {
-					return aaa;
-				}
-				return null;
-			}
 		};
 		mockProviderElement(descriptorTypeId, 10, provider);
 		init();
@@ -703,6 +678,7 @@ public class LaunchBarManager2Test {
 		Listener lis = mock(Listener.class);
 		manager.addListener(lis);
 		doThrow(new NullPointerException()).when(lis).activeLaunchDescriptorChanged();
+		manager.launchObjectAdded(launchObject);
 		manager.launchConfigurationAdded(launchConfig);
 		verify(lis).activeLaunchDescriptorChanged();
 	}
@@ -727,6 +703,7 @@ public class LaunchBarManager2Test {
 		ILaunchConfigurationType lctype2 = mockLCType("lctype2");
 		mockLaunchModes(lctype2, "modex");
 		mockLaunchModes(launchConfigType, "run", "debug", "foo");
+		manager.launchObjectAdded(launchObject);
 		manager.launchConfigurationAdded(launchConfig);
 		ILaunchMode[] launchModes = manager.getLaunchModes();
 		assertEquals(3, launchModes.length);
@@ -760,6 +737,7 @@ public class LaunchBarManager2Test {
 		ILaunchConfigurationType lctype2 = mockLCType("lctype2");
 		ILaunchMode mode = mockLaunchModes(lctype2, "modex")[0];
 		mockLaunchModes(launchConfigType, "run", "debug", "foo");
+		manager.launchObjectAdded(launchObject);
 		manager.launchConfigurationAdded(launchConfig);
 		try {
 			manager.setActiveLaunchMode(mode);
@@ -895,7 +873,7 @@ public class LaunchBarManager2Test {
 		ILaunchMode mode = mockLaunchModes(launchConfigType, "foo")[0];
 		manager.launchObjectAdded(launchObject);
 		manager.launchConfigurationAdded(launchConfig);
-		verify(provider).launchConfigurationAdded(launchConfig);
+		verify(provider).ownsLaunchConfiguration(launchConfig);
 		ILaunchDescriptor[] launchDescriptors = manager.getLaunchDescriptors();
 		assertEquals(1, launchDescriptors.length);
 		assertNotNull(launchDescriptors[0]);
@@ -922,9 +900,9 @@ public class LaunchBarManager2Test {
 
 	@Test
 	public void testLaunchConfigurationAddedBad() throws CoreException {
-		doThrow(new NullPointerException()).when(provider).launchConfigurationAdded(any(ILaunchConfiguration.class));
+		doThrow(new NullPointerException()).when(provider).ownsLaunchConfiguration(any(ILaunchConfiguration.class));
 		manager.launchConfigurationAdded(launchConfig);
-		verify(provider).launchConfigurationAdded(launchConfig);
+		verify(provider).ownsLaunchConfiguration(launchConfig);
 	}
 
 	@Test