From 2a594aa66eab7ba053dd1f846139141d89b55a34 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 11 Sep 2022 09:38:01 +0200 Subject: [PATCH 1/7] fix: [correlation] Smarter count OverCorrelating values --- app/Model/OverCorrelatingValue.php | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/app/Model/OverCorrelatingValue.php b/app/Model/OverCorrelatingValue.php index bfb584183..e5802303b 100644 --- a/app/Model/OverCorrelatingValue.php +++ b/app/Model/OverCorrelatingValue.php @@ -83,16 +83,11 @@ class OverCorrelatingValue extends AppModel return $data; } - public function checkValue($value) + public function findOverCorrelatingValues(array $valuesToCheck): array { - return $this->hasAny(['value' => self::truncate($value)]); - } - - public function findOverCorrelatingValues(array $values_to_check): array - { - $values_to_check_truncated = array_unique(self::truncateValues($values_to_check), SORT_REGULAR); + $valuesToCheck = array_unique(self::truncateValues($valuesToCheck), SORT_REGULAR); return $this->find('column', [ - 'conditions' => ['value' => $values_to_check_truncated], + 'conditions' => ['value' => $valuesToCheck], 'fields' => ['value'], ]); } @@ -134,14 +129,23 @@ class OverCorrelatingValue extends AppModel ]); $this->Attribute = ClassRegistry::init('Attribute'); foreach ($overCorrelations as &$overCorrelation) { + $value = $overCorrelation['OverCorrelatingValue']['value'] . '%'; $count = $this->Attribute->find('count', [ 'recursive' => -1, 'conditions' => [ 'OR' => [ - 'Attribute.value1 LIKE' => $overCorrelation['OverCorrelatingValue']['value'] . '%', - 'Attribute.value2 LIKE' => $overCorrelation['OverCorrelatingValue']['value'] . '%' - ] - ] + 'Attribute.value1 LIKE' => $value, + 'AND' => [ + 'Attribute.value2 LIKE' => $value, + 'NOT' => ['Attribute.type' => Attribute::PRIMARY_ONLY_CORRELATING_TYPES] + ], + ], + 'NOT' => ['Attribute.type' => Attribute::NON_CORRELATING_TYPES], + 'Attribute.disable_correlation' => 0, + 'Event.disable_correlation' => 0, + 'Attribute.deleted' => 0, + ], + 'contain' => ['Event'], ]); $overCorrelation['OverCorrelatingValue']['occurrence'] = $count; } From 881e151ddcbb3f4510a2886a7132ef078c8fd562 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 11 Sep 2022 09:44:22 +0200 Subject: [PATCH 2/7] fix: [UI] Show active tab for over correlations --- app/Controller/CorrelationsController.php | 51 ++++--- app/View/Correlations/over_correlations.ctp | 159 ++++++++++---------- 2 files changed, 108 insertions(+), 102 deletions(-) diff --git a/app/Controller/CorrelationsController.php b/app/Controller/CorrelationsController.php index 4cf2e4ef4..883bc3c57 100644 --- a/app/Controller/CorrelationsController.php +++ b/app/Controller/CorrelationsController.php @@ -83,33 +83,38 @@ class CorrelationsController extends AppController 'page' => 1, 'order' => 'occurrence desc' ]; - foreach (array_keys($query) as $custom_param) { - if (isset($this->params['named'][$custom_param])) { - $query[$custom_param] = $this->params['named'][$custom_param]; - } - } - if (isset($this->params['named']['scope'])) { - $limit = $this->Correlation->OverCorrelatingValue->getLimit(); - if ($this->params['named']['scope'] === 'over_correlating') { - $query['conditions'][] = ['occurrence >=' => $limit]; - } else if ($this->params['named']['scope'] === 'not_over_correlating') { - $query['conditions'][] = ['occurrence <' => $limit]; + foreach ($query as $customParam => $foo) { + if (isset($this->request->params['named'][$customParam])) { + $query[$customParam] = $this->request->params['named'][$customParam]; } } + if (isset($this->request->params['named']['scope'])) { + $limit = $this->Correlation->OverCorrelatingValue->getLimit(); + if ($this->request->params['named']['scope'] === 'over_correlating') { + $scope = 'over_correlating'; + $query['conditions'][] = ['occurrence >=' => $limit]; + } else if ($this->request->params['named']['scope'] === 'not_over_correlating') { + $query['conditions'][] = ['occurrence <' => $limit]; + $scope = 'not_over_correlating'; + } + } else { + $scope = 'all'; + } $data = $this->Correlation->OverCorrelatingValue->getOverCorrelations($query); $data = $this->Correlation->attachExclusionsToOverCorrelations($data); if ($this->_isRest()) { return $this->RestResponse->viewData($data, 'json'); - } else { - $this->__setPagingParams($query['page'], $query['limit'], count($data), 'named'); - $this->set('data', $data); - $this->set('title_for_layout', __('Index of over correlating values')); - $this->set('menuData', [ - 'menuList' => 'correlationExclusions', - 'menuItem' => 'over' - ]); } + + $this->__setPagingParams($query['page'], $query['limit'], count($data), 'named'); + $this->set('data', $data); + $this->set('scope', $scope); + $this->set('title_for_layout', __('Index of over correlating values')); + $this->set('menuData', [ + 'menuList' => 'correlationExclusions', + 'menuItem' => 'over' + ]); } public function switchEngine(string $engine) @@ -205,17 +210,15 @@ class CorrelationsController extends AppController { $this->loadModel('OverCorrelatingValue'); $this->OverCorrelatingValue->generateOccurrencesRouter(); - $message = __('Job queued.'); if (Configure::read('MISP.background_jobs')) { $message = __('Job queued.'); } else { $message = __('Over-correlations counted successfully.'); } - if (!$this->_isRest()) { - $this->Flash->info($message); - $this->redirect(['controller' => 'correlations', 'action' => 'overCorrelations']); - } else { + if ($this->_isRest()) { return $this->RestResponse->saveSuccessResponse('Correlations', 'generateOccurrences', false, $this->response->type(), $message); } + $this->Flash->info($message); + $this->redirect(['controller' => 'correlations', 'action' => 'overCorrelations']); } } diff --git a/app/View/Correlations/over_correlations.ctp b/app/View/Correlations/over_correlations.ctp index 874a1ca35..8b68383f1 100644 --- a/app/View/Correlations/over_correlations.ctp +++ b/app/View/Correlations/over_correlations.ctp @@ -1,89 +1,92 @@ ', empty($ajax) ? ' class="index"' : ''); - echo $this->element('genericElements/IndexTable/index_table', [ - 'data' => [ - 'light_paginator' => 1, - 'data' => $data, - 'top_bar' => [ - 'children' => [ - [ - 'children' => [ - [ - 'type' => 'simple', - 'url' => $baseurl . '/correlations/overCorrelations', - 'text' => __('All') - ], - [ - 'type' => 'simple', - 'url' => $baseurl . '/correlations/overCorrelations/scope:over_correlating', - 'text' => __('Over-correlating') - ], - [ - 'type' => 'simple', - 'url' => $baseurl . '/correlations/overCorrelations/scope:not_over_correlating', - 'text' => __('Not over-correlating') - ], - [ - 'type' => 'simple', - 'url' => $baseurl . '/correlations/generateOccurrences', - 'text' => __('Regenerate occurrence counts') - ] +echo sprintf('', empty($ajax) ? ' class="index"' : ''); +echo $this->element('genericElements/IndexTable/index_table', [ + 'data' => [ + 'light_paginator' => 1, + 'data' => $data, + 'top_bar' => [ + 'children' => [ + [ + 'children' => [ + [ + 'type' => 'simple', + 'url' => $baseurl . '/correlations/overCorrelations', + 'text' => __('All'), + 'active' => $scope === 'all', + ], + [ + 'type' => 'simple', + 'url' => $baseurl . '/correlations/overCorrelations/scope:over_correlating', + 'text' => __('Over-correlating'), + 'active' => $scope === 'over_correlating', + ], + [ + 'type' => 'simple', + 'url' => $baseurl . '/correlations/overCorrelations/scope:not_over_correlating', + 'text' => __('Not over-correlating'), + 'active' => $scope === 'not_over_correlating', + ], + [ + 'type' => 'simple', + 'url' => $baseurl . '/correlations/generateOccurrences', + 'text' => __('Regenerate occurrence counts') ] ] ] - ], - 'fields' => [ - [ - 'name' => 'Value', - 'element' => 'postlink', - 'data_path' => 'OverCorrelatingValue.value', - 'url' => '/attributes/search/results', - 'payload_paths' => [ - 'value' => 'OverCorrelatingValue.value' - ] - ], - [ - 'name' => 'Occurrences', - 'data_path' => 'OverCorrelatingValue.occurrence', - 'class' => 'shortish' - ], - [ - 'name' => 'Blocked by Threshold', - 'data_path' => 'OverCorrelatingValue.over_correlation', - 'class' => 'shortish', - 'element' => 'boolean' - ], - [ - 'name' => 'Excluded by Exclusion List', - 'data_path' => 'OverCorrelatingValue.excluded', - 'class' => 'shortish', - 'element' => 'boolean' + ] + ], + 'fields' => [ + [ + 'name' => 'Value', + 'element' => 'postlink', + 'data_path' => 'OverCorrelatingValue.value', + 'url' => '/attributes/search/results', + 'payload_paths' => [ + 'value' => 'OverCorrelatingValue.value' ] ], - 'title' => empty($ajax) ? $title_for_layout : false, - 'description' => empty($ajax) ? __('The values with the most correlation entries.') : false, - 'pull' => 'right', - 'actions' => [ - [ - 'onclick' => sprintf( - 'openGenericModal(\'%s/correlation_exclusions/add/redirect_controller:correlations/redirect:top/value:[onclick_params_data_path]\');', - $baseurl - ), - 'onclick_params_data_path' => 'OverCorrelatingValue.value', - 'icon' => 'trash', - 'title' => __('Add exclusion entry for value'), - 'complex_requirement' => [ - 'function' => function (array $row) { - return !$row['OverCorrelatingValue']['excluded']; - } - ] + [ + 'name' => 'Occurrences', + 'data_path' => 'OverCorrelatingValue.occurrence', + 'class' => 'shortish' + ], + [ + 'name' => 'Blocked by Threshold', + 'data_path' => 'OverCorrelatingValue.over_correlation', + 'class' => 'shortish', + 'element' => 'boolean' + ], + [ + 'name' => 'Excluded by Exclusion List', + 'data_path' => 'OverCorrelatingValue.excluded', + 'class' => 'shortish', + 'element' => 'boolean' + ] + ], + 'title' => empty($ajax) ? $title_for_layout : false, + 'description' => empty($ajax) ? __('The values with the most correlation entries.') : false, + 'pull' => 'right', + 'actions' => [ + [ + 'onclick' => sprintf( + 'openGenericModal(\'%s/correlation_exclusions/add/redirect_controller:correlations/redirect:top/value:[onclick_params_data_path]\');', + $baseurl + ), + 'onclick_params_data_path' => 'OverCorrelatingValue.value', + 'icon' => 'trash', + 'title' => __('Add exclusion entry for value'), + 'complex_requirement' => [ + 'function' => function (array $row) { + return !$row['OverCorrelatingValue']['excluded']; + } ] ] ] - ]); - echo ''; - if (empty($ajax)) { - echo $this->element('/genericElements/SideMenu/side_menu', $menuData); - } + ] +]); +echo ''; +if (empty($ajax)) { + echo $this->element('/genericElements/SideMenu/side_menu', $menuData); +} From 1519afb48840c5b470577aee2ae519c30f29606e Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 11 Sep 2022 10:24:29 +0200 Subject: [PATCH 3/7] fix: [UI] Correlation for attributes --- .../Events/View/attribute_correlations.ctp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/View/Elements/Events/View/attribute_correlations.ctp b/app/View/Elements/Events/View/attribute_correlations.ctp index 5cb188daf..0927f78e1 100644 --- a/app/View/Elements/Events/View/attribute_correlations.ctp +++ b/app/View/Elements/Events/View/attribute_correlations.ctp @@ -1,7 +1,7 @@ 5) { - $expandButton = __('Show %s more...', $count - 4); + $expandButton = __('Show %s more…', $count - 4); echo sprintf( - '
  • %s
    • ', + '
  • %s
    • ', $linkColour, $expandButton ); } $relatedData = array( - 'Orgc' => !empty($orgTable[$relatedAttribute['org_id']]) ? $orgTable[$relatedAttribute['org_id']] : 'N/A', - 'Date' => isset($relatedAttribute['date']) ? $relatedAttribute['date'] : 'N/A', + 'Orgc' => $orgTable[$relatedAttribute['org_id']] ?? 'N/A', + 'Date' => $relatedAttribute['date'] ?? 'N/A', 'Event' => $relatedAttribute['info'], 'Correlating Value' => $relatedAttribute['value'] ); @@ -59,13 +59,13 @@ } if (!empty($object['correlation_exclusion'])) { echo sprintf( - '%s ', + '%s ', __('The attribute value matches a correlation exclusion rule defined by a site-administrator for this instance. Click the magnifying glass to search for all occurrences of the value.'), __('Excluded.') ); } else if (!empty($object['over_correlation'])) { echo sprintf( - '%s ', + '%s ', __('The instance threshold for the maximum number of correlations for the given attribute value has been exceeded. Click the magnifying glass to search for all occurrences of the value.'), __('Too many correlations.') ); From a3b02cf0370f31d338fe2b2b786066c721891857 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 11 Sep 2022 10:25:09 +0200 Subject: [PATCH 4/7] fix: [internal] Code style --- .../Behavior/NoAclCorrelationBehavior.php | 57 ++++++++------- app/Model/Correlation.php | 69 ++++++++++--------- 2 files changed, 68 insertions(+), 58 deletions(-) diff --git a/app/Model/Behavior/NoAclCorrelationBehavior.php b/app/Model/Behavior/NoAclCorrelationBehavior.php index efdc7fb50..e5db84f75 100644 --- a/app/Model/Behavior/NoAclCorrelationBehavior.php +++ b/app/Model/Behavior/NoAclCorrelationBehavior.php @@ -146,12 +146,18 @@ class NoAclCorrelationBehavior extends ModelBehavior return $this->__config['AttributeFetcher']['fields']; } - private function __collectCorrelations($user, $id, $primary) + /** + * @param array $user + * @param int $eventId + * @param bool $primary + * @return array + */ + private function __collectCorrelations(array $user, $eventId, $primary) { $max_correlations = Configure::read('MISP.max_correlations_per_event') ?: 5000; $source = $primary ? '' : '1_'; $prefix = $primary ? '1_' : ''; - $correlations = $this->Correlation->find('all', [ + return $this->Correlation->find('all', [ 'fields' => [ $source . 'attribute_id', $prefix . 'attribute_id', @@ -159,7 +165,7 @@ class NoAclCorrelationBehavior extends ModelBehavior ], 'conditions' => [ 'OR' => [ - $source . 'event_id' => $id + $source . 'event_id' => $eventId ], 'AND' => [ [ @@ -182,13 +188,18 @@ class NoAclCorrelationBehavior extends ModelBehavior 'order' => false, 'limit' => $max_correlations ]); - return $correlations; } - public function runGetAttributesRelatedToEvent(Model $Model, $user, $id) + /** + * @param Model $Model + * @param array $user + * @param int $id Event ID + * @return array + */ + public function runGetAttributesRelatedToEvent(Model $Model, array $user, $id) { $correlations = []; - $event_ids = []; + $eventIds = []; $temp_correlations = $this->__collectCorrelations($user, $id, false); foreach ($temp_correlations as $temp_correlation) { @@ -196,9 +207,9 @@ class NoAclCorrelationBehavior extends ModelBehavior 'id' => $temp_correlation['Correlation']['event_id'], 'attribute_id' => $temp_correlation['Correlation']['attribute_id'], 'parent_id' => $temp_correlation['Correlation']['1_attribute_id'], - 'value' => $temp_correlation['CorrelationValue']['value'] + 'value' => $temp_correlation['CorrelationValue']['value'], ]; - $event_ids[$temp_correlation['Correlation']['event_id']] = true; + $eventIds[$temp_correlation['Correlation']['event_id']] = true; } $temp_correlations = $this->__collectCorrelations($user, $id, true); @@ -207,15 +218,15 @@ class NoAclCorrelationBehavior extends ModelBehavior 'id' => $temp_correlation['Correlation']['1_event_id'], 'attribute_id' => $temp_correlation['Correlation']['1_attribute_id'], 'parent_id' => $temp_correlation['Correlation']['attribute_id'], - 'value' => $temp_correlation['CorrelationValue']['value'] + 'value' => $temp_correlation['CorrelationValue']['value'], ]; - $event_ids[$temp_correlation['Correlation']['1_event_id']] = true; + $eventIds[$temp_correlation['Correlation']['1_event_id']] = true; } if (empty($correlations)) { return []; } $conditions = [ - 'Event.id' => array_keys($event_ids) + 'Event.id' => array_keys($eventIds) ]; $events = $Model->Event->find('all', array( 'recursive' => -1, @@ -231,9 +242,9 @@ class NoAclCorrelationBehavior extends ModelBehavior continue; } $event = $events[$eventId]; - $correlation['org_id'] = $events[$eventId]['orgc_id']; - $correlation['info'] = $events[$eventId]['info']; - $correlation['date'] = $events[$eventId]['date']; + $correlation['org_id'] = $event['orgc_id']; + $correlation['info'] = $event['info']; + $correlation['date'] = $event['date']; $parentId = $correlation['parent_id']; unset($correlation['parent_id']); $relatedAttributes[$parentId][] = $correlation; @@ -330,19 +341,15 @@ class NoAclCorrelationBehavior extends ModelBehavior } } + /** + * @param Correlation $Model + * @param array $user Not used + * @param int $eventId + * @param array $sgids Not used + * @return array + */ public function fetchRelatedEventIds(Model $Model, array $user, int $eventId, array $sgids) { - // search the correlation table for the event ids of the related events - // Rules: - // 1. Event is owned by the user (org_id matches) - // 2. User is allowed to see both the event and the org: - // a. Event: - // i. Event has a distribution between 1-3 (community only, connected communities, all orgs) - // ii. Event has a sharing group that the user is accessible to view - // b. Attribute: - // i. Attribute has a distribution of 5 (inheritance of the event, for this the event check has to pass anyway) - // ii. Attribute has a distribution between 1-3 (community only, connected communities, all orgs) - // iii. Attribute has a sharing group that the user is accessible to view $primaryEventIds = $this->__filterRelatedEvents($Model, $eventId, true); $secondaryEventIds = $this->__filterRelatedEvents($Model, $eventId, false); return array_unique(array_merge($primaryEventIds, $secondaryEventIds), SORT_REGULAR); diff --git a/app/Model/Correlation.php b/app/Model/Correlation.php index fcfe945b7..96e2deca4 100644 --- a/app/Model/Correlation.php +++ b/app/Model/Correlation.php @@ -17,7 +17,8 @@ class Correlation extends AppModel const CACHE_NAME = 'misp:top_correlations', CACHE_AGE = 'misp:top_correlations_age'; - private $__compositeTypes = []; + /** @var array */ + private $__compositeTypes; public $belongsTo = array( 'Attribute' => [ @@ -828,7 +829,7 @@ class Correlation extends AppModel */ public function getRelatedAttributes($user, $sgids, $attribute, $fields=[], $includeEventData = false) { - if (in_array($attribute['type'], Attribute::NON_CORRELATING_TYPES)) { + if (in_array($attribute['type'], Attribute::NON_CORRELATING_TYPES, true)) { return []; } return $this->runGetRelatedAttributes($user, $sgids, $attribute, $fields, $includeEventData); @@ -836,7 +837,7 @@ class Correlation extends AppModel /** * @param array $user User array - * @param int $eventId List of event IDs + * @param int $eventId Event ID * @param array $sgids List of sharing group IDs * @return array */ @@ -908,40 +909,42 @@ class Correlation extends AppModel public function collectMetrics() { - $results['engine'] = $this->getCorrelationModelName(); - $results['db'] = [ - 'Default' => [ - 'name' => __('Default correlation engine'), - 'tables' => [ - 'default_correlations' => [ - 'id_limit' => 4294967295 - ], - 'correlation_values' => [ - 'id_limit' => 4294967295 + $results = [ + 'engine' => $this->getCorrelationModelName(), + 'db' => [ + 'Default' => [ + 'name' => __('Default correlation engine'), + 'tables' => [ + 'default_correlations' => [ + 'id_limit' => 4294967295 + ], + 'correlation_values' => [ + 'id_limit' => 4294967295 + ] + ] + ], + 'NoAcl' => [ + 'name' => __('No ACL correlation engine'), + 'tables' => [ + 'no_acl_correlations' => [ + 'id_limit' => 4294967295 + ], + 'correlation_values' => [ + 'id_limit' => 4294967295 + ] + ] + ], + 'Legacy' => [ + 'name' => __('Legacy correlation engine (< 2.4.160)'), + 'tables' => [ + 'correlations' => [ + 'id_limit' => 2147483647 + ] ] ] ], - 'NoAcl' => [ - 'name' => __('No ACL correlation engine'), - 'tables' => [ - 'no_acl_correlations' => [ - 'id_limit' => 4294967295 - ], - 'correlation_values' => [ - 'id_limit' => 4294967295 - ] - ] - ], - 'Legacy' => [ - 'name' => __('Legacy correlation engine (< 2.4.160)'), - 'tables' => [ - 'correlations' => [ - 'id_limit' => 2147483647 - ] - ] - ] + 'over_correlations' => $this->OverCorrelatingValue->find('count'), ]; - $results['over_correlations'] = $this->OverCorrelatingValue->find('count'); $this->CorrelationExclusion = ClassRegistry::init('CorrelationExclusion'); $results['excluded_correlations'] = $this->CorrelationExclusion->find('count'); foreach ($results['db'] as &$result) { From 21335d7d1f88dfdbf03f39e1d15c56c401881bc6 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 11 Sep 2022 10:38:09 +0200 Subject: [PATCH 5/7] fix: [internal] Optimise fetching related attributes --- .../Behavior/DefaultCorrelationBehavior.php | 15 ++--- .../Behavior/NoAclCorrelationBehavior.php | 63 ++++++++----------- 2 files changed, 33 insertions(+), 45 deletions(-) diff --git a/app/Model/Behavior/DefaultCorrelationBehavior.php b/app/Model/Behavior/DefaultCorrelationBehavior.php index a5f527e2c..66576b08f 100644 --- a/app/Model/Behavior/DefaultCorrelationBehavior.php +++ b/app/Model/Behavior/DefaultCorrelationBehavior.php @@ -337,7 +337,6 @@ class DefaultCorrelationBehavior extends ModelBehavior [ '1_attribute_id', '1_object_id', - '1_event_id', '1_distribution', '1_object_distribution', '1_event_distribution', @@ -345,12 +344,10 @@ class DefaultCorrelationBehavior extends ModelBehavior '1_object_sharing_group_id', '1_event_sharing_group_id', '1_org_id', - 'value_id' ], [ 'attribute_id', 'object_id', - 'event_id', 'distribution', 'object_distribution', 'event_distribution', @@ -358,11 +355,10 @@ class DefaultCorrelationBehavior extends ModelBehavior 'object_sharing_group_id', 'event_sharing_group_id', 'org_id', - 'value_id' ] ]; $prefixes = ['1_', '']; - $correlated_attribute_ids = []; + $correlatedAttributeIds = []; foreach ($conditions as $k => $condition) { $temp_correlations = $Model->find('all', [ 'recursive' => -1, @@ -376,10 +372,15 @@ class DefaultCorrelationBehavior extends ModelBehavior continue; } } - $correlated_attribute_ids[] = $temp_correlation['Correlation'][$prefixes[$k] . 'attribute_id']; + $correlatedAttributeIds[] = $temp_correlation['Correlation'][$prefixes[$k] . 'attribute_id']; } } } + + if (empty($correlatedAttributeIds)) { + return []; + } + $contain = []; if (!empty($includeEventData)) { $contain['Event'] = [ @@ -402,7 +403,7 @@ class DefaultCorrelationBehavior extends ModelBehavior $relatedAttributes = $Model->Attribute->find('all', [ 'recursive' => -1, 'conditions' => [ - 'Attribute.id' => $correlated_attribute_ids + 'Attribute.id' => $correlatedAttributeIds ], 'fields' => $fields, 'contain' => $contain diff --git a/app/Model/Behavior/NoAclCorrelationBehavior.php b/app/Model/Behavior/NoAclCorrelationBehavior.php index e5db84f75..67408779f 100644 --- a/app/Model/Behavior/NoAclCorrelationBehavior.php +++ b/app/Model/Behavior/NoAclCorrelationBehavior.php @@ -254,54 +254,38 @@ class NoAclCorrelationBehavior extends ModelBehavior /** * @param Correlation $Model - * @param $user - * @param $sgids + * @param array $user Not used + * @param array $sgids Not used * @param array $attribute - * @param array $fields + * @param array $fields Attribute fields to fetch * @param bool $includeEventData * @return array */ public function runGetRelatedAttributes(Model $Model, $user, $sgids, $attribute, $fields = [], $includeEventData = false) { - // LATER getRelatedAttributes($attribute) this might become a performance bottleneck - // prepare the conditions - $conditions = [ - [ + $correlatedAttributeIds = $Model->find('column', [ + 'conditions' => [ 'Correlation.1_event_id !=' => $attribute['event_id'], - 'Correlation.attribute_id' => $attribute['id'] + 'Correlation.attribute_id' => $attribute['id'], ], - [ + 'fields' => ['1_attribute_id'], + ]); + + $correlatedAttributeIds2 = $Model->find('column', [ + 'conditions' => [ 'Correlation.event_id !=' => $attribute['event_id'], - 'Correlation.1_attribute_id' => $attribute['id'] - ] - ]; - $corr_fields = [ - [ - '1_attribute_id', - '1_event_id', - 'value_id' + 'Correlation.1_attribute_id' => $attribute['id'], ], - [ - 'attribute_id', - 'event_id', - 'value_id' - ] - ]; - $prefixes = ['1_', '']; - $correlated_attribute_ids = []; - foreach ($conditions as $k => $condition) { - $temp_correlations = $Model->find('all', [ - 'recursive' => -1, - 'conditions' => $condition, - 'fields' => $corr_fields[$k] - ]); - if (!empty($temp_correlations)) { - foreach ($temp_correlations as $temp_correlation) { - $correlated_attribute_ids[] = $temp_correlation['Correlation'][$prefixes[$k] . 'attribute_id']; - } - } + 'fields' => ['attribute_id'], + ]); + foreach ($correlatedAttributeIds2 as $tempCorrelation) { + $correlatedAttributeIds[] = $tempCorrelation; } - $contain = []; + + if (empty($correlatedAttributeIds)) { + return []; + } + if (!empty($includeEventData)) { $contain['Event'] = [ 'fields' => [ @@ -319,11 +303,14 @@ class NoAclCorrelationBehavior extends ModelBehavior 'Event.org_id' ] ]; + } else { + $contain = []; } + $relatedAttributes = $Model->Attribute->find('all', [ 'recursive' => -1, 'conditions' => [ - 'Attribute.id' => $correlated_attribute_ids + 'Attribute.id' => $correlatedAttributeIds ], 'fields' => $fields, 'contain' => $contain From f8f2e0e43d1e6b04f01545c8a2057b66237606de Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 11 Sep 2022 11:20:06 +0200 Subject: [PATCH 6/7] fix: [correlations] Do not fetch unnecessary data --- app/Controller/AttributesController.php | 3 --- .../Behavior/DefaultCorrelationBehavior.php | 5 ++--- .../Behavior/NoAclCorrelationBehavior.php | 3 +-- app/Model/Event.php | 20 ++++++------------- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/app/Controller/AttributesController.php b/app/Controller/AttributesController.php index 108c06c06..af495018e 100644 --- a/app/Controller/AttributesController.php +++ b/app/Controller/AttributesController.php @@ -1704,9 +1704,6 @@ class AttributesController extends AppController $clusters = []; } - // Fetch correlations in one query - $correlations = $this->Attribute->Event->getRelatedAttributes($user, $attributeIds, false, 'attribute'); - // `attachFeedCorrelations` method expects different attribute format, so we need to transform that, then process // and then take information back to original attribute structure. $fakeEventArray = []; diff --git a/app/Model/Behavior/DefaultCorrelationBehavior.php b/app/Model/Behavior/DefaultCorrelationBehavior.php index 66576b08f..765323f14 100644 --- a/app/Model/Behavior/DefaultCorrelationBehavior.php +++ b/app/Model/Behavior/DefaultCorrelationBehavior.php @@ -204,7 +204,7 @@ class DefaultCorrelationBehavior extends ModelBehavior /** * Fetch correlations for given event. * @param array $user - * @param int $eventId + * @param int|array $eventId * @param array $sgids * @param bool $primary * @return array @@ -245,7 +245,6 @@ class DefaultCorrelationBehavior extends ModelBehavior 'contain' => [ 'CorrelationValue' => [ 'fields' => [ - 'CorrelationValue.id', 'CorrelationValue.value' ] ] @@ -264,7 +263,7 @@ class DefaultCorrelationBehavior extends ModelBehavior /** * @param Correlation $Model * @param array $user - * @param int $id Event ID + * @param int|array $id Event ID * @param array $sgids * @return array */ diff --git a/app/Model/Behavior/NoAclCorrelationBehavior.php b/app/Model/Behavior/NoAclCorrelationBehavior.php index 67408779f..c66a1c265 100644 --- a/app/Model/Behavior/NoAclCorrelationBehavior.php +++ b/app/Model/Behavior/NoAclCorrelationBehavior.php @@ -180,7 +180,6 @@ class NoAclCorrelationBehavior extends ModelBehavior 'contain' => [ 'CorrelationValue' => [ 'fields' => [ - 'CorrelationValue.id', 'CorrelationValue.value' ] ] @@ -193,7 +192,7 @@ class NoAclCorrelationBehavior extends ModelBehavior /** * @param Model $Model * @param array $user - * @param int $id Event ID + * @param int|array $id Event ID * @return array */ public function runGetAttributesRelatedToEvent(Model $Model, array $user, $id) diff --git a/app/Model/Event.php b/app/Model/Event.php index b486ea5d6..2e970cbbe 100755 --- a/app/Model/Event.php +++ b/app/Model/Event.php @@ -689,26 +689,18 @@ class Event extends AppModel } /** + * Get related attributes for event * @param array $user - * @param int|array $id Event ID when $scope is 'event', Attribute ID when $scope is 'attribute' - * @param bool $shadowAttribute - * @param string $scope 'event' or 'attribute' + * @param int|array $eventIds Event IDs * @return array */ - public function getRelatedAttributes(array $user, $id, $shadowAttribute = false, $scope = 'event') + public function getRelatedAttributes(array $user, $eventIds) { - if ($shadowAttribute) { - // no longer supported - return []; - } else { - $parentIdField = '1_attribute_id'; - $correlationModelName = 'Correlation'; - } - if (!isset($this->{$correlationModelName})) { - $this->{$correlationModelName} = ClassRegistry::init($correlationModelName); + if (!isset($this->Correlation)) { + $this->Correlation = ClassRegistry::init('Correlation'); } $sgids = $this->SharingGroup->authorizedIds($user); - $relatedAttributes = $this->{$correlationModelName}->getAttributesRelatedToEvent($user, $id, $sgids); + $relatedAttributes = $this->Correlation->getAttributesRelatedToEvent($user, $eventIds, $sgids); return $relatedAttributes; } From 264263c90fb69e0e2f0a40b0ea9753a5ca66a392 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 11 Sep 2022 18:58:24 +0200 Subject: [PATCH 7/7] chg: [internal] Convert to const --- app/Model/Behavior/NoAclCorrelationBehavior.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/Model/Behavior/NoAclCorrelationBehavior.php b/app/Model/Behavior/NoAclCorrelationBehavior.php index c66a1c265..b3855364c 100644 --- a/app/Model/Behavior/NoAclCorrelationBehavior.php +++ b/app/Model/Behavior/NoAclCorrelationBehavior.php @@ -8,7 +8,7 @@ class NoAclCorrelationBehavior extends ModelBehavior { const TABLE_NAME = 'no_acl_correlations'; - private $__config = [ + const CONFIG = [ 'AttributeFetcher' => [ 'fields' => [ 'Attribute.event_id', @@ -132,18 +132,23 @@ class NoAclCorrelationBehavior extends ModelBehavior } } + /** + * @param Model $Model + * @param string|null $filter + * @return false|mixed + */ public function getContainRules(Model $Model, $filter = null) { if (empty($filter)) { - return $this->__config['AttributeFetcher']['contain']; + return self::CONFIG['AttributeFetcher']['contain']; } else { - return empty($this->__config['AttributeFetcher']['contain'][$filter]) ? false : $this->__config['AttributeFetcher']['contain'][$filter]; + return self::CONFIG['AttributeFetcher']['contain'][$filter] ?? false; } } public function getFieldRules(Model $Model) { - return $this->__config['AttributeFetcher']['fields']; + return self::CONFIG['AttributeFetcher']['fields']; } /**