Skip to content

Supported "Marking" enum php #50311

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Supported "Marking" enum php #50311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7be6124
Supported "Marking" enum php
May 12, 2023
9db3a32
Refactor Marking.php name and types
May 12, 2023
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
61 changes: 44 additions & 17 deletions src/Symfony/Component/Workflow/Marking.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,18 @@
*/
class Marking
{
/**
* @var array<string, int>
*/
private array $places = [];
/**
* @var array<string, \UnitEnum>
*/
private array $enumPlaces = [];
private ?array $context = null;

/**
* @param int[] $representation Keys are the place name and values should be 1
* @param array<string, int> $representation Keys are the place name and values should be 1
*/
public function __construct(array $representation = [])
{
Expand All @@ -31,36 +38,43 @@ public function __construct(array $representation = [])
}
}

/**
* @return void
*/
public function mark(string $place)
public function mark(string|\UnitEnum $place): void
Copy link
Member

Choose a reason for hiding this comment

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

Note that this method is not final and could be overridden downstream. Widening the parameter type and narrowing the native return type are both breaking changes.

Copy link
Author

Choose a reason for hiding this comment

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

I propose to make Marking.php the final class

Copy link
Member

Choose a reason for hiding this comment

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

We cannot do that now, as making a class final is also a breaking change. So we can at most flag it as final using PHPdoc and make it final in 7.0. But that would also mean that we probably need to introduce a MarkingInterface and update the rest of the component to use that typehint, as otherwise this is fully closed for extension.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, you're right, it's better to leave it like that than to make many changes in favor of MarkingInterface

{
$this->places[$place] = 1;
$key = $this->enumOrStringToKey($place);
$this->places[$key] = 1;
if ($place instanceof \UnitEnum) {
$this->enumPlaces[$key] = $place;
}
}

/**
* @return void
*/
public function unmark(string $place)
public function unmark(string|\UnitEnum $place): void
{
unset($this->places[$place]);
$key = $this->enumOrStringToKey($place);
unset(
$this->places[$key],
$this->enumPlaces[$key]
);
}

public function has(string|\UnitEnum $place): bool
{
return isset($this->places[$this->enumOrStringToKey($place)]);
}

/**
* @return bool
* @return array<string, int>
*/
public function has(string $place)
public function getPlaces(): array
{
return isset($this->places[$place]);
return $this->places;
}

/**
* @return array
* @return array<string, \UnitEnum>
*/
public function getPlaces()
public function getEnumPlaces(): array
{
return $this->places;
return $this->enumPlaces;
}

/**
Expand All @@ -78,4 +92,17 @@ public function getContext(): ?array
{
return $this->context;
}

private function enumOrStringToKey(string|\UnitEnum $place): string
{
if (is_string($place)) {
return $place;
}

if ($place instanceof \BackedEnum) {
return (string)$place->value;
}

return $place->name;
}
}
36 changes: 29 additions & 7 deletions src/Symfony/Component/Workflow/MarkingStore/MethodMarkingStore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,42 @@ public function getMarking(object $subject): Marking
}

if ($this->singleState) {
$marking = [(string) $marking => 1];
} elseif (!\is_array($marking)) {
$result = new Marking();
$result->mark($marking);

return $result;
}

if (!\is_array($marking)) {
throw new LogicException(sprintf('The method "%s::%s()" did not return an array and the Workflow\'s Marking store is instantiated with $singleState=false.', get_debug_type($subject), $method));
}

return new Marking($marking);
$result = new Marking();
foreach ($marking as $place => $nbTokenOrEnum) {
if ($nbTokenOrEnum instanceof \UnitEnum) {
$result->mark($nbTokenOrEnum);
} else {
$result->mark($place);
}
}

return $result;
}

public function setMarking(object $subject, Marking $marking, array $context = []): void
{
$marking = $marking->getPlaces();

if ($this->singleState) {
$marking = key($marking);
$enumPlaces = $marking->getEnumPlaces();
if ([] !== $enumPlaces) {
if ($this->singleState) {
$marking = array_values($enumPlaces)[0];
} else {
$marking = $enumPlaces;
}
} else {
$marking = $marking->getPlaces();
if ($this->singleState) {
$marking = key($marking);
}
}

$method = 'set'.ucfirst($this->property);
Expand Down
17 changes: 17 additions & 0 deletions src/Symfony/Component/Workflow/Tests/MarkingEnum.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Workflow\Tests;

enum MarkingEnum: int
{
case FIRST_PLACE = 1;
}
23 changes: 23 additions & 0 deletions src/Symfony/Component/Workflow/Tests/MarkingStore/MethodMarkingStoreTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use PHPUnit\Framework\TestCase;
use Symfony\Component\Workflow\Marking;
use Symfony\Component\Workflow\MarkingStore\MethodMarkingStore;
use Symfony\Component\Workflow\Tests\MarkingEnum;
use Symfony\Component\Workflow\Tests\Subject;

class MethodMarkingStoreTest extends TestCase
Expand Down Expand Up @@ -111,6 +112,28 @@ public function testGetMarkingWithUninitializedProperty2()
$markingStore->getMarking($subject);
}

public function testGetSetMarkingWithSingleEnumState()
{
$subject = new Subject();

$markingStore = new MethodMarkingStore(true);

$marking = $markingStore->getMarking($subject);

$this->assertInstanceOf(Marking::class, $marking);
$this->assertCount(0, $marking->getPlaces());

$marking->mark(MarkingEnum::FIRST_PLACE);

$markingStore->setMarking($subject, $marking);

$this->assertSame(MarkingEnum::FIRST_PLACE, $subject->getMarking());

$marking2 = $markingStore->getMarking($subject);

$this->assertEquals($marking, $marking2);
}

private function createValueObject(string $markingValue): object
{
return new class($markingValue) {
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Workflow/Tests/Subject.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function __construct($marking = null)
$this->context = [];
}

public function getMarking(): string|array|null
public function getMarking(): string|array|null|MarkingEnum
{
return $this->marking;
}
Expand Down