DbObject: remove binary column magic

Binary columns must be configured explicitly, binary keys are now also
possible

fixes #1702

library/Director/Data/Db/DbConnection.php

@@ -3,6 +3,7 @@
 namespace Icinga\Module\Director\Data\Db;
 
 use Icinga\Data\Db\DbConnection as IcingaDbConnection;
+use Zend_Db_Expr;
 
 class DbConnection extends IcingaDbConnection
 {
@@ -16,6 +17,15 @@ class DbConnection extends IcingaDbConnection
         return $this->getDbType() === 'pgsql';
     }
 
+    public function quoteBinary($binary)
+    {
+        if ($this->isPgsql()) {
+            return new Zend_Db_Expr("'\\x" . bin2hex($binary) . "'");
+        }
+
+        return $binary;
+    }
+
     public function hasPgExtension($name)
     {
         $db = $this->db();

library/Director/Data/Db/DbObject.php

@@ -72,6 +72,8 @@ abstract class DbObject
     /** @var bool forbid updates to autoinc values */
     protected $protectAutoinc = true;
 
+    protected $binaryProperties = [];
+
     /**
      * Filled with object instances when prefetchAll is used
      */
@@ -742,7 +744,7 @@ abstract class DbObject
         // TODO: Remove this!
         if ($this->connection->isPgsql()) {
             foreach ($properties as $key => $value) {
-                if (preg_match('/checksum$/', $key)) {
+                if ($this->isBinaryColumn($key)) {
                     $properties[$key] = Util::pgBinEscape($value);
                 }
             }
@@ -751,6 +753,11 @@ abstract class DbObject
         return $this->db->insert($this->table, $properties);
     }
 
+    protected function isBinaryColumn($column)
+    {
+        return in_array($column, $this->binaryProperties);
+    }
+
     /**
      * Store object to database
      *
@@ -920,19 +927,13 @@ abstract class DbObject
                     if ($this->loadedProperties[$k] === null) {
                         $where[] = sprintf('%s IS NULL', $k);
                     } else {
-                        $where[] = $this->db->quoteInto(
+                        $where[] = $this->createQuotedWhere($k, $this->loadedProperties[$k]);
-                            sprintf('%s = ?', $k),
-                            $this->loadedProperties[$k]
-                        );
                     }
                 } else {
                     if ($this->properties[$k] === null) {
                         $where[] = sprintf('%s IS NULL', $k);
                     } else {
-                        $where[] = $this->db->quoteInto(
+                        $where[] = $this->createQuotedWhere($k, $this->properties[$k]);
-                            sprintf('%s = ?', $k),
-                            $this->properties[$k]
-                        );
                     }
                 }
             }
@@ -940,16 +941,27 @@ abstract class DbObject
             return implode(' AND ', $where);
         } else {
             if ($this->hasBeenLoadedFromDb()) {
-                return $this->db->quoteInto(
+                return $this->createQuotedWhere($key, $this->loadedProperties[$key]);
-                    sprintf('%s = ?', $key),
-                    $this->loadedProperties[$key]
-                );
             } else {
+                return $this->createQuotedWhere($key, $this->properties[$key]);
+            }
+        }
+    }
+
+    protected function createQuotedWhere($column, $value)
+    {
         return $this->db->quoteInto(
-                    sprintf('%s = ?', $key),
+            sprintf('%s = ?', $column),
-                    $this->properties[$key]
+            $this->eventuallyQuoteBinary($value, $column)
         );
     }
+
+    protected function eventuallyQuoteBinary($value, $column)
+    {
+        if ($this->isBinaryColumn($column)) {
+            return $this->connection->quoteBinary($value);
+        } else {
+            return $value;
         }
     }
 

library/Director/Db.php

@@ -11,7 +11,6 @@ use Icinga\Module\Director\Data\Db\DbConnection;
 use Icinga\Module\Director\Objects\IcingaEndpoint;
 use Icinga\Module\Director\Objects\IcingaObject;
 use RuntimeException;
-use Zend_Db_Expr;
 use Zend_Db_Select;
 
 class Db extends DbConnection
@@ -711,15 +710,6 @@ class Db extends DbConnection
         }
     }
 
-    public function quoteBinary($binary)
-    {
-        if ($this->isPgsql()) {
-            return new Zend_Db_Expr("'\\x" . bin2hex($binary) . "'");
-        }
-
-        return $binary;
-    }
-
     public function enumDeployedConfigs()
     {
         $db = $this->db();

library/Director/DirectorObject/Automation/Basket.php

@@ -31,6 +31,10 @@ class Basket extends DbObject implements ExportInterface
         'owner_value' => null,
     ];
 
+    protected $binaryProperties = [
+        'uuid'
+    ];
+
     public function getHexUuid()
     {
         return bin2hex($this->get('uuid'));

library/Director/DirectorObject/Automation/BasketContent.php

@@ -17,4 +17,8 @@ class BasketContent extends DbObject
         'summary'  => null,
         'content'  => null,
     ];
+
+    protected $binaryProperties = [
+        'checksum'
+    ];
 }

library/Director/DirectorObject/Automation/BasketSnapshot.php

@@ -65,6 +65,11 @@ class BasketSnapshot extends DbObject
         'ts_create'        => null,
     ];
 
+    protected $binaryProperties = [
+        'basket_uuid',
+        'content_checksum',
+    ];
+
     public static function supports($type)
     {
         return isset(self::$typeClasses[$type]);

library/Director/Objects/DirectorActivityLog.php

@@ -17,7 +17,7 @@ class DirectorActivityLog extends DbObject
 
     protected $autoincKeyName = 'id';
 
-    protected $defaultProperties = array(
+    protected $defaultProperties = [
         'id'              => null,
         'object_name'     => null,
         'action_name'     => null,
@@ -28,7 +28,12 @@ class DirectorActivityLog extends DbObject
         'change_time'     => null,
         'checksum'        => null,
         'parent_checksum' => null,
-    );
+    ];
+
+    protected $binaryProperties = [
+        'checksum',
+        'parent_checksum'
+    ];
 
     /**
      * @param $name
@@ -82,7 +87,7 @@ class DirectorActivityLog extends DbObject
     public static function loadLatest(Db $connection)
     {
         $db = $connection->getDbAdapter();
-        $query = $db->select()->from('director_activity_log', array('id' => 'MAX(id)'));
+        $query = $db->select()->from('director_activity_log', ['id' => 'MAX(id)']);
 
         return static::load($db->fetchOne($query), $connection);
     }

library/Director/Objects/DirectorDeploymentLog.php

@@ -9,7 +9,6 @@ use Icinga\Module\Director\Data\Db\DbObject;
 use Icinga\Module\Director\Db;
 use Icinga\Module\Director\IcingaConfig\IcingaConfig;
 use Icinga\Module\Director\Util;
-use RuntimeException;
 
 class DirectorDeploymentLog extends DbObject
 {
@@ -21,7 +20,7 @@ class DirectorDeploymentLog extends DbObject
 
     protected $config;
 
-    protected $defaultProperties = array(
+    protected $defaultProperties = [
         'id'                     => null,
         'config_checksum'        => null,
         'last_activity_checksum' => null,
@@ -38,7 +37,12 @@ class DirectorDeploymentLog extends DbObject
         'startup_succeeded'      => null,
         'username'               => null,
         'startup_log'            => null,
-    );
+    ];
+
+    protected $binaryProperties = [
+        'config_checksum',
+        'last_activity_checksum'
+    ];
 
     public function getConfigHexChecksum()
     {
@@ -97,6 +101,11 @@ class DirectorDeploymentLog extends DbObject
         return $db->fetchOne($query, $stage);
     }
 
+    /**
+     * @param Db $connection
+     * @return DirectorDeploymentLog
+     * @throws NotFoundError
+     */
     public static function loadLatest(Db $connection)
     {
         $db = $connection->getDbAdapter();

library/Director/Objects/IcingaFlatVar.php

@@ -15,12 +15,17 @@ class IcingaFlatVar extends DbObject
         'flatname_checksum'
     );
 
-    protected $defaultProperties = array(
+    protected $defaultProperties = [
         'var_checksum'      => null,
         'flatname_checksum' => null,
         'flatname'          => null,
         'flatvalue'         => null,
-    );
+    ];
+
+    protected $binaryProperties = [
+        'var_checksum',
+        'flatname_checksum',
+    ];
 
     public static function generateForCustomVar(CustomVariable $var, Db $db)
     {

library/Director/Objects/IcingaVar.php

@@ -4,7 +4,6 @@ namespace Icinga\Module\Director\Objects;
 
 use Icinga\Module\Director\CustomVariable\CustomVariable;
 use Icinga\Module\Director\Data\Db\DbObject;
-use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c;
 use Icinga\Module\Director\Db;
 
 class IcingaVar extends DbObject
@@ -16,13 +15,18 @@ class IcingaVar extends DbObject
     /** @var CustomVariable */
     protected $var;
 
-    protected $defaultProperties = array(
+    protected $defaultProperties = [
         'checksum'          => null,
         'rendered_checksum' => null,
         'varname'           => null,
         'varvalue'          => null,
         'rendered'          => null
-    );
+    ];
+
+    protected $binaryProperties = [
+        'checksum',
+        'rendered_checksum',
+    ];
 
     /**
      * @param CustomVariable $var
@@ -48,10 +52,11 @@ class IcingaVar extends DbObject
     }
 
     /**
-     * @param CustomVariable $var
+     * @param CustomVariable $customVar
      * @param Db $db
      *
      * @return static
+     * @throws \Icinga\Module\Director\Exception\DuplicateKeyException
      */
     public static function generateForCustomVar(CustomVariable $customVar, Db $db)
     {

library/Director/Objects/ImportRun.php

@@ -16,7 +16,7 @@ class ImportRun extends DbObject
     /** @var ImportSource */
     protected $importSource = null;
 
-    protected $defaultProperties = array(
+    protected $defaultProperties = [
         'id'              => null,
         'source_id'       => null,
         'rowset_checksum' => null,
@@ -24,7 +24,11 @@ class ImportRun extends DbObject
         'end_time'        => null,
         // TODO: Check whether succeeded could be dropped
         'succeeded'       => null,
-    );
+    ];
+
+    protected $binaryProperties = [
+        'rowset_checksum',
+    ];
 
     public function prepareImportedObjectQuery($columns = array('object_name'))
     {

library/Director/Util.php

@@ -9,6 +9,7 @@ use Icinga\Exception\NotImplementedError;
 use Icinga\Exception\ProgrammingError;
 use dipl\Html\Html;
 use dipl\Html\Link;
+use RuntimeException;
 use Zend_Db_Expr;
 
 class Util
@@ -19,6 +20,10 @@ class Util
 
     public static function pgBinEscape($binary)
     {
+        if ($binary instanceof Zend_Db_Expr) {
+            throw new RuntimeException('Trying to escape binary twice');
+        }
+
         return new Zend_Db_Expr("'\\x" . bin2hex($binary) . "'");
     }
 

library/Director/Web/Table/BasketSnapshotTable.php

@@ -12,6 +12,8 @@ use RuntimeException;
 
 class BasketSnapshotTable extends ZfQueryBasedTable
 {
+    use DbHelper;
+
     protected $searchColumns = [
         'basket_name',
         'summary'
@@ -87,7 +89,7 @@ class BasketSnapshotTable extends ZfQueryBasedTable
     protected function linkToSnapshot($caption, $row)
     {
         return new Link($caption, 'director/basket/snapshot', [
-            'checksum' => bin2hex($row->content_checksum),
+            'checksum' => bin2hex($this->wantBinaryValue($row->content_checksum)),
             'ts'       => $row->ts_create,
             'name'     => $row->basket_name,
         ]);
@@ -115,7 +117,7 @@ class BasketSnapshotTable extends ZfQueryBasedTable
         )->order('bs.ts_create DESC');
 
         if ($this->basket !== null) {
-            $query->where('b.uuid = ?', $this->basket->get('uuid'));
+            $query->where('b.uuid = ?', $this->quoteBinary($this->basket->get('uuid')));
         }
 
         return $query;

library/Director/Web/Table/BasketTable.php

@@ -13,7 +13,6 @@ class BasketTable extends ZfQueryBasedTable
 
     public function renderRow($row)
     {
-        $hexUuid = bin2hex($row->uuid);
         $tr = $this::row([
             new Link(
                 $row->basket_name,