537836: Transport service has been unregistered

Provide better fallback and more useful logging

Bug: 537836
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=537836
diff --git a/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/Messages.java b/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/Messages.java
index 44272e3..9d97360 100644
--- a/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/Messages.java
+++ b/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/Messages.java
@@ -31,13 +31,25 @@
 
 	public static String ServiceUtil_ignoringIncompatibleServiceProperty;
 
+	public static String TransportFactory_available;
+
+	public static String TransportFactory_DefaultService;
+
+	public static String TransportFactory_DefaultTransportUnavailable_UseFallback;
+
+	public static String TransportFactory_FallbackService;
+
+	public static String TransportFactory_LegacyFallbackCreationError;
+
+	public static String TransportFactory_LegacyFallbacksError;
+
 	public static String TransportFactory_NoLegacyTransportFactoriesError;
 
 	public static String TransportFactory_ServiceErrorAppliedFilter;
 
 	public static String TransportFactory_ServiceErrorDetails;
 
-	public static String TransportFactory_ServiceErrorFilteredService;
+	public static String TransportFactory_ServiceErrorRegisteredService;
 
 	public static String TransportFactory_ServiceErrorNoneAvailable;
 
@@ -47,7 +59,13 @@
 
 	public static String TransportFactory_ServiceErrorUnregistered;
 
+	public static String TransportFactory_StaticFactoryInfo;
+
 	public static String TransportFactory_transportAvailabilityError;
