Skip to content

[Workflow] Remove constraints on transition/place name + Updated Dumper #26079

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

Merged
merged 1 commit into from
Feb 9, 2018
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
5 changes: 0 additions & 5 deletions src/Symfony/Component/Workflow/Definition.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

namespace Symfony\Component\Workflow;

use Symfony\Component\Workflow\Exception\InvalidArgumentException;
use Symfony\Component\Workflow\Exception\LogicException;

/**
Expand Down Expand Up @@ -82,10 +81,6 @@ private function setInitialPlace(string $place = null)

private function addPlace(string $place)
{
if (!preg_match('{^[\w_-]+$}', $place)) {
throw new InvalidArgumentException(sprintf('The place "%s" contains invalid characters.', $place));
}

if (!count($this->places)) {
$this->initialPlace = $place;
}
Expand Down
6 changes: 0 additions & 6 deletions src/Symfony/Component/Workflow/DefinitionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

namespace Symfony\Component\Workflow;

use Symfony\Component\Workflow\Exception\InvalidArgumentException;

/**
* Builds a definition.
*
Expand Down Expand Up @@ -77,10 +75,6 @@ public function setInitialPlace($place)
*/
public function addPlace($place)
{
if (!preg_match('{^[\w_-]+$}', $place)) {
throw new InvalidArgumentException(sprintf('The place "%s" contains invalid characters.', $place));
}

if (!$this->places) {
$this->initialPlace = $place;
}
Expand Down
16 changes: 12 additions & 4 deletions src/Symfony/Component/Workflow/Dumper/GraphvizDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ protected function addPlaces(array $places)
$code = '';

foreach ($places as $id => $place) {
$code .= sprintf(" place_%s [label=\"%s\", shape=circle%s];\n", $this->dotize($id), $id, $this->addAttributes($place['attributes']));
$code .= sprintf(" place_%s [label=\"%s\", shape=circle%s];\n", $this->dotize($id), $this->escape($id), $this->addAttributes($place['attributes']));
}

return $code;
Expand All @@ -121,7 +121,7 @@ protected function addTransitions(array $transitions)
$code = '';

foreach ($transitions as $place) {
$code .= sprintf(" transition_%s [label=\"%s\", shape=box%s];\n", $this->dotize($place['name']), $place['name'], $this->addAttributes($place['attributes']));
$code .= sprintf(" transition_%s [label=\"%s\", shape=box%s];\n", $this->dotize($place['name']), $this->escape($place['name']), $this->addAttributes($place['attributes']));
}

return $code;
Expand Down Expand Up @@ -198,15 +198,23 @@ protected function endDot()
*/
protected function dotize($id)
{
return strtolower(preg_replace('/[^\w]/i', '_', $id));
return hash('sha1', $id);
}

/**
* @internal
*/
protected function escape(string $string): string
{
return addslashes($string);
}

private function addAttributes(array $attributes): string
{
$code = array();

foreach ($attributes as $k => $v) {
$code[] = sprintf('%s="%s"', $k, $v);
$code[] = sprintf('%s="%s"', $k, $this->escape($v));
}

return $code ? ', '.implode(', ', $code) : '';
Expand Down
44 changes: 26 additions & 18 deletions src/Symfony/Component/Workflow/Dumper/PlantUmlDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class PlantUmlDumper implements DumperInterface
qM53XHDUwhY0TAwPug3OG9NonRFhO8ynF3I4unuAMDHmSrXH57V1RGvl9jafuZF9ZhqjWOEh98y0tUYGsUxkBSllIyBdT2oM5Fn2-ut-fzsq_cQNuL6Uvwqr
knh4RrvOKzxZfLV3s0rs_R_1SdYt3VxeQ1_y2_W2
}';
private const INITIAL = 'initial';
private const MARKED = 'marked';
private const INITIAL = '<<initial>>';
private const MARKED = '<<marked>>';

const STATEMACHINE_TRANSITION = 'arrow';
const WORKFLOW_TRANSITION = 'square';
Expand All @@ -45,11 +45,11 @@ class PlantUmlDumper implements DumperInterface
'titleBorderRoundCorner' => 15,
'titleBorderThickness' => 2,
'state' => array(
'BackgroundColor<<'.self::INITIAL.'>>' => '#87b741',
'BackgroundColor<<'.self::MARKED.'>>' => '#3887C6',
'BackgroundColor'.self::INITIAL => '#87b741',
'BackgroundColor'.self::MARKED => '#3887C6',
'BorderColor' => '#3887C6',
'BorderColor<<'.self::MARKED.'>>' => 'Black',
'FontColor<<'.self::MARKED.'>>' => 'White',
'BorderColor'.self::MARKED => 'Black',
'FontColor'.self::MARKED => 'White',
),
'agent' => array(
'BackgroundColor' => '#ffffff',
Expand All @@ -63,7 +63,7 @@ class PlantUmlDumper implements DumperInterface
public function __construct(string $transitionType = null)
{
if (!in_array($transitionType, self::TRANSITION_TYPES)) {
throw new InvalidArgumentException("Transition type '{$transitionType}' does not exist.");
throw new InvalidArgumentException("Transition type '$transitionType' does not exist.");
}
$this->transitionType = $transitionType;
}
Expand All @@ -73,31 +73,36 @@ public function dump(Definition $definition, Marking $marking = null, array $opt
$options = array_replace_recursive(self::DEFAULT_OPTIONS, $options);
$code = $this->initialize($options);
foreach ($definition->getPlaces() as $place) {
$placeEscaped = $this->escape($place);
$code[] =
"state {$place}".
($definition->getInitialPlace() === $place ? ' <<'.self::INITIAL.'>>' : '').
($marking && $marking->has($place) ? ' <<'.self::MARKED.'>>' : '');
"state $placeEscaped".
($definition->getInitialPlace() === $place ? ' '.self::INITIAL : '').
($marking && $marking->has($place) ? ' '.self::MARKED : '');
}
if ($this->isWorkflowTransitionType()) {
foreach ($definition->getTransitions() as $transition) {
$code[] = "agent {$transition->getName()}";
$transitionEscaped = $this->escape($transition->getName());
$code[] = "agent $transitionEscaped";
}
}
foreach ($definition->getTransitions() as $transition) {
$transitionEscaped = $this->escape($transition->getName());
foreach ($transition->getFroms() as $from) {
$fromEscaped = $this->escape($from);
foreach ($transition->getTos() as $to) {
$toEscaped = $this->escape($to);
if ($this->isWorkflowTransitionType()) {
$lines = array(
"{$from} --> {$transition->getName()}",
"{$transition->getName()} --> {$to}",
"$fromEscaped --> $transitionEscaped",
"$transitionEscaped --> $toEscaped",
);
foreach ($lines as $line) {
if (!in_array($line, $code)) {
$code[] = $line;
}
}
} else {
$code[] = "{$from} --> {$to}: {$transition->getName()}";
$code[] = "$fromEscaped --> $toEscaped: $transitionEscaped";
}
}
}
Expand All @@ -114,10 +119,7 @@ private function isWorkflowTransitionType(): bool
private function startPuml(array $options): string
{
$start = '@startuml'.PHP_EOL;

if ($this->isWorkflowTransitionType()) {
$start .= 'allow_mixing'.PHP_EOL;
}
$start .= 'allow_mixing'.PHP_EOL;

if ($options['nofooter'] ?? false) {
return $start;
Expand Down Expand Up @@ -169,4 +171,10 @@ private function initialize(array $options): array

return $code;
}

private function escape(string $string): string
{
// It's not possible to escape property double quote, so let's remove it
return '"'.str_replace('"', '', $string).'"';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ protected function addEdges(array $edges)

foreach ($edges as $id => $edges) {
foreach ($edges as $edge) {
$code .= sprintf(" place_%s -> place_%s [label=\"%s\" style=\"%s\"];\n", $this->dotize($id), $this->dotize($edge['to']), $edge['name'], 'solid');
$code .= sprintf(" place_%s -> place_%s [label=\"%s\" style=\"%s\"];\n", $this->dotize($id), $this->dotize($edge['to']), $this->escape($edge['name']), 'solid');
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@

class DefinitionBuilderTest extends TestCase
{
/**
* @expectedException \Symfony\Component\Workflow\Exception\InvalidArgumentException
*/
public function testAddPlaceInvalidName()
{
$builder = new DefinitionBuilder(array('a"', 'b'));
}

public function testSetInitialPlace()
{
$builder = new DefinitionBuilder(array('a', 'b'));
Expand Down
9 changes: 0 additions & 9 deletions src/Symfony/Component/Workflow/Tests/DefinitionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@ public function testAddPlaces()
$this->assertEquals('a', $definition->getInitialPlace());
}

/**
* @expectedException \Symfony\Component\Workflow\Exception\InvalidArgumentException
*/
public function testAddPlacesInvalidArgument()
{
$places = array('a"', 'e"');
$definition = new Definition($places, array());
}

public function testSetInitialPlace()
{
$places = range('a', 'e');
Expand Down
Loading