Add Config Reader for Java-style .properties files and strings #4824

Merged
merged 14 commits into from Jul 22, 2013

Projects

None yet

7 participants

@mvargeson
Contributor

This pull request adds a Config Reader for Java-style .properties files and strings with corresponding tests.

@coveralls

Coverage Status

Coverage remained the same when pulling 79b8276 on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@samsonasik samsonasik commented on an outdated diff Jul 13, 2013
library/Zend/Config/Reader/JavaProperties.php
@@ -0,0 +1,140 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Config
@samsonasik samsonasik commented on an outdated diff Jul 13, 2013
tests/ZendTest/Config/Reader/JavaPropertiesTest.php
@@ -0,0 +1,85 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Config
@samsonasik samsonasik and 1 other commented on an outdated diff Jul 13, 2013
tests/ZendTest/Config/Reader/JavaPropertiesTest.php
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Config
+ */
+
+namespace ZendTest\Config\Reader;
+
+use Zend\Config\Reader\JavaProperties as JavaProperties;
+
+/**
+ * @category Zend
+ * @package Zend_Config
+ * @subpackage UnitTests
@samsonasik
samsonasik Jul 13, 2013 Contributor

remove @category @package @subpackage

@mvargeson
mvargeson Jul 14, 2013 Contributor

Is the removal of these docblock tags required on all other unit tests (as this was based off another unit test)?

@samsonasik
samsonasik Jul 14, 2013 Contributor

Yes ;)

Matthew Vargeson Fix docblock tags d6deadf
@coveralls

Coverage Status

Coverage remained the same when pulling d6deadf on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@coveralls

Coverage Status

Coverage remained the same when pulling d6deadf on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@coveralls

Coverage Status

Coverage remained the same when pulling 0d98f17 on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@DASPRiD DASPRiD commented on an outdated diff Jul 14, 2013
tests/ZendTest/Config/Reader/JavaPropertiesTest.php
@@ -0,0 +1,81 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace ZendTest\Config\Reader;
+
+use Zend\Config\Reader\JavaProperties as JavaProperties;
@DASPRiD
DASPRiD Jul 14, 2013 Member

Alias not required.

@DASPRiD DASPRiD commented on an outdated diff Jul 14, 2013
tests/ZendTest/Config/Reader/JavaPropertiesTest.php
+ public function testFromString()
+ {
+ $JavaProperties = <<<'ASSET'
+#comment
+!comment
+single.line:test
+path:\\test\\with\\slashes
+multiple:line \
+test
+ASSET;
+
+ $arrayJavaProperties = $this->reader->fromString($JavaProperties);
+
+ $this->assertNotEmpty($arrayJavaProperties);
+ $this->assertEquals($arrayJavaProperties['single.line'], 'test');
+ $this->assertEquals($arrayJavaProperties['path'], '\test\with\slashes');
@DASPRiD
DASPRiD Jul 14, 2013 Member

Those are actually backslashes, not slashes. So the test string is a bit confusing.

@DASPRiD DASPRiD commented on an outdated diff Jul 14, 2013
tests/ZendTest/Config/Reader/JavaPropertiesTest.php
+test
+ASSET;
+
+ $arrayJavaProperties = $this->reader->fromString($JavaProperties);
+
+ $this->assertNotEmpty($arrayJavaProperties);
+ $this->assertEquals($arrayJavaProperties['single.line'], 'test');
+ $this->assertEquals($arrayJavaProperties['path'], '\test\with\slashes');
+ $this->assertEquals($arrayJavaProperties['multiple'], 'line test');
+ }
+
+ public function testInvalidString()
+ {
+ $JavaProperties = '@include:fail.properties';
+
+ $this->setExpectedException('Zend\Config\Exception\RuntimeException');
@DASPRiD
DASPRiD Jul 14, 2013 Member

Please also test for the exception message.

@DASPRiD DASPRiD commented on an outdated diff Jul 14, 2013
library/Zend/Config/Reader/JavaProperties.php
+ || (!$isWaitingOtherLine && strpos($line, "#") === 0)
+ || (!$isWaitingOtherLine && strpos($line, "!") === 0)) {
+ continue;
+ }
+
+ // Add a new key-value pair or append value to a previous pair
+ if (!$isWaitingOtherLine) {
+ $key = substr($line, 0, strpos($line, ':'));
+ $value = substr($line, strpos($line, ':') + 1, strlen($line));
+ } else {
+ $value .= $line;
+ }
+
+ // Check if ends with single '\' (indicating another line is expected)
+ if (strrpos($value, "\\") === strlen($value)-strlen("\\")) {
+ $value = substr($value,0,strlen($value)-1);
@DASPRiD
DASPRiD Jul 14, 2013 Member

Spaces after comma and before and after minus.

@coveralls

Coverage Status

Coverage remained the same when pulling e139fe9 on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@coveralls

Coverage Status

Coverage remained the same when pulling e139fe9 on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@coveralls

Coverage Status

Coverage increased (+0%) when pulling 88d5b03 on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@coveralls

Coverage Status

Coverage decreased (-0%) when pulling 88d5b03 on mvargeson:feature/config-reader-javaproperties into 40585a5 on zendframework:develop.

@jacobkiers jacobkiers commented on an outdated diff Jul 19, 2013
library/Zend/Config/Reader/JavaProperties.php
+ ));
+ }
+
+ $this->directory = dirname($filename);
+
+ $config = $this->parse(file_get_contents($filename));
+
+ return $this->process($config);
+ }
+
+ /**
+ * fromString(): defined by Reader interface.
+ *
+ * @see ReaderInterface::fromString()
+ * @param string $string
+ * @return array|bool
@jacobkiers
jacobkiers Jul 19, 2013 Contributor

While reading this code, it seems to me that this ::fromString() method never returns a boolean. Therefore I would suggest removing that hint. I would also suggest adding an @throws line, since the ::process() method can throw an exception.

@weierophinney weierophinney added a commit that referenced this pull request Jul 22, 2013
@weierophinney weierophinney Merge branch 'feature/4824' into develop
Close #4824
137b31d
@weierophinney weierophinney merged commit 0559ac0 into zendframework:develop Jul 22, 2013

1 check passed

default The Travis CI build passed
Details
@drdev drdev commented on the diff Jul 23, 2013
library/Zend/Config/Reader/JavaProperties.php
+ if (empty($line)
+ || (!$isWaitingOtherLine && strpos($line, "#") === 0)
+ || (!$isWaitingOtherLine && strpos($line, "!") === 0)) {
+ continue;
+ }
+
+ // Add a new key-value pair or append value to a previous pair
+ if (!$isWaitingOtherLine) {
+ $key = substr($line, 0, strpos($line, ':'));
+ $value = substr($line, strpos($line, ':') + 1, strlen($line));
+ } else {
+ $value .= $line;
+ }
+
+ // Check if ends with single '\' (indicating another line is expected)
+ if (strrpos($value, "\\") === strlen($value) - strlen("\\")) {
@drdev
drdev Jul 23, 2013 Contributor

Maybe it's better to get strlen("\\") out of the cycle?

@DASPRiD
DASPRiD Jul 23, 2013 Member

Like, replacing it with 1? I agree.

@weierophinney weierophinney added a commit to zendframework/zend-config that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#4824 from mvargeson/fe…
…ature/config-reader-javaproperties

Add Config Reader for Java-style .properties files and strings
a74508b
@weierophinney weierophinney added a commit to zendframework/zend-config that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'feature/4824' into develop 225858f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment