Fixed loss of other modifiers on keywords/tags search navigation links

source/net/yacy/search/query/QueryParams.java

@@ -980,27 +980,16 @@ public final class QueryParams {
 	protected static void appendNavUrlQueryParams(final StringBuilder sb, final QueryParams theQuery, final String newModifier,
 			final boolean newModifierReplacesOld, final boolean authenticatedFeatures) {
 		if (newModifier == null) {
-            if (!theQuery.modifier.isEmpty()) sb.append("+" + theQuery.modifier.toString());
+            if (!theQuery.modifier.isEmpty()) {
+            	sb.append("+" + theQuery.modifier.toString());
+            }
         } else {
             if (!newModifier.isEmpty()) {
                 if (!theQuery.modifier.isEmpty()) {
                 	sb.append("+" + theQuery.modifier.toString());
                 }
                 if (newModifierReplacesOld) {
-                    int nmpi = newModifier.indexOf(":");
-                    if (nmpi > 0) {
-                        String nmp = newModifier.substring(0, nmpi) + ":";
-                        int i = sb.indexOf(nmp);
-                        if (i > 0) {
-                        	sb.setLength(i);
-                            if (sb.charAt(sb.length() - 1) == ' ') {
-                            	sb.setLength(sb.length() - 1);
-                            }
-                        }
-                        if (sb.charAt(sb.length() - 1) == '+') {
-                        	sb.setLength(sb.length() - 1);
-                        }
-                    }
+                    removeOldModifiersFromNavUrl(sb, newModifier);
                 }
                 try {
                 	sb.append("+" + URLEncoder.encode(newModifier, StandardCharsets.UTF_8.name()));
@@ -1041,4 +1030,38 @@ public final class QueryParams {
         }
 	}
 
+	/**
+	 * Remove from the URL builder any query modifiers with the same name that the new modifier 
+	 * @param sb
+	 *            a StringBuilder holding the search URL navigation being built.
+	 *            Must not be null and contain the URL base and the query string
+	 *            with its eventual modifiers
+	 * @param newModifier
+	 *            a new modifier of form key:value. Must not be null.
+	 */
+	protected static void removeOldModifiersFromNavUrl(final StringBuilder sb, final String newModifier) {
+		int nmpi = newModifier.indexOf(":");
+		if (nmpi > 0) {
+		    final String newModifierKey = newModifier.substring(0, nmpi) + ":";
+		    int sameModifierIndex = sb.indexOf(newModifierKey);
+		    while (sameModifierIndex > 0) {
+		    	final int spaceModifierIndex = sb.indexOf(" ", sameModifierIndex);
+		    	if(spaceModifierIndex > sameModifierIndex) {
+		    		/* There are other modifiers after the matching one : we only remove the old matching modifier */
+		    		sb.delete(sameModifierIndex, spaceModifierIndex + 1);
+		    	} else {
+		    		/* The matching modifier is the last : we truncate the builder */
+		        	sb.setLength(sameModifierIndex);	
+		    	}
+		    	sameModifierIndex = sb.indexOf(newModifierKey);
+		    }
+		    if (sb.charAt(sb.length() - 1) == '+') {
+		    	sb.setLength(sb.length() - 1);
+		    }
+		    if (sb.charAt(sb.length() - 1) == ' ') {
+		    	sb.setLength(sb.length() - 1);
+		    }
+		}
+	}
+
 }

test/java/net/yacy/search/query/QueryParamsTest.java

@@ -23,6 +23,9 @@
 package net.yacy.search.query;
 
 import java.net.MalformedURLException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -233,4 +236,48 @@ public class QueryParamsTest {
 		
 		Assert.assertEquals(QueryParams.catchall_pattern.toString(), filter);
 	}
+	
+	/**
+	 * Test removal of old modifier(s) when building a search navigation URL.
+	 */
+	@Test
+	public void testRemoveOldModifiersFromNavUrl() {
+		final String baseURL = "yacysearch.html?query=test+search+terms";
+		
+		final String newModifier = "keywords:new";
+
+		final Map<String, String> modifiers2Expected = new HashMap<>();
+		/* No existing modifiers */
+		modifiers2Expected.put(baseURL, baseURL);
+		
+		/* No existing modifiers */
+		modifiers2Expected.put(baseURL + "+keywords:old", baseURL);
+		
+		/* One modifier matching the new modifier's name, but with a different value */
+		modifiers2Expected.put(baseURL + "+keywords:old", baseURL);
+		
+		/* One modifier matching the new modifier's name, with the same value */
+		modifiers2Expected.put(baseURL + "+keywords:new", baseURL);
+		
+		/* Two modifiers matching the new modifier's name */
+		modifiers2Expected.put(baseURL + "+keywords:old keywords:new", baseURL);
+		
+		/* One modifier with a different name than the new one */
+		modifiers2Expected.put(baseURL + "+site:example.org", baseURL + "+site:example.org");
+		
+		/* Two modifiers, only one matching the new modifier's name */
+		modifiers2Expected.put(baseURL + "+site:example.org keywords:old", baseURL + "+site:example.org");
+		
+		/* Three modifiers, the one not matching the new modifier's name in the middle of the others */
+		modifiers2Expected.put(baseURL + "+keywords:old site:example.org keywords:other", baseURL + "+site:example.org");
+		
+		/* Three modifiers, only one matching the new modifier's name. The others having two different naming styles. */
+		modifiers2Expected.put(baseURL + "+keywords:old /language/en site:example.org keywords:other", baseURL + "+/language/en site:example.org");
+		
+		for(final Entry<String, String> entry : modifiers2Expected.entrySet()) {
+			StringBuilder sb = new StringBuilder(entry.getKey());		
+			QueryParams.removeOldModifiersFromNavUrl(sb, newModifier);
+			Assert.assertEquals(entry.getValue(), sb.toString());
+		}
+	}
 }