SLING-10610 Support the @ValueFrom suffix for the name parameters (#15)

Also added additional tests for coverage
diff --git a/pom.xml b/pom.xml
index a9119de..9a29b4b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -199,6 +199,12 @@
             <version>1.9.5</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.sling</groupId>
+            <artifactId>org.apache.sling.servlet-helpers</artifactId>
+            <version>1.4.2</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
 </project>
diff --git a/src/main/java/org/apache/sling/servlets/post/impl/helper/DefaultNodeNameGenerator.java b/src/main/java/org/apache/sling/servlets/post/impl/helper/DefaultNodeNameGenerator.java
index 2fcd926..269829c 100644
--- a/src/main/java/org/apache/sling/servlets/post/impl/helper/DefaultNodeNameGenerator.java
+++ b/src/main/java/org/apache/sling/servlets/post/impl/helper/DefaultNodeNameGenerator.java
@@ -54,6 +54,67 @@
     }
 
     /**
+     * Determine the value to use for the specified parameter. This also
+     * considers the parameter with a {@link SlingPostConstants#VALUE_FROM_SUFFIX}
+     *
+     * @param parameters the map of request parameters
+     * @param paramName the parameter to get the value for
+     * @return the value to use for the parameter or null if it could not be determined
+     */
+    protected String getValueToUse(RequestParameterMap parameters, String paramName) {
+        String valueToUse = null;
+        RequestParameter[] pp = parameters.getValues(paramName);
+        if (pp != null) {
+            for (RequestParameter specialParam : pp) {
+                if (specialParam != null && !specialParam.getString().isEmpty()) {
+                    valueToUse = specialParam.getString();
+                }
+
+                if (valueToUse != null) {
+                    if (valueToUse.isEmpty()) {
+                        // empty value is not usable
+                        valueToUse = null;
+                    } else {
+                        // found value, so stop looping
+                        break;
+                    }
+                }
+            }
+        } else {
+            // check for a paramName@ValueFrom param
+            // SLING-130: VALUE_FROM_SUFFIX means take the value of this
+            // property from a different field
+            pp = parameters.getValues(String.format("%s%s", paramName, SlingPostConstants.VALUE_FROM_SUFFIX));
+            if (pp != null) {
+                for (RequestParameter specialParam : pp) {
+                    if (specialParam != null && !specialParam.getString().isEmpty()) {
+                        // retrieve the reference parameter value
+                        RequestParameter[] refParams = parameters.getValues(specialParam.getString());
+                        // @ValueFrom params must have exactly one value, else ignored
+                        if (refParams != null && refParams.length == 1) {
+                            specialParam = refParams[0];
+                            if (specialParam != null && !specialParam.getString().isEmpty()) {
+                                valueToUse = specialParam.getString();
+                            }
+                        }
+                    }
+
+                    if (valueToUse != null) {
+                        if (valueToUse.isEmpty()) {
+                            // empty value is not usable
+                            valueToUse = null;
+                        } else {
+                            // found value, so stop looping
+                            break;
+                        }
+                    }
+                }
+            }
+        }
+        return valueToUse;
+    }
+
+    /**
      * Get a "nice" node name, if possible, based on given request
      *
      * @param request the request
@@ -73,39 +134,22 @@
         // our parameterNames, in order, and has a value
         if (parameters!=null) {
             // we first check for the special sling parameters
-            RequestParameter specialParam = parameters.getValue(SlingPostConstants.RP_NODE_NAME);
-            if ( specialParam != null ) {
-                if ( specialParam.getString() != null && specialParam.getString().length() > 0 ) {
-                    valueToUse = specialParam.getString();
-                    doFilter = false;
-                }
+            valueToUse = getValueToUse(parameters, SlingPostConstants.RP_NODE_NAME);
+            if (valueToUse != null) {
+                doFilter = false;
             }
             if ( valueToUse == null ) {
-                specialParam = parameters.getValue(SlingPostConstants.RP_NODE_NAME_HINT);
-                if ( specialParam != null ) {
-                    if ( specialParam.getString() != null && specialParam.getString().length() > 0 ) {
-                        valueToUse = specialParam.getString();
-                    }
-                }
+                valueToUse = getValueToUse(parameters, SlingPostConstants.RP_NODE_NAME_HINT);
             }
 
             if (valueToUse == null) {
                 for (String param : parameterNames) {
-                    if (valueToUse != null) {
-                        break;
-                    }
                     if (requirePrefix) {
                         param = SlingPostConstants.ITEM_PREFIX_RELATIVE_CURRENT.concat(param);
                     }
-                    final RequestParameter[] pp = parameters.get(param);
-                    if (pp != null) {
-                        for (RequestParameter p : pp) {
-                            valueToUse = p.getString();
-                            if (valueToUse != null && valueToUse.length() > 0) {
-                                break;
-                            }
-                            valueToUse = null;
-                        }
+                    valueToUse = getValueToUse(parameters, param);
+                    if (valueToUse != null) {
+                        break;
                     }
                 }
             }
diff --git a/src/test/java/org/apache/sling/servlets/post/impl/helper/DefaultNodeNameGeneratorTest.java b/src/test/java/org/apache/sling/servlets/post/impl/helper/DefaultNodeNameGeneratorTest.java
new file mode 100644
index 0000000..293ecb7
--- /dev/null
+++ b/src/test/java/org/apache/sling/servlets/post/impl/helper/DefaultNodeNameGeneratorTest.java
@@ -0,0 +1,190 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.servlets.post.impl.helper;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.sling.servlethelpers.MockSlingHttpServletRequest;
+import org.apache.sling.servlets.post.NodeNameGenerator;
+import org.junit.Test;
+
+/**
+ * Test node name generator
+ */
+public class DefaultNodeNameGeneratorTest {
+
+    protected String nodeName(Map<String, Object> parameterMap) {
+        return nodeName(parameterMap, false);
+    }
+    protected String nodeName(Map<String, Object> parameterMap, boolean requirePrefix) {
+        MockSlingHttpServletRequest request = new MockSlingHttpServletRequest(null);
+        request.setParameterMap(parameterMap);
+
+        String basePath = null;
+        NodeNameGenerator defaultNodeNameGenerator = null;
+
+        String[] parameterNames = new String[] {
+                "title",
+                "sling:subject"
+            };
+        int maxNameLength = 10;
+        NodeNameGenerator nodeNameGenerator = new DefaultNodeNameGenerator(
+                parameterNames, 
+                maxNameLength);
+        return nodeNameGenerator.getNodeName(request, basePath, requirePrefix, defaultNodeNameGenerator);
+    }
+
+    protected void assertDefaultName(Map<String, Object> parameterMap) {
+        assertDefaultName(parameterMap, false);
+    }
+    protected void assertDefaultName(Map<String, Object> parameterMap, boolean requirePrefix) {
+        String nodeName = nodeName(parameterMap, requirePrefix);
+        assertTrue(nodeName.matches("\\d+_\\d+"));
+    }
+
+    protected void assertExpectedName(Map<String, Object> parameterMap, String expectedName) {
+        assertExpectedName(parameterMap, expectedName, false);
+    }
+    protected void assertExpectedName(Map<String, Object> parameterMap, String expectedName, boolean requirePrefix) {
+        String nodeName = nodeName(parameterMap, requirePrefix);
+        assertEquals(expectedName, nodeName);
+    }
+
+    @Test
+    public void testNameDefault() {
+        assertDefaultName(Collections.singletonMap("message", "Hello"));
+    }
+
+    @Test
+    public void testNameHint() {
+        assertExpectedName(Collections.singletonMap(":nameHint", "Hello"), "hello");
+    }
+
+    @Test
+    public void testNameHintEmpty() {
+        assertDefaultName(Collections.singletonMap(":nameHint", ""));
+    }
+
+    @Test
+    public void testName() {
+        assertExpectedName(Collections.singletonMap(":name", "Hello"), "Hello");
+    }
+
+    @Test
+    public void testNameEmpty() {
+        assertDefaultName(Collections.singletonMap(":name", ""));
+    }
+
+    @Test
+    public void testNameHintTrimmed() {
+        assertExpectedName(Collections.singletonMap(":nameHint", "HelloWorldTooLong"), "helloworld");
+    }
+
+    @Test
+    public void testNameFromTitle() {
+        assertExpectedName(Collections.singletonMap("title", "Hello"), "hello");
+    }
+
+    @Test
+    public void testNameFromTitleEmpty() {
+        assertDefaultName(Collections.singletonMap("title", ""));
+    }
+
+    @Test
+    public void testNameFromTitleWithPrefix() {
+        // 1. should not find any param and fallback to the default
+        assertDefaultName(Collections.singletonMap("title", "Hello"), true);
+
+        // 2. should find a param and use it
+        assertExpectedName(Collections.singletonMap("./title", "Hello"), "hello", true);
+    }
+
+    @Test
+    public void testNameFromSubject() {
+        Map<String, Object> map = new HashMap<>();
+        map.put("sling:message", "Hello");
+        map.put("sling:subject", "World");
+        assertExpectedName(map, "world");
+    }
+
+    @Test
+    public void testNameHintFilter() {
+        assertExpectedName(Collections.singletonMap(":nameHint", "H$lloW#rld"), "h_llow_rld");
+    }
+
+    /**
+     * SLING-10610
+     */
+    @Test
+    public void testNameHintValueFrom() {
+        Map<String, Object> map = new HashMap<>();
+        map.put(":nameHint@ValueFrom", "message");
+        map.put("message", "Hello");
+        assertExpectedName(map, "hello");
+    }
+
+    @Test
+    public void testNameHintValueFromEmpty() {
+        assertDefaultName(Collections.singletonMap(":nameHint@ValueFrom", ""));
+    }
+
+    @Test
+    public void testNameHintValueFromEmptyRef() {
+        Map<String, Object> map = new HashMap<>();
+        map.put(":nameHint@ValueFrom", "message");
+        map.put("message", "");
+        assertDefaultName(map);
+    }
+
+    /**
+     * SLING-10610
+     */
+    @Test
+    public void testNameValueFrom() {
+        Map<String, Object> map = new HashMap<>();
+        map.put(":name@ValueFrom", "message");
+        map.put("message", "Hello");
+        assertExpectedName(map, "Hello");
+    }
+
+    @Test
+    public void testNameFromSubjectValueFrom() {
+        Map<String, Object> map = new HashMap<>();
+        map.put("sling:message", "Hello");
+        map.put("sling:subject@ValueFrom", "sling:message");
+        assertExpectedName(map, "hello");
+    }
+
+    @Test
+    public void testNameFromSubjectValueFromEmpty() {
+        assertDefaultName(Collections.singletonMap("sling:subject@ValueFrom", ""));
+    }
+
+    @Test
+    public void testNameFromSubjectValueFromEmptyRef() {
+        Map<String, Object> map = new HashMap<>();
+        map.put("sling:message", "");
+        map.put("sling:subject@ValueFrom", "sling:message");
+        assertDefaultName(map);
+    }
+
+}