From 2e431894bf999e70c368263ce0ab6132cac2bb17 Mon Sep 17 00:00:00 2001 From: GideonLeGrange Date: Mon, 8 Sep 2014 13:10:18 +0200 Subject: [PATCH] Some small cleanups and refactoring. Fixed minor potential concurrency issue --- .../me/legrange/mikrotik/ApiConnection.java | 12 ++++- .../mikrotik/impl/ApiCommandException.java | 3 +- .../mikrotik/impl/ApiConnectionImpl.java | 47 ++++++++++--------- .../me/legrange/mikrotik/impl/Command.java | 8 ++-- .../me/legrange/mikrotik/impl/Result.java | 14 +++++- .../legrange/mikrotik/impl/ScanException.java | 2 - 6 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/main/java/me/legrange/mikrotik/ApiConnection.java b/src/main/java/me/legrange/mikrotik/ApiConnection.java index cedd9eb..29fa64c 100644 --- a/src/main/java/me/legrange/mikrotik/ApiConnection.java +++ b/src/main/java/me/legrange/mikrotik/ApiConnection.java @@ -17,6 +17,7 @@ public abstract class ApiConnection { * @param host The host to which to connect. * @param port The TCP port to use. * @return The ApiConnection + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ public static ApiConnection connectTLS(String host, int port) throws MikrotikApiException { return ApiConnectionImpl.connect(host, port, true); @@ -27,6 +28,7 @@ public abstract class ApiConnection { * Create a new API connection to the give device on the default API port, using anonymous TLS for encryption. * @param host The host to which to connect. * @return The ApiConnection + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ public static ApiConnection connectTLS(String host) throws MikrotikApiException { return ApiConnectionImpl.connect(host, DEFAULT_TLS_PORT, true); @@ -38,6 +40,7 @@ public abstract class ApiConnection { * @param host The host to which to connect. * @param port The TCP port to use. * @return The ApiConnection + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ public static ApiConnection connect(String host, int port) throws MikrotikApiException { return ApiConnectionImpl.connect(host, port, false); @@ -47,6 +50,7 @@ public abstract class ApiConnection { * Create a new API connection to the give device on the default API port. * @param host The host to which to connect. * @return The ApiConnection + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem connecting */ public static ApiConnection connect(String host) throws MikrotikApiException { return connect(host, DEFAULT_PORT); @@ -61,6 +65,7 @@ public abstract class ApiConnection { /** * Disconnect from the remote API + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem disconnecting */ public abstract void disconnect() throws MikrotikApiException; @@ -69,12 +74,15 @@ public abstract class ApiConnection { * * @param username - username of the user on the router * @param password - password for the user + * @throws me.legrange.mikrotik.MikrotikApiException + * @throws java.lang.InterruptedException */ public abstract void login(String username, String password) throws MikrotikApiException, InterruptedException; /** execute a command and return a list of results. * @param cmd Command to execute * @return The list of results + * @throws me.legrange.mikrotik.MikrotikApiException */ public abstract List> execute(String cmd) throws MikrotikApiException; @@ -87,7 +95,9 @@ public abstract class ApiConnection { */ public abstract String execute(String cmd, ResultListener lis) throws MikrotikApiException; - /** cancel a command */ + /** cancel a command + * @param tag The tag of the command to cancel + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if there is a problem canceling the command */ public abstract void cancel(String tag) throws MikrotikApiException; /** default TCP port used by Mikrotik API */ diff --git a/src/main/java/me/legrange/mikrotik/impl/ApiCommandException.java b/src/main/java/me/legrange/mikrotik/impl/ApiCommandException.java index 5c4bb78..f694444 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ApiCommandException.java +++ b/src/main/java/me/legrange/mikrotik/impl/ApiCommandException.java @@ -9,7 +9,8 @@ import me.legrange.mikrotik.MikrotikApiException; public class ApiCommandException extends MikrotikApiException { - /** return the tag associated with this exception, if there is one */ + /** return the tag associated with this exception, if there is one + * @return the tag associated with this exception. Null if there is no tag*/ public String getTag() { return tag; } diff --git a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java index 88450dc..e78b4f8 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java +++ b/src/main/java/me/legrange/mikrotik/impl/ApiConnectionImpl.java @@ -1,6 +1,5 @@ package me.legrange.mikrotik.impl; -import me.legrange.mikrotik.*; import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; @@ -9,14 +8,17 @@ import java.net.InetAddress; import java.net.Socket; import java.net.UnknownHostException; import java.util.Arrays; -import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.LinkedBlockingQueue; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; -//import org.bouncycastle.jce.provider.BouncyCastleProvider; +import me.legrange.mikrotik.ApiConnection; +import me.legrange.mikrotik.ApiConnectionException; +import me.legrange.mikrotik.MikrotikApiException; +import me.legrange.mikrotik.ResultListener; /** * The Mikrotik API connection implementation. This is the class used to connect @@ -77,12 +79,15 @@ public final class ApiConnectionImpl extends ApiConnection { * * @param username - username of the user on the router * @param password - password for the user + * @throws me.legrange.mikrotik.MikrotikApiException + * @throws java.lang.InterruptedException */ + @Override public void login(String username, String password) throws MikrotikApiException, InterruptedException { List> list = execute("/login"); Map res = list.get(0); String hash = res.get("ret"); - String chal = Util.hexStrToStr("00") + new String(makePass(password)) + Util.hexStrToStr(hash); + String chal = Util.hexStrToStr("00") + new String(password.toCharArray()) + Util.hexStrToStr(hash); chal = Util.hashMD5(chal); execute("/login name=" + username + " response=00" + chal); } @@ -92,7 +97,9 @@ public final class ApiConnectionImpl extends ApiConnection { * * @param cmd Command to execute * @return The list of results + * @throws me.legrange.mikrotik.MikrotikApiException */ + @Override public List> execute(String cmd) throws MikrotikApiException { return execute(Parser.parse(cmd)); } @@ -105,13 +112,17 @@ public final class ApiConnectionImpl extends ApiConnection { * @return A command object that can be used to cancel the command. * @throws MikrotikApiException */ + @Override public String execute(String cmd, ResultListener lis) throws MikrotikApiException { return execute(Parser.parse(cmd), lis); } /** * cancel a command + * @param tag + * @throws me.legrange.mikrotik.MikrotikApiException Thrown if an error is experienced while canceling the */ + @Override public void cancel(String tag) throws MikrotikApiException { execute(String.format("/cancel tag=%s", tag)); } @@ -137,6 +148,7 @@ public final class ApiConnectionImpl extends ApiConnection { } private ApiConnectionImpl() { + this.listeners = new ConcurrentHashMap<>(); } /** @@ -180,7 +192,7 @@ public final class ApiConnectionImpl extends ApiConnection { */ private Socket openSSLSocket(InetAddress ia, int port) throws IOException { SSLSocket ssl = (SSLSocket) SSLSocketFactory.getDefault().createSocket(ia, port); - List cs = new LinkedList(); + List cs = new LinkedList<>(); // not happy with this code. Without it, SSL throws a "Remote host closed connection during handshake" error // caused by a "SSL peer shut down incorrectly" error for (String s : ssl.getSupportedCipherSuites()) { @@ -192,16 +204,6 @@ public final class ApiConnectionImpl extends ApiConnection { return ssl; } - private char[] makePass(String pass) { - if (true) { - return pass.toCharArray(); - } - char[] res = new char[pass.length() + 1]; - System.arraycopy(pass.toCharArray(), 0, res, 0, pass.length()); - res[pass.length()] = 0x0; - return res; - } - private synchronized String nextTag() { _tag++; return Integer.toHexString(_tag); @@ -213,7 +215,7 @@ public final class ApiConnectionImpl extends ApiConnection { private boolean connected = false; private Reader reader; private Processor processor; - private final Map listeners = new HashMap(); + private final Map listeners; private Integer _tag = 0; /** @@ -253,12 +255,11 @@ public final class ApiConnectionImpl extends ApiConnection { queue.put(ex); } catch (InterruptedException ex2) { } - } catch (ApiConnectionException ex) { - } catch (InterruptedException ex1) { + } catch (ApiConnectionException | InterruptedException ex) { } } } - private LinkedBlockingQueue queue = new LinkedBlockingQueue(40); + private final LinkedBlockingQueue queue = new LinkedBlockingQueue(40); } /** @@ -280,7 +281,6 @@ public final class ApiConnectionImpl extends ApiConnection { continue; } } catch (MikrotikApiException ex) { - ex.printStackTrace(); continue; } ResultListener l = listeners.get(res.getTag()); @@ -443,17 +443,19 @@ public final class ApiConnectionImpl extends ApiConnection { } } } - private List lines = new LinkedList(); + private final List lines = new LinkedList<>(); private String line; } private class SyncListener implements ResultListener { + @Override public synchronized void error(MikrotikApiException ex) { this.err = ex; notify(); } + @Override public synchronized void completed() { notify(); } @@ -467,6 +469,7 @@ public final class ApiConnectionImpl extends ApiConnection { notify(); } + @Override public void receive(Map result) { results.add(result); } @@ -486,7 +489,7 @@ public final class ApiConnectionImpl extends ApiConnection { } return results; } - private List> results = new LinkedList>(); + private final List> results = new LinkedList<>(); private MikrotikApiException err; } } diff --git a/src/main/java/me/legrange/mikrotik/impl/Command.java b/src/main/java/me/legrange/mikrotik/impl/Command.java index 72ec06b..23430f7 100644 --- a/src/main/java/me/legrange/mikrotik/impl/Command.java +++ b/src/main/java/me/legrange/mikrotik/impl/Command.java @@ -73,9 +73,9 @@ class Command { List getParameters() { return params; } - private String cmd; - private List params = new LinkedList(); - private List queries = new LinkedList(); - private List properties = new LinkedList(); + private final String cmd; + private final List params = new LinkedList<>(); + private final List queries = new LinkedList<>(); + private final List properties = new LinkedList<>(); private String tag; } diff --git a/src/main/java/me/legrange/mikrotik/impl/Result.java b/src/main/java/me/legrange/mikrotik/impl/Result.java index 847c9b3..38dbbee 100644 --- a/src/main/java/me/legrange/mikrotik/impl/Result.java +++ b/src/main/java/me/legrange/mikrotik/impl/Result.java @@ -15,6 +15,7 @@ class Result extends Response implements Map { return map.get(key); } + @Override public boolean isEmpty() { return map.isEmpty(); } @@ -24,53 +25,64 @@ class Result extends Response implements Map { return String.format("tag=%s, data=%s", getTag(), map); } + @Override public int size() { return map.size(); } + @Override public boolean containsKey(Object o) { return map.containsKey(o); } + @Override public boolean containsValue(Object o) { return map.containsValue(o); } + @Override public String get(Object o) { return map.get(o); } + @Override public String put(String k, String v) { return map.put(k, v); } + @Override public String remove(Object o) { return map.remove(o); } + @Override public void putAll(Map map) { this.map.putAll(map); } + @Override public void clear() { map.clear(); } + @Override public Set keySet() { return map.keySet(); } + @Override public Collection values() { return map.values(); } + @Override public Set> entrySet() { return map.entrySet(); } Result() { super(null); - this.map = new HashMap(); + this.map = new HashMap<>(); } private final Map map; diff --git a/src/main/java/me/legrange/mikrotik/impl/ScanException.java b/src/main/java/me/legrange/mikrotik/impl/ScanException.java index 1463f2d..01b6cb0 100644 --- a/src/main/java/me/legrange/mikrotik/impl/ScanException.java +++ b/src/main/java/me/legrange/mikrotik/impl/ScanException.java @@ -1,7 +1,5 @@ package me.legrange.mikrotik.impl; -import me.legrange.mikrotik.MikrotikApiException; - /** * Exception thrown if the scanner encounters an error while scanning a command line. * @author GideonLeGrange