+
+	public static String TransportFactory_unavailable;
+
+	public static String TransportFactory_UseLegacyFallback;
 	static {
 		// initialize resource bundle
 		NLS.initializeMessages(BUNDLE_NAME, Messages.class);
diff --git a/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/TransportFactory.java b/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/TransportFactory.java
index e5f285f..6a80ab6 100644
--- a/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/TransportFactory.java
+++ b/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/TransportFactory.java
@@ -19,6 +19,7 @@
 import java.net.URI;

 import java.util.ArrayList;

 import java.util.Collection;

+import java.util.Collections;

 import java.util.HashMap;

 import java.util.HashSet;

 import java.util.List;

@@ -36,6 +37,7 @@
 import org.eclipse.epp.internal.mpc.core.service.ServiceUnavailableException;

 import org.eclipse.epp.mpc.core.service.ITransportFactory;

 import org.eclipse.epp.mpc.core.service.ServiceHelper;

+import org.eclipse.osgi.util.NLS;

 import org.osgi.framework.BundleContext;

 import org.osgi.framework.FrameworkUtil;

 import org.osgi.framework.InvalidSyntaxException;

@@ -73,6 +75,8 @@
 

 	private static final String ECF_EXCLUDES_PROPERTY = "org.eclipse.ecf.provider.filetransfer.excludeContributors"; //$NON-NLS-1$

 

+	private static String lastFallbackTransport = null;

+

 	public static String computeDisabledTransportsFilter() {

 		BundleContext bundleContext = FrameworkUtil.getBundle(TransportFactory.class).getBundleContext();

 		String disabledTransportsStr = bundleContext.getProperty(DISABLED_TRANSPORTS_KEY);

@@ -224,79 +228,145 @@
 	public static org.eclipse.epp.mpc.core.service.ITransport createTransport() {

 		//search for registered factory service

 		BundleContext context = MarketplaceClientCorePlugin.getBundle().getBundleContext();

-		ServiceReference<ITransportFactory> serviceReference = getTransportServiceReference(context);

-		if (serviceReference != null) {

+		Collection<ServiceReference<ITransportFactory>> serviceReferences = getTransportServiceReferences(context);

+

+		MultiStatus serviceError = null;

+		ServiceReference<ITransportFactory> defaultServiceReference = null;

+		for (ServiceReference<ITransportFactory> serviceReference : serviceReferences) {

 			ITransportFactory transportService = context.getService(serviceReference);

 			if (transportService != null) {

 				try {

+					synchronized (TransportFactory.class) {

+						if (serviceError != null) {

+							logTransportServiceFallback(serviceError, defaultServiceReference, serviceReference,

+									transportService);

+						} else {

+							//got our preferred service, reset fallback logging

+							lastFallbackTransport = null;

+						}

+					}

 					return transportService.getTransport();

 				} finally {

-					transportService = null;

 					context.ungetService(serviceReference);

 				}

 			}

+			if (defaultServiceReference == null) {

+				defaultServiceReference = serviceReference;

+			}

+			if (serviceError == null) {

+				serviceError = diagnoseTransportServiceRegistration(context, serviceReference);

+			}

 		}

-		IStatus serviceError = diagnoseTransportServiceRegistration(context, serviceReference);

-		throw new IllegalStateException(new CoreException(serviceError));

+		if (serviceError == null) {

+			serviceError = diagnoseTransportServiceRegistration(context, null);

+		}

+		try {

+			for (ITransportFactory factory : listAvailableFactories()) {

+				try {

+					org.eclipse.epp.mpc.core.service.ITransport transport = factory.getTransport();

+					logTransportServiceFallback(serviceError, defaultServiceReference, null, factory);

+					return transport;

+				} catch (Exception ex) {

+					serviceError.add(new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

+							NLS.bind(Messages.TransportFactory_LegacyFallbackCreationError, factory.getClass().getName()), ex));

+				}

+			}

+		} catch (Exception ex) {

+			serviceError.add(new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

+					Messages.TransportFactory_LegacyFallbacksError, ex));

+		}

+		//We log and throw, because the exception is lacking details

+		MarketplaceClientCore.getLog().log(serviceError);

+		CoreException coreException = new CoreException(serviceError);

+		throw new IllegalStateException(serviceError.toString(), coreException);

 	}

 

-	private static IStatus diagnoseTransportServiceRegistration(BundleContext context,

+	private static void logTransportServiceFallback(MultiStatus serviceError,

+			ServiceReference<ITransportFactory> defaultServiceReference,

+			ServiceReference<ITransportFactory> serviceReference, ITransportFactory factory) {

+		String transportName = factory.getClass().getName();

+		if (lastFallbackTransport != null && lastFallbackTransport.equals(transportName)) {

+			return;

+		}

+		lastFallbackTransport = transportName;

+		MultiStatus transportFallbackStatus = new MultiStatus(MarketplaceClientCore.BUNDLE_ID, 0,

+				NLS.bind(Messages.TransportFactory_DefaultTransportUnavailable_UseFallback, transportName), null);

+		if (defaultServiceReference != null) {

+			transportFallbackStatus.add(new Status(IStatus.INFO, MarketplaceClientCore.BUNDLE_ID,

+					NLS.bind(Messages.TransportFactory_DefaultService, defaultServiceReference.toString())));

+		}

+		if (serviceReference != null) {

+			transportFallbackStatus.add(new Status(IStatus.INFO, MarketplaceClientCore.BUNDLE_ID,

+					NLS.bind(Messages.TransportFactory_FallbackService, serviceReference.toString())));

+		} else {

+			transportFallbackStatus.add(new Status(IStatus.INFO, MarketplaceClientCore.BUNDLE_ID,

+					NLS.bind(Messages.TransportFactory_UseLegacyFallback, transportName)));

+		}

+		transportFallbackStatus.add(serviceError);

+		MarketplaceClientCore.getLog().log(transportFallbackStatus);

+	}

+

+	private static MultiStatus diagnoseTransportServiceRegistration(BundleContext context,

 			ServiceReference<ITransportFactory> serviceReference) {

 		MultiStatus serviceError = null;

 		if (serviceReference != null) {

 			serviceError = new MultiStatus(MarketplaceClientCore.BUNDLE_ID, 0,

 					Messages.TransportFactory_ServiceErrorUnregistered, null);

 			serviceError.add(new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

-					Messages.TransportFactory_ServiceErrorServiceReference + serviceReference));

+					NLS.bind(Messages.TransportFactory_ServiceErrorServiceReference, serviceReference)));

 		} else {

 			serviceError = new MultiStatus(MarketplaceClientCore.BUNDLE_ID, 0,

 					Messages.TransportFactory_ServiceErrorNotFound, null);

-			try {

-				Collection<ServiceReference<ITransportFactory>> allServiceReferences = context

-						.getServiceReferences(ITransportFactory.class, null);

-				if (allServiceReferences.isEmpty()) {

+		}

+		try {

+			Collection<ServiceReference<ITransportFactory>> allServiceReferences = context

+					.getServiceReferences(ITransportFactory.class, null);

+			if (allServiceReferences.isEmpty()) {

+				serviceError.add(new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

+						Messages.TransportFactory_ServiceErrorNoneAvailable));

+			} else {

+				String filter = computeDisabledTransportsFilter();

+				if (!"".equals(filter)) { //$NON-NLS-1$

 					serviceError.add(new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

-							Messages.TransportFactory_ServiceErrorNoneAvailable));

-				} else {

-					String filter = computeDisabledTransportsFilter();

-					serviceError.add(new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

-							Messages.TransportFactory_ServiceErrorAppliedFilter + filter));

+							NLS.bind(Messages.TransportFactory_ServiceErrorAppliedFilter, filter)));

 				}

-				for (ServiceReference<ITransportFactory> availableReference : allServiceReferences) {

-					serviceError.add(new Status(IStatus.INFO, MarketplaceClientCore.BUNDLE_ID,

-							Messages.TransportFactory_ServiceErrorFilteredService + availableReference.toString()));

-				}

