XP Framework: Patch for Issue #15

xp-framework-issue15.diff

diff --git a/core/src/main/php/io/FileNotFoundException.class.php b/core/src/main/php/io/FileNotFoundException.class.php
index aa9b872..6f88e6b 100644
--- a/core/src/main/php/io/FileNotFoundException.class.php
+++ b/core/src/main/php/io/FileNotFoundException.class.php
@@ -10,9 +10,17 @@
    * Indicates the file could not be found
    *
    * @see      xp://io.IOException
-   * @purpose  Exception
    */
   class FileNotFoundException extends IOException {
   
+    /**
+     * Constructor
+     *
+     * @param   string file
+     * @param   lang.Throwable cause default NULL
+     */
+    public function __construct($file, $cause= NULL) {
+      parent::__construct('File "'.$file.'" not found', $cause);
+    }
   }
 ?>
diff --git a/core/src/main/php/util/Properties.class.php b/core/src/main/php/util/Properties.class.php
index ebf4ce2..227f83a 100644
--- a/core/src/main/php/util/Properties.class.php
+++ b/core/src/main/php/util/Properties.class.php
@@ -7,6 +7,11 @@
   uses(
     'io.IOException',
     'io.File',
+    'io.streams.InputStream',
+    'io.streams.MemoryInputStream',
+    'io.streams.MemoryOutputStream',
+    'io.streams.FileInputStream',
+    'text.StreamTokenizer',
     'util.Hashmap'
   );
   
@@ -26,19 +31,16 @@
    * key=value
    * </pre>
    *
