From 4bd5b0d236fd048f9c938f9f018ba8cc5e8cd103 Mon Sep 17 00:00:00 2001 From: SquidDev Date: Sat, 29 Jul 2017 19:06:36 +0100 Subject: [PATCH] Remove HTTPTask, queueing the event when it has finished executing This means we don't have to have lots of shared state between the run and whenFinished method, and allows for easier chaining of futures later on. --- .../computercraft/core/apis/HTTPAPI.java | 30 +++--- .../core/apis/http/HTTPCheck.java | 25 ++--- .../core/apis/http/HTTPExecutor.java | 36 +++++++ .../core/apis/http/HTTPRequest.java | 95 ++++++------------- .../core/apis/http/HTTPTask.java | 61 ------------ 5 files changed, 86 insertions(+), 161 deletions(-) create mode 100644 src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java delete mode 100644 src/main/java/dan200/computercraft/core/apis/http/HTTPTask.java diff --git a/src/main/java/dan200/computercraft/core/apis/HTTPAPI.java b/src/main/java/dan200/computercraft/core/apis/HTTPAPI.java index 67ccc4413..925938629 100644 --- a/src/main/java/dan200/computercraft/core/apis/HTTPAPI.java +++ b/src/main/java/dan200/computercraft/core/apis/HTTPAPI.java @@ -10,18 +10,19 @@ import dan200.computercraft.api.lua.ILuaContext; import dan200.computercraft.api.lua.LuaException; import dan200.computercraft.core.apis.http.HTTPCheck; import dan200.computercraft.core.apis.http.HTTPRequest; -import dan200.computercraft.core.apis.http.HTTPTask; +import dan200.computercraft.core.apis.http.HTTPExecutor; import javax.annotation.Nonnull; import java.net.URL; import java.util.*; +import java.util.concurrent.Future; import static dan200.computercraft.core.apis.ArgumentHelper.*; public class HTTPAPI implements ILuaAPI { private final IAPIEnvironment m_apiEnvironment; - private final List m_httpTasks; + private final List> m_httpTasks; public HTTPAPI( IAPIEnvironment environment ) { @@ -48,15 +49,11 @@ public class HTTPAPI implements ILuaAPI // Wait for all of our http requests synchronized( m_httpTasks ) { - Iterator it = m_httpTasks.iterator(); + Iterator> it = m_httpTasks.iterator(); while( it.hasNext() ) { - final HTTPTask h = it.next(); - if( h.isFinished() ) - { - h.whenFinished( m_apiEnvironment ); - it.remove(); - } + final Future h = it.next(); + if( h.isDone() ) it.remove(); } } } @@ -66,9 +63,9 @@ public class HTTPAPI implements ILuaAPI { synchronized( m_httpTasks ) { - for( HTTPTask r : m_httpTasks ) + for( Future r : m_httpTasks ) { - r.cancel(); + r.cancel( false ); } m_httpTasks.clear(); } @@ -125,10 +122,10 @@ public class HTTPAPI implements ILuaAPI try { URL url = HTTPRequest.checkURL( urlString ); - HTTPRequest request = new HTTPRequest( urlString, url, postString, headers, binary ); + HTTPRequest request = new HTTPRequest( m_apiEnvironment, urlString, url, postString, headers, binary ); synchronized( m_httpTasks ) { - m_httpTasks.add( HTTPTask.submit( request ) ); + m_httpTasks.add( HTTPExecutor.EXECUTOR.submit( request ) ); } return new Object[] { true }; } @@ -147,9 +144,10 @@ public class HTTPAPI implements ILuaAPI try { URL url = HTTPRequest.checkURL( urlString ); - HTTPCheck check = new HTTPCheck( urlString, url ); - synchronized( m_httpTasks ) { - m_httpTasks.add( HTTPTask.submit( check ) ); + HTTPCheck check = new HTTPCheck( m_apiEnvironment, urlString, url ); + synchronized( m_httpTasks ) + { + m_httpTasks.add( HTTPExecutor.EXECUTOR.submit( check ) ); } return new Object[] { true }; } diff --git a/src/main/java/dan200/computercraft/core/apis/http/HTTPCheck.java b/src/main/java/dan200/computercraft/core/apis/http/HTTPCheck.java index ee3da0072..401d11e95 100644 --- a/src/main/java/dan200/computercraft/core/apis/http/HTTPCheck.java +++ b/src/main/java/dan200/computercraft/core/apis/http/HTTPCheck.java @@ -5,14 +5,15 @@ import dan200.computercraft.core.apis.IAPIEnvironment; import java.net.URL; -public class HTTPCheck implements HTTPTask.IHTTPTask +public class HTTPCheck implements Runnable { + private final IAPIEnvironment environment; private final String urlString; private final URL url; - private String error; - public HTTPCheck( String urlString, URL url ) + public HTTPCheck( IAPIEnvironment environment, String urlString, URL url ) { + this.environment = environment; this.urlString = urlString; this.url = url; } @@ -22,24 +23,12 @@ public class HTTPCheck implements HTTPTask.IHTTPTask { try { - HTTPRequest.checkHost( url ); + HTTPRequest.checkHost( url.getHost() ); + environment.queueEvent( "http_check", new Object[] { urlString, true } ); } catch( HTTPRequestException e ) { - error = e.getMessage(); - } - } - - @Override - public void whenFinished( IAPIEnvironment environment ) - { - if( error == null ) - { - environment.queueEvent( "http_check", new Object[] { urlString, true } ); - } - else - { - environment.queueEvent( "http_check", new Object[] { urlString, false, error } ); + environment.queueEvent( "http_check", new Object[] { urlString, false, e.getMessage() } ); } } } diff --git a/src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java b/src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java new file mode 100644 index 000000000..75c44eb05 --- /dev/null +++ b/src/main/java/dan200/computercraft/core/apis/http/HTTPExecutor.java @@ -0,0 +1,36 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2017. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ + +package dan200.computercraft.core.apis.http; + +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.ThreadFactoryBuilder; + +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +/** + * Just a shared object for executing simple HTTP related tasks. + */ +public final class HTTPExecutor +{ + public static final ListeningExecutorService EXECUTOR = MoreExecutors.listeningDecorator( new ThreadPoolExecutor( + 4, Integer.MAX_VALUE, + 60L, TimeUnit.SECONDS, + new SynchronousQueue(), + new ThreadFactoryBuilder() + .setDaemon( true ) + .setPriority( Thread.MIN_PRIORITY + (Thread.NORM_PRIORITY - Thread.MIN_PRIORITY) / 2 ) + .setNameFormat( "ComputerCraft-HTTP-%d" ) + .build() + ) ); + + private HTTPExecutor() + { + } +} diff --git a/src/main/java/dan200/computercraft/core/apis/http/HTTPRequest.java b/src/main/java/dan200/computercraft/core/apis/http/HTTPRequest.java index ff106af4b..a12929091 100644 --- a/src/main/java/dan200/computercraft/core/apis/http/HTTPRequest.java +++ b/src/main/java/dan200/computercraft/core/apis/http/HTTPRequest.java @@ -25,7 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -public class HTTPRequest implements HTTPTask.IHTTPTask +public class HTTPRequest implements Runnable { public static URL checkURL( String urlString ) throws HTTPRequestException { @@ -55,11 +55,11 @@ public class HTTPRequest implements HTTPTask.IHTTPTask return url; } - public static InetAddress checkHost( URL url ) throws HTTPRequestException + public static InetAddress checkHost( String host ) throws HTTPRequestException { try { - InetAddress resolved = InetAddress.getByName( url.getHost() ); + InetAddress resolved = InetAddress.getByName( host ); if( !ComputerCraft.http_whitelist.matches( resolved ) || ComputerCraft.http_blacklist.matches( resolved ) ) { throw new HTTPRequestException( "Domain not permitted" ); @@ -73,22 +73,16 @@ public class HTTPRequest implements HTTPTask.IHTTPTask } } + private final IAPIEnvironment m_environment; private final URL m_url; private final String m_urlString; private final String m_postText; private final Map m_headers; - - private boolean m_success = false; - private String m_encoding; - private byte[] m_result; private boolean m_binary; - private int m_responseCode = -1; - private Map m_responseHeaders; - private String m_errorMessage; - public HTTPRequest( String urlString, URL url, final String postText, final Map headers, boolean binary ) throws HTTPRequestException + public HTTPRequest( IAPIEnvironment environment, String urlString, URL url, final String postText, final Map headers, boolean binary ) throws HTTPRequestException { - // Parse the URL + m_environment = environment; m_urlString = urlString; m_url = url; m_binary = binary; @@ -96,28 +90,19 @@ public class HTTPRequest implements HTTPTask.IHTTPTask m_headers = headers; } - public InputStream getContents() - { - byte[] result = m_result; - if( result != null ) - { - return new ByteArrayInputStream( result ); - } - return null; - } - @Override public void run() { // First verify the address is allowed. try { - checkHost( m_url ); + checkHost( m_url.getHost() ); } catch( HTTPRequestException e ) { - m_success = false; - m_errorMessage = e.getMessage(); + // Queue the failure event if not. + String error = e.getMessage(); + m_environment.queueEvent( "http_failure", new Object[] { m_urlString, error == null ? "Could not connect" : error, null } ); return; } @@ -186,58 +171,36 @@ public class HTTPRequest implements HTTPTask.IHTTPTask byte[] result = ByteStreams.toByteArray( is ); is.close(); - // We completed - m_success = responseSuccess; - m_result = result; - m_responseCode = connection.getResponseCode(); - m_encoding = connection.getContentEncoding(); - + // We've got some sort of response, so let's build a resulting object. Joiner joiner = Joiner.on( ',' ); - Map headers = m_responseHeaders = new HashMap(); + Map headers = new HashMap(); for( Map.Entry> header : connection.getHeaderFields().entrySet() ) { headers.put( header.getKey(), joiner.join( header.getValue() ) ); } + InputStream contents = new ByteArrayInputStream( result ); + ILuaObject stream = wrapStream( + m_binary ? new BinaryInputHandle( contents ) : new EncodedInputHandle( contents, connection.getContentEncoding() ), + connection.getResponseCode(), headers + ); + connection.disconnect(); // disconnect + + // Queue the appropriate event. + if( responseSuccess ) + { + m_environment.queueEvent( "http_success", new Object[] { m_urlString, stream } ); + } + else + { + m_environment.queueEvent( "http_failure", new Object[] { m_urlString, "Could not connect", stream } ); + } } catch( IOException e ) { // There was an error - m_success = false; - } - } - - @Override - public void whenFinished( IAPIEnvironment environment ) - { - final String url = m_urlString; - if( m_success ) - { - // Queue the "http_success" event - InputStream contents = getContents(); - Object result = wrapStream( - m_binary ? new BinaryInputHandle( contents ) : new EncodedInputHandle( contents, m_encoding ), - m_responseCode, m_responseHeaders - ); - environment.queueEvent( "http_success", new Object[] { url, result } ); - } - else - { - // Queue the "http_failure" event - String error = "Could not connect"; - if( m_errorMessage != null ) error = m_errorMessage; - - InputStream contents = getContents(); - Object result = null; - if( contents != null ) - { - result = wrapStream( - m_binary ? new BinaryInputHandle( contents ) : new EncodedInputHandle( contents, m_encoding ), - m_responseCode, m_responseHeaders - ); - } - environment.queueEvent( "http_failure", new Object[] { url, error, result } ); + m_environment.queueEvent( "http_failure", new Object[] { m_urlString, "Could not connect", null } ); } } diff --git a/src/main/java/dan200/computercraft/core/apis/http/HTTPTask.java b/src/main/java/dan200/computercraft/core/apis/http/HTTPTask.java deleted file mode 100644 index c510bf5d5..000000000 --- a/src/main/java/dan200/computercraft/core/apis/http/HTTPTask.java +++ /dev/null @@ -1,61 +0,0 @@ -package dan200.computercraft.core.apis.http; - -import com.google.common.util.concurrent.ThreadFactoryBuilder; -import dan200.computercraft.core.apis.IAPIEnvironment; - -import java.util.concurrent.*; - -/** - * A task which executes asynchronously on a new thread. - * - * This functions very similarly to a {@link Future}, but with an additional - * method which is called on the main thread when the task is completed. - */ -public class HTTPTask -{ - public interface IHTTPTask extends Runnable - { - void whenFinished( IAPIEnvironment environment ); - } - - private static final ExecutorService httpThreads = new ThreadPoolExecutor( - 4, Integer.MAX_VALUE, - 60L, TimeUnit.SECONDS, - new SynchronousQueue(), - new ThreadFactoryBuilder() - .setDaemon( true ) - .setPriority( Thread.MIN_PRIORITY + (Thread.NORM_PRIORITY - Thread.MIN_PRIORITY) / 2 ) - .setNameFormat( "ComputerCraft-HTTP-%d" ) - .build() - ); - - private final Future future; - private final IHTTPTask task; - - private HTTPTask( Future future, IHTTPTask task ) - { - this.future = future; - this.task = task; - } - - public static HTTPTask submit( IHTTPTask task ) - { - Future future = httpThreads.submit( task ); - return new HTTPTask( future, task ); - } - - public void cancel() - { - future.cancel( false ); - } - - public boolean isFinished() - { - return future.isDone(); - } - - public void whenFinished( IAPIEnvironment environment ) - { - task.whenFinished( environment ); - } -}