From cdcfe03d67a01e90ecb92175fd02b1c3e271c6cb Mon Sep 17 00:00:00 2001 From: Eric Date: Tue, 18 Sep 2018 15:33:00 +0200 Subject: [PATCH] =?UTF-8?q?N=C2=B0931:=20Tag=20max=20number=20and=20max=20?= =?UTF-8?q?length=20per=20class=20and=20field=20(defined=20in=20xml)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/attributedef.class.inc.php | 34 +++++++++++---- core/ormtagset.class.inc.php | 32 +++++++++++++- core/tagsetfield.class.inc.php | 23 ++++++---- dictionaries/en.dictionary.itop.core.php | 4 +- dictionaries/fr.dictionary.itop.core.php | 4 +- setup/compiler.class.inc.php | 6 +++ test/core/TagSetFieldDataTest.php | 32 +++++++++++++- test/core/ormTagSetTest.php | 53 ++++++++++++++++++------ 8 files changed, 153 insertions(+), 35 deletions(-) diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index 153ea2356..7b35a4fb0 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -6712,7 +6712,7 @@ class AttributeTagSet extends AttributeDBFieldVoid static public function ListExpectedParams() { - return array_merge(parent::ListExpectedParams(), array("is_null_allowed")); + return array_merge(parent::ListExpectedParams(), array('is_null_allowed', 'tag_max_nb', 'tag_code_max_len')); } public function GetDefaultValue(DBObject $oHostObject = null) @@ -6725,6 +6725,16 @@ class AttributeTagSet extends AttributeDBFieldVoid return $this->Get("is_null_allowed"); } + public function GetTagMaxNb() + { + return $this->Get('tag_max_nb'); + } + + public function GetTagCodeMaxLength() + { + return $this->Get('tag_code_max_len'); + } + public function GetEditClass() { return "TagSet"; @@ -6748,14 +6758,15 @@ class AttributeTagSet extends AttributeDBFieldVoid protected function GetSQLCol($bFullSpec = false) { - return 'VARCHAR(255)' + $iLen = $this->GetMaxSize(); + return "VARCHAR($iLen)" .CMDBSource::GetSqlStringColumnDefinition() .($bFullSpec ? $this->GetSQLColSpec() : ''); } public function GetMaxSize() { - return 255; + return $iLen = ($this->GetTagMaxNb() * $this->GetTagCodeMaxLength()) + 1; } public function RequiresIndex() @@ -6806,7 +6817,7 @@ class AttributeTagSet extends AttributeDBFieldVoid $aTagCodes = explode(' ', "$sValue"); $sAttCode = $this->GetCode(); $sClass = MetaModel::GetAttributeOrigin($this->GetHostClass(), $sAttCode); - $oTagSet = new ormTagSet($sClass, $sAttCode); + $oTagSet = new ormTagSet($sClass, $sAttCode, $this->GetTagMaxNb()); $aGoodTags = array(); foreach($aTagCodes as $sTagCode) { @@ -6817,6 +6828,11 @@ class AttributeTagSet extends AttributeDBFieldVoid if ($oTagSet->IsValidTag($sTagCode)) { $aGoodTags[] = $sTagCode; + if (count($aGoodTags) === $this->GetTagMaxNb()) + { + // extra tags are ignored + continue; + } } else { @@ -6841,7 +6857,7 @@ class AttributeTagSet extends AttributeDBFieldVoid public function MakeRealValue($proposedValue, $oHostObj) { $oTagSet = new ormTagSet(MetaModel::GetAttributeOrigin($this->GetHostClass(), $this->GetCode()), - $this->GetCode()); + $this->GetCode(), $this->GetTagMaxNb()); if (is_string($proposedValue) && !empty($proposedValue)) { $aTagCodes = explode(' ', "$proposedValue"); @@ -6879,7 +6895,7 @@ class AttributeTagSet extends AttributeDBFieldVoid if ($bLocalizedValue && !empty($sProposedValue)) { $oTagSet = new ormTagSet(MetaModel::GetAttributeOrigin($this->GetHostClass(), $this->GetCode()), - $this->GetCode()); + $this->GetCode(), $this->GetTagMaxNb()); $aLabels = explode($sSepItem, $sProposedValue); $aCodes = array(); foreach($aLabels as $sTagLabel) @@ -6897,7 +6913,7 @@ class AttributeTagSet extends AttributeDBFieldVoid public function GetNullValue() { - return new ormTagSet(MetaModel::GetAttributeOrigin($this->GetHostClass(), $this->GetCode()), $this->GetCode()); + return new ormTagSet(MetaModel::GetAttributeOrigin($this->GetHostClass(), $this->GetCode()), $this->GetCode(), $this->GetTagMaxNb()); } public function IsNull($proposedValue) @@ -7025,7 +7041,7 @@ class AttributeTagSet extends AttributeDBFieldVoid $aTagCodes = explode(' ', $value); $aValues = array(); $oTagSet = new ormTagSet(MetaModel::GetAttributeOrigin($this->GetHostClass(), $this->GetCode()), - $this->GetCode()); + $this->GetCode(), $this->GetTagMaxNb()); foreach($aTagCodes as $sTagCode) { try @@ -7225,7 +7241,7 @@ class AttributeTagSet extends AttributeDBFieldVoid */ public function FromJSONToValue($json) { - $oSet = new ormTagSet($this->GetHostClass(), $this->GetCode()); + $oSet = new ormTagSet($this->GetHostClass(), $this->GetCode(), $this->GetTagMaxNb()); $oSet->SetValue($json); return $oSet; diff --git a/core/ormtagset.class.inc.php b/core/ormtagset.class.inc.php index f434d20c8..8246a6c95 100644 --- a/core/ormtagset.class.inc.php +++ b/core/ormtagset.class.inc.php @@ -52,6 +52,11 @@ final class ormTagSet */ private $aModified = array(); + /** + * @var int Max number of tags in collection + */ + private $iLimit; + /** * __toString magical function overload. */ @@ -73,10 +78,11 @@ final class ormTagSet * * @param string $sClass * @param string $sAttCode + * @param int $iLimit * * @throws \Exception */ - public function __construct($sClass, $sAttCode) + public function __construct($sClass, $sAttCode, $iLimit = 12) { $this->sAttCode = $sAttCode; @@ -86,6 +92,7 @@ final class ormTagSet throw new Exception("ormTagSet: field {$sClass}:{$sAttCode} is not a tag"); } $this->sClass = $sClass; + $this->iLimit = $iLimit; } /** @@ -119,8 +126,16 @@ final class ormTagSet } $oTags = array(); + $iCount = 0; + $bError = false; foreach($aTagCodes as $sTagCode) { + $iCount++; + if ($iCount > $this->iLimit) + { + $bError = true; + continue; + } $oTag = $this->GetTagFromCode($sTagCode); $oTags[$sTagCode] = $oTag; } @@ -130,6 +145,16 @@ final class ormTagSet $this->aAdded = array(); $this->aModified = array(); $this->aOriginalObjects = $oTags; + + if ($bError) + { + throw new CoreException("Maximum number of tags ({$this->iLimit}) reached for {$this->sClass}:{$this->sAttCode}"); + } + } + + private function GetCount() + { + return count($this->aPreserved) + count($this->aAdded) - count($this->aRemoved); } /** @@ -314,6 +339,10 @@ final class ormTagSet */ public function AddTag($sTagCode) { + if ($this->GetCount() === $this->iLimit) + { + throw new CoreException("Maximum number of tags ({$this->iLimit}) reached for {$this->sClass}:{$this->sAttCode}"); + } if ($this->IsTagInList($this->aPreserved, $sTagCode) || $this->IsTagInList($this->aAdded, $sTagCode)) { // nothing to do, already existing tag @@ -481,4 +510,5 @@ final class ormTagSet { return MetaModel::GetTagDataClass($this->sClass, $this->sAttCode); } + } \ No newline at end of file diff --git a/core/tagsetfield.class.inc.php b/core/tagsetfield.class.inc.php index 48c337cff..0087beef6 100644 --- a/core/tagsetfield.class.inc.php +++ b/core/tagsetfield.class.inc.php @@ -154,24 +154,35 @@ abstract class TagSetFieldData extends cmdbAbstractObject * @throws \MySQLException * @throws \MySQLHasGoneAwayException * @throws \OQLException + * @throws \Exception */ public function DoCheckToWrite() { - // Check that code and labels are uniques + $sClass = $this->Get('tag_class'); + $sAttCode = $this->Get('tag_attcode'); + $iMaxLen = 20; + $oAttDef = MetaModel::GetAttributeDef($sClass, $sAttCode); + if ($oAttDef instanceof AttributeTagSet) + { + $iMaxLen = $oAttDef->GetTagCodeMaxLength(); + } + $sTagCode = $this->Get('tag_code'); // Check tag_code syntax - if (!preg_match("@^[a-zA-Z0-9]{3,20}$@", $sTagCode)) + if (!preg_match("@^[a-zA-Z0-9]{3,$iMaxLen}$@", $sTagCode)) { - $this->m_aCheckIssues[] = Dict::S('Core:TagSetFieldData:ErrorTagCodeSyntax'); + $this->m_aCheckIssues[] = Dict::Format('Core:TagSetFieldData:ErrorTagCodeSyntax', $iMaxLen); } $sTagLabel = $this->Get('tag_label'); - if (empty($sTagLabel) || (strpos($sTagLabel, "|") !== false)) + $sSepItem = MetaModel::GetConfig()->Get('tag_set_item_separator'); + if (empty($sTagLabel) || (strpos($sTagLabel, $sSepItem) !== false)) { // Label must not contain | character - $this->m_aCheckIssues[] = Dict::S('Core:TagSetFieldData:ErrorTagLabelSyntax'); + $this->m_aCheckIssues[] = Dict::Format('Core:TagSetFieldData:ErrorTagLabelSyntax', $sSepItem); } + // Check that code and labels are uniques $id = $this->GetKey(); $sClassName = get_class($this); if (empty($id)) @@ -188,8 +199,6 @@ abstract class TagSetFieldData extends cmdbAbstractObject $this->m_aCheckIssues[] = Dict::S('Core:TagSetFieldData:ErrorDuplicateTagCodeOrLabel'); } // Clear cache - $sClass = $this->Get('tag_class'); - $sAttCode = $this->Get('tag_attcode'); $sTagDataClass = MetaModel::GetTagDataClass($sClass, $sAttCode); unset(self::$m_aAllowedValues[$sTagDataClass]); diff --git a/dictionaries/en.dictionary.itop.core.php b/dictionaries/en.dictionary.itop.core.php index a59c8fcfd..fb8671639 100644 --- a/dictionaries/en.dictionary.itop.core.php +++ b/dictionaries/en.dictionary.itop.core.php @@ -919,8 +919,8 @@ Dict::Add('EN US', 'English', 'English', array( 'Class:TagSetFieldData+' => '', 'Core:TagSetFieldData:ErrorDeleteUsedTag' => 'Used tags cannot be deleted', 'Core:TagSetFieldData:ErrorDuplicateTagCodeOrLabel' => 'Tags codes or labels must be unique', - 'Core:TagSetFieldData:ErrorTagCodeSyntax' => 'Tags code should contain between 3 and 20 alphanumeric characters', - 'Core:TagSetFieldData:ErrorTagLabelSyntax' => 'Tags label should not contain | nor be empty', + 'Core:TagSetFieldData:ErrorTagCodeSyntax' => 'Tags code should contain between 3 and %1$d alphanumeric characters', + 'Core:TagSetFieldData:ErrorTagLabelSyntax' => 'Tags label should not contain \'%1$s\' nor be empty', 'Core:TagSetFieldData:ErrorCodeUpdateNotAllowed' => 'Tags code cannot be changed', 'Core:TagSetFieldData:WhereIsThisTagTab' => 'Tag usage (%1$d)', 'Core:TagSetFieldData:NoEntryFound' => 'No entry found for this tag', diff --git a/dictionaries/fr.dictionary.itop.core.php b/dictionaries/fr.dictionary.itop.core.php index 599fe217a..ad08377ad 100644 --- a/dictionaries/fr.dictionary.itop.core.php +++ b/dictionaries/fr.dictionary.itop.core.php @@ -770,8 +770,8 @@ Opérateurs :
'Class:TagSetFieldData+' => '', 'Core:TagSetFieldData:ErrorDeleteUsedTag' => 'Impossible de supprimer une étiquette utilisée', 'Core:TagSetFieldData:ErrorDuplicateTagCodeOrLabel' => 'Les codes et noms des étiquettes doivent être unique', - 'Core:TagSetFieldData:ErrorTagCodeSyntax' => 'Le code de l\'étiquette doit contenir entre 3 et 20 caractères alphanumériques.', - 'Core:TagSetFieldData:ErrorTagLabelSyntax' => 'Le nom de l\'étiquette ne doit pas être vide ni contenir le caractère \'|\'', + 'Core:TagSetFieldData:ErrorTagCodeSyntax' => 'Le code de l\'étiquette doit contenir entre 3 et %1$d caractères alphanumériques.', + 'Core:TagSetFieldData:ErrorTagLabelSyntax' => 'Le nom de l\'étiquette ne doit pas être vide ni contenir le caractère \'%1$s\'', 'Core:TagSetFieldData:ErrorCodeUpdateNotAllowed' => 'Le code de l\'étiquette ne peut pas être changé', 'Core:TagSetFieldData:WhereIsThisTagTab' => 'Utilisation (%1$d)', 'Core:TagSetFieldData:NoEntryFound' => 'Pas d\'utilisation de cette étiquette', diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index c42277369..c69ec826e 100644 --- a/setup/compiler.class.inc.php +++ b/setup/compiler.class.inc.php @@ -1395,6 +1395,12 @@ EOF $aParameters['sql'] = $this->GetMandatoryPropString($oField, 'sql'); $aParameters['is_null_allowed'] = $this->GetPropBoolean($oField, 'is_null_allowed', false); $aParameters['depends_on'] = $sDependencies; + $aParameters['tag_max_nb'] = $this->GetPropNumber($oField, 'tag_max_nb', 12); + $aParameters['tag_code_max_len'] = $this->GetPropNumber($oField, 'tag_code_max_len', 20); + if ($aParameters['tag_code_max_len'] > 255) + { + $aParameters['tag_code_max_len'] = 255; + } } else { diff --git a/test/core/TagSetFieldDataTest.php b/test/core/TagSetFieldDataTest.php index a0dc514b3..4ba862d46 100644 --- a/test/core/TagSetFieldDataTest.php +++ b/test/core/TagSetFieldDataTest.php @@ -164,7 +164,6 @@ class TagSetFieldDataTest extends ItopDataTestCase 'No _' => array('tag_1'), 'No -' => array('tag-1'), 'No %' => array('tag%1'), - 'Less than 21 chars' => array('012345678901234567890'), 'At least 3 chars' => array(''), 'At least 3 chars 1' => array('a'), 'At least 3 chars 2' => array('ab'), @@ -197,4 +196,35 @@ class TagSetFieldDataTest extends ItopDataTestCase $oTagData->Set('tag_code', 'tag2'); $oTagData->DBWrite(); } + + /** + * Check that the code length is correctly checked + * + * @throws \CoreException + */ + public function testMaxTagCodeLength() + { + /** @var \AttributeTagSet $oAttdef */ + $oAttdef = \MetaModel::GetAttributeDef(TAG_CLASS, TAG_ATTCODE); + + $iMaxLength = $oAttdef->GetTagCodeMaxLength(); + $sTagCode = str_repeat('a', $iMaxLength); + + // Should work + $this->CreateTagData(TAG_CLASS, TAG_ATTCODE, $sTagCode, $sTagCode); + + // Too long + $sTagCode = str_repeat('a', $iMaxLength + 1); + try + { + $this->CreateTagData(TAG_CLASS, TAG_ATTCODE, $sTagCode, $sTagCode); + } catch (\CoreException $e) + { + $this->debug('Awaited: '.$e->getMessage()); + static::assertTrue(true); + return; + } + // Failed + static::assertFalse(true); + } } diff --git a/test/core/ormTagSetTest.php b/test/core/ormTagSetTest.php index 1b3f02ee3..e37617644 100644 --- a/test/core/ormTagSetTest.php +++ b/test/core/ormTagSetTest.php @@ -32,6 +32,8 @@ use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use Exception; use ormTagSet; +define('MAX_TAGS', 12); + /** * Tests of the ormTagSet class * @@ -57,13 +59,13 @@ class ormTagSetTest extends ItopDataTestCase public function testGetTagDataClass() { - $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); static::assertEquals($oTagSet->GetTagDataClass(), 'TagSetFieldDataFor_Ticket_tagfield'); } public function testGetValue() { - $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); static::assertEquals($oTagSet->GetValue(), array()); $oTagSet->AddTag('tag1'); @@ -75,7 +77,7 @@ class ormTagSetTest extends ItopDataTestCase public function testAddTag() { - $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet->AddTag('tag1'); static::assertEquals($oTagSet->GetValue(), array('tag1')); @@ -90,13 +92,38 @@ class ormTagSetTest extends ItopDataTestCase static::assertEquals($oTagSet->GetValue(), array('tag1', 'tag2')); } + + /** + * @expectedException \CoreException + * @throws \CoreException + * @throws \CoreUnexpectedValue + */ + public function testMaxTagLimit() + { + $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE, 3); + + $oTagSet->SetValue(array('tag1', 'tag2', 'tag3')); + + static::assertEquals($oTagSet->GetValue(), array('tag1', 'tag2', 'tag3')); + + try + { + $oTagSet->SetValue(array('tag1', 'tag2', 'tag3', 'tag4')); + } + catch (\CoreException $e) + { + $this->debug('Awaited: '.$e->getMessage()); + throw $e; + } + } + public function testEquals() { - $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet1->AddTag('tag1'); static::assertTrue($oTagSet1->Equals($oTagSet1)); - $oTagSet2 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet2 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet2->SetValue(array('tag1')); static::assertTrue($oTagSet1->Equals($oTagSet2)); @@ -107,7 +134,7 @@ class ormTagSetTest extends ItopDataTestCase public function testSetValue() { - $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet->SetValue(array('tag1')); static::assertEquals($oTagSet->GetValue(), array('tag1')); @@ -119,7 +146,7 @@ class ormTagSetTest extends ItopDataTestCase public function testRemoveTag() { - $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet->RemoveTag('tag_unknown'); static::assertEquals($oTagSet->GetValue(), array()); @@ -146,10 +173,10 @@ class ormTagSetTest extends ItopDataTestCase public function testGetDelta() { - $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet1->SetValue(array('tag1', 'tag2')); - $oTagSet2 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet2 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet2->SetValue(array('tag1', 'tag3', 'tag4')); $aDelta = $oTagSet1->GetDelta($oTagSet2); @@ -160,10 +187,10 @@ class ormTagSetTest extends ItopDataTestCase public function testApplyDelta() { - $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet1->SetValue(array('tag1', 'tag2')); - $oTagSet2 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet2 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet2->SetValue(array('tag1', 'tag3', 'tag4')); $aDelta = $oTagSet1->GetDelta($oTagSet2); @@ -184,7 +211,7 @@ class ormTagSetTest extends ItopDataTestCase */ public function testGetModified($aInitialTags, $aDiffAndExpectedTags) { - $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet1->SetValue($aInitialTags); foreach($aDiffAndExpectedTags as $aTestItem) @@ -236,7 +263,7 @@ class ormTagSetTest extends ItopDataTestCase */ public function testBulkModify($aInitialTags, $aDelta, $aExpectedTags) { - $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE); + $oTagSet1 = new ormTagSet(TAG_CLASS, TAG_ATTCODE, MAX_TAGS); $oTagSet1->SetValue($aInitialTags); $oTagSet1->ApplyDelta($aDelta);