[client] Rewrite netrules coloring and error message generation

This fixes some corder-case bugs and improves UX a bit by not popping up
500 message boxes if there are a lot of errors. It's probably still not
glitch free, but getting this right seems just a pita.

1 files changed, 150 insertions, 144 deletions

diff --git a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/control/NetrulesConfigurator.java b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/control/NetrulesConfigurator.java
index 67291c19..4603cffd 100644
--- a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/control/NetrulesConfigurator.java
+++ b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/control/NetrulesConfigurator.java
@@ -2,25 +2,29 @@ package org.openslx.dozmod.gui.control;
 
 import java.awt.Color;
 import java.awt.Insets;
+import java.awt.event.KeyAdapter;
+import java.awt.event.KeyEvent;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.EventListener;
 import java.util.EventObject;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 
 import javax.swing.BorderFactory;
 import javax.swing.JPanel;
 import javax.swing.JScrollPane;
 import javax.swing.JTextPane;
+import javax.swing.SwingUtilities;
 import javax.swing.UIManager;
 import javax.swing.event.EventListenerList;
 import javax.swing.text.BadLocationException;
+import javax.swing.text.DefaultStyledDocument;
 import javax.swing.text.SimpleAttributeSet;
 import javax.swing.text.StyleConstants;
-import javax.swing.text.StyledDocument;
 
 import org.apache.log4j.Logger;
 import org.openslx.bwlp.thrift.iface.NetDirection;
@@ -29,6 +33,7 @@ import org.openslx.dozmod.gui.Gui;
 import org.openslx.dozmod.gui.helper.GridManager;
 import org.openslx.dozmod.gui.helper.MessageType;
 import org.openslx.dozmod.gui.helper.TextChangeListener;
+import org.openslx.util.Util;
 
 /**
  * Widget for netrules configuration of lectures
@@ -46,6 +51,17 @@ public class NetrulesConfigurator extends NetrulesConfiguratorLayout {
 	 * -direction "in"
 	 */
 	private static final String FIELD_DELIMITER = "\\s+";