-   * @test      xp://net.xp_framework.unittest.util.PropertiesTest
-   * @purpose   Wrapper around parse_ini_file
+   * @test    xp://net.xp_framework.unittest.util.PropertyWritingTest
+   * @test    xp://net.xp_framework.unittest.util.StringBasedPropertiesTest
+   * @test    xp://net.xp_framework.unittest.util.FileBasedPropertiesTest
+   * @see     php://parse_ini_file
    */
   class Properties extends Object {
     public
       $_file    = '',
       $_data    = NULL;
 
-    private static $INI_PARSE_BUG= FALSE;
-    static function __static() {
-      self::$INI_PARSE_BUG= version_compare(PHP_VERSION, '5.3.0', 'lt');
-    }
-      
     /**
      * Constructor
      *
@@ -49,96 +51,125 @@
     }
     
     /**
+     * Load from an input stream, e.g. a file
+     *
+     * @param   io.streams.InputStream in
+     * @throws  io.IOException
+     * @throws  lang.FormatException
+     */
+    public function load(InputStream $in) {
+      $s= new StreamTokenizer($in, "\r\n");
+      $this->_data= array();
+      $section= NULL;
+      while ($s->hasMoreTokens()) {
+        if ('' === ($t= $s->nextToken())) continue;                // Empty lines
+        $c= $t{0};
+        if (';' === $c || '#' === $c) {                            // One line comments
+          continue;
+        } else if ('[' === $c) {
+          if (FALSE === ($p= strrpos($t, ']'))) {
+            throw new FormatException('Unclosed section "'.$t.'"');
+          }
+          $section= substr($t, 1, $p- 1);
+          $this->_data[$section]= array();
+        } else if (FALSE !== ($p= strpos($t, '='))) {
+          $key= trim(substr($t, 0, $p));
+          $value= trim(substr($t, $p+ 1));
+          if (strlen($value) && ('"' === ($q= $value{0}))) {       // Quoted strings
+            if (FALSE === ($p= strrpos($value, $q, 1))) {
+              $value= substr($value, 1)."\n".$s->nextToken($q);
+            } else {
+              $value= substr($value, 1, $p- 1);
+            }
+          } else if (FALSE !== ($p= strpos($value, ';'))) {        // Comments at end of line
+            $value= trim(substr($value, 0, $p));
+          }
+
+          // Arrays and maps: key[], key[0], key[assoc]
+          if (']' === substr($key, -1)) {
+            if (FALSE === ($p= strpos($key, '['))) {
+              throw new FormatException('Invalid key "'.$key.'"');
+            }
+            $offset= substr($key, $p+ 1, -1);
+            $key= substr($key, 0, $p);
+            if (!isset($this->_data[$section][$key])) {
+              $this->_data[$section][$key]= array();
+            }
+            if ('' === $offset) {
+              $this->_data[$section][$key][]= $value;
+            } else {
+              $this->_data[$section][$key][$offset]= $value;
+            }
+          } else {
+            $this->_data[$section][$key]= $value;
+          }
+        } else if ('' !== trim($t)) {
+          throw new FormatException('Invalid line "'.$t.'"');
+        }
+      }
+    }
+
+    /**
+     * Store to an output stream, e.g. a file
+     *
+     * @param   io.streams.OutputStream out
+     * @throws  io.IOException
+     */
+    public function store(OutputStream $out) {
+      foreach (array_keys($this->_data) as $section) {
+        $out->write(sprintf("[%s]\n", $section));
+        
+        foreach ($this->_data[$section] as $key => $val) {
+          if (';' == $key{0}) {
+            $out->write(sprintf("\n; %s\n", $val)); 
+          } else {
+            if ($val instanceof Hashmap) {
+              $str= '';
+              foreach ($val->keys() as $k) {
+                $str.= '|'.$k.':'.$val->get($k);
+              }
+              $val= (string)substr($str, 1);
+            } 
+            if (is_array($val)) $val= implode('|', $val);
+            if (is_string($val)) $val= '"'.$val.'"';
+            $out->write(sprintf(
+              "%s=%s\n",
+              $key,
+              strval($val)
+            ));
+          }
+        }
+        $out->write("\n");
+      }
+    }
+    
+    /**
      * Create a property file from an io.File object
      *
+     * @deprecated  Use load() method instead
      * @param   io.File file
      * @return  util.Properties
      * @throws  io.IOException in case the file given does not exist
      */
     public static function fromFile(File $file) {
-      if (!$file->exists()) {
-        throw new IOException('The file "'.$file->getURI().'" could not be read');
-      }
-      return new self($file->getURI());
+      $self= new self($file->getURI());
+      $self->load($file->getInputStream());
+      return $self;
     }
 
     /**
      * Create a property file from a string
      *
+     * @deprecated  Use load() method instead
      * @param   string str
      * @return  util.Properties
      */
     public static function fromString($str) {
       $self= new self(NULL);
-      $self->_data= self::parse($str);
+      $self->load(new MemoryInputStream($str));
       return $self;
     }
-    
-    /**
-     * Parse from a string
-     *
-     * @param   string str
-     * @return  [:var] data
-     */
-    protected static function parse($str) {
-      $section= NULL;
-      $data= array();
-      if ($t= strtok($str, "\r\n")) do {
-        switch ($t{0}) {
-          case ';':
-          case '#':
-            break;
-
-          case '[':
-            $p= strpos($t, '[');
-            $section= substr($t, $p+ 1, strpos($t, ']', $p)- 1);
-            $data[$section]= array();
-            break;
-
-          default:
-            if (FALSE === ($p= strpos($t, '='))) break;
-            $key= trim(substr($t, 0, $p));
-            $value= trim(substr($t, $p+ 1), ' ');
-
-            // Check for string quotations
-            if (strlen($value) && ('"' == ($quote= $value{0}))) {
-
-              // For multiline values, get next token by quote
-              if (FALSE === strrpos($value, $quote, 1))
-                $value.= "\n".strtok($quote);
-
-              $value= trim($value, $quote);
-              $value= trim(substr($value, 0, ($p= strpos($value, '"')) !== FALSE
-                ? $p : strlen($value)
-              ), $quote);
-
-            // Check for comment
-            } else if (FALSE !== ($p= strpos($value, ';'))) {
-              $value= trim(substr($value, 0, $p));
-            }
 
-            // Arrays and maps: key[], key[0], key[assoc]
-            if (']' === substr($key, -1)) {
-              $p= strpos($key, '[');
-              $offset= substr($key, $p+ 1, -1);
-              $key= substr($key, 0, $p);
-              if (!isset($data[$section][$key])) {
-                $data[$section][$key]= array();
-              }
-              if ('' === $offset) {
-                $data[$section][$key][]= $value;
-              } else {
-                $data[$section][$key][$offset]= $value;
-              }
-            } else {
-              $data[$section][$key]= $value;
-            }
-            break;
-        }
-      } while ($t= strtok("\r\n"));
-      return $data;
-    }
-    
     /**
      * Retrieves the file name containing the properties
      *
@@ -154,9 +185,12 @@
      * @throws  io.IOException if the property file could not be created
      */
     public function create() {
-      $fd= new File($this->_file);
-      $fd->open(FILE_MODE_WRITE);
-      $fd->close();
+      if (NULL !== $this->_file) {
+        $fd= new File($this->_file);
+        $fd->open(FILE_MODE_WRITE);
+        $fd->close();
+      }
+      $this->_data= array();
     }
     
     /**
@@ -176,20 +210,7 @@
      */
     protected function _load($force= FALSE) {
       if (!$force && NULL !== $this->_data) return;
-      
-      if (self::$INI_PARSE_BUG) {
-        $this->_data= $this->parse(file_get_contents($this->_file));
-        $error= xp::errorAt(__FILE__, __LINE__ - 1);
-      } else {
-        $this->_data= parse_ini_file($this->_file, TRUE);
-        $error= xp::errorAt(__FILE__, __LINE__ - 1);
-      }
-      if ($error) {
-        $e= new IOException('The file "'.$this->_file.'" could not be read');
-        xp::gc(__FILE__);
-        $this->_data= NULL;
-        throw $e;
-      }
+      $this->load(new FileInputStream($this->_file));
     }
     
     /**
@@ -203,37 +224,12 @@
     /**
      * Save properties to the file
      *
+     * @deprecated  Use store() method instead
      * @throws  io.IOException if the property file could not be written
      */
     public function save() {
       $fd= new File($this->_file);
-      $fd->open(FILE_MODE_WRITE);
-      
-      foreach (array_keys($this->_data) as $section) {
-        $fd->write(sprintf("[%s]\n", $section));
-        
-        foreach ($this->_data[$section] as $key => $val) {
-          if (';' == $key{0}) {
-            $fd->write(sprintf("\n; %s\n", $val)); 
-          } else {
-            if ($val instanceof Hashmap) {
-              $str= '';
-              foreach ($val->keys() as $k) {
-                $str.= '|'.$k.':'.$val->get($k);
-              }
-              $val= substr($str, 1);
-            }
-            if (is_array($val)) $val= implode('|', $val);
-            if (is_string($val)) $val= '"'.$val.'"';
-            $fd->write(sprintf(
-              "%s=%s\n",
-              $key,
-              strval($val)
-            ));
-          }
-        }
-        $fd->write("\n");
-      }
+      $this->store($fd->getOutputStream());
       $fd->close();
     }
 
diff --git a/core/src/resources/unittest/util.ini b/core/src/resources/unittest/util.ini
index 5d917eb..6c0a383 100644
--- a/core/src/resources/unittest/util.ini
+++ b/core/src/resources/unittest/util.ini
@@ -15,5 +15,8 @@ class="net.xp_framework.unittest.util.FileBasedPropertiesTest"
 [properties:fromstring]
 class="net.xp_framework.unittest.util.StringBasedPropertiesTest"
 
+[properties:write]
+class="net.xp_framework.unittest.util.PropertyWritingTest"
+
 [timer]
 class="net.xp_framework.unittest.util.TimerTest"
diff --git a/core/src/test/php/net/xp_framework/unittest/tests/UnittestRunnerTest.class.php b/core/src/test/php/net/xp_framework/unittest/tests/UnittestRunnerTest.class.php
index 18448b4..f9e1308 100644
--- a/core/src/test/php/net/xp_framework/unittest/tests/UnittestRunnerTest.class.php
+++ b/core/src/test/php/net/xp_framework/unittest/tests/UnittestRunnerTest.class.php
@@ -136,7 +136,7 @@
     public function nonExistantProperties() {
       $return= $this->runner->run(array('@@NON-EXISTANT@@.ini'));
       $this->assertEquals(1, $return);
-      $this->assertOnStream($this->err, '@@NON-EXISTANT@@.ini" could not be read');
+      $this->assertOnStream($this->err, '*** File "@@NON-EXISTANT@@.ini" not found');
       $this->assertEquals('', $this->out->getBytes());
     }
 
diff --git a/core/src/test/php/net/xp_framework/unittest/util/AbstractPropertiesTest.class.php b/core/src/test/php/net/xp_framework/unittest/util/AbstractPropertiesTest.class.php
index 5ec203f..7ce6789 100644
--- a/core/src/test/php/net/xp_framework/unittest/util/AbstractPropertiesTest.class.php
+++ b/core/src/test/php/net/xp_framework/unittest/util/AbstractPropertiesTest.class.php
@@ -176,6 +176,38 @@ range="1..5"
         $p->readRange('section', 'range')
       );
     }
