From 3bfb1416c1939cf6eb40db54fcc9cbd9c3688cf7 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Wed, 29 Jul 2015 13:46:15 -0600 Subject: [PATCH 01/10] Proof of concept reporter interface This is my first attempt at implementing the idea in issue #100 --- src/Report.php | 65 ++++++++++++++++++++++++++++++++ src/ReportMessage.php | 66 +++++++++++++++++++++++++++++++++ src/ReporterInterface.php | 16 ++++++++ src/Result.php | 17 ++++++++- src/Step.php | 2 +- src/Step/ConverterStep.php | 3 +- src/Step/FilterStep.php | 3 +- src/Step/MappingStep.php | 3 +- src/Step/ValidatorStep.php | 3 +- src/Step/ValueConverterStep.php | 8 +++- src/Workflow/StepAggregator.php | 8 +++- 11 files changed, 186 insertions(+), 8 deletions(-) create mode 100644 src/Report.php create mode 100644 src/ReportMessage.php create mode 100644 src/ReporterInterface.php diff --git a/src/Report.php b/src/Report.php new file mode 100644 index 00000000..8d118b16 --- /dev/null +++ b/src/Report.php @@ -0,0 +1,65 @@ +row = $row; + } + + /** + * @return integer + */ + public function getRow() + { + return $this->row; + } + + /** + * @param integer $row + * @return Report + */ + public function setRow($row) + { + $this->row = $row; + return $this; + } + + /** + * @return mixed + */ + public function getMessages() + { + return $this->messages; + } + + /** + * @param mixed $messages + * @return Report + */ + public function setMessages($messages) + { + $this->messages = $messages; + return $this; + } + + /** + * @param ReportMessage $message + */ + public function addMessage(ReportMessage $message) + { + $this->messages[] = $message; + } +} \ No newline at end of file diff --git a/src/ReportMessage.php b/src/ReportMessage.php new file mode 100644 index 00000000..bffb0c14 --- /dev/null +++ b/src/ReportMessage.php @@ -0,0 +1,66 @@ +message = $message; + $this->column = $column; + } + + /** + * @return mixed + */ + public function getColumn() + { + return $this->column; + } + + /** + * @param mixed $column + * @return ReportMessage + */ + public function setColumn($column) + { + $this->column = $column; + return $this; + } + + /** + * @return mixed + */ + public function getMessage() + { + return $this->message; + } + + /** + * @param mixed $message + * @return ReportMessage + */ + public function setMessage($message) + { + $this->message = $message; + return $this; + } + + /** + * @return mixed + */ + public function isException() + { + return ($this->message instanceof Exception); + } +} \ No newline at end of file diff --git a/src/ReporterInterface.php b/src/ReporterInterface.php new file mode 100644 index 00000000..891f640c --- /dev/null +++ b/src/ReporterInterface.php @@ -0,0 +1,16 @@ +name = $name; $this->startTime = $startTime; @@ -70,6 +75,7 @@ public function __construct($name, \DateTime $startTime, \DateTime $endTime, $to $this->errorCount = count($exceptions); $this->successCount = $totalCount - $this->errorCount; $this->exceptions = $exceptions; + $this->reports = $reports; } /** @@ -143,4 +149,13 @@ public function getExceptions() { return $this->exceptions; } + + /** + * @return \SplObjectStorage + */ + public function getReports() + { + return $this->reports; + } + } diff --git a/src/Step.php b/src/Step.php index fb1a9b98..5f2e404b 100644 --- a/src/Step.php +++ b/src/Step.php @@ -14,5 +14,5 @@ interface Step * * @return boolean False return value means the item should be skipped */ - public function process(&$item); + public function process(&$item, Report $report = null); } diff --git a/src/Step/ConverterStep.php b/src/Step/ConverterStep.php index 974fd809..ab38ebcf 100644 --- a/src/Step/ConverterStep.php +++ b/src/Step/ConverterStep.php @@ -2,6 +2,7 @@ namespace Ddeboer\DataImport\Step; +use Ddeboer\DataImport\Report; use Ddeboer\DataImport\Step; use Ddeboer\DataImport\Exception\UnexpectedTypeException; @@ -40,7 +41,7 @@ public function add(callable $converter) /** * {@inheritdoc} */ - public function process(&$item) + public function process(&$item, Report $report = null) { foreach ($this->converters as $converter) { $item = call_user_func($converter, $item); diff --git a/src/Step/FilterStep.php b/src/Step/FilterStep.php index 847b24a4..1821b000 100644 --- a/src/Step/FilterStep.php +++ b/src/Step/FilterStep.php @@ -2,6 +2,7 @@ namespace Ddeboer\DataImport\Step; +use Ddeboer\DataImport\Report; use Ddeboer\DataImport\Step; /** @@ -35,7 +36,7 @@ public function add(callable $filter, $priority = null) /** * {@inheritdoc} */ - public function process(&$item) + public function process(&$item, Report $report = null) { foreach (clone $this->filters as $filter) { if (false === call_user_func($filter, $item)) { diff --git a/src/Step/MappingStep.php b/src/Step/MappingStep.php index d98fb275..656b4a3a 100644 --- a/src/Step/MappingStep.php +++ b/src/Step/MappingStep.php @@ -3,6 +3,7 @@ namespace Ddeboer\DataImport\Step; use Ddeboer\DataImport\Exception\MappingException; +use Ddeboer\DataImport\Report; use Ddeboer\DataImport\Step; use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException; use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException; @@ -51,7 +52,7 @@ public function map($from, $to) * * @throws MappingException */ - public function process(&$item) + public function process(&$item, Report $report = null) { try { foreach ($this->mappings as $from => $to) { diff --git a/src/Step/ValidatorStep.php b/src/Step/ValidatorStep.php index 43cbda23..225c9a18 100644 --- a/src/Step/ValidatorStep.php +++ b/src/Step/ValidatorStep.php @@ -2,6 +2,7 @@ namespace Ddeboer\DataImport\Step; +use Ddeboer\DataImport\Report; use Symfony\Component\Validator\ValidatorInterface; use Symfony\Component\Validator\Constraints; use Symfony\Component\Validator\Constraint; @@ -81,7 +82,7 @@ public function getViolations() /** * {@inheritdoc} */ - public function process(&$item) + public function process(&$item, Report $report = null) { $constraints = new Constraints\Collection($this->constraints); $list = $this->validator->validateValue($item, $constraints); diff --git a/src/Step/ValueConverterStep.php b/src/Step/ValueConverterStep.php index 3855f11f..adddd35d 100644 --- a/src/Step/ValueConverterStep.php +++ b/src/Step/ValueConverterStep.php @@ -2,6 +2,9 @@ namespace Ddeboer\DataImport\Step; +use Ddeboer\DataImport\Report; +use Ddeboer\DataImport\ReporterInterface; +use Ddeboer\DataImport\ReportMessage; use Ddeboer\DataImport\Step; use Symfony\Component\PropertyAccess\PropertyAccessor; @@ -31,7 +34,7 @@ public function add($property, callable $converter) /** * {@inheritdoc} */ - public function process(&$item) + public function process(&$item, Report $report = null) { $accessor = new PropertyAccessor(); @@ -40,6 +43,9 @@ public function process(&$item) $orgValue = $accessor->getValue($item, $property); $value = call_user_func($converter, $orgValue); $accessor->setValue($item,$property,$value); + if($report !== null && $converter instanceof ReporterInterface && $converter->hasMessage()) { + $report->addMessage(new ReportMessage($converter->getMessage(),$property)); + } } } diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index db319b83..b9ca568e 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -5,6 +5,7 @@ use Ddeboer\DataImport\Exception; use Ddeboer\DataImport\Exception\UnexpectedTypeException; use Ddeboer\DataImport\Reader; +use Ddeboer\DataImport\Report; use Ddeboer\DataImport\Result; use Ddeboer\DataImport\Step; use Ddeboer\DataImport\Step\PriorityStep; @@ -109,6 +110,7 @@ public function process() { $count = 0; $exceptions = new \SplObjectStorage(); + $reports = new \SplObjectStorage(); $startTime = new \DateTime; foreach ($this->writers as $writer) { @@ -132,8 +134,10 @@ public function process() } try { + $report = new Report($index); + foreach (clone $this->steps as $step) { - if (false === $step->process($item)) { + if (false === $step->process($item, $report)) { continue 2; } } @@ -145,6 +149,8 @@ public function process() foreach ($this->writers as $writer) { $writer->writeItem($item); } + + $reports->attach($report,$index); } catch(Exception $e) { if (!$this->skipItemOnFailure) { throw $e; From ab89587f7d3e7dedcf453fb230508052390a22c5 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Wed, 29 Jul 2015 13:56:45 -0600 Subject: [PATCH 02/10] add interface handling to filter and converter steps --- src/Step/ConverterStep.php | 4 ++++ src/Step/FilterStep.php | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/Step/ConverterStep.php b/src/Step/ConverterStep.php index ab38ebcf..7c9e331c 100644 --- a/src/Step/ConverterStep.php +++ b/src/Step/ConverterStep.php @@ -45,6 +45,10 @@ public function process(&$item, Report $report = null) { foreach ($this->converters as $converter) { $item = call_user_func($converter, $item); + + if($report !== null && $converter instanceof ReporterInterface && $converter->hasMessage()) { + $report->addMessage(new ReportMessage($converter->getMessage())); + } } return true; diff --git a/src/Step/FilterStep.php b/src/Step/FilterStep.php index 1821b000..b069211c 100644 --- a/src/Step/FilterStep.php +++ b/src/Step/FilterStep.php @@ -40,6 +40,10 @@ public function process(&$item, Report $report = null) { foreach (clone $this->filters as $filter) { if (false === call_user_func($filter, $item)) { + if($report !== null && $filter instanceof ReporterInterface && $filter->hasMessage()) { + $report->addMessage(new ReportMessage($filter->getMessage())); + } + return false; } } From 845a83964e6fc700cd9f9f7e7168b0c23ea9108d Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Wed, 5 Aug 2015 11:51:41 -0600 Subject: [PATCH 03/10] Added severity level This allows a report message to have a severity. --- src/Report.php | 15 +++++++++++++-- src/ReportMessage.php | 22 +++++++++++++++++++++- src/ReporterInterface.php | 5 +++++ src/Step/ConverterStep.php | 2 +- src/Step/FilterStep.php | 2 +- src/Step/ValueConverterStep.php | 2 +- 6 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/Report.php b/src/Report.php index 8d118b16..c69be129 100644 --- a/src/Report.php +++ b/src/Report.php @@ -40,9 +40,20 @@ public function setRow($row) /** * @return mixed */ - public function getMessages() + public function getMessages($severity = null) { - return $this->messages; + if($severity === null) { + return $this->messages; + } + + $messages = array(); + foreach($this->messages as $message) { + if($message->getSeverity() == $severity) { + $messages[] = $message; + } + } + + return $messages; } /** diff --git a/src/ReportMessage.php b/src/ReportMessage.php index bffb0c14..b7111981 100644 --- a/src/ReportMessage.php +++ b/src/ReportMessage.php @@ -13,10 +13,12 @@ class ReportMessage { private $column; private $message; + private $severity; - public function __construct($message, $column = null) + public function __construct($message, $severity, $column = null) { $this->message = $message; + $this->severity = $severity; $this->column = $column; } @@ -46,6 +48,24 @@ public function getMessage() return $this->message; } + /** + * @return mixed + */ + public function getSeverity() + { + return $this->severity; + } + + /** + * @param integer $severity + * @return ReportMessage + */ + public function setSeverity($severity) + { + $this->severity = $severity; + return $this; + } + /** * @param mixed $message * @return ReportMessage diff --git a/src/ReporterInterface.php b/src/ReporterInterface.php index 891f640c..633422fa 100644 --- a/src/ReporterInterface.php +++ b/src/ReporterInterface.php @@ -11,6 +11,11 @@ interface ReporterInterface { + const INFO = 0; + const WARNING = 1; + const ERROR = 2; + public function hasMessage(); public function getMessage(); + public function getSeverity(); } \ No newline at end of file diff --git a/src/Step/ConverterStep.php b/src/Step/ConverterStep.php index 7c9e331c..41e4b6ac 100644 --- a/src/Step/ConverterStep.php +++ b/src/Step/ConverterStep.php @@ -47,7 +47,7 @@ public function process(&$item, Report $report = null) $item = call_user_func($converter, $item); if($report !== null && $converter instanceof ReporterInterface && $converter->hasMessage()) { - $report->addMessage(new ReportMessage($converter->getMessage())); + $report->addMessage(new ReportMessage($converter->getMessage(),$converter->getSeverity())); } } diff --git a/src/Step/FilterStep.php b/src/Step/FilterStep.php index b069211c..0fee160e 100644 --- a/src/Step/FilterStep.php +++ b/src/Step/FilterStep.php @@ -41,7 +41,7 @@ public function process(&$item, Report $report = null) foreach (clone $this->filters as $filter) { if (false === call_user_func($filter, $item)) { if($report !== null && $filter instanceof ReporterInterface && $filter->hasMessage()) { - $report->addMessage(new ReportMessage($filter->getMessage())); + $report->addMessage(new ReportMessage($filter->getMessage(),$filter->getSeverity())); } return false; diff --git a/src/Step/ValueConverterStep.php b/src/Step/ValueConverterStep.php index adddd35d..87164a73 100644 --- a/src/Step/ValueConverterStep.php +++ b/src/Step/ValueConverterStep.php @@ -44,7 +44,7 @@ public function process(&$item, Report $report = null) $value = call_user_func($converter, $orgValue); $accessor->setValue($item,$property,$value); if($report !== null && $converter instanceof ReporterInterface && $converter->hasMessage()) { - $report->addMessage(new ReportMessage($converter->getMessage(),$property)); + $report->addMessage(new ReportMessage($converter->getMessage(),$converter->getSeverity(),$property)); } } } From 2e6be680755c8be934580b0b7d21c8767950cf1a Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Wed, 5 Aug 2015 11:52:47 -0600 Subject: [PATCH 04/10] docblock updated --- src/Report.php | 3 +++ src/Result.php | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Report.php b/src/Report.php index c69be129..1f8882b3 100644 --- a/src/Report.php +++ b/src/Report.php @@ -14,6 +14,9 @@ class Report private $row; private $messages; + /** + * @param $row + */ public function __construct($row) { $this->row = $row; diff --git a/src/Result.php b/src/Result.php index 32213845..72646961 100644 --- a/src/Result.php +++ b/src/Result.php @@ -64,6 +64,7 @@ class Result * @param \DateTime $endTime * @param integer $totalCount * @param \SplObjectStorage $exceptions + * @param \SplObjectStorage $reports */ public function __construct($name, \DateTime $startTime, \DateTime $endTime, $totalCount, \SplObjectStorage $exceptions, \SplObjectStorage $reports = null) { @@ -157,5 +158,4 @@ public function getReports() { return $this->reports; } - } From ae4558cc5974ed88cb5e68ae6b809fad73934005 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Wed, 5 Aug 2015 11:53:08 -0600 Subject: [PATCH 05/10] actually pass the reports to the result object --- src/Workflow/StepAggregator.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index b9ca568e..3df59fe1 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -149,8 +149,6 @@ public function process() foreach ($this->writers as $writer) { $writer->writeItem($item); } - - $reports->attach($report,$index); } catch(Exception $e) { if (!$this->skipItemOnFailure) { throw $e; @@ -160,6 +158,8 @@ public function process() $this->logger->error($e->getMessage()); } + $reports->attach($report,$index); + $count++; } @@ -167,7 +167,7 @@ public function process() $writer->finish(); } - return new Result($this->name, $startTime, new \DateTime, $count, $exceptions); + return new Result($this->name, $startTime, new \DateTime, $count, $exceptions, $reports); } /** From 147715af9c49eb7b690f1322062381eb838c2e9f Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Wed, 5 Aug 2015 22:57:05 -0600 Subject: [PATCH 06/10] fix constructor, added typehint and dockblocks --- src/Report.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Report.php b/src/Report.php index 1f8882b3..b9bcbb5d 100644 --- a/src/Report.php +++ b/src/Report.php @@ -11,7 +11,14 @@ class Report { + /** + * @var integer $row + */ private $row; + + /** + * @var array $messages + */ private $messages; /** @@ -20,6 +27,7 @@ class Report public function __construct($row) { $this->row = $row; + $this->messages = array(); } /** @@ -63,7 +71,7 @@ public function getMessages($severity = null) * @param mixed $messages * @return Report */ - public function setMessages($messages) + public function setMessages(array $messages) { $this->messages = $messages; return $this; @@ -76,4 +84,5 @@ public function addMessage(ReportMessage $message) { $this->messages[] = $message; } -} \ No newline at end of file +} + From 70e714fde15ae525b8b8cc4c39da4a8d504367cc Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Tue, 18 Aug 2015 12:29:30 -0600 Subject: [PATCH 07/10] Only add report message when it exists --- src/Report.php | 8 ++++++++ src/Workflow/StepAggregator.php | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Report.php b/src/Report.php index b9bcbb5d..93fc9046 100644 --- a/src/Report.php +++ b/src/Report.php @@ -84,5 +84,13 @@ public function addMessage(ReportMessage $message) { $this->messages[] = $message; } + + /** + * @return boolean + */ + public function hasMessages() + { + return (!empty($this->messages)); + } } diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index 3df59fe1..6363ab2a 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -158,7 +158,9 @@ public function process() $this->logger->error($e->getMessage()); } - $reports->attach($report,$index); + if ($report->hasMessages()) { + $reports->attach($report,$index); + } $count++; } From 80f8347484063f66e57579c74b19e2ebcb545645 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Tue, 1 Sep 2015 16:45:45 -0600 Subject: [PATCH 08/10] Fix imported count when exceptions occur When an exception occurs we need to record any messages and then continue the loop, otherwise the imported count gets incremented erroneously. --- src/Workflow/StepAggregator.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index 6363ab2a..1823f8ae 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -156,6 +156,11 @@ public function process() $exceptions->attach($e, $index); $this->logger->error($e->getMessage()); + + if ($report->hasMessages()) { + $reports->attach($report,$index); + } + continue; } if ($report->hasMessages()) { From c6afc5bcf0010f99811cd8920afaa9441a74fff7 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Tue, 10 Nov 2015 18:54:25 +0100 Subject: [PATCH 09/10] fix missing use statements --- src/Step/ConverterStep.php | 2 ++ src/Step/FilterStep.php | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/Step/ConverterStep.php b/src/Step/ConverterStep.php index 41e4b6ac..151d618d 100644 --- a/src/Step/ConverterStep.php +++ b/src/Step/ConverterStep.php @@ -5,6 +5,8 @@ use Ddeboer\DataImport\Report; use Ddeboer\DataImport\Step; use Ddeboer\DataImport\Exception\UnexpectedTypeException; +use Ddeboer\DataImport\ReporterInterface; +use Ddeboer\DataImport\ReportMessage; /** * @author Markus Bachmann diff --git a/src/Step/FilterStep.php b/src/Step/FilterStep.php index 0fee160e..78da0e18 100644 --- a/src/Step/FilterStep.php +++ b/src/Step/FilterStep.php @@ -4,6 +4,8 @@ use Ddeboer\DataImport\Report; use Ddeboer\DataImport\Step; +use Ddeboer\DataImport\ReporterInterface; +use Ddeboer\DataImport\ReportMessage; /** * @author Markus Bachmann @@ -51,3 +53,4 @@ public function process(&$item, Report $report = null) return true; } } + From 8a70918f7d89d60366b5e07d7b768330ad27c860 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Wed, 11 Nov 2015 11:07:56 +0100 Subject: [PATCH 10/10] Catch any filter based error messages Sometimes a filter may provide detailed information regarding why it is rejecting a particular row. Without detecting that a message exists it will be lost. --- src/Workflow/StepAggregator.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index 1823f8ae..9c865acb 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -138,6 +138,9 @@ public function process() foreach (clone $this->steps as $step) { if (false === $step->process($item, $report)) { + if ($report->hasMessages()) { + $reports->attach($report,$index); + } continue 2; } }