+	
+	private static final Color FOREGROUND_TEXT_COLOR;
+	
+	static {
+		Color fgOrigColor = UIManager.getDefaults().getColor("ColorChooser.foreground");
+		if (fgOrigColor == null) {
+			// use black as fallback
+			fgOrigColor = Color.BLACK;
+		}
+		FOREGROUND_TEXT_COLOR = fgOrigColor;
+	}
 
 	public NetrulesConfigurator() {
 		super();
@@ -53,12 +69,47 @@ public class NetrulesConfigurator extends NetrulesConfiguratorLayout {
 		final TextChangeListener docListener = new TextChangeListener() {
 			@Override
 			public void changed() {
-				fireNetrulesConfigurationChangeEvent(new NetrulesConfigurationChangeEvent(
-						new Object()));
+				fireNetrulesConfigurationChangeEvent(new NetrulesConfigurationChangeEvent(NetrulesConfigurator.this));
 			}
 		};
+		final SimpleAttributeSet as = new SimpleAttributeSet();
+		StyleConstants.setForeground(as, FOREGROUND_TEXT_COLOR);
 		tpNetworkRules.getDocument().addDocumentListener(docListener);
-
+		tpNetworkRules.addKeyListener(new KeyAdapter() {
+			@Override
+			public void keyTyped(KeyEvent e) {
+				SwingUtilities.invokeLater(new Runnable() {
+					@Override
+					public void run() {
+						int pos = tpNetworkRules.getCaretPosition();
+						if (pos < 0) {
+							return;
+						}
+						// Odd: On windows, getText() returns the text with CRLF line breaks,
+						// regardless of what you put into the text box.
+						// The offsets passed to setCharAttrs() below and the caret position
+						// you get from getCaretPosition() however have to adhere to the
+						// text version with just LF, exactly how we created the document.
+						String text = tpNetworkRules.getText().replace("\r", "");
+						if (pos >= text.length()) {
+							return;
+						}
+						int start = text.lastIndexOf('\n', pos == 0 ? 0 : pos - 1);
+						int end = text.indexOf('\n', pos);
+						if (start == -1) {
+							start = 0;
+						}
+						if (end == -1) {
+							end = text.length() - 1;
+						}
+						if (end <= start) {
+							return;
+						}
+						tpNetworkRules.getStyledDocument().setCharacterAttributes(start, end - start, as, true);
+					}
+				});
+			}
+		});
 	}
 
 	public boolean hasChanged() {
@@ -73,8 +124,7 @@ public class NetrulesConfigurator extends NetrulesConfiguratorLayout {
 	 */
 	public List<NetRule> getState() {
 		// cleanup the TextPane for network rules if needed
-		String input = tpNetworkRules.getText().trim();
-		return parseNetRules(input);
+		return parseNetRules(false);
 	}
 
 	/**
@@ -131,72 +181,61 @@ public class NetrulesConfigurator extends NetrulesConfiguratorLayout {
 	 * @return list of netrules if successful. If any errors occured while
 	 *         parsing, null is returned.
 	 */
-	public List<NetRule> parseNetRules(final String rawNetRules) {
-		if (rawNetRules == null)
-			return null;
+	public List<NetRule> parseNetRules(boolean silent) {
+		String rawNetRules = tpNetworkRules.getText().trim();
 		List<NetRule> rulesList = new ArrayList<NetRule>();
 		if (rawNetRules.isEmpty()) {
 			return rulesList;
 		}
-		// prune the text first
-		String prunedRawNetRules = rawNetRules.replaceAll("(?m)^\\s*", "");
-		prunedRawNetRules = prunedRawNetRules.replaceAll("(?m)\\s*$", "");
 
 		// split it line by line
-		List<String> netRules = Arrays.asList(prunedRawNetRules.split("\n"));
-		for (int i = 0; i < netRules.size(); i++) {
-			final String ruleLine = netRules.get(i);
-			if (ruleLine == null) {
-				continue;
-			}
-			if (ruleLine.isEmpty()) {
-				netRules.remove(i);
-				continue;
-			}
+		boolean invalid = false; // True if the rules are invalid and null should be returned
+		DefaultStyledDocument newdoc = new DefaultStyledDocument(); // Used to build new document with colors
+		StringBuilder errors = new StringBuilder(); // Error messages to show (if not silent)
+		int lineNo = 0; // Show line numbers in error messages
+		Set<String> warnedHosts = new HashSet<>(); // Ask only once about each unknown host
+		for (String ruleLine : rawNetRules.split("[\r\n]+")) {
+			Color lineColor = null;
 			LOGGER.debug("Parsing rule: " + ruleLine);
 			// split the fields and check if we have 3 as expected.
-			String[] fields = ruleLine.split(FIELD_DELIMITER);
+			String[] fields = ruleLine.trim().split(FIELD_DELIMITER);
 			if (fields.length != 3) {
-				markText(ruleLine, Color.RED);
+				lineNo += addLine(newdoc, ruleLine, Color.RED, true);
 				// log numbers for fields independently...
-				LOGGER.debug("Invalid number of fields! Expected 3, got: "
-						+ fields.length);
-				Gui.showMessageBox(
-						"Ungültige Syntax: Bitte definieren Sie Ihre Regel im Format: <host> <port> [in|out|both]",
-						MessageType.ERROR, null, null);
-				break;
+				LOGGER.debug("Invalid number of fields! Expected 3, got: " + fields.length);
+				if (fields.length > 3) {
+					errors.append("Zeile " + lineNo + ": Zu viele Felder.\n");
+				} else {
+					errors.append("Zeile " + lineNo + ": Zu wenig Felder.\n");
+				}
+				invalid = true;
+				continue;
 			}
+			// Have 3 fields, pretty up
+			String ruleDirection = fields[2].toUpperCase();
+			ruleLine = fields[0] + " \t " + fields[1] + " \t " + ruleDirection;
 
 			// start to check fields one by one from the last to the first ....
 			// check net direction: accept either 'in' or 'out' (case
 			// insensitive)
 			// TODO support combined 'in/out' rules
-			String ruleDirection = fields[2].toLowerCase();
-			if (!ruleDirection.matches("^(\\bin\\b|\\bout\\b)$")) {
-				markText(ruleLine, Color.RED);
-				LOGGER.debug("Invalid net direction! Expected 'in' or out'. Got: "
-						+ ruleDirection);
-				Gui.showMessageBox(
-						"Ungültige Richtung: Bitte nutzen Sie 'in' bzw. 'out'.",
-						MessageType.ERROR, null, null);
-				break;
-			}
-			// check port: accept if > -2 and < 65535
-			int port = -2;
-			try {
-				port = Integer.parseInt(fields[1]);
-			} catch (NumberFormatException e) {
-				LOGGER.error("Could not parse '" + fields[1] + "' to an int.");
+			if (!ruleDirection.equals("IN") && !ruleDirection.equals("OUT")) {
+				lineNo += addLine(newdoc, ruleLine, Color.RED, true);
+				LOGGER.debug("Invalid net direction! Expected 'in' or out'. Got: " + ruleDirection);
+				errors.append("Zeile " + lineNo + ": Ungültige Richtung. Bitte nutzen Sie 'IN' bzw. 'OUT'.\n");
+				invalid = true;
+				continue;
 			}
+			// check port: accept if >= 0 and <= 65535
+			int port = Util.parseInt(fields[1], -1);
 
-			// TODO: port = -1 for all ports?
-			if (port <= -2 || port > 65535) {
-				markText(ruleLine, Color.RED);
+			// port = 0 means match only host (all protocols and ports)
+			if (port < 0 || port > 65535) {
+				lineNo += addLine(newdoc, ruleLine, Color.RED, true);
 				LOGGER.debug("Invalid port! Got: " + port);
-				Gui.showMessageBox(
-						"Ungültiger Port! Bitte nutzen Sie einen Port aus dem Bereich [0-65536].",
-						MessageType.ERROR, null, null);
-				break;
+				errors.append("Zeile " + lineNo + ": Ungültiger Port. Gültiger Bereich ist 0-65535.\n");
+				invalid = true;
+				continue;
 			}
 			// check hostname: bit more to do here
 			// for IPs and/or resolvable hostnames we make use of java.net's
@@ -207,6 +246,8 @@ public class NetrulesConfigurator extends NetrulesConfiguratorLayout {
 			// try/catch-block
 			InetAddress ruleHost = null;
 			try {
+				// TODO: Find a way to reliably set a timeout (external DNS library?)
+				// Otherwise just remove this check
 				ruleHost = InetAddress.getByName(fields[0]);
 			} catch (UnknownHostException e) {
 				// might be good to see this exception in the log file
@@ -215,38 +256,59 @@ public class NetrulesConfigurator extends NetrulesConfiguratorLayout {
 			if (ruleHost == null) {
 				// either invalid IP-Address or an invalid resolvable hostname
 				// however it might also be a non-resolvable hostname that would
-				// be
-				// valid in the actual pool-rooms, so lets check its syntax
+				// be valid in the actual pool-rooms, so lets check its syntax
 				// according to: http://tools.ietf.org/html/rfc1034#section-3.1
 				LOGGER.debug("Invalid host/IP! Got: " + fields[0]);
-				if (checkHostnameSimple(fields[0])) {
-					markText(ruleLine, Color.ORANGE);
-					if (!Gui.showMessageBox(
-							"Konnte '"
-									+ fields[0]
+				String checkRes = checkHostnameSimple(fields[0]);
+				if (checkRes == null) {
+					if (!silent
+							&& !warnedHosts.contains(fields[0])
+							&& !Gui.showMessageBox("Konnte '" + fields[0]
 									+ "' nicht auflösen. Wollen Sie diesen Hostnamen trotzdem verwenden?",
-							MessageType.WARNING_RETRY, null, null)) {
-						break;
+									MessageType.WARNING_RETRY, null, null)) {
+						invalid = true;
 					}
+					warnedHosts.add(fields[0]);
+					lineColor = Color.ORANGE;
 				} else {
-					markText(ruleLine, Color.RED);
+					lineNo += addLine(newdoc, ruleLine, Color.RED, true);
+					errors.append("Zeile " + lineNo + ": " + checkRes + "\n");
+					invalid = true;
 					continue;
 				}
-			} else {
-				markText(ruleLine, Color.GREEN);
 			}
-			// valid, put it in the list
-			rulesList.add(new NetRule(NetDirection.valueOf(fields[2]
-					.toUpperCase()), fields[0], port));
+			// Made it to here - line is valid
+			lineNo += addLine(newdoc, ruleLine, lineColor, false);
+			rulesList.add(new NetRule(NetDirection.valueOf(ruleDirection), fields[0], port));
 		}
-		if (netRules.size() == rulesList.size()) {
-			// pruned rules were successfully parsed so they are valid: set the
-			// textpane to it
-			tpNetworkRules.setText(prunedRawNetRules);
-			LOGGER.debug("Success");
-			return rulesList;
+		if (!silent && errors.length() != 0) {
+			Gui.showMessageBox("Fehler beim Auswerten der angegebenen Netzwerkregeln.\n\n" + errors.toString()
+					+ "\nBitte geben Sie die Regeln zeilenweise im Format\n"
+					+ "<host> <port> <IN|OUT>\n"
+					+ "an.",
+					MessageType.ERROR, null, null);
 		}
-		return null;
+		tpNetworkRules.setDocument(newdoc);
+		if (invalid) {
+			return null;
+		}
+		// Success
+		return rulesList;
+	}
+
+	private int addLine(DefaultStyledDocument doc, String line, Color color, boolean bold) {
+		if (color == null) {
+			color = FOREGROUND_TEXT_COLOR;
+		}
+		SimpleAttributeSet attrs = new SimpleAttributeSet();
+		StyleConstants.setForeground(attrs, color);
+		StyleConstants.setBold(attrs, bold);
+		try {
+			doc.insertString(doc.getLength(), line + "\n", attrs);
+		} catch (BadLocationException e) {
+			LOGGER.warn("Cannot append to new textbox document", e);
+		}
+		return 1;
 	}
 
 	/**
@@ -258,86 +320,30 @@ public class NetrulesConfigurator extends NetrulesConfiguratorLayout {
 	 * can only contain digits, letters and hyphen. (Note: we also accept
 	 * forward slash to accept subnets!)
 	 * 
-	 * @param hostname
-	 *            as String to check the syntax of
-	 * @return
+	 * @param hostname the hostname to check for syntactical validity
+	 * @return null if valid, error string otherwise
 	 */
-	private boolean checkHostnameSimple(final String hostname) {
+	private String checkHostnameSimple(final String hostname) {
 		if (hostname.length() > 254) {
-			Gui.showMessageBox("Hostname ist zu lang!", MessageType.ERROR,
-					null, null);
-			return false;
+			return "Hostname ist zu lang.";
 		}
 		// split by '.' to get domain levels
+		boolean allNumeric = true;
 		String[] domainLabels = hostname.split("\\.");
 		for (String domainLabel : domainLabels) {
 			if (domainLabel.length() > 63) {
 				// fail since domain level should be max 63 chars
-				Gui.showMessageBox("Domain-Ebene '" + domainLabel
-						+ "' länger als 63 Zeichen!", MessageType.ERROR,
-						null, null);
-				return false;
+				return "Domain-Ebene '" + domainLabel + "' länger als 63 Zeichen.";
 			}
-			// length is ok, check for invalid characters
-			for (int i = 0; i < domainLabel.length(); i++) {
-				Character c = Character.valueOf(domainLabel.charAt(i));
-				// only accepts numbers, letters, hyphen
-				// forward slash as a special case to accept subnets...
-				if (!(Character.isDigit(c) || Character.isLetter(c)
-						|| c.equals('-') || c.equals('/'))) {
-					Gui.showMessageBox("Ungültiges Zeichen '" + c
-							+ "' in hostname!", MessageType.ERROR, null, null);
-					return false;
-				}
+			if (Util.parseInt(domainLabel, -1) == -1) {
+				allNumeric = false;
 			}
+			// checking for valid chars is pointless with punycode
 		}
-		return true;
-	}
-
-	/**
-	 * Searches the given txt within tpNetworkRules and changes its color to the
-	 * given color
-	 * 
-	 * @param txt
-	 *            text to search within the tpNetworkRules
-	 * @param color
-	 *            to set the given txt to
-	 */
-	// TODO still buggy with text colors: the marking works fine
-	// but when trying to input new text, funny things happen
-	private void markText(final String txt, final Color color) {
-		// get original foreground color
-		Color fgOrigColor = UIManager.getDefaults().getColor("ColorChooser.foreground");
-		if (fgOrigColor == null) {
-			// use white as fallback
-			fgOrigColor = Color.WHITE;
+		if (allNumeric && domainLabels.length != 4) {
+			return "Unvollständige IP-Adresse.";
 		}
-		SimpleAttributeSet set = new SimpleAttributeSet();
-		StyleConstants.setForeground(set, color);
-		StyleConstants.setBold(set, color == Color.red);
-		StyledDocument doc = tpNetworkRules.getStyledDocument();
-		try {
-			for (int pos = 0; pos < doc.getLength() - txt.length() + 1; ++pos) {
-				String current = doc.getText(pos, txt.length());
-				if (current.endsWith("\n")) {
-					current = current.substring(0, current.length() - 1);
-				}
-				if (current.equals(txt)) {
-					doc.setCharacterAttributes(pos, txt.length(), set, true);
-					break;
-				}
-			}
-		} catch (BadLocationException e) {
-			LOGGER.error(
-					"Failed to set '" + txt + "' to color " + color.toString(),
-					e);
-		}
-		// resetting the char attr to what they were before (buggy)
-
-		tpNetworkRules.setStyledDocument(doc);
-		StyleConstants.setForeground(set, fgOrigColor);
-		StyleConstants.setBold(set, false);
-		tpNetworkRules.setCharacterAttributes(set, true);
+		return null;
 	}
 
 	/**