From 87f1d3d97b066916f8a4b46312086f6ff91228ca Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 19 Apr 2023 09:01:59 +0200 Subject: [PATCH 1/6] Binary: Fix hex filter support --- src/Behavior/Binary.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Behavior/Binary.php b/src/Behavior/Binary.php index b465dd6..f2c096e 100644 --- a/src/Behavior/Binary.php +++ b/src/Behavior/Binary.php @@ -93,7 +93,7 @@ public function rewriteCondition(Condition $condition, $relation = null) if (isset($this->rewriteSubjects[$column])) { $value = $condition->getValue(); - if (empty($this->properties) && is_resource($value)) { + if (! empty($this->properties) && is_resource($value)) { // Only for PostgreSQL. throw new UnexpectedValueException(sprintf('Unexpected resource for %s', $column)); } @@ -105,11 +105,11 @@ public function rewriteCondition(Condition $condition, $relation = null) * no further adjustments are needed as ctype_xdigit returns false for binary and bytea hex strings. */ if (ctype_xdigit($value)) { - if (empty($this->properties) && substr($value, 0, 2) !== '\\x') { - // Only for PostgreSQL. - $condition->setValue(sprintf('\\x%s', $value)); - } else { + if (empty($this->properties)) { $condition->setValue(hex2bin($value)); + } elseif (substr($value, 0, 2) !== '\\x') { + // PostgreSQL. + $condition->setValue(sprintf('\\x%s', $value)); } } } From 9cb2d7ec5cf2c5aff05cf7816ec40e52c09029f4 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 19 Apr 2023 09:56:14 +0200 Subject: [PATCH 2/6] Make Binary::setQuery() fluent --- src/Behavior/Binary.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Behavior/Binary.php b/src/Behavior/Binary.php index f2c096e..29b1893 100644 --- a/src/Behavior/Binary.php +++ b/src/Behavior/Binary.php @@ -81,6 +81,8 @@ public function setQuery(Query $query) // Only process properties if the adapter is PostgreSQL. $this->properties = []; } + + return $this; } public function rewriteCondition(Condition $condition, $relation = null) From 73191ca9ae76fd781a18c837fe2a22d6e4b33386 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 19 Apr 2023 09:56:34 +0200 Subject: [PATCH 3/6] Binary: Add tests --- tests/Behavior/BinaryTest.php | 110 ++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/Behavior/BinaryTest.php diff --git a/tests/Behavior/BinaryTest.php b/tests/Behavior/BinaryTest.php new file mode 100644 index 0000000..2d4a079 --- /dev/null +++ b/tests/Behavior/BinaryTest.php @@ -0,0 +1,110 @@ +assertSame( + static::TEST_BINARY_VALUE, + $this->behavior()->retrieveProperty(static::TEST_BINARY_VALUE, static::TEST_COLUMN) + ); + } + + public function testRetrievePropertyReturnsStreamContentsIfAdapterIsPostgreSQL(): void + { + $stream = fopen('php://temp', 'r+'); + fputs($stream, static::TEST_BINARY_VALUE); + rewind($stream); + + $this->assertSame( + static::TEST_BINARY_VALUE, + $this->behavior(true)->retrieveProperty($stream, static::TEST_COLUMN) + ); + } + + public function testPersistPropertyReturnsVanillaValueIfAdapterIsNotPostgreSQL(): void + { + $this->assertSame( + static::TEST_BINARY_VALUE, + $this->behavior()->persistProperty(static::TEST_BINARY_VALUE, static::TEST_COLUMN) + ); + } + + public function testPersistPropertyReturnsByteaHexStringIfAdapterIsPostgreSQL(): void + { + $this->assertSame( + sprintf('\\x%s', static::TEST_HEX_VALUE), + $this->behavior(true)->persistProperty(static::TEST_BINARY_VALUE, static::TEST_COLUMN) + ); + } + + public function testRewriteConditionTransformsHexStringToBinaryIfAdapterIsNotPostgreSQL(): void + { + $c = $this->condition(); + + $this->behavior()->rewriteCondition($c); + $this->assertSame(static::TEST_BINARY_VALUE, $c->getValue()); + } + + public function testRewriteConditionTransformsHexStringToByteaHexStringIfAdapterIsPostgreSQL(): void + { + $c = $this->condition(); + + $this->behavior(true)->rewriteCondition($c); + $this->assertSame( + sprintf('\\x%s', static::TEST_HEX_VALUE), + $c->getValue() + ); + } + + /** + * We expect `retrieveProperty()` to return stream contents when the adapter is `PostgreSQL`. + * Working with streams in other functions is neither expected nor supported. + */ + public function testPersistPropertyThrowsExceptionIfAdapterIsPostgreSQLAndValueIsResource(): void + { + $this->expectException(ValueConversionException::class); + $this->behavior(true)->persistProperty(fopen('php://temp', 'r'), static::TEST_COLUMN); + } + + /** + * @see testPersistPropertyThrowsExceptionIfAdapterIsPostgreSQLAndValueIsResource() + */ + public function testRewriteConditionThrowsExceptionIfAdapterIsPostgreSQLAndValueIsResource(): void + { + $this->expectException(UnexpectedValueException::class); + $this->behavior(true)->rewriteCondition($this->condition(fopen('php://temp', 'r'))); + } + + protected function behavior(bool $postgres = false): Binary + { + return (new Binary([static::TEST_COLUMN])) + ->setQuery( + (new Query()) + ->setDb($postgres ? new Connection(['db' => 'pgsql']) : new TestConnection()) + ); + } + + protected function condition($value = self::TEST_HEX_VALUE): Equal + { + $c = new Equal(static::TEST_COLUMN, $value); + $c->metaData()->set('columnName', static::TEST_COLUMN); + + return $c; + } +} From 97564f1c70b173d65301b74fdb9322690e9498d1 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 19 Apr 2023 10:34:58 +0200 Subject: [PATCH 4/6] Binary: Don't reset properties when mysql adapter is used --- src/Behavior/Binary.php | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/Behavior/Binary.php b/src/Behavior/Binary.php index 29b1893..807a783 100644 --- a/src/Behavior/Binary.php +++ b/src/Behavior/Binary.php @@ -18,18 +18,15 @@ */ class Binary extends PropertyBehavior implements QueryAwareBehavior, RewriteFilterBehavior { - /** - * Properties for {@see rewriteCondition()} to support hex filters for each adapter - * - * Set in {@see setQuery()} from the {@see $properties} array because the latter is reset for adapters other than - * {@see Pgsql}, so {@see fromDb()} and {@see toDb()} don't run for them. - * - * @var array - */ - protected $rewriteSubjects; + /** @var bool Whether the query is using a pgsql adapter */ + protected $isPostgres = true; public function fromDb($value, $key, $_) { + if (! $this->isPostgres) { + return $value; + } + if ($value !== null) { if (is_resource($value)) { return stream_get_contents($value); @@ -46,6 +43,10 @@ public function fromDb($value, $key, $_) */ public function toDb($value, $key, $_) { + if (! $this->isPostgres) { + return $value; + } + if (is_resource($value)) { throw new ValueConversionException(sprintf('Unexpected resource for %s', $key)); } @@ -75,12 +76,7 @@ public function toDb($value, $key, $_) public function setQuery(Query $query) { - $this->rewriteSubjects = $this->properties; - - if (! $query->getDb()->getAdapter() instanceof Pgsql) { - // Only process properties if the adapter is PostgreSQL. - $this->properties = []; - } + $this->isPostgres = $query->getDb()->getAdapter() instanceof Pgsql; return $this; } @@ -92,11 +88,10 @@ public function rewriteCondition(Condition $condition, $relation = null) * {@see \ipl\Orm\Compat\FilterProcessor::requireAndResolveFilterColumns()} */ $column = $condition->metaData()->get('columnName'); - if (isset($this->rewriteSubjects[$column])) { + if (isset($this->properties[$column])) { $value = $condition->getValue(); - if (! empty($this->properties) && is_resource($value)) { - // Only for PostgreSQL. + if ($this->isPostgres && is_resource($value)) { throw new UnexpectedValueException(sprintf('Unexpected resource for %s', $column)); } @@ -107,10 +102,9 @@ public function rewriteCondition(Condition $condition, $relation = null) * no further adjustments are needed as ctype_xdigit returns false for binary and bytea hex strings. */ if (ctype_xdigit($value)) { - if (empty($this->properties)) { + if (! $this->isPostgres) { $condition->setValue(hex2bin($value)); } elseif (substr($value, 0, 2) !== '\\x') { - // PostgreSQL. $condition->setValue(sprintf('\\x%s', $value)); } } From 5240661e9cdc0019ea1f046078d00c86222208c1 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Tue, 25 Apr 2023 13:32:19 +0200 Subject: [PATCH 5/6] Use the original filter value in `Binary#rewriteCondition()` --- src/Behavior/Binary.php | 2 +- src/Compat/FilterProcessor.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Behavior/Binary.php b/src/Behavior/Binary.php index 807a783..dd67410 100644 --- a/src/Behavior/Binary.php +++ b/src/Behavior/Binary.php @@ -89,7 +89,7 @@ public function rewriteCondition(Condition $condition, $relation = null) */ $column = $condition->metaData()->get('columnName'); if (isset($this->properties[$column])) { - $value = $condition->getValue(); + $value = $condition->metaData()->get('originalValue'); if ($this->isPostgres && is_resource($value)) { throw new UnexpectedValueException(sprintf('Unexpected resource for %s', $column)); diff --git a/src/Compat/FilterProcessor.php b/src/Compat/FilterProcessor.php index 635d31d..0cea081 100644 --- a/src/Compat/FilterProcessor.php +++ b/src/Compat/FilterProcessor.php @@ -111,6 +111,8 @@ protected function requireAndResolveFilterColumns(Filter\Rule $filter, Query $qu } $subjectBehaviors = $resolver->getBehaviors($subject); + // This is only used within the Binary behavior in rewriteCondition(). + $filter->metaData()->set('originalValue', $filter->getValue()); try { // Prepare filter as if it were final to allow full control for rewrite filter behaviors From 1a26879295900ce5f5e3f69877ae340320d9269a Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Tue, 25 Apr 2023 13:37:50 +0200 Subject: [PATCH 6/6] tests: Fix binary tests --- tests/Behavior/BinaryTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Behavior/BinaryTest.php b/tests/Behavior/BinaryTest.php index 2d4a079..36c12c4 100644 --- a/tests/Behavior/BinaryTest.php +++ b/tests/Behavior/BinaryTest.php @@ -103,7 +103,9 @@ protected function behavior(bool $postgres = false): Binary protected function condition($value = self::TEST_HEX_VALUE): Equal { $c = new Equal(static::TEST_COLUMN, $value); - $c->metaData()->set('columnName', static::TEST_COLUMN); + $c->metaData() + ->set('originalValue', $value) + ->set('columnName', static::TEST_COLUMN); return $c; }