From 438f2650dbbb6d8e01460caa9a39203bce09af10 Mon Sep 17 00:00:00 2001
From: Matthias Jentsch <matthias.jentsch@netways.de>
Date: Mon, 10 Aug 2015 15:13:59 +0200
Subject: [PATCH] Conform to coding guidelines

Use exclusively getters and setters for accessing class variables. Add better documentation to INI parser and writer classes.
---
 library/Icinga/File/Ini/Dom/Comment.php   |  19 +++-
 library/Icinga/File/Ini/Dom/Directive.php | 104 ++++++++++++++++++----
 library/Icinga/File/Ini/Dom/Document.php  |  42 ++++++++-
 library/Icinga/File/Ini/Dom/Section.php   |  74 ++++++++++++---
 library/Icinga/File/Ini/IniParser.php     |  19 ++--
 library/Icinga/File/Ini/IniWriter.php     |   5 +-
 6 files changed, 221 insertions(+), 42 deletions(-)

diff --git a/library/Icinga/File/Ini/Dom/Comment.php b/library/Icinga/File/Ini/Dom/Comment.php
index 9cd9e416f..a1f6c04d2 100644
--- a/library/Icinga/File/Ini/Dom/Comment.php
+++ b/library/Icinga/File/Ini/Dom/Comment.php
@@ -3,14 +3,31 @@
 
 namespace Icinga\File\Ini\Dom;
 
+/**
+ * A single comment-line in an INI file
+ */
 class Comment
 {
     /**
+     * The comment text
+     *
      * @var string
      */
-    public $content;
+    protected $content;
 
     /**
+     * Set the text content of this comment
+     *
+     * @param $content
+     */
+    public function setContent($content)
+    {
+        $this->content = $content;
+    }
+
+    /**
+     * Render this comment into INI markup
+     *
      * @return string
      */
     public function render()
diff --git a/library/Icinga/File/Ini/Dom/Directive.php b/library/Icinga/File/Ini/Dom/Directive.php
index 97bc6d37c..0014d25bb 100644
--- a/library/Icinga/File/Ini/Dom/Directive.php
+++ b/library/Icinga/File/Ini/Dom/Directive.php
@@ -3,42 +3,57 @@
 
 namespace Icinga\File\Ini\Dom;
 
+use Icinga\Exception\ConfigurationError;
+
+/**
+ * A key value pair in a Section
+ */
 class Directive
 {
     /**
+     * The value of this configuration directive
+     *
      * @var string
      */
     protected $key;
 
     /**
+     * The immutable name of this configuration directive
+     *
      * @var string
      */
     protected $value;
 
     /**
-     * @var array
-     */
-    public $commentsPre;
-
-    /**
-     * @var string
-     */
-    public $commentPost;
-
-    /**
-     * @param   string    $key
+     * Comments added one line before this directive
      *
-     * @throws  Exception
+     * @var Comment[]   The comment lines
+     */
+    protected $commentsPre = null;
+
+    /**
+     * Comment added at the end of the same line
+     *
+     * @var Comment
+     */
+    protected $commentPost = null;
+
+    /**
+     * @param   string    $key  The name of this configuration directive
+     *
+     * @throws  ConfigurationError
      */
     public function __construct($key)
     {
         $this->key = trim($key);
         if (strlen($this->key) < 1) {
-            throw new Exception(sprintf('Ini parser error: empty key.'));
+            throw new ConfigurationError(sprintf('Ini error: empty directive key.'));
         }
     }
 
     /**
+     * Return the name of this directive
+     *
      * @return string
      */
     public function getKey()
@@ -47,7 +62,9 @@ class Directive
     }
 
     /**
-     * @return string
+     * Return the value of this configuration directive
+     *
+     * @return  string
      */
     public function getValue()
     {
@@ -55,7 +72,9 @@ class Directive
     }
 
     /**
-     * @param string    $value
+     * Set the value of this configuration directive
+     *
+     * @param   string      $value
      */
     public function setValue($value)
     {
@@ -63,7 +82,39 @@ class Directive
     }
 
     /**
-     * @return string
+     * Set the comments to be rendered on the line before this directive
+     *
+     * @param   Comment[]   $comments
+     */
+    public function setCommentsPre(array $comments)
+    {
+        $this->commentsPre = $comments;
+    }
+
+    /**
+     * Return the comments to be rendered on the line before this directive
+     *
+     * @return Comment[]
+     */
+    public function getCommentsPre()
+    {
+        return $this->commentsPre;
+    }
+
+    /**
+     * Set the comment rendered on the same line of this directive
+     *
+     * @param   Comment     $comment
+     */
+    public function setCommentPost(Comment $comment)
+    {
+        $this->commentPost = $comment;
+    }
+
+    /**
+     * Render this configuration directive into INI markup
+     *
+     * @return  string
      */
     public function render()
     {
@@ -82,16 +133,35 @@ class Directive
         return $str;
     }
 
+    /**
+     * Assure that the given identifier contains no newlines and pending or trailing whitespaces
+     *
+     * @param   $str    The string to sanitize
+     *
+     * @return string
+     */
     protected function sanitizeKey($str)
     {
         return trim(str_replace(PHP_EOL, ' ', $str));
     }
 
+    /**
+     * Escape the significant characters in directive values, normalize line breaks and assure that
+     * the character contains no linebreaks
+     *
+     * @param   $str    The string to sanitize
+     *
+     * @return mixed|string
+     */
     protected function sanitizeValue($str)
     {
         $str = trim($str);
         $str = str_replace('\\', '\\\\', $str);
         $str = str_replace('"', '\\"', $str);
-        return str_replace(PHP_EOL, ' ', $str);
+
+        // line breaks in the value should always match the current system EOL sequence
+        // to assure editable configuration files
+        $str = preg_replace("/(\r\n)|(\n)/", PHP_EOL, $str);
+        return $str;
     }
 }
