Skip to content

Commit 9c2a33d

Browse files
yhabteabyhabteab
committed
Optimize imports & cleanup some codes
1 parent d2b5622 commit 9c2a33d

File tree

10 files changed

+85
-80
lines changed

10 files changed

+85
-80
lines changed

‎application/clicommands/CheckCommand.php‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66

77
use Icinga\Application\Logger;
88
use Icinga\Module\X509\Command;
9-
use Icinga\Module\X509\Job;
109
use Icinga\Module\X509\Model\X509Certificate;
10+
use Icinga\Module\X509\Model\X509CertificateChain;
1111
use Icinga\Module\X509\Model\X509Target;
1212
use ipl\Sql\Expression;
1313
use ipl\Stdlib\Filter;
@@ -89,36 +89,28 @@ public function hostAction()
8989
]);
9090

9191
// Sub queries for (valid_from, valid_to) columns
92-
$validFrom = X509Certificate::on($conn)
93-
->with(['chain', 'issuer_certificate'])
94-
->columns([
95-
new Expression('MAX(GREATEST(%s, %s))', ['valid_from', 'issuer_certificate.valid_from'])
96-
]);
97-
98-
$validFrom->getResolver()->setAliasPrefix('sub_');
92+
$validFrom = $targets->createSubQuery(new X509Certificate(), 'chain.certificate');
9993
$validFrom
94+
->columns([new Expression('MAX(GREATEST(%s, %s))', ['valid_from', 'issuer_certificate.valid_from'])])
10095
->getSelectBase()
101-
->where(new Expression(
102-
'sub_certificate_link.certificate_chain_id = target_chain.id'
103-
));
96+
->resetWhere()
97+
->where(new Expression('sub_certificate_link.certificate_chain_id = target_chain.id'));
10498

10599
$validTo = clone $validFrom;
106-
$validTo->columns([
107-
new Expression('MIN(LEAST(%s, %s))', ['valid_to', 'issuer_certificate.valid_to'])
108-
]);
100+
$validTo->columns([new Expression('MIN(LEAST(%s, %s))', ['valid_to', 'issuer_certificate.valid_to'])]);
109101

110-
list($validFromSelect, $validFromValues) = $validFrom->dump();
111-
list($validToSelect, $validToValues) = $validTo->dump();
102+
list($validFromSelect, $_) = $validFrom->dump();
103+
list($validToSelect, $_) = $validTo->dump();
112104
$targets
113105
->withColumns([
114-
'valid_from' => new Expression("$validFromSelect", null, ...$validFromValues),
115-
'valid_to' => new Expression("$validToSelect", null, ...$validToValues)
106+
'valid_from' => new Expression($validFromSelect),
107+
'valid_to' => new Expression($validToSelect)
116108
])
117109
->getSelectBase()
118110
->where(new Expression('target_chain_link.order = 0'));
119111

