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

Fixed Tautrimas comments and added basic pipeline documentation. #103

Merged
merged 3 commits into from
Jan 20, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions EventListener/AbstractConsumeEventListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use ONGR\ConnectionsBundle\Pipeline\Event\ItemPipelineEvent;

/**
* AbstractConsumeEventListener class.
* Handles basic item skipping when there is nothing to do on skip.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence becomes clear after reading it several times and also reading the code. Not sure if that's so great, but it's not that bad, either.

*/
abstract class AbstractConsumeEventListener
{
Expand All @@ -25,7 +25,7 @@ abstract class AbstractConsumeEventListener
*/
public function onConsume(ItemPipelineEvent $event)
{
if ($event->getItemSkipException()) {
if ($event->getSkipException()) {
$this->skip($event);
} else {
$this->consume($event);
Expand Down
12 changes: 6 additions & 6 deletions Pipeline/Event/ItemPipelineEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ItemPipelineEvent extends Event
/**
* @var ItemSkipException
*/
private $itemSkipException;
private $skipException;

/**
* @param mixed $item
Expand Down Expand Up @@ -79,19 +79,19 @@ public function setOutput($output)
/**
* @return ItemSkipException
*/
public function getItemSkipException()
public function getSkipException()
{
return $this->itemSkipException;
return $this->skipException;
}

/**
* @param ItemSkipException $itemSkipException
* @param ItemSkipException $skipException
*
* @return $this
*/
public function setItemSkipException(ItemSkipException $itemSkipException)
public function setSkipException(ItemSkipException $skipException)
{
$this->itemSkipException = $itemSkipException;
$this->skipException = $skipException;

return $this;
}
Expand Down
7 changes: 6 additions & 1 deletion Pipeline/ItemSkipException.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
namespace ONGR\ConnectionsBundle\Pipeline;

/**
* Skip Item Exception.
* Exception for skipping items.
*
* This exception (or exception derived from this)
* should be thrown inside pipeline modifier
* to indicate that this item should be skipped.
* Thrown exception will be available for consumers.
*/
class ItemSkipException extends \Exception
{
Expand Down
2 changes: 1 addition & 1 deletion Pipeline/Pipeline.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public function start()
$itemEvent
);
} catch (ItemSkipException $itemSkipException) {
$itemEvent->setItemSkipException($itemSkipException);
$itemEvent->setSkipException($itemSkipException);
}

$dispatcher->dispatch(
Expand Down
40 changes: 40 additions & 0 deletions Resources/doc/pipeline.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
Pipeline
========

Pipeline is used to process data with 5 events:

- source
- start
- modify
- consume
- finish

Pipeline starts with source event which provides all data which should be processed.
Then start event is fired to indicate that items are about to come
Copy link
Contributor

Choose a reason for hiding this comment

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

Then start event is fired to indicate that items are about to come_,_

after that pipeline iterates through all items from all sources by calling modify and consume events
Copy link
Contributor

Choose a reason for hiding this comment

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

after that pipeline iterates through all items from all sources, calling modify and consume events with each item [as a parameter?]

with each item. After the iteration finish event is fired to notify that no more items will follow.
Copy link
Contributor

Choose a reason for hiding this comment

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

After the iterations are finished, finish event is fired to notify that no more items will follow.


Pipeline can have any number of listeners for each event but for functioning pipeline
at least one source and consume listener should be provided.

Listeners should listen to ongr.pipeline.<PipelineName>.<Target>.<Event> events.
Example:

.. code-block:: yaml

test.import.modifier:
class: %ongr_connections.import.modifier.class%
tags:
- { name: kernel.event_listener, event: ongr.pipeline.import.default.modify, method: onModify }
..

Item skipping
-------------
If item for some reason should not be consumed without stopping pipeline ItemSkipException can be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

If item for some reason should be skipped without stopping pipeline, ItemSkipException can be used.

(double negatives are confusing :( )


When modifier throws `ItemSkipException` pipeline catches it and sets skipException in `ItemPipelineEvent`.
Copy link
Contributor

Choose a reason for hiding this comment

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

When ItemSkipException is thrown by the modifier, pipeline catches it and sets skipException value in ItemPipelineEvent.

If `AbstractConsumeEventListener` is used `skip` method will called if skipException exception set otherwise `consume`
Copy link
Contributor

Choose a reason for hiding this comment

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

If AbstractConsumeEventListener is used and skipException exception is set, skip method will be called. Otherwise consume will be invoked.

method is called.



41 changes: 22 additions & 19 deletions Tests/Unit/Pipeline/PipelineTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,30 @@
use ONGR\ConnectionsBundle\Pipeline\PipelineFactory;
use Symfony\Component\EventDispatcher\EventDispatcher;

/**
* PipelineTest class.
*/
class PipelineTest extends \PHPUnit_Framework_TestCase
{
/**
* Pipeline data provider.
*
* @return array
*/
public function pipelineData()
{
// Modifier skips items which are string 'skip', anything else gets consumed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Modifier skips items which are equal to 'skip', anything else gets consumed.

sounds a bit better IMHO.

return [
// Case #0: No data so then should be 0 consumed and skipped items.
Copy link
Contributor

Choose a reason for hiding this comment

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

Case #0: No data. Results should be: 0 consumed and skipped items.

[[], 0, 0],
// Case #1: All data should be consumed so 1 consume and 0 skips.
Copy link
Contributor

Choose a reason for hiding this comment

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

Case #1: All data should be consumed, so 1 consume and 0 skips.

[['consume'], 1, 0],
// Case #2: All data should be skipped so 0 consumes and 1 skip.
Copy link
Contributor

Choose a reason for hiding this comment

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

Case #2: All data should be skipped, so 0 consumes and 1 skip.

[['skip'], 0, 1],
// Case #3: Data with consume and skips. 1 consume and 2 skips.
[['skip', 'consume', 'skip'], 1, 2],
// Case #3: Data with consumes and skip. 2 consumes and 1 skip.
[['consume', 'skip', 'consume'], 2, 1],
];
}

/**
* Tests pipeline.
*
Expand Down Expand Up @@ -70,20 +89,4 @@ public function onModify(ItemPipelineEvent $event)
throw new ItemSkipException();
}
}

/**
* Pipeline data provider.
*
* @return array
*/
public function pipelineData()
{
return [
[[], 0, 0],
[['consume'], 1, 0],
[['skip'], 0, 1],
[['skip', 'consume', 'skip'], 1, 2],
[['consume', 'skip', 'consume'], 2, 1],
];
}
}
4 changes: 2 additions & 2 deletions Tests/Unit/Pipeline/PipelineTestConsumer.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ class PipelineTestConsumer extends AbstractConsumeEventListener
*/
public function consume(ItemPipelineEvent $event)
{
++$this->consumeCalled;
$this->consumeCalled++;
}

/**
* {@inheritdoc}
*/
public function skip(ItemPipelineEvent $event)
{
++$this->skipCalled;
$this->skipCalled++;
}

/**
Expand Down