diff --git a/library/Icinga/File/Ini/Dom/Document.php b/library/Icinga/File/Ini/Dom/Document.php
index 2f606a1f5..69b628916 100644
--- a/library/Icinga/File/Ini/Dom/Document.php
+++ b/library/Icinga/File/Ini/Dom/Document.php
@@ -6,16 +6,22 @@ namespace Icinga\File\Ini\Dom;
 class Document
 {
     /**
-     * @var array
+     * The sections of this INI file
+     *
+     * @var Section[]
      */
     protected $sections = array();
 
     /**
-     * @var array
+     * The comemnts at file end that belong to no particular section
+     *
+     * @var Comment[]
      */
-    public $commentsDangling;
+    protected $commentsDangling;
 
     /**
+     * Append a section to the end of this INI file
+     *
      * @param Section $section
      */
     public function addSection(Section $section)
@@ -24,6 +30,8 @@ class Document
     }
 
     /**
+     * Return whether this INI file has the section with the given key
+     *
      * @param   string  $name
      *
      * @return  bool
@@ -34,6 +42,8 @@ class Document
     }
 
     /**
+     * Return the section with the given name
+     *
      * @param   string  $name
      *
      * @return Section
@@ -44,6 +54,8 @@ class Document
     }
 
     /**
+     * Set the section with the given name
+     *
      * @param string  $name
      * @param Section $section
      *
@@ -55,6 +67,8 @@ class Document
     }
 
     /**
+     * Remove the section with the given name
+     *
      * @param string $name
      */
     public function removeSection($name)
@@ -63,6 +77,28 @@ class Document
     }
 
     /**
+     * Set the dangling comments at file end that belong to no particular directive
+     *
+     * @param Comment[] $comments
+     */
+    public function setCommentsDangling(array $comments)
+    {
+        $this->commentsDangling = $comments;
+    }
+
+    /**
+     * Get the dangling comments at file end that belong to no particular directive
+     *
+     * @return array
+     */
+    public function getCommentsDangling()
+    {
+        return $this->commentsDangling;
+    }
+
+    /**
+     * Render this document into the corresponding INI markup
+     *
      * @return string
      */
     public function render()
diff --git a/library/Icinga/File/Ini/Dom/Section.php b/library/Icinga/File/Ini/Dom/Section.php
index e46d4f8ca..dbc9188cb 100644
--- a/library/Icinga/File/Ini/Dom/Section.php
+++ b/library/Icinga/File/Ini/Dom/Section.php
@@ -5,30 +5,43 @@ namespace Icinga\File\Ini\Dom;
 
 use Icinga\Exception\ConfigurationError;
 
+/**
+ * A section in an INI file
+ */
 class Section
 {
     /**
+     * The immutable name of this section
+     *
      * @var string
      */
     protected $name;
 
     /**
-     * @var array
+     * All configuration directives of this section
+     *
+     * @var Directive[]
      */
     protected $directives = array();
 
     /**
-     * @var array
+     * Comments added one line before this section
+     *
+     * @var Comment[]
      */
-    public $commentsPre;
+    protected $commentsPre;
 
     /**
+     * Comment added at the end of the same line
+     *
      * @var string
      */
-    public $commentPost;
+    protected $commentPost;
 
     /**
-     * @param   string      $name
+     * @param   string  $name       The immutable name of this section
+     *
+     * @throws  ConfigurationError  When the section name is empty
      */
     public function __construct($name)
     {
@@ -39,7 +52,9 @@ class Section
     }
 
     /**
-     * @param Directive $directive
+     * Append a directive to the end of this section
+     *
+     * @param   Directive   $directive  The directive to append
      */
     public function addDirective(Directive $directive)
     {
@@ -47,7 +62,9 @@ class Section
     }
 
     /**
-     * @param string    $key
+     * Remove the directive with the given name
+     *
+     * @param   string      $key        They name of the directive to remove
      */
     public function removeDirective($key)
     {
@@ -55,7 +72,9 @@ class Section
     }
 
     /**
-     * @param   string  $key
+     * Return whether this section has a directive with the given key
+     *
+     * @param   string  $key            The name of the directive
      *
      * @return  bool
      */
@@ -65,6 +84,8 @@ class Section
     }
 
     /**
+     * Get the directive with the given key
+     *
      * @param $key  string
      *
      * @return Directive
@@ -75,7 +96,9 @@ class Section
     }
 
     /**
-     * @return string
+     * Return the name of this section
+     *
+     * @return string   The name
      */
     public function getName()
     {
@@ -83,6 +106,28 @@ class Section
     }
 
     /**
+     * Set the comments to be rendered on the line before this section
+     *
+     * @param   Comment[]   $comments
+     */
+    public function setCommentsPre(array $comments)
+    {
+        $this->commentsPre = $comments;
+    }
+
+    /**
+     * Set the comment rendered on the same line of this section
+     *
+     * @param   Comment     $comment
+     */
+    public function setCommentPost(Comment $comment)
+    {
+        $this->commentPost = $comment;
+    }
+
+    /**
+     * Render this section into INI markup
+     *
      * @return string
      */
     public function render()
