From 7026d316319ba7b064a63c10d46033ba55f365c3 Mon Sep 17 00:00:00 2001 From: madflow Date: Wed, 15 Jun 2016 13:44:29 +0200 Subject: [PATCH] Integrated second batch of changes #135 Newline Fix test, use reflection for protected static members --- .../Exception/Border/InvalidNameException.php | 3 -- .../Border/InvalidStyleException.php | 2 -- .../Border/InvalidWidthException.php | 2 -- src/Spout/Writer/ODS/Helper/BorderHelper.php | 4 +-- src/Spout/Writer/Style/Border.php | 2 +- src/Spout/Writer/Style/BorderBuilder.php | 24 +++++++------- src/Spout/Writer/Style/BorderPart.php | 21 ++++++++----- src/Spout/Writer/XLSX/Helper/BorderHelper.php | 13 +++++++- src/Spout/Writer/XLSX/Helper/StyleHelper.php | 7 +++-- .../Spout/Writer/ODS/WriterWithStyleTest.php | 31 ++++++++++++++----- tests/Spout/Writer/Style/BorderTest.php | 15 +++++---- 11 files changed, 76 insertions(+), 48 deletions(-) diff --git a/src/Spout/Writer/Exception/Border/InvalidNameException.php b/src/Spout/Writer/Exception/Border/InvalidNameException.php index 0037668..13ac06c 100644 --- a/src/Spout/Writer/Exception/Border/InvalidNameException.php +++ b/src/Spout/Writer/Exception/Border/InvalidNameException.php @@ -7,13 +7,10 @@ use Box\Spout\Writer\Style\BorderPart; class InvalidNameException extends WriterException { - public function __construct($name) { - $msg = '%s is not a valid name identifier for a border. Valid identifiers are: %s.'; parent::__construct(sprintf($msg, $name, implode(',', BorderPart::getAllowedNames()))); - } } diff --git a/src/Spout/Writer/Exception/Border/InvalidStyleException.php b/src/Spout/Writer/Exception/Border/InvalidStyleException.php index b372fa0..c684339 100644 --- a/src/Spout/Writer/Exception/Border/InvalidStyleException.php +++ b/src/Spout/Writer/Exception/Border/InvalidStyleException.php @@ -9,10 +9,8 @@ class InvalidStyleException extends WriterException { public function __construct($name) { - $msg = '%s is not a valid style identifier for a border. Valid identifiers are: %s.'; parent::__construct(sprintf($msg, $name, implode(',', BorderPart::getAllowedStyles()))); - } } diff --git a/src/Spout/Writer/Exception/Border/InvalidWidthException.php b/src/Spout/Writer/Exception/Border/InvalidWidthException.php index b1fb162..32f092b 100644 --- a/src/Spout/Writer/Exception/Border/InvalidWidthException.php +++ b/src/Spout/Writer/Exception/Border/InvalidWidthException.php @@ -7,12 +7,10 @@ use Box\Spout\Writer\Style\BorderPart; class InvalidWidthException extends WriterException { - public function __construct($name) { $msg = '%s is not a valid width identifier for a border. Valid identifiers are: %s.'; parent::__construct(sprintf($msg, $name, implode(',', BorderPart::getAllowedWidths()))); - } } diff --git a/src/Spout/Writer/ODS/Helper/BorderHelper.php b/src/Spout/Writer/ODS/Helper/BorderHelper.php index 1ccc8de..f3c3fed 100644 --- a/src/Spout/Writer/ODS/Helper/BorderHelper.php +++ b/src/Spout/Writer/ODS/Helper/BorderHelper.php @@ -26,7 +26,7 @@ class BorderHelper * * @var array */ - public static $widthMap = [ + protected static $widthMap = [ Border::WIDTH_THIN => '0.75pt', Border::WIDTH_MEDIUM => '1.75pt', Border::WIDTH_THICK => '2.5pt', @@ -37,7 +37,7 @@ class BorderHelper * * @var array */ - public static $styleMap = [ + protected static $styleMap = [ Border::STYLE_SOLID => 'solid', Border::STYLE_DASHED => 'dashed', Border::STYLE_DOTTED => 'dotted', diff --git a/src/Spout/Writer/Style/Border.php b/src/Spout/Writer/Style/Border.php index f0140d3..b68ec80 100644 --- a/src/Spout/Writer/Style/Border.php +++ b/src/Spout/Writer/Style/Border.php @@ -28,7 +28,7 @@ class Border protected $parts = []; /** - * @param array $borderParts + * @param array|void $borderParts */ public function __construct(array $borderParts = []) { diff --git a/src/Spout/Writer/Style/BorderBuilder.php b/src/Spout/Writer/Style/BorderBuilder.php index 017646b..83d797a 100644 --- a/src/Spout/Writer/Style/BorderBuilder.php +++ b/src/Spout/Writer/Style/BorderBuilder.php @@ -18,9 +18,9 @@ class BorderBuilder } /** - * @param string $style Border style @see BorderPart::allowedStyles - * @param string $color Border A RGB color code - * @param string $width Border width @see BorderPart::allowedWidths + * @param string|void $style Border style @see BorderPart::allowedStyles + * @param string|void $color Border A RGB color code + * @param string|void $width Border width @see BorderPart::allowedWidths * @return self */ public function setBorderTop($style = Border::STYLE_SOLID, $color = Color::BLACK, $width = Border::WIDTH_THICK) @@ -30,9 +30,9 @@ class BorderBuilder } /** - * @param string $style Border style @see BorderPart::allowedStyles - * @param string $color Border A RGB color code - * @param string $width Border width @see BorderPart::allowedWidths + * @param string|void $style Border style @see BorderPart::allowedStyles + * @param string|void $color Border A RGB color code + * @param string|void $width Border width @see BorderPart::allowedWidths * @return self */ public function setBorderRight($style = Border::STYLE_SOLID, $color = Color::BLACK, $width = Border::WIDTH_THICK) @@ -42,9 +42,9 @@ class BorderBuilder } /** - * @param string $style Border style @see BorderPart::allowedStyles - * @param string $color Border A RGB color code - * @param string $width Border width @see BorderPart::allowedWidths + * @param string|void $style Border style @see BorderPart::allowedStyles + * @param string|void $color Border A RGB color code + * @param string|void $width Border width @see BorderPart::allowedWidths * @return self */ public function setBorderBottom($style = Border::STYLE_SOLID, $color = Color::BLACK, $width = Border::WIDTH_THICK) @@ -54,9 +54,9 @@ class BorderBuilder } /** - * @param string $style Border style @see BorderPart::allowedStyles - * @param string $color Border A RGB color code - * @param string $width Border width @see BorderPart::allowedWidths + * @param string|void $style Border style @see BorderPart::allowedStyles + * @param string|void $color Border A RGB color code + * @param string|void $width Border width @see BorderPart::allowedWidths * @return self */ public function setBorderLeft($style = Border::STYLE_SOLID, $color = Color::BLACK, $width = Border::WIDTH_THICK) diff --git a/src/Spout/Writer/Style/BorderPart.php b/src/Spout/Writer/Style/BorderPart.php index 096c2a6..42951d8 100644 --- a/src/Spout/Writer/Style/BorderPart.php +++ b/src/Spout/Writer/Style/BorderPart.php @@ -62,9 +62,9 @@ class BorderPart ]; /** - * @param string $name @see BorderPart::allowedNames - * @param string $style @see BorderPart::allowedStyles - * @param string $width @see BorderPart::allowedWidths + * @param string $name @see BorderPart::$allowedNames + * @param string $style @see BorderPart::$allowedStyles + * @param string $width @see BorderPart::$allowedWidths * @param string $color A RGB color code * @throws InvalidNameException * @throws InvalidStyleException @@ -87,7 +87,9 @@ class BorderPart } /** - * @param string $name + * @param string $name The name of the border part @see BorderPart::$allowedNames + * @throws InvalidNameException + * @return void */ public function setName($name) { @@ -106,7 +108,9 @@ class BorderPart } /** - * @param string $style + * @param string $style The style of the border part @see BorderPart::$allowedStyles + * @throws InvalidStyleException + * @return void */ public function setStyle($style) { @@ -125,7 +129,8 @@ class BorderPart } /** - * @param string $color + * @param string $color The color of the border part @see Color::rgb() + * @return void */ public function setColor($color) { @@ -141,7 +146,9 @@ class BorderPart } /** - * @param string $width + * @param string $width The width of the border part @see BorderPart::$allowedWidths + * @throws InvalidWidthException + * @return void */ public function setWidth($width) { diff --git a/src/Spout/Writer/XLSX/Helper/BorderHelper.php b/src/Spout/Writer/XLSX/Helper/BorderHelper.php index b940e99..ad63aea 100644 --- a/src/Spout/Writer/XLSX/Helper/BorderHelper.php +++ b/src/Spout/Writer/XLSX/Helper/BorderHelper.php @@ -41,7 +41,7 @@ class BorderHelper */ public static function serializeBorderPart(BorderPart $borderPart) { - $borderStyle = self::$xlsxStyleMap[$borderPart->getStyle()][$borderPart->getWidth()]; + $borderStyle = self::getBorderStyle($borderPart); $colorEl = $borderPart->getColor() ? sprintf('', $borderPart->getColor()) : ''; $partEl = sprintf( @@ -54,4 +54,15 @@ class BorderHelper return $partEl . PHP_EOL; } + + /** + * Get the style definition from the style map + * + * @param BorderPart $borderPart + * @return string + */ + protected static function getBorderStyle(BorderPart $borderPart) + { + return self::$xlsxStyleMap[$borderPart->getStyle()][$borderPart->getWidth()]; + } } diff --git a/src/Spout/Writer/XLSX/Helper/StyleHelper.php b/src/Spout/Writer/XLSX/Helper/StyleHelper.php index fa33e16..3c2b3d1 100644 --- a/src/Spout/Writer/XLSX/Helper/StyleHelper.php +++ b/src/Spout/Writer/XLSX/Helper/StyleHelper.php @@ -100,10 +100,13 @@ EOD; */ protected function getBordersSectionContent() { - $content = ''; + $registeredStyles = $this->getRegisteredStyles(); + $registeredStylesCount = count($registeredStyles); + + $content = ''; /** @var \Box\Spout\Writer\Style\Style $style */ - foreach ($this->getRegisteredStyles() as $style) { + foreach ($registeredStyles as $style) { $border = $style->getBorder(); if ($border) { $content .= ''; diff --git a/tests/Spout/Writer/ODS/WriterWithStyleTest.php b/tests/Spout/Writer/ODS/WriterWithStyleTest.php index 91f16f0..273528a 100644 --- a/tests/Spout/Writer/ODS/WriterWithStyleTest.php +++ b/tests/Spout/Writer/ODS/WriterWithStyleTest.php @@ -268,30 +268,45 @@ class WriterWithStyleTest extends \PHPUnit_Framework_TestCase $this->writeToODSFileWithMultipleStyles($dataRows, $fileName, $styles); $styleElements = $this->getCellStyleElementsFromContentXmlFile($fileName); + $this->assertEquals(3, count($styleElements), 'There should be 3 styles)'); + // Use reflection for protected members here + $widthMapReflection = new \ReflectionProperty('Box\Spout\Writer\ODS\Helper\BorderHelper', 'widthMap'); + $widthMapReflection->setAccessible(true); + $widthMap = $widthMapReflection->getValue(); + + $styleMapReflection = new \ReflectionProperty('Box\Spout\Writer\ODS\Helper\BorderHelper', 'styleMap'); + $styleMapReflection->setAccessible(true); + $styleMap = $styleMapReflection->getValue(); + $expectedFirst = sprintf( '%s %s #%s', - BorderHelper::$widthMap[Border::WIDTH_THICK], - BorderHelper::$styleMap[Border::STYLE_SOLID], + $widthMap[Border::WIDTH_THICK], + $styleMap[Border::STYLE_SOLID], Color::GREEN ); - $this->assertEquals($expectedFirst, $styleElements[1] + $actualFirst = $styleElements[1] ->getElementsByTagName('table-cell-properties') ->item(0) - ->getAttribute('fo:border-bottom')); + ->getAttribute('fo:border-bottom'); + + $this->assertEquals($expectedFirst, $actualFirst); $expectedThird = sprintf( '%s %s #%s', - BorderHelper::$widthMap[Border::WIDTH_THIN], - BorderHelper::$styleMap[Border::STYLE_DASHED], + $widthMap[Border::WIDTH_THIN], + $styleMap[Border::STYLE_DASHED], Color::RED ); - $this->assertEquals($expectedThird, $styleElements[2] + + $actualThird = $styleElements[2] ->getElementsByTagName('table-cell-properties') ->item(0) - ->getAttribute('fo:border-top')); + ->getAttribute('fo:border-top'); + + $this->assertEquals($expectedThird, $actualThird); } diff --git a/tests/Spout/Writer/Style/BorderTest.php b/tests/Spout/Writer/Style/BorderTest.php index a73498e..bb768b1 100644 --- a/tests/Spout/Writer/Style/BorderTest.php +++ b/tests/Spout/Writer/Style/BorderTest.php @@ -89,19 +89,17 @@ class BorderTest extends \PHPUnit_Framework_TestCase public function testAnyCombinationOfAllowedBorderPartsParams() { $color = Color::BLACK; - foreach(BorderPart::getAllowedNames() as $allowedName) - { - foreach(BorderPart::getAllowedStyles() as $allowedStyle) - { - foreach(BorderPart::getAllowedWidths() as $allowedWidth) - { + foreach (BorderPart::getAllowedNames() as $allowedName) { + foreach (BorderPart::getAllowedStyles() as $allowedStyle) { + foreach (BorderPart::getAllowedWidths() as $allowedWidth) { $borderPart = new BorderPart($allowedName, $allowedStyle, $color, $allowedWidth); $border = new Border(); $border->addPart($borderPart); $this->assertEquals(1, count($border->getParts())); - $part = $border->getParts()[$allowedName]; /** @var $part BorderPart */ + $part = $border->getParts()[$allowedName]; + $this->assertEquals($allowedStyle, $part->getStyle()); $this->assertEquals($allowedWidth, $part->getWidth()); $this->assertEquals($color, $part->getColor()); @@ -109,4 +107,5 @@ class BorderTest extends \PHPUnit_Framework_TestCase } } } -} \ No newline at end of file +} +