Merge pull request #105 from Icinga/fix-binary-behaviour

Make `Binary` behaviour extensible which might also be used for mysql adapter

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,9 @@ public function toDb($value, $key, $_)
 
     public function setQuery(Query $query)
     {
-        $this->rewriteSubjects = $this->properties;
+        $this->isPostgres = $query->getDb()->getAdapter() instanceof Pgsql;
 
-        if (! $query->getDb()->getAdapter() instanceof Pgsql) {
-            // Only process properties if the adapter is PostgreSQL.
-            $this->properties = [];
-        }
+        return $this;
     }
 
     public function rewriteCondition(Condition $condition, $relation = null)
@@ -90,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])) {
-            $value = $condition->getValue();
+        if (isset($this->properties[$column])) {
+            $value = $condition->metaData()->get('originalValue');
 
-            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));
             }
 
@@ -105,11 +102,10 @@ 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 (! $this->isPostgres) {
                     $condition->setValue(hex2bin($value));
+                } elseif (substr($value, 0, 2) !== '\\x') {
+                    $condition->setValue(sprintf('\\x%s', $value));
                 }
             }
         }

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

tests/Behavior/BinaryTest.php

@@ -0,0 +1,112 @@
+<?php
+
+namespace ipl\Tests\Orm;
+
+use ipl\Orm\Behavior\Binary;
+use ipl\Orm\Exception\ValueConversionException;
+use ipl\Orm\Query;
+use ipl\Sql\Connection;
+use ipl\Stdlib\Filter\Equal;
+use UnexpectedValueException;
+
+class BinaryTest extends \PHPUnit\Framework\TestCase
+{
+    protected const TEST_BINARY_VALUE = 'value';
+
+    protected const TEST_HEX_VALUE = '76616c7565';
+
+    protected const TEST_COLUMN = 'column';
+
+    public function testRetrievePropertyReturnsVanillaValueIfAdapterIsNotPostgreSQL(): void
+    {
+        $this->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('originalValue', $value)
+            ->set('columnName', static::TEST_COLUMN);
+
+        return $c;
+    }
+}