From 3abdf1b539845bd991ce2ee81975863fcc10b16c Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Fri, 21 Aug 2015 11:43:35 -0600 Subject: [PATCH 1/4] Improve Result interface Allow the Offset filter to stop the process function loop. The impetus is the lack of detail the result object contains - particularly when using the Offset filter. When processing a file in chunks the StepAggregator reports the number of successfully imported results and errors but not the number of skipped. This allows a user to seek the reader, and then process a batch and then stop. Receiving an accurate set of processed,skipped and errors. --- src/Exception/StopException.php | 10 ++++++++++ src/Filter/OffsetFilter.php | 13 ++++++++++++- src/Result.php | 17 ++++++++++++----- src/Workflow/StepAggregator.php | 16 +++++++++++++--- tests/Filter/OffsetFilterTest.php | 18 ++++++++++++++++++ tests/ResultTest.php | 14 +++++++------- 6 files changed, 72 insertions(+), 16 deletions(-) create mode 100644 src/Exception/StopException.php diff --git a/src/Exception/StopException.php b/src/Exception/StopException.php new file mode 100644 index 00000000..7518206b --- /dev/null +++ b/src/Exception/StopException.php @@ -0,0 +1,10 @@ +offset = $offset; $this->limit = $limit; + $this->stopOnMaxLimit = ($limit>0 && $stopOnLimit); } /** @@ -52,6 +59,10 @@ public function __invoke(array $item) { // In case we've already filtered up to limited if ($this->maxLimitHit) { + if($this->stopOnMaxLimit) { + throw new StopException(); + } + return false; } diff --git a/src/Result.php b/src/Result.php index dc9e7426..c8d6f062 100644 --- a/src/Result.php +++ b/src/Result.php @@ -57,18 +57,25 @@ class Result * @param string $name * @param \DateTime $startTime * @param \DateTime $endTime - * @param integer $totalCount + * @param integer $processed + * @param integer $imported + * @param integer $skipped + * @param integer $errors * @param \SplObjectStorage $exceptions */ - public function __construct($name, \DateTime $startTime, \DateTime $endTime, $totalCount, \SplObjectStorage $exceptions) + public function __construct($name, \DateTime $startTime, \DateTime $endTime, $processed, $imported, $skipped, $errors, \SplObjectStorage $exceptions) { $this->name = $name; $this->startTime = $startTime; $this->endTime = $endTime; $this->elapsed = $startTime->diff($endTime); - $this->totalProcessedCount = $totalCount; - $this->errorCount = count($exceptions); - $this->successCount = $totalCount - $this->errorCount; + + //Should expect $processed = $errors+$imported+$skipped + $this->totalProcessedCount = $processed; + $this->errorCount = $errors; + $this->successCount = $imported; + $this->skippedCount = $skipped; + $this->exceptions = $exceptions; } diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index db319b83..6d653556 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -107,7 +107,11 @@ public function addWriter(Writer $writer) */ public function process() { - $count = 0; + $processed = 0; + $skipped = 0; + $errors = 0; + $imported = 0; + $exceptions = new \SplObjectStorage(); $startTime = new \DateTime; @@ -131,9 +135,12 @@ public function process() break; } + $processed++; + try { foreach (clone $this->steps as $step) { if (false === $step->process($item)) { + $skipped++; continue 2; } } @@ -145,23 +152,26 @@ public function process() foreach ($this->writers as $writer) { $writer->writeItem($item); } + } catch(Exception\StopException $e) { + break; } catch(Exception $e) { if (!$this->skipItemOnFailure) { throw $e; } + $errors++; $exceptions->attach($e, $index); $this->logger->error($e->getMessage()); } - $count++; + $imported++; } foreach ($this->writers as $writer) { $writer->finish(); } - return new Result($this->name, $startTime, new \DateTime, $count, $exceptions); + return new Result($this->name, $startTime, new \DateTime, $processed, $imported, $skipped, $errors, $exceptions); } /** diff --git a/tests/Filter/OffsetFilterTest.php b/tests/Filter/OffsetFilterTest.php index 99dda153..9c6bf6d2 100644 --- a/tests/Filter/OffsetFilterTest.php +++ b/tests/Filter/OffsetFilterTest.php @@ -48,4 +48,22 @@ public function testOffsetWithMaxCount() $resultItems = $this->applyFilter(new OffsetFilter(1, 1), $items); $this->assertEquals($resultItems, array('second')); } + + /** + * @expectedException \Ddeboer\DataImport\Exception\StopException + */ + public function testMaxCountStop() + { + $items = array('first','second','third','fourth'); + $this->applyFilter(new OffsetFilter(0, 2, true), $items); + } + + /** + * @expectedException \Ddeboer\DataImport\Exception\StopException + */ + public function testOffsetWithMaxCountStop() + { + $items = array('first','second','third','fourth'); + $this->applyFilter(new OffsetFilter(1, 1, true), $items); + } } diff --git a/tests/ResultTest.php b/tests/ResultTest.php index 73274eaa..792f6d6a 100644 --- a/tests/ResultTest.php +++ b/tests/ResultTest.php @@ -13,13 +13,13 @@ class ResultTest extends \PHPUnit_Framework_TestCase { public function testResultName() { - $result = new Result('export', new \DateTime, new \DateTime, 10, new \SplObjectStorage()); + $result = new Result('export', new \DateTime, new \DateTime, 10, 10, 0, 0, new \SplObjectStorage()); $this->assertSame('export', $result->getName()); } public function testResultCounts() { - $result = new Result('export', new \DateTime, new \DateTime, 10, new \SplObjectStorage()); + $result = new Result('export', new \DateTime, new \DateTime, 10, 10, 0, 0, new \SplObjectStorage()); $this->assertSame(10, $result->getTotalProcessedCount()); $this->assertSame(10, $result->getSuccessCount()); $this->assertSame(0, $result->getErrorCount()); @@ -27,7 +27,7 @@ public function testResultCounts() $exceptions = new \SplObjectStorage(); $exceptions->attach(new \Exception()); $exceptions->attach(new \Exception()); - $result = new Result('export', new \DateTime, new \DateTime, 10, $exceptions); + $result = new Result('export', new \DateTime, new \DateTime, 10, 8, 0, 2, $exceptions); $this->assertSame(10, $result->getTotalProcessedCount()); $this->assertSame(8, $result->getSuccessCount()); $this->assertSame(2, $result->getErrorCount()); @@ -39,7 +39,7 @@ public function testDates() $startDate = new \DateTime("22-07-2014 22:00"); $endDate = new \DateTime("22-07-2014 23:30"); - $result = new Result('export', $startDate, $endDate, 10, new \SplObjectStorage()); + $result = new Result('export', $startDate, $endDate, 10, 10, 0, 0, new \SplObjectStorage()); $this->assertSame($startDate, $result->getStartTime()); $this->assertSame($endDate, $result->getEndTime()); @@ -52,13 +52,13 @@ public function testHasErrorsReturnsTrueIfAnyExceptions() $exceptions->attach(new \Exception()); $exceptions->attach(new \Exception()); - $result = new Result('export', new \DateTime, new \DateTime, 10, $exceptions); + $result = new Result('export', new \DateTime, new \DateTime, 10, 10, 0, 2, $exceptions); $this->assertTrue($result->hasErrors()); } public function testHasErrorsReturnsFalseIfNoExceptions() { - $result = new Result('export', new \DateTime, new \DateTime, 10, new \SplObjectStorage()); + $result = new Result('export', new \DateTime, new \DateTime, 10, 10, 0, 0, new \SplObjectStorage()); $this->assertFalse($result->hasErrors()); } @@ -68,7 +68,7 @@ public function testGetExceptions() $exceptions->attach(new \Exception()); $exceptions->attach(new \Exception()); - $result = new Result('export', new \DateTime, new \DateTime, 10, $exceptions); + $result = new Result('export', new \DateTime, new \DateTime, 10, 10, 0, 2, $exceptions); $this->assertSame($exceptions, $result->getExceptions()); } } From e6c4101c3aaca184812fefc3d0edef960f054d0c Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Fri, 21 Aug 2015 13:13:36 -0600 Subject: [PATCH 2/4] add missing getter for skippedCount --- src/Result.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Result.php b/src/Result.php index c8d6f062..48f9b41c 100644 --- a/src/Result.php +++ b/src/Result.php @@ -48,6 +48,11 @@ class Result */ protected $totalProcessedCount = 0; + /** + * @var integer + */ + protected $skippedCount = 0; + /** * @var \SplObjectStorage */ @@ -135,6 +140,14 @@ public function getTotalProcessedCount() return $this->totalProcessedCount; } + /** + * @return int + */ + public function getSkippedCount() + { + return $this->skippedCount; + } + /** * @return boolean */ From 6930a446c8b334f3df95d0c9df593b36616b0112 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Tue, 1 Sep 2015 14:37:54 -0600 Subject: [PATCH 3/4] Fix inaccurate processed count on StopException When a filter throws the StopException we need to decrement the processed counter to match the actual count. --- src/Workflow/StepAggregator.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index 6d653556..2fe09301 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -153,6 +153,7 @@ public function process() $writer->writeItem($item); } } catch(Exception\StopException $e) { + $processed--; break; } catch(Exception $e) { if (!$this->skipItemOnFailure) { From e226d9c19040d5a8a781c19ce29e2a4bdd850be2 Mon Sep 17 00:00:00 2001 From: "Nathanael d. Noblet" Date: Tue, 1 Sep 2015 17:02:47 -0600 Subject: [PATCH 4/4] Fix imported count when exceptions occur When an exception occurs we need continue the loop but not increment the imported counter. --- src/Workflow/StepAggregator.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Workflow/StepAggregator.php b/src/Workflow/StepAggregator.php index 2fe09301..11aeab40 100644 --- a/src/Workflow/StepAggregator.php +++ b/src/Workflow/StepAggregator.php @@ -163,6 +163,8 @@ public function process() $errors++; $exceptions->attach($e, $index); $this->logger->error($e->getMessage()); + + continue; } $imported++;