From 80963b1c3390b01df210d6ccdfe532ed2931b963 Mon Sep 17 00:00:00 2001 From: Carsten Reckord Date: Thu, 30 May 2019 19:53:31 +0200 Subject: Bug 544447 - [provider] Implement filetransfer provider using HttpClient 4.5 API Make sure connection is closed in hardClose(). There are four cases to consider: 1. Transfer is complete 2. Transfer was cancelled 3. There was an exception during transfer 4. The transfer was paused In case 3, hardClose() is called from cancel() directly, in the other cases it gets called from the finally block in performFileTransfer(). Cases 1-3 are easy: In case 1, the stream encountered EOF, isDone() == true, and the connection was already closed as a result. In case 2 and 3, it is obvious that we want the connection closed as a result. In case 4, we might want to resume later, but when resuming, we are actually creating a new HttpGet request and get a new HttpResponse as a result. So we also want to close the previous one when pausing. Change-Id: I3daf3c34d0784e5c19aa38441ba373422796f538 Signed-off-by: Carsten Reckord --- .../HttpClientRetrieveFileTransfer.java | 9 +- .../eclipse/ecf/tests/filetransfer/Activator.java | 19 ++++ .../filetransfer/URLRetrievePauseResumeTest.java | 103 ++++++++++++++++----- 3 files changed, 101 insertions(+), 30 deletions(-) diff --git a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient45/src/org/eclipse/ecf/provider/filetransfer/httpclient45/HttpClientRetrieveFileTransfer.java b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient45/src/org/eclipse/ecf/provider/filetransfer/httpclient45/HttpClientRetrieveFileTransfer.java index 672897916..4fb577732 100644 --- a/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient45/src/org/eclipse/ecf/provider/filetransfer/httpclient45/HttpClientRetrieveFileTransfer.java +++ b/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient45/src/org/eclipse/ecf/provider/filetransfer/httpclient45/HttpClientRetrieveFileTransfer.java @@ -183,16 +183,11 @@ public class HttpClientRetrieveFileTransfer extends AbstractRetrieveFileTransfer protected void hardClose() { // changed for addressing bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=389292 if (getMethod != null) { - // First, if !isDone and paused - if (!isDone() && isPaused()) - getMethod.abort(); - // release in any case - // getMethod.releaseConnection(); - // and set to null + getMethod.abort(); getMethod = null; } - if (httpResponse != null && isDone() && !isPaused()) { + if (httpResponse != null) { try { httpResponse.close(); } catch (final IOException e) { diff --git a/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/Activator.java b/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/Activator.java index db7b6881f..0404abb30 100755 --- a/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/Activator.java +++ b/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/Activator.java @@ -16,7 +16,10 @@ import org.eclipse.ecf.provider.filetransfer.IFileTransferProtocolToFactoryMappe import org.osgi.framework.Bundle; import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; +import org.osgi.framework.BundleException; +import org.osgi.framework.ServiceReference; import org.osgi.util.tracker.ServiceTracker; +import org.osgi.util.tracker.ServiceTrackerCustomizer; /** * The activator class controls the plug-in life cycle @@ -62,6 +65,22 @@ public class Activator implements BundleActivator { } catch (NoClassDefFoundError e) { System.out.println("Proxy API not available...continuing with testing without it"); } + + startFiletransferProviderBundle(); + } + + private void startFiletransferProviderBundle() throws BundleException { + Bundle[] bundles = Activator.getDefault().getBundle().getBundleContext().getBundles(); + Bundle filetransferProviderBundle = null; + for (Bundle bundle : bundles) { + if ("org.eclipse.ecf.provider.filetransfer".equals(bundle.getSymbolicName())) { + filetransferProviderBundle = bundle; + break; + } + } + if (filetransferProviderBundle.getState() != Bundle.ACTIVE) { + filetransferProviderBundle.start(); + } } /* diff --git a/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/URLRetrievePauseResumeTest.java b/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/URLRetrievePauseResumeTest.java index 88c42c4cd..3811dafb9 100644 --- a/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/URLRetrievePauseResumeTest.java +++ b/tests/bundles/org.eclipse.ecf.tests.filetransfer/src/org/eclipse/ecf/tests/filetransfer/URLRetrievePauseResumeTest.java @@ -12,8 +12,17 @@ package org.eclipse.ecf.tests.filetransfer; import java.io.File; +import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.security.DigestInputStream; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import org.eclipse.ecf.filetransfer.IFileTransferListener; import org.eclipse.ecf.filetransfer.IFileTransferPausable; @@ -27,17 +36,19 @@ import org.eclipse.ecf.filetransfer.events.IIncomingFileTransferReceiveStartEven import org.eclipse.ecf.filetransfer.identity.FileIDFactory; import org.eclipse.ecf.filetransfer.service.IRetrieveFileTransfer; import org.eclipse.ecf.tests.ContainerAbstractTestCase; +import org.osgi.framework.Bundle; public class URLRetrievePauseResumeTest extends ContainerAbstractTestCase { - private static final String HTTP_RETRIEVE = "http://ftp.osuosl.org/pub/eclipse/rt/ecf/3.14.0/site.p2/artifacts.jar"; - //private static final String HTTP_RETRIEVE = "http://ftp.osuosl.org/pub/eclipse/rt/ecf/3.5.4/site.p2/plugins/org.eclipse.ecf.doc_1.3.0.v20111230-0120.jar"; - - //private static final String HTTP_RETRIEVE = "http://mirror.csclub.uwaterloo.ca/eclipse/eclipse/downloads/drops/R-3.7.1-201109091335/eclipse-platform-3.7.1-win32.zip"; + // We need something sufficiently large to take a few seconds to download. + // Currently, around 15-20 MB seem fairly safe. + private static final String HTTP_RETRIEVE = "http://mirror.csclub.uwaterloo.ca/eclipse/rt/ecf/3.14.4/org.eclipse.ecf.sdk_3.14.4.v20181013-2146.zip"; + private static final String HTTP_RETRIEVE_MD5 = "a6a8a64fd5784165f7e7ba0d6c7ffa1b"; private static final String FILENAME = "foo.zip"; private static final int PAUSE_TIME = 10000; + private static final double RESUMED_DOWNLOAD_AMOUNT_THRESHOLD = 1.5; private IRetrieveFileTransfer transferInstance; @@ -60,6 +71,9 @@ public class URLRetrievePauseResumeTest extends ContainerAbstractTestCase { */ protected void tearDown() throws Exception { super.tearDown(); + if (session != null) { + session.cancel(); + } session = null; pausable = null; if (incomingFile != null) @@ -89,9 +103,13 @@ public class URLRetrievePauseResumeTest extends ContainerAbstractTestCase { protected void testReceiveHttp(String url) throws Exception { assertNotNull(transferInstance); + final AtomicBoolean wasPaused = new AtomicBoolean(false); + final AtomicBoolean wasResumed = new AtomicBoolean(false); + final AtomicLong downloaded = new AtomicLong(0); final IFileTransferListener listener = new IFileTransferListener() { public void handleTransferEvent(IFileTransferEvent event) { if (event instanceof IIncomingFileTransferReceiveResumedEvent) { + wasResumed.set(true); try { IIncomingFileTransferReceiveResumedEvent rse = (IIncomingFileTransferReceiveResumedEvent) event; session = rse.receive(outs); @@ -105,8 +123,7 @@ public class URLRetrievePauseResumeTest extends ContainerAbstractTestCase { outs = new FileOutputStream(incomingFile); session = rse.receive(outs); pausable = (IFileTransferPausable) session.getAdapter(IFileTransferPausable.class); - if (pausable == null) - fail("pausable is null"); + assertNotNull("pausable is null", pausable); } catch (IOException e) { fail(e.getLocalizedMessage()); } @@ -114,6 +131,7 @@ public class URLRetrievePauseResumeTest extends ContainerAbstractTestCase { System.out.println("data=" + event); } else if (event instanceof IIncomingFileTransferReceivePausedEvent) { System.out.println("paused=" + event); + wasPaused.set(true); } else if (event instanceof IIncomingFileTransferReceiveDoneEvent) { closeOutputStream(); System.out.println("done=" + event); @@ -121,30 +139,44 @@ public class URLRetrievePauseResumeTest extends ContainerAbstractTestCase { notify.notify(); } } + if (session != null) { + long bytesReceived = session.getBytesReceived(); + downloaded.set(bytesReceived); + } } }; transferInstance.sendRetrieveRequest(FileIDFactory.getDefault().createFileID(transferInstance.getRetrieveNamespace(), url), listener, null); // Now if we can do pausing, then pause, wait a while and resume - if (pausable != null) { - Thread.sleep(500); - System.out.println("pausable.pause()=" + pausable.pause()); - System.out.println("Pausing " + PAUSE_TIME / 1000 + " seconds"); - Thread.sleep(PAUSE_TIME); - final boolean success = pausable.resume(); - System.out.println("pausable.resume()=" + success); - if (!success) { - System.out.println("session=" + session); - final Exception e = session.getException(); - System.out.println(" exception=" + e); - if (e != null) - e.printStackTrace(); - System.out.println(" isDone=" + session.isDone()); - return; - } - System.out.println(); + Thread.sleep(500); + assertNotNull("pausable is null", pausable); + System.out.println("pausable.pause()"); + boolean paused = pausable.pause(); + assertTrue("Pause failed", paused); + Thread.sleep(500); + assertTrue(wasPaused.get()); + long downloadedUntilPause = downloaded.get(); + System.out.println("Pausing " + PAUSE_TIME / 1000 + " seconds"); + Thread.sleep(PAUSE_TIME - 500); + long downloadedBeforeResume = session.getBytesReceived(); + assertEquals("Download continued before resume", downloadedUntilPause, downloadedBeforeResume); + final boolean success = pausable.resume(); + System.out.println("pausable.resume()=" + success); + if (!success) { + System.out.println("session=" + session); + final Exception e = session.getException(); + System.out.println(" exception=" + e); + if (e != null) + e.printStackTrace(); + System.out.println(" isDone=" + session.isDone()); + fail("Resume failed"); } + Thread.sleep(500); + assertTrue("Resume event was not received", wasResumed.get()); + long downloadedAfterResume = downloaded.get(); + assertTrue("Download continued before resume", downloadedAfterResume < (1+RESUMED_DOWNLOAD_AMOUNT_THRESHOLD) * downloadedUntilPause); + System.out.println(); synchronized (notify) { notify.wait(); @@ -153,6 +185,31 @@ public class URLRetrievePauseResumeTest extends ContainerAbstractTestCase { final Exception e = session.getException(); if (e != null) throw e; + + assertTrue("Nothing received after resume", downloaded.get() > downloadedUntilPause); + assertEquals("File corrupted", HTTP_RETRIEVE_MD5, downloadChecksum()); + Thread.sleep(500);//WIP + } + + private String downloadChecksum() throws IOException, NoSuchAlgorithmException { + MessageDigest md = MessageDigest.getInstance("MD5"); + DigestInputStream dis = null; + try + { + dis = new DigestInputStream(new FileInputStream(incomingFile), md); + byte[] buffer = new byte[1024]; + while(dis.read(buffer) != -1) {} + } finally { + if (dis != null) { + dis.close(); + } + } + byte[] digest = md.digest(); + StringBuilder sb = new StringBuilder(); + for (byte b : digest) { + sb.append(String.format("%02x", b)); + } + return sb.toString(); } public void testReceiveFile() throws Exception { -- cgit v1.2.3