120112
if ($ip !== null) {
121-
$targets->filter(Filter::equal('ip', Job::binary($ip)));
113+
$targets->filter(Filter::equal('ip', $ip));
122114
}
123115
if ($hostname !== null) {
124116
$targets->filter(Filter::equal('hostname', $hostname));

‎application/controllers/CertificateController.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use Icinga\Module\X509\CertificateDetails;
99
use Icinga\Module\X509\Controller;
1010
use Icinga\Module\X509\Model\X509Certificate;
11-
use ipl\Sql;
1211
use ipl\Stdlib\Filter;
1312

1413
class CertificateController extends Controller

‎application/controllers/CertificatesController.php‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ public function indexAction()
6262
} else {
6363
$this->addControl($searchBar);
6464
$this->sendMultipartUpdate();
65+
6566
return;
6667
}
6768
} else {
@@ -77,14 +78,7 @@ public function indexAction()
7778
$this->addControl($limitControl);
7879
$this->addControl($searchBar);
7980

80-
// List of allowed columns to be exported
81-
$exportable = array_flip([
82-
'id', 'subject', 'issuer', 'version', 'self_signed', 'ca', 'trusted',
83-
'pubkey_algo', 'pubkey_bits', 'signature_algo', 'signature_hash_algo',
84-
'valid_from', 'valid_to'
85-
]);
86-
87-
$this->handleFormatRequest($certificates, function (Query $certificates) use ($exportable) {
81+
$this->handleFormatRequest($certificates, function (Query $certificates) {
8882
/** @var X509Certificate $cert */
8983
foreach ($certificates as $cert) {
9084
$cert['valid_from'] = (new \DateTime())
@@ -94,7 +88,7 @@ public function indexAction()
9488
->setTimestamp($cert['valid_to'])
9589
->format('l F jS, Y H:i:s e');
9690

97-
yield array_intersect_key(iterator_to_array($cert), $exportable);
91+
yield array_intersect_key(iterator_to_array($cert), array_flip($cert->getExportableColumns()));
9892
}
9993
});
10094

@@ -107,10 +101,11 @@ public function indexAction()
107101

108102
public function completeAction()
109103
{
110-
$suggestions = new ObjectSuggestions();
111-
$suggestions->setModel(X509Certificate::class);
112-
$suggestions->forRequest($this->getServerRequest());
113-
$this->getDocument()->add($suggestions);
104+
$this->getDocument()->add(
105+
(new ObjectSuggestions())
106+
->setModel(X509Certificate::class)
107+
->forRequest($this->getServerRequest())
108+
);
114109
}
115110

116111
public function searchEditorAction()

‎application/controllers/UsageController.php‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ public function indexAction()
7878
} else {
7979
$this->addControl($searchBar);
8080
$this->sendMultipartUpdate();
81+
8182
return;
8283
}
8384
} else {
@@ -93,13 +94,7 @@ public function indexAction()
9394
$this->addControl($limitControl);
9495
$this->addControl($searchBar);
9596

96-
$exportable = array_flip([
97-
'valid', 'hostname', 'ip', 'port', 'subject', 'issuer', 'version',
98-
'self_signed', 'ca', 'trusted', 'pubkey_algo', 'pubkey_bits',
99-
'signature_algo', 'signature_hash_algo', 'valid_from', 'valid_to'
100-
]);
101-
102-
$this->handleFormatRequest($targets, function (Query $targets) use ($conn, $exportable) {
97+
$this->handleFormatRequest($targets, function (Query $targets) {
10398
foreach ($targets as $usage) {
10499
$usage['valid_from'] = (new \DateTime())
105100
->setTimestamp($usage['valid_from'])
@@ -113,7 +108,10 @@ public function indexAction()
113108
$usage->port = $usage->chain->target->port;
114109
$usage->valid = $usage->chain->valid;
115110

116-
yield array_intersect_key(iterator_to_array($usage), $exportable);
111+
yield array_intersect_key(
112+
iterator_to_array($usage),
113+
array_flip(array_merge(['valid', 'hostname', 'ip', 'port'], $usage->getExportableColumns()))
114+
);
117115
}
118116
});
119117

@@ -126,10 +124,11 @@ public function indexAction()
126124

127125
public function completeAction()
128126
{
129-
$suggestions = new ObjectSuggestions();
130-
$suggestions->setModel(X509Certificate::class);
131-
$suggestions->forRequest($this->getServerRequest());
132-
$this->getDocument()->add($suggestions);
127+
$this->getDocument()->add(
128+
(new ObjectSuggestions())
129+
->setModel(X509Certificate::class)
130+
->forRequest($this->getServerRequest())
131+
);
133132
}
134133

135134
public function searchEditorAction()

‎library/X509/CertificateDetails.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
namespace Icinga\Module\X509;
66

77
use DateTime;
8+
use Icinga\Module\X509\Model\X509Certificate;
89
use ipl\Html\BaseHtmlElement;
910
use ipl\Html\Html;
10-
use ipl\Orm\Model;
1111

1212
/**
1313
* Widget to display X.509 certificate details
@@ -23,7 +23,7 @@ class CertificateDetails extends BaseHtmlElement
2323
*/
2424
protected $cert;
2525

26-
public function setCert(Model $cert)
26+
public function setCert(X509Certificate $cert)
2727
{
2828
$this->cert = $cert;
2929

‎library/X509/DataTable.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected function renderRow($row)
8484
$cells = [];
8585

8686
foreach ($this->columns as $key => $column) {
87-
if (! is_int($key) && isset($row->$key)) {
87+
if (! is_int($key) && property_exists($row, $key)) {
8888
$data = $row[$key];
8989
} else {
9090
$data = null;

‎library/X509/Job.php‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,7 @@ protected function getScanTargets(): Generator
196196
} else {
197197
$targets = X509Target::on($this->db)->columns(['id', 'ip', 'hostname', 'port']);
198198
if ($this->sinceLastScan) {
199-
// The filter processor doesn't allow us to filter using expressions
200-
$targets
201-
->getSelectBase()
202-
->where(new Expression('last_scan < %d', [$this->sinceLastScan->getTimestamp()]));
199+
$targets->filter(Filter::lessThan('last_scan', $this->sinceLastScan->getTimestamp()));
203200
}
204201

205202
foreach ($targets as $target) {
@@ -438,10 +435,10 @@ protected function processChain($target, $chain)
438435
$this->db->transaction(function () use ($target, $chain) {
439436
$row = X509Target::on($this->db)->columns(['id']);
440437

441-
$filter = Filter::all();
442-
$filter->add(Filter::equal('ip', static::binary($target->ip)));
443-
$filter->add(Filter::equal('port', $target->port));
444-
$filter->add(Filter::equal('hostname', $target->hostname));
438+
$filter = Filter::all()
439+
->add(Filter::equal('ip', $target->ip))
440+
->add(Filter::equal('port', $target->port))
441+
->add(Filter::equal('hostname', $target->hostname));
445442

446443
$row->filter($filter);
447444

@@ -463,13 +460,14 @@ protected function processChain($target, $chain)
463460

464461
$chainUptodate = false;
465462

466-
$lastChain = X509CertificateChain::on($this->db)->columns(['id']);
467-
$lastChain
463+
$lastChain = X509CertificateChain::on($this->db)
464+
->columns(['id'])
468465
->filter(Filter::equal('target_id', $targetId))
469466
->orderBy('id', SORT_DESC)
470-
->limit(1);
467+
->limit(1)
468+
->first();
471469

472-
if (($lastChain = $lastChain->first())) {
470+
if ($lastChain) {
473471
$lastFingerprints = X509Certificate::on($this->db)->utilize('chain');
474472
$lastFingerprints
475473
->columns(['fingerprint'])
@@ -495,7 +493,7 @@ protected function processChain($target, $chain)
495493
}
496494

497495
if ($chainUptodate) {
498-
$chainId = (int) $lastChain->id;
496+
$chainId = $lastChain->id;
499497
} else {
500498
// TODO: https://github.com/Icinga/ipl-orm/pull/78
501499
$this->db->insert(

‎library/X509/Model/X509Certificate.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,30 @@ public function getSearchColumns()
8686
return ['subject', 'issuer'];
8787
}
8888

89+
/**
90+
* Get list of allowed columns to be exported
91+
*
92+
* @return string[]
93+
*/
94+
public function getExportableColumns(): array
95+
{
96+
return [
97+
'id',
98+
'subject',
99+
'issuer',
100+
'version',
101+
'self_signed',
102+
'ca',
103+
'trusted',
104+
'pubkey_algo',
105+
'pubkey_bits',
106+
'signature_algo',
107+
'signature_hash_algo',
108+
'valid_from',
109+
'valid_to'
110+
];
111+
}
112+
89113
public function createBehaviors(Behaviors $behaviors)
90114
{
91115
$behaviors->add(new Binary([

‎library/X509/ProvidedHook/HostsImportSource.php‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@
44

55
namespace Icinga\Module\X509\ProvidedHook;
66

7-
use Icinga\Module\X509\DbTool;
87
use Icinga\Module\X509\Job;
98
use Icinga\Module\X509\Model\X509Target;
109
use ipl\Sql;
11-
use ipl\Stdlib\Filter;
1210

1311
class HostsImportSource extends X509ImportSource
1412
{
@@ -29,11 +27,11 @@ public function fetchData()
2927

3028
if ($this->getDb()->getAdapter() instanceof Sql\Adapter\Pgsql) {
3129
$targets->withColumns([
32-
'host_ports' => new Sql\Expression('ARRAY_TO_STRING(ARRAY_AGG(DISTINCT port), \',\')')
30+
'host_ports' => new Sql\Expression("ARRAY_TO_STRING(ARRAY_AGG(DISTINCT port), ',')")
3331
]);
3432
} else {
3533
$targets->withColumns([
36-
'host_ports' => new Sql\Expression('GROUP_CONCAT(DISTINCT port SEPARATOR \',\')')
34+
'host_ports' => new Sql\Expression("GROUP_CONCAT(DISTINCT port SEPARATOR ',')")
3735
]);
3836
}
3937

@@ -60,12 +58,12 @@ public function fetchData()
6058
$target->host_name_or_ip = $target->host_ip;
6159
}
6260

63-
unset($target->ip); // Isn't needed any more!!
64-
unset($target->chain); // We don't need any relation properties anymore
61+
// Target ip is now obsolete and must not be included in the results.
62+
// The relation is only used to utilize the query and must not be in the result set as well.
63+
unset($target->ip);
64+
unset($target->chain);
6565

66-
$properties = iterator_to_array($target);
67-
68-
$results[$target->host_name_or_ip] = (object) $properties;
66+
$results[$target->host_name_or_ip] = (object) iterator_to_array($target);
6967
}
7068

7169
return $results;

‎library/X509/ProvidedHook/ServicesImportSource.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ public function fetchData()
5050
if ($this->getDb()->getAdapter() instanceof Sql\Adapter\Pgsql) {
5151
$targets
5252
->withColumns([
53-
'cert_fingerprint' => new Sql\Expression('ENCODE(%s, \'hex\')', [
53+
'cert_fingerprint' => new Sql\Expression("ENCODE(%s, 'hex')", [
5454
'chain.certificate.fingerprint'
5555
]),
5656
'cert_dn' => new Sql\Expression(
57-
'ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, \'=\', %s)), \',\')',
57+
"ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, '=', %s)), ',')",
5858
[
5959
'chain.certificate.dn.key',
6060
'chain.certificate.dn.value'
@@ -65,13 +65,13 @@ public function fetchData()
6565
->groupBy(['target_chain_certificate.id', 'target_chain_certificate_issuer_certificate.id']);
6666

6767
$certAltName->columns([
68-
new Sql\Expression('ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, \':\', %s)), \',\')', ['type', 'value'])
68+
new Sql\Expression("ARRAY_TO_STRING(ARRAY_AGG(CONCAT(%s, ':', %s)), ',')", ['type', 'value'])
6969
]);
7070
} else {
7171
$targets->withColumns([
7272
'cert_fingerprint' => new Sql\Expression('HEX(%s)', ['chain.certificate.fingerprint']),
7373
'cert_dn' => new Sql\Expression(
74-
'GROUP_CONCAT(CONCAT(%s, \'=\', %s) SEPARATOR \',\')',
74+
"GROUP_CONCAT(CONCAT(%s, '=', %s) SEPARATOR ',')",
7575
[
7676
'chain.certificate.dn.key',
7777
'chain.certificate.dn.value'
@@ -80,7 +80,7 @@ public function fetchData()
8080
]);
8181

8282
$certAltName->columns([
83-
new Sql\Expression('GROUP_CONCAT(CONCAT(%s, \':\', %s) SEPARATOR \',\')', ['type', 'value'])
83+
new Sql\Expression("GROUP_CONCAT(CONCAT(%s, ':', %s) SEPARATOR ',')", ['type', 'value'])
8484
]);
8585
}
8686

@@ -101,12 +101,12 @@ public function fetchData()
101101
$target->host_port
102102
);
103103

104-
unset($target->ip); // Isn't needed any more!!
105-
unset($target->chain); // We don't need any relation properties anymore
104+
// Target ip is now obsolete and must not be included in the results.
105+
// The relation is only used to utilize the query and must not be in the result set as well.
106+
unset($target->ip);
107+
unset($target->chain);
106108

107-
$properties = iterator_to_array($target);
108-
109-
$results[$target->host_name_ip_and_port] = (object) $properties;
109+
$results[$target->host_name_ip_and_port] = (object) iterator_to_array($target);
110110
}
111111

112112
return $results;

0 commit comments

Comments
 (0)