From 1c30412abdea62a3727058b9ad23a203310d6954 Mon Sep 17 00:00:00 2001 From: Thomas Gelf Date: Sat, 20 Aug 2022 20:40:15 +0200 Subject: [PATCH] DirectorActivityLog, others: constants, cleanup --- application/clicommands/SyncruleCommand.php | 7 +- application/forms/SyncCheckForm.php | 13 ++- library/Director/Db/Branch/BranchActivity.php | 7 +- .../Director/Objects/DirectorActivityLog.php | 61 ++++++----- library/Director/Objects/SyncRun.php | 7 ++ .../Director/Web/Widget/ActivityLogInfo.php | 21 ++-- .../Director/Web/Widget/SyncRunDetails.php | 101 +++++++++++------- 7 files changed, 130 insertions(+), 87 deletions(-) diff --git a/application/clicommands/SyncruleCommand.php b/application/clicommands/SyncruleCommand.php index 961683df3..37a3f0e93 100644 --- a/application/clicommands/SyncruleCommand.php +++ b/application/clicommands/SyncruleCommand.php @@ -3,6 +3,7 @@ namespace Icinga\Module\Director\Clicommands; use Icinga\Module\Director\Cli\Command; +use Icinga\Module\Director\Objects\DirectorActivityLog; use Icinga\Module\Director\Objects\IcingaObject; use Icinga\Module\Director\Objects\SyncRule; use RuntimeException; @@ -98,9 +99,9 @@ protected function getExpectedModificationCounts(SyncRule $rule) } return (object) [ - 'create' => $create, - 'modify' => $modify, - 'delete' => $delete, + DirectorActivityLog::ACTION_CREATE => $create, + DirectorActivityLog::ACTION_MODIFY => $modify, + DirectorActivityLog::ACTION_DELETE => $delete, ]; } diff --git a/application/forms/SyncCheckForm.php b/application/forms/SyncCheckForm.php index 28ab5e275..8fb3bd0ca 100644 --- a/application/forms/SyncCheckForm.php +++ b/application/forms/SyncCheckForm.php @@ -2,6 +2,7 @@ namespace Icinga\Module\Director\Forms; +use Icinga\Module\Director\Objects\DirectorActivityLog; use Icinga\Module\Director\Objects\SyncRule; use Icinga\Module\Director\Web\Form\DirectorForm; @@ -31,16 +32,20 @@ public function onSuccess() $this->notifySuccess( $this->translate(('This Sync Rule would apply new changes')) ); - $sum = array('create' => 0, 'modify' => 0, 'delete' => 0); + $sum = [ + DirectorActivityLog::ACTION_CREATE => 0, + DirectorActivityLog::ACTION_MODIFY => 0, + DirectorActivityLog::ACTION_DELETE => 0 + ]; // TODO: Preview them? Like "hosta, hostb and 4 more would be... foreach ($this->rule->getExpectedModifications() as $object) { if ($object->shouldBeRemoved()) { - $sum['delete']++; + $sum[DirectorActivityLog::ACTION_DELETE]++; } elseif (! $object->hasBeenLoadedFromDb()) { - $sum['create']++; + $sum[DirectorActivityLog::ACTION_CREATE]++; } elseif ($object->hasBeenModified()) { - $sum['modify']++; + $sum[DirectorActivityLog::ACTION_MODIFY]++; } } diff --git a/library/Director/Db/Branch/BranchActivity.php b/library/Director/Db/Branch/BranchActivity.php index 19b8391c2..097af2682 100644 --- a/library/Director/Db/Branch/BranchActivity.php +++ b/library/Director/Db/Branch/BranchActivity.php @@ -9,6 +9,7 @@ use Icinga\Module\Director\Data\Json; use Icinga\Module\Director\Data\SerializableValue; use Icinga\Module\Director\Db; +use Icinga\Module\Director\Objects\DirectorActivityLog; use Icinga\Module\Director\Objects\IcingaObject; use InvalidArgumentException; use Ramsey\Uuid\Uuid; @@ -19,9 +20,9 @@ class BranchActivity { const DB_TABLE = 'director_branch_activity'; - const ACTION_CREATE = 'create'; - const ACTION_MODIFY = 'modify'; - const ACTION_DELETE = 'delete'; + const ACTION_CREATE = DirectorActivityLog::ACTION_CREATE; + const ACTION_MODIFY = DirectorActivityLog::ACTION_MODIFY; + const ACTION_DELETE = DirectorActivityLog::ACTION_DELETE; /** @var int */ protected $timestampNs; diff --git a/library/Director/Objects/DirectorActivityLog.php b/library/Director/Objects/DirectorActivityLog.php index 9e00b0eca..ffc0c92e6 100644 --- a/library/Director/Objects/DirectorActivityLog.php +++ b/library/Director/Objects/DirectorActivityLog.php @@ -4,13 +4,19 @@ use Icinga\Module\Director\Data\Db\DbObject; use Icinga\Module\Director\Db; -use Icinga\Module\Director\Util; use Icinga\Authentication\Auth; use Icinga\Application\Icinga; use Icinga\Application\Logger; class DirectorActivityLog extends DbObject { + const ACTION_CREATE = 'create'; + const ACTION_DELETE = 'delete'; + const ACTION_MODIFY = 'modify'; + + /** @deprecated */ + const AUDIT_REMOVE = 'remove'; + protected $table = 'director_activity_log'; protected $keyName = 'id'; @@ -104,25 +110,25 @@ public static function logCreation(IcingaObject $object, Db $db) $type = $object->getTableName(); $newProps = $object->toJson(null, true); - $data = array( + $data = [ 'object_name' => $name, - 'action_name' => 'create', + 'action_name' => self::ACTION_CREATE, 'author' => static::username(), 'object_type' => $type, 'new_properties' => $newProps, 'change_time' => date('Y-m-d H:i:s'), 'parent_checksum' => $db->getLastActivityChecksum() - ); + ]; $data['checksum'] = sha1(json_encode($data), true); $data['parent_checksum'] = hex2bin($data['parent_checksum']); - static::audit($db, array( - 'action' => 'create', + static::audit($db, [ + 'action' => self::ACTION_CREATE, 'object_type' => $type, 'object_name' => $name, 'new_props' => $newProps, - )); + ]); return static::create($data)->store($db); } @@ -134,27 +140,27 @@ public static function logModification(IcingaObject $object, Db $db) $oldProps = json_encode($object->getPlainUnmodifiedObject()); $newProps = $object->toJson(null, true); - $data = array( + $data = [ 'object_name' => $name, - 'action_name' => 'modify', + 'action_name' => self::ACTION_MODIFY, 'author' => static::username(), 'object_type' => $type, 'old_properties' => $oldProps, 'new_properties' => $newProps, 'change_time' => date('Y-m-d H:i:s'), 'parent_checksum' => $db->getLastActivityChecksum() - ); + ]; $data['checksum'] = sha1(json_encode($data), true); $data['parent_checksum'] = hex2bin($data['parent_checksum']); - static::audit($db, array( - 'action' => 'modify', + static::audit($db, [ + 'action' => self::ACTION_MODIFY, 'object_type' => $type, 'object_name' => $name, 'old_props' => $oldProps, 'new_props' => $newProps, - )); + ]); return static::create($data)->store($db); } @@ -165,45 +171,42 @@ public static function logRemoval(IcingaObject $object, Db $db) $type = $object->getTableName(); $oldProps = json_encode($object->getPlainUnmodifiedObject()); - $data = array( + $data = [ 'object_name' => $name, - 'action_name' => 'delete', + 'action_name' => self::ACTION_DELETE, 'author' => static::username(), 'object_type' => $type, 'old_properties' => $oldProps, 'change_time' => date('Y-m-d H:i:s'), 'parent_checksum' => $db->getLastActivityChecksum() - ); + ]; $data['checksum'] = sha1(json_encode($data), true); $data['parent_checksum'] = hex2bin($data['parent_checksum']); - static::audit($db, array( - 'action' => 'remove', + static::audit($db, [ + 'action' => self::AUDIT_REMOVE, 'object_type' => $type, 'object_name' => $name, 'old_props' => $oldProps - )); + ]); return static::create($data)->store($db); } public static function audit(Db $db, $properties) { - if ($db->settings()->enable_audit_log !== 'y') { + if ($db->settings()->get('enable_audit_log') !== 'y') { return; } - $log = array(); - $properties = array_merge( - array( - 'username' => static::username(), - 'address' => static::ip(), - ), - $properties - ); + $log = []; + $properties = array_merge([ + 'username' => static::username(), + 'address' => static::ip(), + ], $properties); - foreach ($properties as $key => & $val) { + foreach ($properties as $key => $val) { $log[] = "$key=" . json_encode($val); } diff --git a/library/Director/Objects/SyncRun.php b/library/Director/Objects/SyncRun.php index 44c09ac90..62f7378de 100644 --- a/library/Director/Objects/SyncRun.php +++ b/library/Director/Objects/SyncRun.php @@ -36,4 +36,11 @@ public static function start(SyncRule $rule) $rule->getConnection() ); } + + public function countActivities() + { + return (int) $this->get('objects_deleted') + + (int) $this->get('objects_created') + + (int) $this->get('objects_modified'); + } } diff --git a/library/Director/Web/Widget/ActivityLogInfo.php b/library/Director/Web/Widget/ActivityLogInfo.php index 4a8634873..8454b2632 100644 --- a/library/Director/Web/Widget/ActivityLogInfo.php +++ b/library/Director/Web/Widget/ActivityLogInfo.php @@ -3,6 +3,7 @@ namespace Icinga\Module\Director\Web\Widget; use gipfl\Json\JsonString; +use Icinga\Module\Director\Objects\DirectorActivityLog; use ipl\Html\HtmlDocument; use ipl\Html\HtmlElement; use Icinga\Date\DateFormatter; @@ -343,7 +344,6 @@ protected function objectKey() /** * @param Url|null $url * @return Tabs - * @throws ProgrammingError */ public function getTabs(Url $url = null) { @@ -357,13 +357,12 @@ public function getTabs(Url $url = null) /** * @param Url $url * @return Tabs - * @throws ProgrammingError */ public function createTabs(Url $url) { $entry = $this->entry; $tabs = new Tabs(); - if ($entry->action_name === 'modify') { + if ($entry->action_name === DirectorActivityLog::ACTION_MODIFY) { $tabs->add('diff', [ 'label' => $this->translate('Diff'), 'url' => $url->without('show')->with('id', $entry->id) @@ -372,7 +371,10 @@ public function createTabs(Url $url) $this->defaultTab = 'diff'; } - if (in_array($entry->action_name, ['create', 'modify'])) { + if (in_array($entry->action_name, [ + DirectorActivityLog::ACTION_CREATE, + DirectorActivityLog::ACTION_MODIFY, + ])) { $tabs->add('new', [ 'label' => $this->translate('New object'), 'url' => $url->with(['id' => $entry->id, 'show' => 'new']) @@ -383,7 +385,10 @@ public function createTabs(Url $url) } } - if (in_array($entry->action_name, ['delete', 'modify'])) { + if (in_array($entry->action_name, [ + DirectorActivityLog::ACTION_DELETE, + DirectorActivityLog::ACTION_MODIFY, + ])) { $tabs->add('old', [ 'label' => $this->translate('Former object'), 'url' => $url->with(['id' => $entry->id, 'show' => 'old']) @@ -572,13 +577,13 @@ public function hasBeenDisabled() public function getTitle() { switch ($this->entry->action_name) { - case 'create': + case DirectorActivityLog::ACTION_CREATE: $msg = $this->translate('%s "%s" has been created'); break; - case 'delete': + case DirectorActivityLog::ACTION_DELETE: $msg = $this->translate('%s "%s" has been deleted'); break; - case 'modify': + case DirectorActivityLog::ACTION_MODIFY: $msg = $this->translate('%s "%s" has been modified'); break; default: diff --git a/library/Director/Web/Widget/SyncRunDetails.php b/library/Director/Web/Widget/SyncRunDetails.php index 90eb1efe2..408e8f6b2 100644 --- a/library/Director/Web/Widget/SyncRunDetails.php +++ b/library/Director/Web/Widget/SyncRunDetails.php @@ -2,18 +2,21 @@ namespace Icinga\Module\Director\Web\Widget; +use Icinga\Module\Director\Objects\DirectorActivityLog; use ipl\Html\HtmlDocument; use Icinga\Module\Director\Db; use Icinga\Module\Director\Objects\SyncRun; -use ipl\Html\Html; use gipfl\IcingaWeb2\Link; use gipfl\Translation\TranslationHelper; use gipfl\IcingaWeb2\Widget\NameValueTable; +use function sprintf; class SyncRunDetails extends NameValueTable { use TranslationHelper; + const URL_ACTIVITIES = 'director/config/activities'; + /** @var SyncRun */ protected $run; @@ -22,22 +25,20 @@ public function __construct(SyncRun $run) $this->run = $run; $this->getAttributes()->add('data-base-target', '_next'); // eigentlich nur runSummary $this->addNameValuePairs([ - $this->translate('Start time') => $run->start_time, - $this->translate('Duration') => sprintf('%.2fs', $run->duration_ms / 1000), - $this->translate('Activity') => $this->runSummary($run) + $this->translate('Start time') => $run->get('start_time'), + $this->translate('Duration') => sprintf('%.2fs', $run->get('duration_ms') / 1000), + $this->translate('Activity') => $this->runSummary($run) ]); } /** * @param SyncRun $run * @return array - * @throws \Icinga\Exception\IcingaException - * @throws \Icinga\Exception\ProgrammingError */ protected function runSummary(SyncRun $run) { $html = []; - $total = $run->objects_deleted + $run->objects_created + $run->objects_modified; + $total = $run->countActivities(); if ($total === 0) { $html[] = $this->translate('No changes have been made'); } else { @@ -52,47 +53,49 @@ protected function runSummary(SyncRun $run) /** @var Db $db */ $db = $run->getConnection(); - if ($run->last_former_activity === null) { + $formerId = $db->fetchActivityLogIdByChecksum($run->get('last_former_activity')); + if ($formerId === null) { return $html; } - $formerId = $db->fetchActivityLogIdByChecksum($run->last_former_activity); - $lastId = $db->fetchActivityLogIdByChecksum($run->last_related_activity); + $lastId = $db->fetchActivityLogIdByChecksum($run->get('last_related_activity')); - $idRangeEx = sprintf( - 'id>%d&id<=%d', - $formerId, - $lastId - ); - $activityUrl = 'director/config/activities'; + if ($formerId !== $lastId) { + $idRangeEx = sprintf( + 'id>%d&id<=%d', + $formerId, + $lastId + ); + } else { + $idRangeEx = null; + } $links = new HtmlDocument(); $links->setSeparator(', '); - if ($run->objects_created > 0) { - $links->add(new Link( - sprintf('%d created', $run->objects_created), - $activityUrl, - ['action' => 'create', 'idRangeEx' => $idRangeEx] - )); - } - if ($run->objects_modified > 0) { - $links->add(new Link( - sprintf('%d modified', $run->objects_modified), - $activityUrl, - ['action' => 'modify', 'idRangeEx' => $idRangeEx] - )); - } - if ($run->objects_deleted > 0) { - $links->add(new Link( - sprintf('%d deleted', $run->objects_deleted), - $activityUrl, - ['action' => 'delete', 'idRangeEx' => $idRangeEx] - )); - } + $links->add([ + $this->activitiesLink( + 'objects_created', + $this->translate('%d created'), + DirectorActivityLog::ACTION_CREATE, + $idRangeEx + ), + $this->activitiesLink( + 'objects_modified', + $this->translate('%d modified'), + DirectorActivityLog::ACTION_MODIFY, + $idRangeEx + ), + $this->activitiesLink( + 'objects_deleted', + $this->translate('%d deleted'), + DirectorActivityLog::ACTION_DELETE, + $idRangeEx + ), + ]); - if (count($links) > 1) { + if ($idRangeEx && count($links) > 1) { $links->add(new Link( - 'Show all actions', - $activityUrl, + $this->translate('Show all actions'), + self::URL_ACTIVITIES, ['idRangeEx' => $idRangeEx] )); } @@ -105,4 +108,22 @@ protected function runSummary(SyncRun $run) return $html; } + + protected function activitiesLink($key, $label, $action, $rangeFilter) + { + $count = $this->run->get($key); + if ($count > 0) { + if ($rangeFilter) { + return new Link( + sprintf($label, $count), + self::URL_ACTIVITIES, + ['action' => $action, 'idRangeEx' => $rangeFilter] + ); + } + + return sprintf($label, $count); + } + + return null; + } }