@@ -90,7 +135,9 @@ class Section
         $dirs = '';
         $i = 0;
         foreach ($this->directives as $directive) {
-            $dirs .= (($i++ > 0 && ! empty($directive->commentsPre)) ? PHP_EOL : '') . $directive->render() . PHP_EOL;
+            $comments = $directive->getCommentsPre();
+            $dirs .= (($i++ > 0 && ! empty($comments)) ? PHP_EOL : '')
+                    . $directive->render() . PHP_EOL;
         }
         $cms = '';
         if (! empty($this->commentsPre)) {
@@ -106,6 +153,13 @@ class Section
         return $cms . sprintf('[%s]', $this->sanitize($this->name)) . $post . PHP_EOL . $dirs;
     }
 
+    /**
+     * Escape the significant characters in sections and normalize line breaks
+     *
+     * @param   $str    The string to sanitize
+     *
+     * @return  mixed
+     */
     protected function sanitize($str)
     {
         $str = trim($str);
diff --git a/library/Icinga/File/Ini/IniParser.php b/library/Icinga/File/Ini/IniParser.php
index 85ab20e18..43c96836d 100644
--- a/library/Icinga/File/Ini/IniParser.php
+++ b/library/Icinga/File/Ini/IniParser.php
@@ -93,7 +93,7 @@ class IniParser
                         $token .= $s;
                     } else {
                         $sec = new Section($token);
-                        $sec->commentsPre = $coms;
+                        $sec->setCommentsPre($coms);
                         $doc->addSection($sec);
                         $dir = null;
                         $coms = array();
@@ -108,8 +108,7 @@ class IniParser
                         $token .= $s;
                     } else {
                         $dir = new Directive($token);
-                        $dir->commentsPre = $coms;
-
+                        $dir->setCommentsPre($coms);
                         if (isset($sec)) {
                             $sec->addDirective($dir);
                         } else {
@@ -177,16 +176,16 @@ class IniParser
                         $token .= $s;
                     } else {
                         $com = new Comment();
-                        $com->content = $token;
+                        $com->setContent($token);
                         $token = '';
 
                         // Comments at the line end belong to the current line's directive or section. Comments
                         // on empty lines belong to the next directive that shows up.
                         if ($state === self::COMMENT_END) {
                             if (isset($dir)) {
-                                $dir->commentPost = $com;
+                                $dir->setCommentPost($com);
                             } else {
-                                $sec->commentPost = $com;
+                                $sec->setCommentPost($com);
                             }
                         } else {
                             $coms[] = $com;
@@ -212,12 +211,12 @@ class IniParser
             case self::COMMENT:
             case self::COMMENT_END:
                 $com = new Comment();
-                $com->content = $token;
+                $com->setContent($token);
                 if ($state === self::COMMENT_END) {
                     if (isset($dir)) {
-                        $dir->commentPost = $com;
+                        $dir->setCommentPost($com);
                     } else {
-                        $sec->commentPost = $com;
+                        $sec->setCommentPost($com);
                     }
                 } else {
                     $coms[] = $com;
@@ -236,7 +235,7 @@ class IniParser
                 self::throwParseError('File ended in unterminated state ' . $state, $line);
         }
         if (! empty($coms)) {
-            $doc->commentsDangling = $coms;
+            $doc->setCommentsDangling($coms);
         }
         return $doc;
     }
diff --git a/library/Icinga/File/Ini/IniWriter.php b/library/Icinga/File/Ini/IniWriter.php
index ab024bf62..8bf8241aa 100644
--- a/library/Icinga/File/Ini/IniWriter.php
+++ b/library/Icinga/File/Ini/IniWriter.php
@@ -117,7 +117,10 @@ class IniWriter
     protected function updateSectionOrder(Config $newconfig, Document $oldDoc)
     {
         $doc = new Document();
-        $doc->commentsDangling = $oldDoc->commentsDangling;
+        $dangling = $oldDoc->getCommentsDangling();
+        if (isset($dangling)) {
+            $doc->setCommentsDangling($dangling);
+        }
         foreach ($newconfig->toArray() as $section => $directives) {
             $doc->addSection($oldDoc->getSection($section));
         }