Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.1 #3

Merged
merged 22 commits into from
Jun 9, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ vendor
composer.lock
phpunit.xml
~$*.xlsx

/nbproject
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# CHANGELOG

## 1.1

### Features

* CSV support

### BC Breaks

* WorkbookInterface is now SpreasheetInterface
* WorkbookLoaderInterface is now SpreadsheetLoaderInterface

44 changes: 42 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Akeneo Spreadsheet Parser

This component is designed to extract data from spreadsheets, while being easy on resources, even for large files.

The actual version of the spreadsheet parser only works with xlsx files.
The actual version of the spreadsheet parser works with csv and xlsx files.

[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/akeneo/spreadsheet-parser/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/akeneo/spreadsheet-parser/?branch=master)

Expand All @@ -18,7 +18,7 @@ From your application root:
Usage
-----

To extract data from a spreadsheet, use the following code:
To extract data from an XLSX spreadsheet, use the following code:

<?php

Expand All @@ -31,3 +31,43 @@ To extract data from a spreadsheet, use the following code:
foreach ($workbook->createIterator($myWorksheetIndex) as $rowIndex => $values) {
var_dump($rowIndex, $values);
}


By using the CSV parser options, you can specify the format of your CSV file :

<?php

use Akeneo\Component\SpreadsheetParser\SpreadsheetParser;

$workbook = SpreadsheetParser::open('myfile.csv');

$iterator = $workbook->createIterator(
0,
[
'encoding' => 'UTF-8',
'length' => null,
'delimiter' => ',',
'enclosure' => '"',
'escape' => '\\'
]
);


foreach ($workbook->createIterator($myWorksheetIndex) as $rowIndex => $values) {
var_dump($rowIndex, $values);
}


Running the tests
-----------------

To run unit tests, use phpspec:

$ php bin/phpspec run


To run integration tests, use phpunit:

$ phpunit


6 changes: 5 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
"homepage": "http://akeneo.com"
}
],
"require": {
"php": ">=5.4.0",
"symfony/options-resolver": "2.*"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might prefer to rely on ~2.4.0 (otherwise you'll get 2.5, 2.6, etc.).

},
"require-dev": {
"phpspec/phpspec": "2.0.*@dev"
},
Expand All @@ -26,7 +30,7 @@
},
"extra": {
"branch-alias": {
"dev-master": "1.0.x-dev"
"dev-master": "1.1.x-dev"
}
},
"config": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

namespace spec\Akeneo\Component\SpreadsheetParser\Csv;

use PhpSpec\ObjectBehavior;

