Improved blacklist entries editing operations :

- Fixes issue #160 : handle properly syntax exceptions with a user
friendly message
- Fixes loss of information on multiple blacklist entries editions
- Fixes loss of entries when moving entries from one list to another

htroot/Blacklist_p.html

@@ -69,6 +69,9 @@
         <legend>Edit list <b><em>#[currentBlacklist]#</em></b></legend>
       <!-- Blacklist configuration -->
       #(edit)#
+      
+      #(addError)#::<div class="alert alert-danger" role="alert">Could not add entry <code>#[entry]#</code>. Please check syntax.</div>#(/addError)#
+      #(moveError)#::<div class="alert alert-danger" role="alert">An error occurred while moving entries to the target list.</div>#(/moveError)#
 
       <form action="Blacklist_p.html" method="post" enctype="multipart/form-data" accept-charset="UTF-8">
         <p>Add new pattern:</p>
@@ -146,6 +149,15 @@
 
       ::
       <p>Edit existing pattern(s):</p>
+      #(editError)#::<div class="alert alert-danger" role="alert">An error occurred while editing the following entries. Please check syntax.
+      	<ul>
+      	  #{list}#
+          	<li><code>#[item]#</code></li>
+          #{/list}#
+          #(hasMore)#::<li>#[more]# more...</li>#(/hasMore)#
+        </ul>
+      </div>
+      #(/editError)#
       <form name="editBlacklistEntry" action="Blacklist_p.html" method="post" enctype="multipart/form-data">
         <div>
           <input type="hidden" name="currentBlacklist" value="#[currentBlacklist]#" />

htroot/Blacklist_p.java

@@ -32,7 +32,10 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import net.yacy.cora.protocol.RequestHeader;
 import net.yacy.cora.util.ConcurrentLog;
@@ -234,10 +237,9 @@ public class Blacklist_p {
                 		WorkTables.TABLE_API_TYPE_CONFIGURATION,
                 		"add to blacklist '" + blacklistToUse + "': " + blentry);
 
