From c250bfe574c99e65ece8c390ccfb7f485ff3cd6a Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Wed, 30 Jul 2014 10:53:08 -0400
Subject: [PATCH] Smarter array handling in config writer (with tests).

---
 module/VuFind/src/VuFind/Config/Writer.php    | 38 ++++++++++++----
 .../src/VuFindTest/Config/WriterTest.php      | 45 +++++++++++++++++++
 2 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/module/VuFind/src/VuFind/Config/Writer.php b/module/VuFind/src/VuFind/Config/Writer.php
index a653c21315b..491e2a3c360 100644
--- a/module/VuFind/src/VuFind/Config/Writer.php
+++ b/module/VuFind/src/VuFind/Config/Writer.php
@@ -214,6 +214,32 @@ class Writer
         return $key . $tabStr . "= ". $this->buildContentValue($value);
     }
 
+    /**
+     * support method for buildContent -- format an array into lines
+     *
+     * @param string $key   Configuration key
+     * @param array  $value Configuration value
+     *
+     * @return string       Formatted line
+     */
+    protected function buildContentArrayLines($key, $value)
+    {
+        $expectedKey = 0;
+        $content = '';
+        foreach ($value as $key2 => $subValue) {
+            // We just want to use "[]" if this is a standard array with consecutive
+            // keys; however, if we have non-numeric keys or out-of-order keys, we
+            // want to retain those values as-is.
+            $subKey = (is_int($key2) && $key2 == $expectedKey)
+                ? ''
+                : (is_int($key2) ? $key2 : "'{$key2}'");    // quote string keys
+            $content .= $this->buildContentLine("{$key}[{$subKey}]", $subValue);
+            $content .= "\n";
+            $expectedKey++;
+        }
+        return $content;
+    }
+
     /**
      * write an ini file, adapted from
      * http://php.net/manual/function.parse-ini-file.php
@@ -244,12 +270,7 @@ class Writer
                     $settingComments = array();
                 }
                 if (is_array($elem2)) {
-                    for ($i = 0; $i < count($elem2); $i++) {
-                        $content .= $this->buildContentLine(
-                            $key2 . "[]", $elem2[$i]
-                        );
-                        $content .= "\n";
-                    }
+                    $content .= $this->buildContentArrayLines($key2, $elem2);
                 } else {
                     $content .= $this->buildContentLine($key2, $elem2);
                 }
@@ -259,8 +280,9 @@ class Writer
                 $content .= "\n";
             }
         }
-
-        $content .= $comments['after'];
+        if (isset($comments['after'])) {
+            $content .= $comments['after'];
+        }
         return $content;
     }
 }
\ No newline at end of file
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Config/WriterTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Config/WriterTest.php
index 6c7319fe048..440db135f5d 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/Config/WriterTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Config/WriterTest.php
@@ -94,6 +94,51 @@ class WriterTest extends \VuFindTest\Unit\TestCase
         $this->assertEquals($cfg, $test->getContent());
     }
 
+    /**
+     * Test constructing text from a non-associative array.
+     *
+     * @return void
+     */
+    public function testStandardArray()
+    {
+        $cfg = array('Test' => array('test' => array('val1', 'val2')));
+        $test = new Writer('fake.ini', $cfg);
+        $expected = "[Test]\ntest[]           = \"val1\"\n"
+            . "test[]           = \"val2\"\n\n";
+        $this->assertEquals($expected, $test->getContent());
+    }
+
+    /**
+     * Test constructing text from a non-associative array with
+     * non-consecutive keys.
+     *
+     * @return void
+     */
+    public function testOutOfOrderArray()
+    {
+        $cfg = array('Test' => array('test' => array(6 => 'val1', 8 => 'val2')));
+        $test = new Writer('fake.ini', $cfg);
+        $expected = "[Test]\ntest[6]          = \"val1\"\n"
+            . "test[8]          = \"val2\"\n\n";
+        $this->assertEquals($expected, $test->getContent());
+    }
+
+    /**
+     * Test constructing text from an associative array.
+     *
+     * @return void
+     */
+    public function testAssocArray()
+    {
+        $cfg = array(
+            'Test' => array('test' => array('key1' => 'val1', 'key2' => 'val2'))
+        );
+        $test = new Writer('fake.ini', $cfg);
+        $expected = "[Test]\ntest['key1']     = \"val1\"\n"
+            . "test['key2']     = \"val2\"\n\n";
+        $this->assertEquals($expected, $test->getContent());
+    }
+
     /**
      * Test setting a value.
      *
-- 
GitLab