composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-02-13 07:24:13 +01:00
Code Issues Packages Projects Releases Wiki Activity

N°2555 action_rule performance

Browse Source 
* do not execute scope_oql that have no conditions
* pick the first result only
This commit is contained in:
Pierre Goiffon
2019-10-24 10:54:30 +02:00
parent 9aeba1df9b
commit 5c483efd15
4 changed files with 67 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
core/coreexception.class.inc.php
View File

@@ -188,7 +188,7 @@ class CoreOqlException extends CoreException
}
class CoreOqlMultipleResultsFoundException extends CoreOqlException
class CoreOqlMultipleResultsForbiddenException extends CoreOqlException
{
}

11
core/dbsearch.class.php
View File

@@ -1077,16 +1077,16 @@ abstract class DBSearch
* @param array $aSearchParams
*
* @return null|\DBObject query result
* @throws \CoreOqlMultipleResultsFoundException if multiple results found and parameter enforce the check
* @throws \CoreOqlMultipleResultsForbiddenException if multiple results found and parameter enforce the check
* @throws \CoreException
* @throws \CoreUnexpectedValue
* @throws \MySQLException
*/
public function GetFirstResult($bMustHaveOneResultMax = true, $aOrderBy = array(), $aSearchParams = array())
{
$oSet = new DBObjectSet($this, array(), $aSearchParams);
$oSet = new DBObjectSet($this, array(), $aSearchParams, null, 2);
$oFirstResult = $oSet->Fetch();
if ($oFirstResult === null)
if ($oFirstResult === null) // useless but here for readability ;)
{
return null;
}
@@ -1094,9 +1094,10 @@ abstract class DBSearch
if ($bMustHaveOneResultMax)
{
$oSecondResult = $oSet->Fetch();
if ($oSecondResult != null)
if ($oSecondResult !== null)
{
throw new CoreOqlMultipleResultsFoundException('TODO');
throw new CoreOqlMultipleResultsForbiddenException(
'Search returned multiple results, this is forbidden. Query was: '.$this->ToOQL());
}
}

67
datamodels/2.x/itop-portal-base/portal/src/Helper/ContextManipulatorHelper.php
View File

@@ -22,19 +22,22 @@
namespace Combodo\iTop\Portal\Helper;
use ModuleDesign;
use BinaryExpression;
use Combodo\iTop\Portal\Brick\BrickCollection;
use Exception;
use DOMNodeList;
use DOMFormatException;
use Symfony\Component\Routing\RouterInterface;
use UserRights;
use CoreOqlMultipleResultsForbiddenException;
use CoreUnexpectedValue;
use DBObject;
use DBSearch;
use DBObjectSet;
use BinaryExpression;
use DOMFormatException;
use DOMNodeList;
use Exception;
use FieldExpression;
use IssueLog;
use ModuleDesign;
use ScalarExpression;
use Symfony\Component\Routing\RouterInterface;
use TrueExpression;
use UserRights;
/**
* Class ContextManipulatorHelper
@@ -342,6 +345,15 @@ class ContextManipulatorHelper
}
}
$oSearchRootCondition = $oSearch->GetCriteria();
if (($oSearchRootCondition === null) || ($oSearchRootCondition instanceof TrueExpression))
{
// N°2555 : disallow searches without any condition
$sErrMsg = "Portal query was stopped: action_rule '$sId' searches for '$sSearchClass' without any condition is forbidden";
IssueLog::Error($sErrMsg);
throw new CoreUnexpectedValue($sErrMsg);
}
// Checking for silos
$oScopeSearch = $this->oScopeValidator->GetScopeFilterForProfiles(UserRights::ListProfiles(), $sSearchClass,
UR_ACTION_READ);
@@ -351,27 +363,34 @@ class ContextManipulatorHelper
}
// Retrieving source object(s) and applying rules
$oSet = new DBObjectSet($oSearch, array(), $aSearchParams);
while ($oSourceObject = $oSet->Fetch())
try
{
// Changing behaviour to remove usage of ObjectCopier as its now being integrated in the core
// Old code : iTopObjectCopier::PrepareObject($aRule, $oObject, $oSourceObject);
// Presets
if (isset($aRule['preset']) && !empty($aRule['preset']))
{
$oObject->ExecActions($aRule['preset'], array('source' => $oSourceObject));
}
// Retrofits
if (isset($aRule['retrofit']) && !empty($aRule['retrofit']))
{
$oSourceObject->ExecActions($aRule['retrofit'], array('source' => $oObject));
}
$oSourceObject = $oSearch->GetFirstResult(true, array(), $aSearchParams);
}
catch (CoreOqlMultipleResultsForbiddenException $e)
{
// N°2555 : disallow searches returning more than 1 result
$sErrMsg = "Portal query was stopped: action_rule '$sId' gives more than 1 '$sSearchClass' result";
IssueLog::Error($sErrMsg);
throw new CoreUnexpectedValue($sErrMsg);
}
if ($oSourceObject === null)
{
continue;
}
// Presets
if (isset($aRule['preset']) && !empty($aRule['preset']))
{
$oObject->ExecActions($aRule['preset'], array('source' => $oSourceObject));
}
// Retrofits
if (isset($aRule['retrofit']) && !empty($aRule['retrofit']))
{
$oSourceObject->ExecActions($aRule['retrofit'], array('source' => $oObject));
}
}
else
{
// Changing behaviour to remove usage of ObjectCopier as its now being integrated in the core
// Old code : iTopObjectCopier::PrepareObject($aRule, $oObject, $oObject);
// Presets
if (isset($aRule['preset']) && !empty($aRule['preset']))
{

29
test/core/DBSearchTest.php
View File

@@ -29,7 +29,7 @@ namespace Combodo\iTop\Test\UnitTest\Core;
use CMDBSource;
use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
use CoreOqlMultipleResultsFoundException;
use CoreOqlMultipleResultsForbiddenException;
use DBSearch;
use Exception;
use Expression;
@@ -525,7 +525,7 @@ class DBSearchTest extends ItopDataTestCase
*
* @param string $sOql query to test
* @param bool $bMustHaveOneResultMax arg passed to the tested function
* @param int $iReturn 0 if should return null, 1 if should return object, -1 if should throw
* @param int $sReturn
*
* @throws \CoreException
* @throws \CoreUnexpectedValue
@@ -534,7 +534,7 @@ class DBSearchTest extends ItopDataTestCase
*
* @covers DBSearch::GetFirstResult()
*/
public function testGetFirstResult($sOql, $bMustHaveOneResultMax, $iReturn)
public function testGetFirstResult($sOql, $bMustHaveOneResultMax, $sReturn)
{
$oSearch = DBSearch::FromOQL($sOql);
@@ -543,21 +543,21 @@ class DBSearchTest extends ItopDataTestCase
{
$oFirstResult = $oSearch->GetFirstResult($bMustHaveOneResultMax);
}
catch (CoreOqlMultipleResultsFoundException $e)
catch (CoreOqlMultipleResultsForbiddenException $e)
{
$oFirstResult = null;
$bHasThrownException = true;
}
switch ($iReturn)
switch ($sReturn)
{
case -1:
case 'exception':
self::assertEquals(true, $bHasThrownException, 'Exception raised');
break;
case 0:
case 'null':
self::assertNull($oFirstResult, 'Null returned');
break;
case 1:
case 'object':
self::assertInternalType('object', $oFirstResult, 'Object returned');
break;
}
@@ -569,22 +569,27 @@ class DBSearchTest extends ItopDataTestCase
'One result' => array(
'SELECT Person WHERE id = 1',
false,
1,
'object',
),
'Multiple results, no exception' => array(
'SELECT Person',
false,
1,
'object',
),
'Multiple results, with exception' => array(
'SELECT Person',
true,
-1,
'exception',
),
'Multiple results with "WHERE 1", with exception' => array(
'SELECT Person WHERE 1',
true,
'exception',
),
'No result' => array(
'SELECT Person WHERE id = -1',
true,
0,
'null',
),
);
}