class RowIteratorFactorySpec extends ObjectBehavior
{
public function let()
{
$this->beConstructedWith(
'spec\Akeneo\Component\SpreadsheetParser\Csv\StubRowIterator'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you pass the class to create namespace in the ctor?

Is it to override it easily? Cause it seems pretty hackish to me. Instead I would override the whole factory if I need to change the created object.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is really easy to override, with the component and with the bundle. It also makes testing the factory easier. This is a design pattern, it is not hackish ;)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instanciating a class through a variable value is (new $class()). What if
class ctor have arguments? Then you're screwed and need to override the
factory. That's why I think factory should contain hardcoded instanciation
of one given class.

2014-06-09 11:14 GMT+02:00 Antoine Guigan [email protected]:

In spec/Akeneo/Component/SpreadsheetParser/Csv/RowIteratorFactorySpec.php:

@@ -0,0 +1,46 @@
+<?php
+
+namespace spec\Akeneo\Component\SpreadsheetParser\Csv;
+
+use PhpSpec\ObjectBehavior;
+
+class RowIteratorFactorySpec extends ObjectBehavior
+{

  • public function let()
  • {
  •    $this->beConstructedWith(
    
  •        'spec\Akeneo\Component\SpreadsheetParser\Csv\StubRowIterator'
    

It is really easy to override, with the component and with the bundle. It
also makes testing the factory easier. This is a design pattern, it is not
hackish ;)


Reply to this email directly or view it on GitHub
https://github.com/akeneo/spreadsheet-parser/pull/3/files#r13533637.

Gildas Quéméner

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree, for two reasons :

  • this way, i can define a stub in my specs, and can test the constructor of the objects is called with the right arguments. Without this, there is no way to have a strict and complete unit test on the factories
  • most of the time, as an integrator, I do not need more dependencies in the classes I extend. Having a parameter for the class of the object helps me to go quicker.

All of this makes more sense if you look at the bundle : https://github.com/akeneo/SpreadsheetParserBundle/blob/1.1/Resources/config/xlsx.yml

);
}

public function it_is_initializable()
{
$this->shouldHaveType('Akeneo\Component\SpreadsheetParser\Csv\RowIteratorFactory');
}

public function it_creates_row_iterators(
) {
$iterator = $this->create('path', ['options']);
$iterator->getPath()->shouldReturn('path');
$iterator->getOptions()->shouldReturn(['options']);
}
}

class StubRowIterator
{
protected $path;
protected $options;
public function __construct($path, $options)
{
$this->path = $path;
$this->options = $options;
}
public function getOptions()
{
return $this->options;
}
public function getPath()
{
return $this->path;
}
}
80 changes: 80 additions & 0 deletions spec/Akeneo/Component/SpreadsheetParser/Csv/RowIteratorSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

namespace spec\Akeneo\Component\SpreadsheetParser\Csv;

use PhpSpec\ObjectBehavior;

class RowIteratorSpec extends ObjectBehavior
{
protected $values = [
['value', 'enclosed value', '15'],
['', 'value2', '']
];

public function it_is_initializable()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove public keyword

{
$this->beConstructedWith('path', []);
$this->shouldHaveType('Akeneo\Component\SpreadsheetParser\Csv\RowIterator');
}

public function it_parses_csv_files()
{
$this->beConstructedWith(__DIR__ . '/fixtures/test.csv' , []);
$this->rewind();
foreach ($this->values as $i=>$row) {
$this->key()->shouldReturn($i);
$this->valid()->shouldReturn(true);
$this->current()->shouldReturn($row);
$this->next();
}
$this->valid()->shouldReturn(false);
}

public function it_can_be_rewinded()
{
$this->beConstructedWith(__DIR__ . '/fixtures/test.csv' , []);
$this->rewind();
$this->current()->shouldReturn($this->values[0]);
$this->next();
$this->rewind();
$this->current()->shouldReturn($this->values[0]);
}

public function it_accepts_options()
{
$this->beConstructedWith(
__DIR__ . '/fixtures/with_options.csv',
[
'delimiter' => '|',
'enclosure' => "@"
]
);
$this->rewind();
foreach ($this->values as $i => $row) {
$this->key()->shouldReturn($i);
$this->valid()->shouldReturn(true);
$this->current()->shouldReturn($row);
$this->next();
}
$this->valid()->shouldReturn(false);
}

public function it_converts_between_encodings()
{
$this->beConstructedWith(
__DIR__ . '/fixtures/iso-8859-15.csv',
[
'encoding' => 'iso-8859-15'
]
);
$values = [['é', 'è', '€']];
$this->rewind();
foreach ($values as $i => $row) {
$this->key()->shouldReturn($i);
$this->valid()->shouldReturn(true);
$this->current()->shouldReturn($row);
$this->next();
}
$this->valid()->shouldReturn(false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

namespace spec\Akeneo\Component\SpreadsheetParser\Csv;

use PhpSpec\ObjectBehavior;
use Akeneo\Component\SpreadsheetParser\Csv\RowIteratorFactory;

class SpreadsheetLoaderSpec extends ObjectBehavior
{
public function let(
RowIteratorFactory $rowIteratorFactory
) {
$this->beConstructedWith(
$rowIteratorFactory,
'spec\Akeneo\Component\SpreadsheetParser\Csv\StubSpreadsheet',
'sheet'
);
}

public function it_is_initializable()
{
$this->shouldHaveType('Akeneo\Component\SpreadsheetParser\Csv\SpreadsheetLoader');
}

public function it_creates_spreadsheet_objects(
RowIteratorFactory $rowIteratorFactory
) {
$spreadsheet = $this->open('path');
$spreadsheet->getPath()->shouldReturn('path');
$spreadsheet->getSheetName()->shouldReturn('sheet');
$spreadsheet->getRowIteratorFactory()->shouldReturn($rowIteratorFactory);
}
}

class StubSpreadsheet
{
protected $rowIteratorFactory;
protected $sheetName;
protected $path;
public function __construct($rowIteratorFactory, $sheetName, $path)
{
$this->rowIteratorFactory = $rowIteratorFactory;
$this->sheetName = $sheetName;
$this->path = $path;
}
public function getRowIteratorFactory()
{
return $this->rowIteratorFactory;
}
public function getSheetName()
{
return $this->sheetName;
}
public function getPath()
{
return $this->path;
}
}
47 changes: 47 additions & 0 deletions spec/Akeneo/Component/SpreadsheetParser/Csv/SpreadsheetSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace spec\Akeneo\Component\SpreadsheetParser\Csv;

use PhpSpec\ObjectBehavior;
use Akeneo\Component\SpreadsheetParser\Csv\RowIterator;
use Akeneo\Component\SpreadsheetParser\Csv\RowIteratorFactory;

class SpreadsheetSpec extends ObjectBehavior
{
public function let(RowIteratorFactory $rowIteratorFactory)
{
$this->beConstructedWith($rowIteratorFactory, 'sheet', 'path');
}

public function it_is_initializable()
{
$this->shouldHaveType('Akeneo\Component\SpreadsheetParser\Csv\Spreadsheet');
}

public function it_returns_the_worksheet_list()
{
$this->getWorksheets()->shouldReturn(['sheet']);
}

public function it_creates_row_iterators(
RowIteratorFactory $rowIteratorFactory,
RowIterator $rowIterator1,
RowIterator $rowIterator2
) {
$rowIteratorFactory->create('path', ['options1'])->willReturn($rowIterator1);
$rowIteratorFactory->create('path', ['options2'])->willReturn($rowIterator2);

$this->createRowIterator(0, ['options1'])->shouldReturn($rowIterator1);
$this->createRowIterator(1, ['options2'])->shouldReturn($rowIterator2);
}

public function it_finds_a_worksheet_index_by_name()
{
$this->getWorksheetIndex('sheet')->shouldReturn(0);
}

public function it_returns_false_if_a_worksheet_does_not_exist()
{
$this->getWorksheetIndex('sheet3')->shouldReturn(false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
�,�,�
2 changes: 2 additions & 0 deletions spec/Akeneo/Component/SpreadsheetParser/Csv/fixtures/test.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
value,"enclosed value",15
,value2,
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
value|@enclosed value@|15
|value2|
35 changes: 35 additions & 0 deletions spec/Akeneo/Component/SpreadsheetParser/SpreadsheetLoaderSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

namespace spec\Akeneo\Component\SpreadsheetParser;

use Akeneo\Component\SpreadsheetParser\SpreadsheetInterface;
use Akeneo\Component\SpreadsheetParser\SpreadsheetLoaderInterface;
use PhpSpec\ObjectBehavior;

class SpreadsheetLoaderSpec extends ObjectBehavior
{
public function let(
SpreadsheetLoaderInterface $loader,
SpreadsheetLoaderInterface $otherLoader,
SpreadsheetInterface $spreadsheet
) {
$this->addLoader('extension', $loader)->addLoader('other_extension', $otherLoader);
$loader->open('file.extension')->willReturn($spreadsheet);
}

public function it_is_initializable()
{
$this->shouldHaveType('Akeneo\Component\SpreadsheetParser\SpreadsheetLoader');
}

public function it_uses_the_loader_corresponding_to_the_file_extension(
SpreadsheetInterface $spreadsheet
) {
$this->open('file.extension')->shouldReturn($spreadsheet);
}

public function it_throws_an_exception_if_no_loader_is_available()
{
$this->shouldThrow('\InvalidArgumentException')->duringOpen('file');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ArchiveSpec extends ObjectBehavior
{
public function let()
{
$this->beConstructedWith(__DIR__ . '/../fixtures/test.zip');
$this->beConstructedWith(__DIR__ . '/fixtures/test.zip');
}

public function it_is_initializable()
Expand Down
Loading