-                final String temp = BlacklistHelper.addBlacklistEntry(blacklistToUse, blentry, header);
+                if(!BlacklistHelper.addBlacklistEntry(blacklistToUse, blentry, header)) {
-                if (temp != null) {
+                	prop.put(DISABLED + EDIT + "addError", true);
-                    prop.put(serverObjects.ACTION_LOCATION, temp);
+                	prop.put(DISABLED + EDIT + "addError_entry", blentry);
-                    return prop;
                 }
 
                 Switchboard.urlBlacklist.clear();
@@ -270,15 +272,15 @@ public class Blacklist_p {
                         !targetBlacklist.equals(blacklistToUse)) {
                     String temp;
                     for (final String selectedBlacklistEntry : selectedBlacklistEntries) {
-                        if ((temp = BlacklistHelper.addBlacklistEntry(targetBlacklist, selectedBlacklistEntry, header)) != null) {
+                    	/* Removal must be done first, otherwise add operation will not be performed because the entry will be detected as already present */
-                            prop.put(serverObjects.ACTION_LOCATION, temp);
-                            return prop;
-                        }
-
                         if ((temp = BlacklistHelper.deleteBlacklistEntry(blacklistToUse, selectedBlacklistEntry, header)) != null) {
                             prop.put(serverObjects.ACTION_LOCATION, temp);
                             return prop;
-
+                        }
+                        
+                        if (!BlacklistHelper.addBlacklistEntry(targetBlacklist, selectedBlacklistEntry, header)) {
+                        	prop.put(DISABLED + EDIT + "moveError", true);
+                        	break;
                         }
                     }
                 }
@@ -294,30 +296,29 @@ public class Blacklist_p {
 
                 blacklistToUse = post.get("currentBlacklist", "").trim();
 
-                final String[] editedBlacklistEntries = post.getAll("editedBlacklistEntry.*");
+                final Map<String, String> editedBlacklistEntries = post.getMatchingEntries("editedBlacklistEntry.*");
 
                 // if edited entry has been posted, save changes
-                if (editedBlacklistEntries.length > 0) {
+                if (editedBlacklistEntries.size() > 0) {
 
-                    final String[] selectedBlacklistEntries = post.getAll("selectedBlacklistEntry.*");
+                    final Map<String, String> selectedBlacklistEntries = post.getMatchingEntries("selectedBlacklistEntry.*");
 
-                    if (selectedBlacklistEntries.length != editedBlacklistEntries.length) {
+                    if (selectedBlacklistEntries.size() != editedBlacklistEntries.size()) {
                         prop.put(serverObjects.ACTION_LOCATION, "");
                         return prop;
                     }
 
                     String temp = null;
+                    final HashMap<String, String> selected2EditedErrors = new HashMap<>();
+                    for (final Entry<String, String> selectedEntry : selectedBlacklistEntries.entrySet()) {
 
-                    for (int i = 0; i < selectedBlacklistEntries.length; i++) {
+                    	final String editedEntryValue = editedBlacklistEntries.get(selectedEntry.getKey().replace("selectedBlacklistEntry.", "editedBlacklistEntry."));
-
+                        if (!selectedEntry.getValue().equals(editedEntryValue)) {
-                        if (!selectedBlacklistEntries[i].equals(editedBlacklistEntries[i])) {
-
-                            if ((temp = BlacklistHelper.deleteBlacklistEntry(blacklistToUse, selectedBlacklistEntries[i], header)) != null) {
-                                prop.put(serverObjects.ACTION_LOCATION, temp);
-                                return prop;
-                            }
 
-                            if ((temp = BlacklistHelper.addBlacklistEntry(blacklistToUse, editedBlacklistEntries[i], header)) != null) {
+                        	/* Add first, to detect any eventual syntax errors before removing the old entry */
+                            if (!BlacklistHelper.addBlacklistEntry(blacklistToUse, editedEntryValue, header)) {
+                            	selected2EditedErrors.put(selectedEntry.getValue(), editedEntryValue);
+                            } else if ((temp = BlacklistHelper.deleteBlacklistEntry(blacklistToUse, selectedEntry.getValue(), header)) != null) {
                                 prop.put(serverObjects.ACTION_LOCATION, temp);
                                 return prop;
                             }
@@ -326,7 +327,39 @@ public class Blacklist_p {
 
                     Switchboard.urlBlacklist.clear();
                     ListManager.reloadBlacklists();
-                    prop.putHTML(DISABLED + EDIT + "currentBlacklist", blacklistToUse);
+                    if(selected2EditedErrors.isEmpty()) {
+                        prop.putHTML(DISABLED + EDIT + "currentBlacklist", blacklistToUse);
+                    } else {
+                    	/* At least one error occurred : display again entries with errors for edition */
+                    	prop.put(DISABLED + EDIT + "editError", true);
+                    	
+                    	final int maxDisplayedErrors = 10;
+                    	
+                    	int i = 0;
+                        for (final Entry<String, String> selected2Edited : selected2EditedErrors.entrySet()) {
+                        	/* We do not use here putHTML as we don't want '+' characters to be interpreted as application/x-www-form-urlencoded encoding */
+                            prop.put(DISABLED + EDIT + "editList_" + i + "_item", CharacterCoding.unicode2html(selected2Edited.getKey(), true));
+                            prop.put(DISABLED + EDIT + "editList_" + i + "_count", i);
+                            
+                        	/* We do not use here putHTML as we don't want '+' characters to be interpreted as application/x-www-form-urlencoded encoding */
+                            if(i < maxDisplayedErrors) {
+                            	prop.put(DISABLED + EDIT + "editError_list_" + i + "_item", CharacterCoding.unicode2html(selected2Edited.getValue(), true));
+                            }
+                            i++;
+                        }
+						if (selected2EditedErrors.size() > maxDisplayedErrors) {
+							prop.put(DISABLED + EDIT + "editError_hasMore", true);
+							prop.put(DISABLED + EDIT + "editError_hasMore_more",
+									selected2EditedErrors.size() - maxDisplayedErrors);
+						} else {
+							prop.put(DISABLED + EDIT + "editError_hasMore", false);
+						}
+                        prop.putHTML(DISABLED + EDIT + "currentBlacklist", blacklistToUse);
+                        prop.put(DISABLED + "edit", "1");
+                        prop.put(DISABLED + EDIT + "editList", selected2EditedErrors.size());
+                        prop.put(DISABLED + EDIT + "editError_list", Math.min(maxDisplayedErrors, selected2EditedErrors.size()));
+                    }
+                    
 
                 // else return entry to be edited
                 } else {

htroot/IndexControlRWIs_p.java

@@ -32,6 +32,7 @@ import java.util.Collection;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
+import java.util.regex.PatternSyntaxException;
 
 import net.yacy.cora.date.GenericFormatter;
 import net.yacy.cora.document.analysis.Classification.ContentDomain;
@@ -448,7 +449,7 @@ public class IndexControlRWIs_p {
                                                 blacklist,
                                                 url.getHost(),
                                                 ".*");
-                                        } catch (PunycodeException e) {
+                                        } catch (final PunycodeException | PatternSyntaxException e) {
                                             ConcurrentLog.warn(APP_NAME,
                                                             "Unable to add blacklist entry to blacklist "
                                                                             + supportedBlacklistType, e);

htroot/api/blacklists/add_entry_p.java

@@ -31,7 +31,7 @@ public class add_entry_p {
             		WorkTables.TABLE_API_TYPE_CONFIGURATION,
 					"add to blacklist '" + blacklistToUse + "': " + entry);
             
-            if (BlacklistHelper.addBlacklistEntry(blacklistToUse, entry, header) == null) {
+            if (BlacklistHelper.addBlacklistEntry(blacklistToUse, entry, header)) {
                 prop.put(XML_ITEM_STATUS, RESULT_SUCCESS);
 
                 Switchboard.urlBlacklist.clear();

source/net/yacy/repository/Blacklist.java

@@ -329,10 +329,11 @@ public class Blacklist {
 	 *            source file
 	 * @param items
 	 *            blacklist host/path items to add
-	 * @throws PunycodeException
+	 * @throws PunycodeException when a entry domain name could not be Punycode encoded
+	 * @throws PatternSyntaxException when an entry regular expression is not valid
 	 */
 	public final void add(final BlacklistType blacklistType, final String blacklistToUse,
-			final Collection<BlacklistHostAndPath> items) throws PunycodeException {
+			final Collection<BlacklistHostAndPath> items) throws PunycodeException, PatternSyntaxException {
 
 		if (items != null) {
 			PrintWriter pw = null;
@@ -409,9 +410,11 @@ public class Blacklist {
      * @param blacklistToUse source file
      * @param host
      * @param path
-     * @throws PunycodeException
+	 * @throws PunycodeException when a entry domain name could not be Punycode encoded
+	 * @throws PatternSyntaxException when an entry regular expression is not valid
      */
-    public final void add(final BlacklistType blacklistType, final String blacklistToUse, final String host, final String path) throws PunycodeException {
+	public final void add(final BlacklistType blacklistType, final String blacklistToUse, final String host,
+			final String path) throws PunycodeException, PatternSyntaxException {
     	final Collection<BlacklistHostAndPath> oneItemList = new ArrayList<>();
     	oneItemList.add(new BlacklistHostAndPath(host, path));
         this.add(blacklistType, blacklistToUse, oneItemList);

source/net/yacy/repository/BlacklistHelper.java

@@ -2,6 +2,7 @@ package net.yacy.repository;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 
 import net.yacy.cora.document.id.Punycode.PunycodeException;
 import net.yacy.cora.protocol.RequestHeader;
@@ -56,50 +57,42 @@ public final class BlacklistHelper {
     /**
      * Adds a new entry to the chosen blacklist.
      * @param blacklistToUse the name of the blacklist the entry is to be added to
-     * @param newEntry the entry that is to be added
+     * @param entry the entry that is to be added
-     * @param header
+     * @param header the current HTTP request headers
      * @param supportedBlacklistTypes
-     * @return null if no error occurred, else a String to put into LOCATION
+     * @return true when no error occurred and the entry was successfully added
      */
-	public static String addBlacklistEntry(
+	public static boolean addBlacklistEntry(
 	        final String blacklistToUse,
 	        final String entry,
 	        final RequestHeader header) {
     	String newEntry = entry;
 
-    	String location = null;
+		if (blacklistToUse == null || blacklistToUse.isEmpty() || newEntry == null || newEntry.isEmpty()) {
-        if (blacklistToUse == null || blacklistToUse.isEmpty()) {
+			return false;
-        	location = header.getPathInfo();
+		}
-        } else if (newEntry == null || newEntry.isEmpty()) {
-        	location = header.getPathInfo() + "?selectList=&selectedListName=" + blacklistToUse;
-        }
         
-    	if(location != null) {
-    		if(location.startsWith("/")) {
-    	    	/* Remove the starting "/" to redirect to a relative location for easier reverse proxy integration */
-    			location = location.substring(1, location.length());
-    		}
-            return location;
-    	}
-    	
     	newEntry = prepareEntry(newEntry);
 
         int pos = newEntry.indexOf('/',0);
         String host = newEntry.substring(0, pos);
         String path = newEntry.substring(pos + 1);
-        
+
+		boolean success = false;
         for (final BlacklistType supportedBlacklistType : BlacklistType.values()) {
         	if (ListManager.listSetContains(supportedBlacklistType + ".BlackLists", blacklistToUse)) {
             	try {
                     Switchboard.urlBlacklist.add(supportedBlacklistType, blacklistToUse, host, path);
-                } catch (PunycodeException e) {
+                    success = true;
-                    ConcurrentLog.warn(APP_NAME, "Unable to add blacklist entry to blacklist " + supportedBlacklistType, e);
+                } catch (final PunycodeException | PatternSyntaxException e) {
+					ConcurrentLog.info(APP_NAME, "Unable to add blacklist entry to blacklist " + supportedBlacklistType,
+							e);
                 }
             }
         }
         
         SearchEventCache.cleanupEvents(true);
-        return null;
+        return success;
     }
 	
     /**

source/net/yacy/server/serverObjects.java

@@ -59,6 +59,7 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 
 import net.yacy.cora.document.encoding.UTF8;
 import net.yacy.cora.document.id.MultiProtocolURL;
@@ -485,8 +486,12 @@ public class serverObjects implements Serializable, Cloneable {
         return s.equals("true") || s.equals("on") || s.equals("1");
     }
 
-    // returns a set of all values where their key mappes the keyMapper
+    /**
-    public String[] getAll(final String keyMapper) {
+     * @param keyMapper a regular expression for keys matching 
+     * @return a set of all values where their key mappes the keyMapper
+     * @throws PatternSyntaxException when the keyMapper syntax is not valid
+     */
+    public String[] getAll(final String keyMapper) throws PatternSyntaxException {
         // the keyMapper may contain regular expressions as defined in String.matches
         // this method is particulary useful when parsing the result of checkbox forms
         final List<String> v = new ArrayList<String>();
@@ -498,6 +503,25 @@ public class serverObjects implements Serializable, Cloneable {
 
         return v.toArray(new String[0]);
     }
+    
+    /**
+     * @param keyMapper a regular expression for keys matching
+     * @return a map of keys/values where keys matches the keyMapper
+     * @throws PatternSyntaxException when the keyMapper syntax is not valid
+     */
+    public Map<String, String> getMatchingEntries(final String keyMapper) throws PatternSyntaxException  {
+        // the keyMapper may contain regular expressions as defined in String.matches
+        // this method is particulary useful when parsing the result of checkbox forms
+    	final Pattern p = Pattern.compile(keyMapper);
+        final Map<String, String> map = new HashMap<>();
+        for (final Map.Entry<String, String> entry: entrySet()) {
+            if (entry.getKey().matches(keyMapper)) {
+            	map.put(entry.getKey(), entry.getValue());
+            }
+        }
+
+        return map;
+    }
 
     // put all elements of another hashtable into the own table
     public void putAll(final serverObjects add) {