Skip to content

Commit 18e3c00

Browse files
oleibmanoleibman
committed
Performance Improvements for Csv Reader
Fix #460. Another in the "better late than never" series, closed as stale in June 2018. Ods Writer and Ods Reader handle booleans differently; what is worse, neither of them do it correctly. They will now match the behavior of LibreOffice. Reporter said that part of the xml would vary depending on locale; I believe that part is never actually used, but I do emulate that behavior.
1 parent 1b68270 commit 18e3c00

File tree

6 files changed

+91
-19
lines changed

6 files changed

+91
-19
lines changed

‎src/PhpSpreadsheet/Reader/Ods.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ public function loadIntoExisting(string $filename, Spreadsheet $spreadsheet): Sp
492492
break;
493493
case 'boolean':
494494
$type = DataType::TYPE_BOOL;
495-
$dataValue = ($allCellDataText == 'TRUE') ? true : false;
495+
$dataValue = ($cellData->getAttributeNS($officeNs, 'boolean-value') === 'true') ? true : false;
496496

497497
break;
498498
case 'percentage':

‎src/PhpSpreadsheet/Writer/Ods/Content.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Writer\Ods;
44

5+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
56
use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException;
67
use PhpOffice\PhpSpreadsheet\Cell\Cell;
78
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
@@ -209,8 +210,8 @@ private function writeCells(XMLWriter $objWriter, RowCellIterator $cells): void
209210
switch ($cell->getDataType()) {
210211
case DataType::TYPE_BOOL:
211212
$objWriter->writeAttribute('office:value-type', 'boolean');
212-
$objWriter->writeAttribute('office:value', $cell->getValueString());
213-
$objWriter->writeElement('text:p', $cell->getValueString());
213+
$objWriter->writeAttribute('office:boolean-value', $cell->getValue() ? 'true' : 'false');
214+
$objWriter->writeElement('text:p', Calculation::getInstance()->getLocaleBoolean($cell->getValue() ? 'TRUE' : 'FALSE'));
214215

215216
break;
216217
case DataType::TYPE_ERROR:

‎tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php

-6
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,15 @@ class CalculationFunctionListTest extends TestCase
1313
{
1414
private string $compatibilityMode;
1515

16-
private string $locale;
17-
1816
protected function setUp(): void
1917
{
2018
$this->compatibilityMode = Functions::getCompatibilityMode();
21-
$calculation = Calculation::getInstance();
22-
$this->locale = $calculation->getLocale();
2319
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
2420
}
2521

2622
protected function tearDown(): void
2723
{
2824
Functions::setCompatibilityMode($this->compatibilityMode);
29-
$calculation = Calculation::getInstance();
30-
$calculation->setLocale($this->locale);
3125
}
3226

3327
/**

‎tests/PhpSpreadsheetTests/Calculation/CalculationTest.php

-6
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,15 @@ class CalculationTest extends TestCase
1515
{
1616
private string $compatibilityMode;
1717

18-
private string $locale;
19-
2018
protected function setUp(): void
2119
{
2220
$this->compatibilityMode = Functions::getCompatibilityMode();
23-
$calculation = Calculation::getInstance();
24-
$this->locale = $calculation->getLocale();
2521
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
2622
}
2723

2824
protected function tearDown(): void
2925
{
3026
Functions::setCompatibilityMode($this->compatibilityMode);
31-
$calculation = Calculation::getInstance();
32-
$calculation->setLocale($this->locale);
3327
}
3428

3529
/**

‎tests/PhpSpreadsheetTests/Reader/Ods/BooleanDataTest.php

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods;
6+
7+
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
8+
use PhpOffice\PhpSpreadsheet\Reader\Ods as OdsReader;
9+
use PhpOffice\PhpSpreadsheet\Shared\File;
10+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
11+
use PhpOffice\PhpSpreadsheet\Writer\Ods as OdsWriter;
12+
use PHPUnit\Framework\TestCase;
13+
14+
class BooleanDataTest extends TestCase
15+
{
16+
private string $tempfile = '';
17+
18+
private string $locale;
19+
20+
protected function setUp(): void
21+
{
22+
$calculation = Calculation::getInstance();
23+
$this->locale = $calculation->getLocale();
24+
}
25+
26+
protected function tearDown(): void
27+
{
28+
$calculation = Calculation::getInstance();
29+
$calculation->setLocale($this->locale);
30+
if ($this->tempfile !== '') {
31+
unlink($this->tempfile);
32+
$this->tempfile = '';
33+
}
34+
}
35+
36+
public function testBooleanData(): void
37+
{
38+
$spreadsheetOld = new Spreadsheet();
39+
$sheetOld = $spreadsheetOld->getActiveSheet();
40+
$sheetOld->getCell('A1')->setValue(true);
41+
$sheetOld->getCell('A2')->setValue(false);
42+
$writer = new OdsWriter($spreadsheetOld);
43+
$this->tempfile = File::temporaryFileName();
44+
$writer->save($this->tempfile);
45+
$spreadsheetOld->disconnectWorksheets();
46+
$reader = new OdsReader();
47+
$spreadsheet = $reader->load($this->tempfile);
48+
$sheet = $spreadsheet->getActiveSheet();
49+
self::assertTrue($sheet->getCell('A1')->getValue());
50+
self::assertFalse($sheet->getCell('A2')->getValue());
51+
$spreadsheet->disconnectWorksheets();
52+
$zipFile = 'zip://' . $this->tempfile . '#content.xml';
53+
$contents = (string) file_get_contents($zipFile);
54+
self::assertStringContainsString('<text:p>TRUE</text:p>', $contents);
55+
self::assertStringContainsString('<text:p>FALSE</text:p>', $contents);
56+
}
57+
58+
public function testBooleanDataGerman(): void
59+
{
60+
$calculation = Calculation::getInstance();
61+
$calculation->setLocale('de');
62+
$spreadsheetOld = new Spreadsheet();
63+
$sheetOld = $spreadsheetOld->getActiveSheet();
64+
$sheetOld->getCell('A1')->setValue(true);
65+
$sheetOld->getCell('A2')->setValue(false);
66+
$writer = new OdsWriter($spreadsheetOld);
67+
$this->tempfile = File::temporaryFileName();
68+
$writer->save($this->tempfile);
69+
$spreadsheetOld->disconnectWorksheets();
70+
$reader = new OdsReader();
71+
$spreadsheet = $reader->load($this->tempfile);
72+
$sheet = $spreadsheet->getActiveSheet();
73+
self::assertTrue($sheet->getCell('A1')->getValue());
74+
self::assertFalse($sheet->getCell('A2')->getValue());
75+
$spreadsheet->disconnectWorksheets();
76+
$zipFile = 'zip://' . $this->tempfile . '#content.xml';
77+
$contents = (string) file_get_contents($zipFile);
78+
self::assertStringContainsString('<text:p>WAHR</text:p>', $contents);
79+
self::assertStringContainsString('<text:p>FALSCH</text:p>', $contents);
80+
self::assertStringNotContainsString('<text:p>TRUE</text:p>', $contents);
81+
self::assertStringNotContainsString('<text:p>FALSE</text:p>', $contents);
82+
}
83+
}

‎tests/data/Writer/Ods/content-with-data.xml

+4-4
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,11 @@
9595
<table:table-cell table:number-columns-repeated="1017"/>
9696
</table:table-row>
9797
<table:table-row>
98-
<table:table-cell office:value="1" office:value-type="boolean" table:style-name="ce0">
99-
<text:p>1</text:p>
98+
<table:table-cell office:boolean-value="true" office:value-type="boolean" table:style-name="ce0">
99+
<text:p>TRUE</text:p>
100100
</table:table-cell>
101-
<table:table-cell office:value="" office:value-type="boolean" table:style-name="ce0">
102-
<text:p/>
101+
<table:table-cell office:boolean-value="false" office:value-type="boolean" table:style-name="ce0">
102+
<text:p>FALSE</text:p>
103103
</table:table-cell>
104104
<table:table-cell office:value="1 1" office:value-type="string" table:formula="of:=IF([.A3]; CONCATENATE([.A1]; &quot; &quot;; [.A2]); CONCATENATE([.A2]; &quot; &quot;; [.A1]))" table:style-name="ce10">
105105
<text:p>1 1</text:p>

0 commit comments

Comments
 (0)