Merge pull request #3442 from gturri/fix3436

Add tests on sectionID and fixes #3436

2 files changed, 52 insertions, 8 deletions

diff --git a/_test/tests/inc/pageutils_sectionid.test.php b/_test/tests/inc/pageutils_sectionid.test.php
new file mode 100644
index 000000000..f7cfcc68a
--- /dev/null
+++ b/_test/tests/inc/pageutils_sectionid.test.php
@@ -0,0 +1,43 @@
+<?php
+
+class sectionid_test extends Dokuwikitest
+{
+    /**
+     * DataProvider
+     *
+     * @return Generator|array
+     * @see testSectionidsAreUnique
+     */
+    public function provideTestData(){
+        // Each test case represents a sequence of sections title
+        return [
+            [['A', 'A', 'A1']],
+            [['A', 'A1', 'A']]
+        ];
+    }
+
+    /**
+     * @dataProvider provideTestData
+     * @param array $titles
+     */
+    function testSectionidsAreUnique($titles)
+    {
+        $check = array();
+        $alreadyGeneratedIds = array();
+        foreach($titles as $title){
+            $newId = sectionID($title, $check);
+            $this->assertNotContains($newId, $alreadyGeneratedIds, "id $newId has been generated twice. The 2nd time it was for the title $title");
+            $alreadyGeneratedIds []= $newId;
+        }
+    }
+
+    /**
+     * The convention in the code is to pass $check=false when we're not interested in having
+     * unique sectionID. This test ensures that this type of call is correctly handled
+     */
+    function testSectionIDCanBeCalledWithNonArrayCheck(){
+        $check = false;
+        $this->assertEquals("abc", sectionID("abc", $check), "Passing \$check=false shouldn't lead to an error");
+        $this->assertEquals("abc", sectionID("abc", $check), "Passing \$check=false shouldn't try to deduplicate id");
+    }
+}
diff --git a/inc/pageutils.php b/inc/pageutils.php
index d4a8bb7c4..c281ffc5b 100644
--- a/inc/pageutils.php
+++ b/inc/pageutils.php
@@ -226,7 +226,7 @@ function noNSorNS($id) {
  * Creates a XHTML valid linkid from a given headline title
  *
  * @param string  $title   The headline title
- * @param array|bool   $check   Existing IDs (title => number)
+ * @param array|bool   $check   Existing IDs
  * @return string the title
  *
  * @author Andreas Gohr <andi@splitbrain.org>
@@ -241,15 +241,16 @@ function sectionID($title,&$check) {
     }
 
     if(is_array($check)){
-        // make sure tiles are unique
-        if (!array_key_exists ($title,$check)) {
-            $check[$title] = 0;
-        } else {
-            $title .= ++ $check[$title];
+        $suffix=0;
+        $candidateTitle = $title;
+        while(in_array($candidateTitle, $check)){
+          $candidateTitle = $title . ++$suffix;
         }
+        $check []= $candidateTitle;
+        return $candidateTitle;
+    } else {
+      return $title;
     }
-
-    return $title;
 }
 
 /**