From 143baad700b84ab70155663b9aff99b4b67a70b1 Mon Sep 17 00:00:00 2001 From: Itai Lulu Koren Date: Sun, 16 Jan 2022 09:01:27 +0200 Subject: [PATCH 1/2] return NodeResult object instead of boolean in ConditionManager use identifier for second (compare) value use explicit value for first value --- src/DataSource.php | 22 +++++--- src/LogicTreeFacade.php | 10 ++-- src/Node/AbstractNode.php | 4 +- src/Node/Combine.php | 22 +++++--- src/Node/CombineInterface.php | 22 ++++++-- src/Node/Condition.php | 84 +++++++++++++++++++++++++----- src/Node/ConditionInterface.php | 63 ++++++++++++++++++++-- src/Node/NodeInterface.php | 31 ++++++++++- src/Operator/OperatorInterface.php | 6 +++ src/Service/ConditionManager.php | 57 +++++++++++++++----- tests/LogicTreeFacadeTest.php | 4 +- 11 files changed, 271 insertions(+), 54 deletions(-) diff --git a/src/DataSource.php b/src/DataSource.php index a198a24..17c6945 100644 --- a/src/DataSource.php +++ b/src/DataSource.php @@ -26,7 +26,7 @@ public function getData(): array return $this->data; } - public function setData(iterable $data): void + public function setData(iterable $data): DataSource { if (!is_array($data) && !($data instanceof ArrayAccess)) { throw new InvalidArgumentException('Data must be an array or implements ArrayAccess.'); @@ -37,34 +37,44 @@ public function setData(iterable $data): void foreach ($data as $key => $value) { $this->setValue($key, $value); } + + return $this; } - public function addData(iterable $data): void + public function addData(iterable $data): DataSource { foreach ($data as $key => $value) { $this->setValue($key, $value); } + + return $this; } - public function getValue(string $key): mixed + public function getValue(string $key) { return $this->data[$key] ?? null; } - public function setValue(string $key, mixed $value): void + public function setValue(string $key, $value): DataSource { $this->data[$key] = $value; + + return $this; } - public function unsetValue(string $key): void + public function unsetValue(string $key): DataSource { unset($this->data[$key]); + + return $this; } - public function unsetValues(array $keys): void + public function unsetValues(array $keys): DataSource { foreach ($keys as $key) { $this->unsetValue($key); } + + return $this; } } diff --git a/src/LogicTreeFacade.php b/src/LogicTreeFacade.php index fa885db..a514f5e 100644 --- a/src/LogicTreeFacade.php +++ b/src/LogicTreeFacade.php @@ -6,8 +6,8 @@ namespace LogicTree; -use LogicTree\Node\CombineInterface; -use LogicTree\Node\ConditionInterface; +use LogicTree\Node\NodeInterface; +use LogicTree\Node\NodeResult; use LogicTree\Operator\OperatorInterface; use LogicTree\Operator\OperatorPool; use LogicTree\Operator\OperatorType; @@ -23,9 +23,11 @@ public function __construct( $this->conditionManager = $conditionManager ?? new ConditionManager($this->operatorPool); } - public function addOperator(OperatorType $type, string $operatorCode, OperatorInterface $operator): void + public function addOperator(OperatorType $type, string $operatorCode, OperatorInterface $operator): LogicTreeFacade { $this->operatorPool->addOperator($type, $operatorCode, $operator); + + return $this; } public function createDataSource(iterable $data): DataSource @@ -33,7 +35,7 @@ public function createDataSource(iterable $data): DataSource return new DataSource($data); } - public function executeCombineConditions(CombineInterface|ConditionInterface $node, DataSource $dataSource): bool + public function executeCombineConditions(NodeInterface $node, DataSource $dataSource): NodeResult { return $this->conditionManager->execute($node, $dataSource); } diff --git a/src/Node/AbstractNode.php b/src/Node/AbstractNode.php index c3589d3..9ce9639 100644 --- a/src/Node/AbstractNode.php +++ b/src/Node/AbstractNode.php @@ -15,9 +15,11 @@ public function getParent(): ?CombineInterface return $this->parent; } - public function setParent(?CombineInterface $combine): void + public function setParent(?CombineInterface $combine): static { $this->parent = $combine; + + return $this; } public function hasParent(): bool diff --git a/src/Node/Combine.php b/src/Node/Combine.php index 72b885e..21b973e 100644 --- a/src/Node/Combine.php +++ b/src/Node/Combine.php @@ -32,23 +32,27 @@ public function getChildren(): array return $this->nodes; } - public function setChildren(array $children): void + public function setChildren(array $children): static { $this->nodes = []; foreach ($children as $child) { $this->addChild($child); } + + return $this; } - public function addChild(CombineInterface|ConditionInterface $condition): void + public function addChild(NodeInterface $node): static { - if ($condition === $this) { + if ($node === $this) { throw new LogicException('Child node cannot be the current instance of itself.'); } - $condition->setParent($this); - $this->nodes[] = $condition; + $node->setParent($this); + $this->nodes[] = $node; + + return $this; } public function getCount(): int @@ -66,9 +70,11 @@ public function isInvert(): bool return $this->isInvert; } - public function setIsInvert(bool $isInvert): void + public function setIsInvert(bool $isInvert): static { $this->isInvert = $isInvert; + + return $this; } public function getOperator(): string @@ -76,8 +82,10 @@ public function getOperator(): string return $this->operator; } - public function setOperator(string $operator): void + public function setOperator(string $operator): static { $this->operator = $operator; + + return $this; } } diff --git a/src/Node/CombineInterface.php b/src/Node/CombineInterface.php index 8896b79..aed665e 100644 --- a/src/Node/CombineInterface.php +++ b/src/Node/CombineInterface.php @@ -22,28 +22,44 @@ public function getChildren(): array; * Set the logic structure as conditions or combinations * * @param NodeInterface[] $children + * @return self */ - public function setChildren(array $children): void; + public function setChildren(array $children): static; /** * Add a logic structure as condition or combination + * + * @param NodeInterface $node + * @return self */ - public function addChild(CombineInterface|ConditionInterface $node): void; + public function addChild(NodeInterface $node): static; /** * Retrieve the count of children nodes + * + * @return int */ public function getCount(): int; + /** + * Check if it has children + * + * @return bool + */ public function hasChildren(): bool; /** * Check if the result should be inverted + * + * @return bool */ public function isInvert(): bool; /** * Set is result of combination inverted + * + * @param bool $isInvert + * @return self */ - public function setIsInvert(bool $isInvert): void; + public function setIsInvert(bool $isInvert): static; } diff --git a/src/Node/Condition.php b/src/Node/Condition.php index 9f8f048..e23cda4 100644 --- a/src/Node/Condition.php +++ b/src/Node/Condition.php @@ -9,13 +9,17 @@ final class Condition extends AbstractNode implements ConditionInterface { public function __construct( - private string $valueIdentifier, + private ?string $firstValueIdentifier, private string $operator, - private mixed $valueCompare + private mixed $secondValue = null, + private mixed $firstValue = null, + private ?string $secondValueIdentifier = null, ) { - $this->setValueIdentifier($valueIdentifier); + $this->setFirstValueIdentifier($firstValueIdentifier); + $this->setFirstValue($firstValue); $this->setOperator($operator); - $this->setValueCompare($valueCompare); + $this->setSecondValueIdentifier($secondValueIdentifier); + $this->setSecondValue($secondValue); } public function getOperator(): string @@ -23,28 +27,82 @@ public function getOperator(): string return $this->operator; } - public function setOperator(string $operator): void + public function setOperator(string $operator): static { $this->operator = $operator; + + return $this; + } + + /** + * @return string|null + */ + public function getFirstValueIdentifier(): ?string + { + return $this->firstValueIdentifier; + } + + /** + * @param string|null $identifier + * @return $this + */ + public function setFirstValueIdentifier(?string $identifier): static + { + $this->firstValueIdentifier = $identifier; + return $this; + } + + /** + * @return mixed + */ + public function getFirstValue(): mixed + { + return $this->firstValue; + } + + /** + * @param mixed $value + * @return $this + */ + public function setFirstValue(mixed $value): static + { + $this->firstValue = $value; + return $this; } - public function getValueIdentifier(): string + /** + * @return string|null + */ + public function getSecondValueIdentifier(): ?string { - return $this->valueIdentifier; + return $this->secondValueIdentifier; } - public function setValueIdentifier(string $identifier): void + /** + * @param ?string $identifier + * @return $this + */ + public function setSecondValueIdentifier(?string $identifier): static { - $this->valueIdentifier = $identifier; + $this->secondValueIdentifier = $identifier; + return $this; } - public function getValueCompare(): mixed + /** + * @return mixed + */ + public function getSecondValue(): mixed { - return $this->valueCompare;//ToDo: study to be fetched from datasource also? + return $this->secondValue; } - public function setValueCompare(mixed $value): void + /** + * @param mixed $value + * @return $this + */ + public function setSecondValue(mixed $value): static { - $this->valueCompare = $value; + $this->secondValue = $value; + return $this; } } diff --git a/src/Node/ConditionInterface.php b/src/Node/ConditionInterface.php index a2efc55..5937f39 100644 --- a/src/Node/ConditionInterface.php +++ b/src/Node/ConditionInterface.php @@ -11,11 +11,66 @@ */ interface ConditionInterface extends NodeInterface { - public function getValueIdentifier(): string; + /** + * Retrieve the first value key identifier to compare + * + * @return ?string + */ + public function getFirstValueIdentifier(): ?string; - public function setValueIdentifier(string $identifier): void; + /** + * Set the first value key identifier to compare + * + * @param ?string $identifier + * @return self + */ + public function setFirstValueIdentifier(?string $identifier): static; + + /** + * Retrieve the first value to compare + * + * @return mixed + */ + public function getFirstValue(): mixed; + + /** + * Set the first value to compare + * + * @param mixed $value + * @return self + */ + public function setFirstValue(mixed $value): static; + + + /** + * Retrieve the second value key identifier to compare + * + * @return ?string + */ + public function getSecondValueIdentifier(): ?string; + + /** + * Set the second value key identifier to compare + * + * @param string $identifier + * @return self + */ + public function setSecondValueIdentifier(string $identifier): static; + + /** + * Retrieve the second value to compare + * + * @return mixed + */ + public function getSecondValue(): mixed; + + /** + * Set the second value to compare + * + * @param mixed $value + * @return self + */ + public function setSecondValue(mixed $value): static; - public function getValueCompare(): mixed; - public function setValueCompare(mixed $value): void; } diff --git a/src/Node/NodeInterface.php b/src/Node/NodeInterface.php index 03e6861..70a9f1a 100644 --- a/src/Node/NodeInterface.php +++ b/src/Node/NodeInterface.php @@ -11,13 +11,40 @@ */ interface NodeInterface { + /** + * Retrieve the operator + * + * @return string + */ public function getOperator(): string; - public function setOperator(string $operator): void; + /** + * Set the operator + * + * @param string $operator + * @return self + */ + public function setOperator(string $operator): static; + /** + * Retrieve the parent combine + * + * @return CombineInterface|null + */ public function getParent(): ?CombineInterface; - public function setParent(?CombineInterface $condition): void; + /** + * Set the parent combine + * + * @param CombineInterface|null $combine + * @return self + */ + public function setParent(?CombineInterface $combine): static; + /** + * Check if it has an existing parent + * + * @return bool + */ public function hasParent(): bool; } diff --git a/src/Operator/OperatorInterface.php b/src/Operator/OperatorInterface.php index 28f86b1..de0e476 100644 --- a/src/Operator/OperatorInterface.php +++ b/src/Operator/OperatorInterface.php @@ -11,5 +11,11 @@ */ interface OperatorInterface { + /** + * Execute operator for the expressions + * + * @param mixed ...$expressions + * @return bool + */ public function execute(mixed ...$expressions): bool; } diff --git a/src/Service/ConditionManager.php b/src/Service/ConditionManager.php index fbd297d..5b592e5 100644 --- a/src/Service/ConditionManager.php +++ b/src/Service/ConditionManager.php @@ -10,6 +10,7 @@ use LogicTree\Node\CombineInterface; use LogicTree\Node\ConditionInterface; use LogicTree\Node\NodeInterface; +use LogicTree\Node\NodeResult; use LogicTree\Operator\OperatorInterface; use LogicTree\Operator\OperatorPool; use LogicTree\Operator\OperatorType; @@ -23,29 +24,61 @@ class ConditionManager { public function __construct(private OperatorPool $operatorPool) {} - public function execute(CombineInterface|ConditionInterface $node, DataSource $dataSource): bool + public function execute(NodeInterface $node, DataSource $dataSource): NodeResult { return match (true) { $node instanceof CombineInterface => $this->executeCombine($node, $dataSource), - $node instanceof ConditionInterface => $this->executeCondition($node, $dataSource) + $node instanceof ConditionInterface => $this->executeCondition($node, $dataSource), + default => new NodeResult(true) }; } - private function executeCombine(CombineInterface $combine, DataSource $dataSource): bool + private function executeCombine(CombineInterface $combine, DataSource $dataSource): NodeResult { - return $combine->isInvert() xor $this->resolveOperator(OperatorType::Logical, $combine)->execute( - ...array_map( - fn (CombineInterface|ConditionInterface $node) => $this->execute($node, $dataSource), - $combine->getChildren() - ) + $children = array_map( + fn(NodeInterface $node) => $this->execute($node, $dataSource), + $combine->getChildren() + ); + + $isMatched = $this + ->resolveOperator(OperatorType::Logical, $combine) + ->execute( + ...array_map( + fn(NodeResult $nodeResult) => $nodeResult->isMatched(), + $children + ) + ); + + return new NodeResult( + matched: $combine->isInvert() xor $isMatched, + node: $combine, + children: $children ); } - private function executeCondition(ConditionInterface $condition, DataSource $dataSource): bool + private function executeCondition(ConditionInterface $condition, DataSource $dataSource): NodeResult { - return $this->resolveOperator(OperatorType::Comparator, $condition)->execute( - $dataSource->getValue($condition->getValueIdentifier()), - $condition->getValueCompare() + $firstValue = $condition->getFirstValue(); + $firstValueIdentifier = $condition->getFirstValueIdentifier(); + $secondValue = $condition->getSecondValue(); + $secondValueIdentifier = $condition->getSecondValueIdentifier(); + + if(is_null($firstValue) && !is_null($firstValueIdentifier)) { + $firstValue = $dataSource->getValue($firstValueIdentifier); + $condition->setFirstValue($firstValue); + } + if(is_null($secondValue) && !is_null($secondValueIdentifier)) { + $secondValue = $dataSource->getValue($secondValueIdentifier); + $condition->setSecondValue($secondValue); + } + + $result = $this + ->resolveOperator(OperatorType::Comparator, $condition) + ->execute($firstValue, $secondValue); + + return new NodeResult( + matched: $result, + node: $condition ); } diff --git a/tests/LogicTreeFacadeTest.php b/tests/LogicTreeFacadeTest.php index b408546..6ed9790 100644 --- a/tests/LogicTreeFacadeTest.php +++ b/tests/LogicTreeFacadeTest.php @@ -25,9 +25,9 @@ public function testSimpleCase(): void { $expr2 = new Condition('value_2', 'gt', 5); // true $exprA = new Condition('value_1', 'regexp', '/^[A-Z]+/'); // true $exprB = new Condition('value_2', 'lt', 1); // false - $combA = new Combine('or', true, [$exprA, $exprB]); // true OR false + $combA = new Combine('or', false, [$exprA, $exprB]); // true OR false $logicTree = new Combine('and', false, [$expr1, $expr2, $combA]); // true AND true AND true - $this->assertTrue($logicTreeFacade->executeCombineConditions($logicTree, $dataSource)); + $this->assertTrue($logicTreeFacade->executeCombineConditions($logicTree, $dataSource)->isMatched()); } } \ No newline at end of file From 0ae9c803eb1101573cc4e2bd1ab20353db3f3a7c Mon Sep 17 00:00:00 2001 From: Itai Lulu Koren Date: Sat, 15 Jan 2022 01:03:03 +0200 Subject: [PATCH 2/2] fixed RegexpOperator (added strict comparison) --- src/Operator/Comparator/RegexpOperator.php | 2 +- tests/Operator/Comparator/RegexpOperatorTest.php | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 tests/Operator/Comparator/RegexpOperatorTest.php diff --git a/src/Operator/Comparator/RegexpOperator.php b/src/Operator/Comparator/RegexpOperator.php index 976ce07..74195c2 100644 --- a/src/Operator/Comparator/RegexpOperator.php +++ b/src/Operator/Comparator/RegexpOperator.php @@ -18,6 +18,6 @@ final class RegexpOperator extends AbstractCompareTwo public function executeComparison(mixed $expr1, mixed $expr2): bool { - return (bool) preg_match($expr2, $expr1); + return preg_match($expr2, $expr1) !== false; } } diff --git a/tests/Operator/Comparator/RegexpOperatorTest.php b/tests/Operator/Comparator/RegexpOperatorTest.php new file mode 100644 index 0000000..893d79e --- /dev/null +++ b/tests/Operator/Comparator/RegexpOperatorTest.php @@ -0,0 +1,15 @@ +assertTrue((new RegexpOperator())->executeComparison('toto', '/^[A-Z]+/')); + } + +} \ No newline at end of file