Fix to a serious bug with adding attributes via the API and performance fixes

- due to a bug, setting an attribute ID in the /attributes/add API call can lead to overwriting an existing attribute

performance improvements:

- massive improvements to the correlation performance
- improvements to the attribute validation process
pull/652/head
Iglocska 2015-09-07 10:06:34 +02:00
parent 5f6c840211
commit e7b54c2c61
4 changed files with 65 additions and 67 deletions

View File

@ -1 +1 @@
{"major":2, "minor":3, "hotfix":123}
{"major":2, "minor":3, "hotfix":124}

View File

@ -124,7 +124,7 @@ class AttributesController extends AppController {
$this->Event->set('timestamp', $date->getTimestamp());
$this->Event->set('published', 0);
$this->Event->save($this->Event->data, array('fieldList' => array('published', 'timestamp', 'info')));
if (isset($this->request->data['Attribute']['id'])) unset($this->request->data['Attribute']['id']);
//
// multiple attributes in batch import
//

View File

@ -408,7 +408,9 @@ class Attribute extends AppModel {
}
}
// update correlation... (only needed here if there's an update)
$this->__beforeSaveCorrelation($this->data['Attribute']);
if ($this->id || !empty($this->data['Attribute']['id'])) {
$this->__beforeSaveCorrelation($this->data['Attribute']);
}
// always return true after a beforeSave()
return true;
}
@ -978,65 +980,60 @@ class Attribute extends AppModel {
public function __afterSaveCorrelation($a) {
// Don't do any correlation if the type is a non correlating type
if (!in_array($a['type'], $this->nonCorrelatingTypes)) {
$event = $this->Event->find('first', array(
'recursive' => -1,
'fields' => array('Event.distribution', 'Event.id', 'Event.info', 'Event.org', 'Event.date'),
'conditions' => array('id' => $a['event_id'])
));
$this->Correlation = ClassRegistry::init('Correlation');
// When we add/update an attribute we need to
// - (beforeSave) (update-only) clean up the relation of the old value: remove the existing relations related to that attribute, we DO have a reference, the id
// - remove the existing relations for that value1 or value2, we do NOT have an id reference, but we have a value1/value2 field to search for
// ==> DELETE FROM correlations WHERE value = $value1 OR value = $value2 */
$dummy = $this->Correlation->deleteAll(array('Correlation.value' => array($a['value1'], $a['value2'])));
// now build a correlation array of things that will need to be added in the db
// we do this twice, once for value1 and once for value2
$correlations = array(); // init variable
$value_names = array ('value1', 'value2');
// do the correlation for value1 and value2, this needs to be done separately
foreach ($value_names as $value_name) {
if (empty($a[$value_name])) continue; // do not correlate if attribute is empty
$params = array(
'conditions' => array(
'OR' => array(
'Attribute.value1' => $a[$value_name],
'Attribute.value2' => $a[$value_name]
),
'AND' => array(
'Attribute.type !=' => $this->nonCorrelatingTypes,
)),
'recursive' => -1,
'fields' => array('Attribute.event_id', 'Attribute.id', 'Attribute.distribution'),
'contain' => array('Event' => array('fields' => array('Event.id', 'Event.date', 'Event.info', 'Event.org', 'Event.distribution'))),
//'fields' => '', // we want to have the Attribute AND Event, so do not filter here
);
// search for the related attributes for that "value(1|2)"
$attributes = $this->find('all', $params);
// build the correlations, each attribute should have a relation in both directions
// this is why we have a double loop.
// The result is that for each Attribute pair we want: A1-A2, A2-A1 and so on,
// In total that's N * (N-1) rows (minus the ones from the same event) (with N the number of related attributes)
$attributes_right = $attributes;
foreach ($attributes as $attribute) {
foreach ($attributes_right as $attribute_right) {
if ($attribute['Attribute']['event_id'] == $attribute_right['Attribute']['event_id']) {
// do not build a relation between the same attributes
// or attributes from the same event
continue;
}
$is_private = ($attribute_right['Event']['distribution'] == 0) || ($attribute_right['Attribute']['distribution'] == 0);
$correlations[] = array(
'value' => $a[$value_name],
'1_event_id' => $attribute['Attribute']['event_id'],
'1_attribute_id' => $attribute['Attribute']['id'],
'event_id' => $attribute_right['Attribute']['event_id'],
'attribute_id' => $attribute_right['Attribute']['id'],
'org' => $attribute_right['Event']['org'],
'private' => $is_private,
'date' => $attribute_right['Event']['date'],
'info' => $attribute_right['Event']['info'],
);
}
}
$correlations = array();
$fields = array('value1', 'value2');
$correlatingValues = array($a['value1']);
if (!empty($a['value2'])) $correlatingValues[] = $a['value2'];
foreach ($correlatingValues as $k => $cV) {
$correlatingAttributes[$k] = $this->find('all', array(
'conditions' => array(
'OR' => array(
'Attribute.value1' => $cV,
'Attribute.value2' => $cV
),
'AND' => array(
'Attribute.type !=' => $this->nonCorrelatingTypes,
'Attribute.id !=' => $a['id'],
),
),
'recursive => -1',
'fields' => array('Attribute.event_id', 'Attribute.id', 'Attribute.distribution'),
'contain' => array('Event' => array('fields' => array('Event.id', 'Event.date', 'Event.info', 'Event.org', 'Event.distribution'))),
));
}
$correlations = array();
foreach ($correlatingAttributes as $k => $cA) {
foreach ($cA as $corr) {
$correlations[] = array(
'value' => $correlatingValues[$k],
'1_event_id' => $event['Event']['id'],
'1_attribute_id' => $a['id'],
'event_id' => $corr['Attribute']['event_id'],
'attribute_id' => $corr['Attribute']['id'],
'org' => $corr['Event']['org'],
'private' => ($corr['Attribute']['distribution'] == 0 || $corr['Event']['distribution'] == 0) ? true : false,
'date' => $corr['Event']['date'],
'info' => $corr['Event']['info'],
);
$correlations[] = array(
'value' => $correlatingValues[$k],
'1_event_id' => $corr['Event']['id'],
'1_attribute_id' => $corr['Attribute']['id'],
'event_id' => $a['event_id'],
'attribute_id' => $a['id'],
'org' => $event['Event']['org'],
'private' => ($a['distribution'] == 0 || $event['Event']['distribution'] == 0) ? true : false,
'date' => $event['Event']['date'],
'info' => $event['Event']['info'],
);
}
}
// save the new correlations to the database in a single shot
$this->Correlation->saveMany($correlations);
}
}
@ -1415,12 +1412,9 @@ class Attribute extends AppModel {
public function reportValidationIssuesAttributes($eventId) {
$conditions = array();
if ($eventId && is_numeric($eventId)) $conditions = array('event_id' => $eventId);
// for efficiency reasons remove the unique requirement
$this->validator()->remove('value', 'unique');
// get all attributes..
$attributes = $this->find('all', array('recursive' => -1, 'fields' => array('id'), 'conditions' => $conditions));
// for all attributes..
$result = array();
$i = 0;

View File

@ -7,6 +7,13 @@ App::uses('Regexp', 'Model');
*
*/
class RegexpBehavior extends ModelBehavior {
private $__allRegexp = array();
public function setup(Model $model, $config = null) {
$regexp = new Regexp();
$this->__allRegexp = $regexp->find('all');
}
/**
* replace the current value according to the regexp rules, or block blacklisted regular expressions
@ -15,10 +22,7 @@ class RegexpBehavior extends ModelBehavior {
* @param unknown_type $array
*/
public function runRegexp(Model $Model, $type, $value) {
$regexp = new Regexp();
$allRegexp = $regexp->find('all');
foreach ($allRegexp as $regexp) {
foreach ($this->__allRegexp as $regexp) {
if (!empty($regexp['Regexp']['replacement']) && !empty($regexp['Regexp']['regexp']) && ($regexp['Regexp']['type'] === 'ALL' || $regexp['Regexp']['type'] === $type)) {
$value = preg_replace($regexp['Regexp']['regexp'], $regexp['Regexp']['replacement'], $value);
}