+
+    /**
+     * Test simple reading of range
+     *
+     */
+    #[@test]
+    public function readRangeUpperBoundaryLessThanLower() {
+      $p= $this->newPropertiesFrom('
+[section]
+range="1..0"
+      ');
+      $this->assertEquals(
+        range(1, 0),
+        $p->readRange('section', 'range')
+      );
+    }
+
+    /**
+     * Test simple reading of range
+     *
+     */
+    #[@test]
+    public function readRangeNegativeNumbers() {
+      $p= $this->newPropertiesFrom('
+[section]
+range="-3..3"
+      ');
+      $this->assertEquals(
+        range(-3, 3),
+        $p->readRange('section', 'range')
+      );
+    }
     
     /**
      * Test simple reading of integer
@@ -390,5 +422,58 @@ third line';
 
       $this->assertEquals($expected, $p->readString('section', 'key'));
     }
+
+    /**
+     * Test a property file where everything is indented from the left
+     *
+     */
+    #[@test]
+    public function identedKey() {
+      $p= $this->newPropertiesFrom('
+[section]
+  key1="value1"
+  key2="value2"
+      ');
+      $this->assertEquals(
+        array('key1' => 'value1', 'key2' => 'value2'), 
+        $p->readSection('section')
+      );
+    }
+
+    /**
+     * Test a property file where a key without value exists
+     *
+     */
+    #[@test, @expect('lang.FormatException')]
+    public function malformedLine() {
+      $p= $this->newPropertiesFrom('
+[section]
+foo
+      ');
+    }
+
+    /**
+     * Test a property file where a key without value exists
+     *
+     */
+    #[@test, @expect('lang.FormatException')]
+    public function malformedKey() {
+      $p= $this->newPropertiesFrom('
+[section]
+foo]=value
+      ');
+    }
+
+    /**
+     * Malformed section (unclosed brackets)
+     *
+     */
+    #[@test, @expect('lang.FormatException')]
+    public function malformedSection() {
+      $p= $this->newPropertiesFrom('
+[section
+foo=bar
+      ');
+    }
   }
 ?>
diff --git a/core/src/test/php/net/xp_framework/unittest/util/FileBasedPropertiesTest.class.php b/core/src/test/php/net/xp_framework/unittest/util/FileBasedPropertiesTest.class.php
index 08295f6..d7c80a7 100644
--- a/core/src/test/php/net/xp_framework/unittest/util/FileBasedPropertiesTest.class.php
+++ b/core/src/test/php/net/xp_framework/unittest/util/FileBasedPropertiesTest.class.php
@@ -26,6 +26,7 @@
         public function __construct($stream) { $this->stream= $stream; }
         public function exists() { return NULL !== $this->stream; }
         public function getURI() { return Streams::readableUri($this->stream); }
+        public function getInputStream() { return $this->stream; }
       }');
     }
   
diff --git a/core/src/test/php/net/xp_framework/unittest/util/PropertyWritingTest.class.php b/core/src/test/php/net/xp_framework/unittest/util/PropertyWritingTest.class.php
index e69de29..a84a810 100644
--- a/core/src/test/php/net/xp_framework/unittest/util/PropertyWritingTest.class.php
+++ b/core/src/test/php/net/xp_framework/unittest/util/PropertyWritingTest.class.php
@@ -0,0 +1,221 @@
+<?php
+/* This class is part of the XP framework
+ *
+ * $Id$
+ */
+
+  uses(
+    'unittest.TestCase',
+    'util.Properties',
+    'util.Hashmap'
+  );
+
+  /**
+   * Testcase for util.Properties class.
+   *
+   * @see      xp://util.Properties
+   */
+  class PropertyWritingTest extends TestCase {
+    protected $fixture= NULL;
+    
+    /**
+     * Creates a new, empty properties file as fixture
+     *
+     */
+    public function setUp() {
+      $this->fixture= new Properties(NULL);
+      $this->fixture->create();
+    }
+    
+    /**
+     * Verifies the saved property file equals a given expected source string
+     *
+     * @param   string expected
+     * @throws  unittest.AssertionFailedError
+     */
+    protected function assertSavedFixtureEquals($expected) {
+      $out= new MemoryOutputStream();
+      $this->fixture->store($out);
+      $this->assertEquals(preg_replace('/^ +/m', '', trim($expected)), trim($out->getBytes())); 
+    }
+  
+    /**
+     * Test writing a string
+     *
+     */
+    #[@test]
+    public function string() {
+      $this->fixture->writeString('section', 'key', 'value');
+      $this->assertSavedFixtureEquals('
+        [section]
+        key="value"
+      ');
+    }
+
+    /**
+     * Test writing a string
+     *
+     */
+    #[@test]
+    public function emptyString() {
+      $this->fixture->writeString('section', 'key', '');
+      $this->assertSavedFixtureEquals('
+        [section]
+        key=""
+      ');
+    }
+
+    /**
+     * Test writing an integer
+     *
+     */
+    #[@test]
+    public function integer() {
+      $this->fixture->writeInteger('section', 'key', 1);
+      $this->assertSavedFixtureEquals('
+        [section]
+        key=1
+      ');
+    }
+
+    /**
+     * Test writing a float
+     *
+     */
+    #[@test]
+    public function float() {
+      $this->fixture->writeFloat('section', 'key', 1.5);
+      $this->assertSavedFixtureEquals('
+        [section]
+        key=1.5
+      ');
+    }
+
+    /**
+     * Test writing a bool
+     *
+     */
+    #[@test]
+    public function boolTrue() {
+      $this->fixture->writeFloat('section', 'key', TRUE);
+      $this->assertSavedFixtureEquals('
+        [section]
+        key=1
+      ');
+    }
+
+    /**
+     * Test writing a bool
+     *
+     */
+    #[@test]
+    public function boolFalse() {
+      $this->fixture->writeFloat('section', 'key', FALSE);
+      $this->assertSavedFixtureEquals('
+        [section]
+        key=0
+      ');
+    }
+
+    /**
+     * Test writing an array
+     *
+     */
+    #[@test]
+    public function intArray() {
+      $this->fixture->writeArray('section', 'key', array(1, 2, 3));
+      $this->assertSavedFixtureEquals('
+        [section]
+        key="1|2|3"
+      ');
+    }
+
+    /**
+     * Test writing an array
+     *
+     */
+    #[@test]
+    public function emptyArray() {
+      $this->fixture->writeArray('section', 'key', array());
+      $this->assertSavedFixtureEquals('
+        [section]
+        key=""
+      ');
+    }
+
+    /**
+     * Test writing a hashmap
+     *
+     */
+    #[@test]
+    public function hashmapOneElement() {
+      $h= new HashMap();
+      $h->put('color', 'green');
+      $this->fixture->writeHash('section', 'key', $h);
+      $this->assertSavedFixtureEquals('
+        [section]
+        key="color:green"
+      ');
+    }
+
+    /**
+     * Test writing a hashmap
+     *
+     */
+    #[@test]
+    public function hashmapTwoElements() {
+      $h= new HashMap();
+      $h->put('color', 'green');
+      $h->put('size', 'L');
+      $this->fixture->writeHash('section', 'key', $h);
+      $this->assertSavedFixtureEquals('
+        [section]
+        key="color:green|size:L"
+      ');
+    }
+
+    /**
+     * Test writing a hashmap
+     *
+     */
+    #[@test]
+    public function emptyHashmap() {
+      $this->fixture->writeHash('section', 'key', new HashMap());
+      $this->assertSavedFixtureEquals('
+        [section]
+        key=""
+      ');
+    }
+
+    /**
+     * Test writing a comment
+     *
+     */
+    #[@test]
+    public function comment() {
+      $this->fixture->writeComment('section', 'Hello');
+      $this->assertSavedFixtureEquals('
+        [section]
+
+        ; Hello
+      ');
+    }
+
+    /**
+     * Test writing a comment
+     *
+     */
+    #[@test]
+    public function comments() {
+      $this->fixture->writeComment('section', 'Hello');
+      $this->fixture->writeComment('section', 'World');
+      $this->assertSavedFixtureEquals('
+        [section]
+
+        ; Hello
+
+        ; World
+      ');
+    }
+  }
+?>

Includes a fix for writing empty HashMap instances in save()

What this patch parses differently than parse_ini_file() is, e.g.

devbar=TRUE

PHP's native function yields array('devbar' => '1') for this, the XP Framework's implementation array('devbar' => 'TRUE'). When reading the value with readBool() though, this does not make a difference.