-			} catch (InvalidSyntaxException e) {

-				//Unreachable

-				serviceError.add(

-						new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

-								Messages.TransportFactory_ServiceErrorDetails, e));

 			}

+			for (ServiceReference<ITransportFactory> availableReference : allServiceReferences) {

+				serviceError.add(new Status(IStatus.INFO, MarketplaceClientCore.BUNDLE_ID,

+						NLS.bind(Messages.TransportFactory_ServiceErrorRegisteredService,

+								availableReference.toString())));

+			}

+			for (ITransportFactory factory : listAvailableFactories(true)) {

+				serviceError.add(new Status(IStatus.INFO, MarketplaceClientCore.BUNDLE_ID,

+						NLS.bind(Messages.TransportFactory_StaticFactoryInfo, factory.getClass().getName(),

+								((TransportFactory) factory).isAvailable() ? Messages.TransportFactory_available : Messages.TransportFactory_unavailable)));

+			}

+		} catch (Exception e) {

+			serviceError.add(new Status(IStatus.ERROR, MarketplaceClientCore.BUNDLE_ID,

+					Messages.TransportFactory_ServiceErrorDetails, e));

 		}

 		return serviceError;

 	}

 

-	public static ServiceReference<ITransportFactory> getTransportServiceReference(BundleContext context) {

-		ServiceReference<ITransportFactory> serviceReference = null;

+	public static Collection<ServiceReference<ITransportFactory>> getTransportServiceReferences(BundleContext context) {

 		String disabledTransportsFilter = computeDisabledTransportsFilter();

 		if ("".equals(disabledTransportsFilter)) { //$NON-NLS-1$

-			serviceReference = context.getServiceReference(ITransportFactory.class);

-		} else {

-			try {

-				Collection<ServiceReference<ITransportFactory>> serviceReferences = context

-						.getServiceReferences(ITransportFactory.class, disabledTransportsFilter);

-				if (!serviceReferences.isEmpty()) {

-					serviceReference = serviceReferences.iterator().next();

-				}

-			} catch (InvalidSyntaxException e) {

-				MarketplaceClientCore.error(e);

-				serviceReference = context.getServiceReference(ITransportFactory.class);

-			}

+			disabledTransportsFilter = null;

 		}

-		return serviceReference;

+		try {

+			return context.getServiceReferences(ITransportFactory.class, disabledTransportsFilter);

+		} catch (InvalidSyntaxException e) {

+			MarketplaceClientCore.error(e);

+			ServiceReference<ITransportFactory> serviceReference = context.getServiceReference(ITransportFactory.class);

+			return serviceReference == null ? Collections.emptySet() : Collections.singleton(serviceReference);

+		}

 	}

 

 	public static List<ITransportFactory> listAvailableFactories() {

+		return listAvailableFactories(false);

+	}

+

+	private static List<ITransportFactory> listAvailableFactories(boolean includeUnavailable) {

 		List<ITransportFactory> factories = new ArrayList<>();

 		for (String factoryClass : factoryClasses) {

 			TransportFactory factory;

@@ -308,7 +378,7 @@
 				continue;

 			}

 			try {

-				if (factory.isAvailable()) {

+				if (includeUnavailable || factory.isAvailable()) {

 					factories.add(factory);

 				}

 			} catch (Throwable t) {

diff --git a/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/messages.properties b/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/messages.properties
index 1f39871..3b033c0 100644
--- a/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/messages.properties
+++ b/org.eclipse.epp.mpc.core/src/org/eclipse/epp/internal/mpc/core/util/messages.properties
@@ -5,12 +5,21 @@
 FallbackTransportFactory_disablingTransport=Disabling transport {0} - too many failures
 FallbackTransportFactory_fallbackStream=Failed to stream using {0} - falling back to {1}
 ServiceUtil_ignoringIncompatibleServiceProperty=Ignoring value '{0}' for service property '{1}' - incompatible type.
+TransportFactory_available=available
+TransportFactory_DefaultService=Default Service:
+TransportFactory_DefaultTransportUnavailable_UseFallback=Default Marketplace Transport unavailable. Falling back to {0}
+TransportFactory_FallbackService=Fallback Service:
+TransportFactory_LegacyFallbackCreationError=Failed to create legacy transport using factory {0}
+TransportFactory_LegacyFallbacksError=Failed to create legacy transports
 TransportFactory_NoLegacyTransportFactoriesError=No transport factories found for legacy support
-TransportFactory_ServiceErrorAppliedFilter=Some registered services have been filtered. Applied filter: 
+TransportFactory_ServiceErrorAppliedFilter=Some registered services have been filtered. Applied filter: {0}
 TransportFactory_ServiceErrorDetails=Error getting service details
-TransportFactory_ServiceErrorFilteredService=Filtered service: 
+TransportFactory_ServiceErrorRegisteredService=Registered service: {0}
 TransportFactory_ServiceErrorNoneAvailable=No service registrations available
 TransportFactory_ServiceErrorNotFound=No transport service found
-TransportFactory_ServiceErrorServiceReference=Service reference: 
+TransportFactory_ServiceErrorServiceReference=Service reference: {0}
 TransportFactory_ServiceErrorUnregistered=Transport service has been unregistered
+TransportFactory_StaticFactoryInfo=Static factory: {0} ({1})
 TransportFactory_transportAvailabilityError=Skipping transport type {0} - an error occured while checking its availability
+TransportFactory_unavailable=unavailable
+TransportFactory_UseLegacyFallback=Static factory: {0}
diff --git a/org.eclipse.epp.mpc.tests/src/org/eclipse/epp/mpc/tests/util/TransportFactoryTest.java b/org.eclipse.epp.mpc.tests/src/org/eclipse/epp/mpc/tests/util/TransportFactoryTest.java
index 28f22dd..807f91b 100644
--- a/org.eclipse.epp.mpc.tests/src/org/eclipse/epp/mpc/tests/util/TransportFactoryTest.java
+++ b/org.eclipse.epp.mpc.tests/src/org/eclipse/epp/mpc/tests/util/TransportFactoryTest.java
@@ -13,16 +13,8 @@
  *******************************************************************************/
 package org.eclipse.epp.mpc.tests.util;
 
-import static org.hamcrest.Matchers.hasItem;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.not;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
@@ -75,7 +67,7 @@
 import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Matchers;
+import org.mockito.ArgumentMatchers;
 import org.mockito.Mockito;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.FrameworkUtil;
@@ -265,7 +257,7 @@
 		ITransport secondaryTransport = Mockito.mock(ITransport.class);
 		InputStream expectedResultStream = new ByteArrayInputStream("Secondary transport".getBytes("UTF-8"));
 		Mockito.when(secondaryFactory.getTransport()).thenReturn(secondaryTransport);
-		Mockito.when(secondaryTransport.stream(Matchers.<URI> any(), Matchers.<IProgressMonitor> any())).thenReturn(
+		Mockito.when(secondaryTransport.stream(ArgumentMatchers.<URI> any(), ArgumentMatchers.<IProgressMonitor> any())).thenReturn(
 				expectedResultStream);
 
 		FallbackTransportFactory fallbackTransportFactory = new FallbackTransportFactory();
@@ -279,18 +271,18 @@
 	@Test
 	public void testHttpClientCustomizer() throws Exception {
 		final HttpClientCustomizer customizer = Mockito.mock(HttpClientCustomizer.class);
-		Mockito.when(customizer.customizeBuilder(Matchers.any())).thenAnswer(invocation -> {
+		Mockito.when(customizer.customizeBuilder(ArgumentMatchers.any())).thenAnswer(invocation -> {
 			HttpClientBuilder builder = (HttpClientBuilder) invocation.getArguments()[0];
 			return builder == null ? null
 					: builder.addInterceptorFirst((HttpRequestInterceptor) (request, context) -> request.addHeader(
 							"X-Customizer-Test", "true"));
 		});
-		Mockito.when(customizer.customizeCredentialsProvider(Matchers.any())).thenReturn(null);
+		Mockito.when(customizer.customizeCredentialsProvider(ArgumentMatchers.any())).thenReturn(null);
 
 		HttpRequest request = interceptRequest(customizer).getInterceptedRequest();
 
-		Mockito.verify(customizer).customizeBuilder(Matchers.any());
-		Mockito.verify(customizer).customizeCredentialsProvider(Matchers.any());
+		Mockito.verify(customizer).customizeBuilder(ArgumentMatchers.any());
+		Mockito.verify(customizer).customizeCredentialsProvider(ArgumentMatchers.any());
 
 		assertThat(request.getFirstHeader("X-Customizer-Test"), LambdaMatchers.<Header, String> map(x -> x == null
 				? null : x.getValue())
@@ -395,9 +387,12 @@
 
 	private static String getTransportServiceName() {
 		BundleContext bundleContext = FrameworkUtil.getBundle(TransportFactory.class).getBundleContext();
-		ServiceReference<ITransportFactory> transportServiceReference = TransportFactory.getTransportServiceReference(
+		Collection<ServiceReference<ITransportFactory>> transportServiceReferences = TransportFactory
+				.getTransportServiceReferences(
 				bundleContext);
-		if (transportServiceReference != null) {
+		if (!transportServiceReferences.isEmpty()) {
+			ServiceReference<ITransportFactory> transportServiceReference = transportServiceReferences.iterator()
+					.next();
 			String transportServiceName = (String) transportServiceReference.getProperty(
 					ComponentConstants.COMPONENT_NAME);
 			bundleContext.ungetService(transportServiceReference);