From 81d0093e471ca31aae726c3f82145e92a5de9f70 Mon Sep 17 00:00:00 2001 From: pbarroca <pbarroca@afi-sa.fr> Date: Tue, 22 Nov 2016 14:54:39 +0100 Subject: [PATCH] hotline #50500 : fix id_thesaurus overflow --- VERSIONS_HOTLINE/50500 | 1 + .../php/classes/KohaRecordIntegrationTest.php | 109 +++++++++++++++++- library/Class/CodifThesaurus.php | 23 ++-- .../PseudoNotice/FacettesVisitor.php | 6 +- .../CmsControllerCustomFieldsTest.php | 45 ++++++++ tests/library/Class/CodifThesaurusTest.php | 38 ++++++ 6 files changed, 209 insertions(+), 13 deletions(-) create mode 100644 VERSIONS_HOTLINE/50500 diff --git a/VERSIONS_HOTLINE/50500 b/VERSIONS_HOTLINE/50500 new file mode 100644 index 00000000000..565cd501af8 --- /dev/null +++ b/VERSIONS_HOTLINE/50500 @@ -0,0 +1 @@ + - ticket #50500 : Indexation : correction d'une erreur lorsqu'un niveau thesaurus dépassait 9999 éléments \ No newline at end of file diff --git a/cosmogramme/tests/php/classes/KohaRecordIntegrationTest.php b/cosmogramme/tests/php/classes/KohaRecordIntegrationTest.php index 70ceb22dd5d..7e48a68c458 100644 --- a/cosmogramme/tests/php/classes/KohaRecordIntegrationTest.php +++ b/cosmogramme/tests/php/classes/KohaRecordIntegrationTest.php @@ -217,8 +217,15 @@ class KohaRecordIntegrationVagabondWithThesaurusOn702DollarATest extends KohaRec 'code' => 'summary' ]); + $this->fixture('Class_CodifThesaurus', + ['id' => 6, + 'libelle' => 'Test', + 'id_thesaurus' => 'TEST', + 'id_origine' => null, + 'code' => 'TEST', + 'rules' => '{"label":"610$a"}']); - $loader = Storm_Test_ObjectWrapper::onLoaderOfModel('Class_CodifThesaurus'); + $loader = $this->onLoaderOfModel('Class_CodifThesaurus'); $loader ->whenCalled('findFirstBy') ->with(['code' => 'authors', @@ -243,12 +250,14 @@ class KohaRecordIntegrationVagabondWithThesaurusOn702DollarATest extends KohaRec $this->assertEquals('Vagabond n° 4', $this->_notice->getTitrePrincipal()); } + /** @test */ public function noSumm0002ShouldBeCreated() { $entry = Class_CodifThesaurus::findAllBy(['libelle' => 'd\'après l\'oeuvre d\'Eiji Yoshikawa, "Miyamoto Musashi"']); $this->assertCount(1, $entry); } + /** @test */ public function facetsShouldContainsHAUTH0001() { $this->assertContains('HAUTH0001', $this->_notice->getFacetCodes()); @@ -268,6 +277,7 @@ class KohaRecordIntegrationVagabondWithThesaurusOn702DollarATest extends KohaRec $this->_notice->getFacettes()); } + /** @test */ public function entryForYoshikawaShouldBeCreated() { $entry = Class_CodifThesaurus::findFirstBy(['id_thesaurus' => 'AUTH0002']); @@ -276,6 +286,101 @@ class KohaRecordIntegrationVagabondWithThesaurusOn702DollarATest extends KohaRec $entry->getLibelle()); } + + /** @test */ + public function shouldCreateThesaurusFor610_a() { + $this->assertNotNull(Class_CodifThesaurus::findFirstBy(['id_thesaurus' => 'TEST0001', + 'libelle' => 'Manga'])); + } } -?> \ No newline at end of file + + +class KohaRecordIntegrationVagabondWithTooMany610aTest extends KohaRecordIntegrationTestCase { + public function setUp() { + parent::setUp(); + + $this->fixture('Class_CodifThesaurus', + ['id' => 1, + 'libelle' => 'Author', + 'id_thesaurus' => 'AUTH', + 'id_origine' => null, + 'code' => 'authors', + 'rules' => '{"label":"702$a"}']); + + + $marcel = $this->fixture('Class_CodifThesaurus', + ['id' => 2, + 'libelle' => 'Marcel', + 'id_thesaurus' => 'AUTH0001', + 'id_origine' => 'MARCEL', + 'code' => 'authors', + 'rules' => null]); + + + $docu = $this->fixture('Class_CodifThesaurus', + ['id' => 3, + 'libelle' => 'Document', + 'id_thesaurus' => 'DOCU', + 'id_origine' => null, + 'code' => 'document', + 'rules' => '{"label":" 99$t "}']); + + $this->fixture('Class_CodifThesaurus', + ['id' => 4, + 'libelle' => 'Summary', + 'id_thesaurus' => 'SUMM', + 'id_origine' => null, + 'code' => 'summary', + 'rules' => '{"label":" 200$g "}']); + + $this->fixture('Class_CodifThesaurus', + ['id' => 5, + 'libelle' => 'd\'après l\'oeuvre d\'Eiji Yoshikawa, "Miyamoto Musashi"', + 'id_thesaurus' => 'SUMM0001', + 'id_origine' => 'D\'APRèS L\'OEUVRE D\'E', + 'code' => 'summary' + ]); + + $this->fixture('Class_CodifThesaurus', + ['id' => 6, + 'libelle' => 'Test', + 'id_thesaurus' => 'TEST', + 'id_origine' => null, + 'code' => 'TEST', + 'rules' => '{"label":"610$a"}']); + + $this->fixture('Class_CodifThesaurus', + ['id_thesaurus' => 'TEST9999', + 'id_origine' => 'RIME', + 'libelle' => 'rime', + 'code' => 'TEST', + 'id' => 10722, + 'libelle_facette' => 'rime', + 'rules' => null]); + + $loader = $this->onLoaderOfModel('Class_CodifThesaurus'); + $loader + ->whenCalled('findFirstBy') + ->with(['code' => 'authors', + 'where' => 'id_thesaurus like "AUTH%"', + 'order' => 'id_thesaurus desc']) + ->answers($marcel) + + ->whenCalled('findFirstBy') + ->with(['code' => 'document', + 'where' => 'id_thesaurus like "DOCU%"', + 'order' => 'id_thesaurus desc']) + ->answers($docu); + + + $this->loadNotice('unimarc_vagabond'); + $this->_notice = Class_Notice::find(1); + } + + + /** @test */ + public function shouldNotCreateThesaurusFor610_a() { + $this->assertNull(Class_CodifThesaurus::findFirstBy(['libelle' => 'Manga'])); + } +} \ No newline at end of file diff --git a/library/Class/CodifThesaurus.php b/library/Class/CodifThesaurus.php index 11188fedaf4..2e6aa9f83ec 100644 --- a/library/Class/CodifThesaurus.php +++ b/library/Class/CodifThesaurus.php @@ -256,6 +256,7 @@ class Class_CodifThesaurus extends Storm_Model_Abstract { const COLUMN_ORIGIN_SIZE = 20, + ID_KEY_LENGTH = 4, CODE_FACETTE = 'H', CODE_CATALOGUE = 'Catalogue', @@ -328,10 +329,15 @@ class Class_CodifThesaurus extends Storm_Model_Abstract { public function newChildEntry() { $loader = $this->getLoader(); $code = $this->getCode(); - return $loader->newInstance(['code' => $code, - 'id_thesaurus' => $loader->findNextThesaurusChildId( - $code, - $this->getIdThesaurus())]); + + $parent_id_length = strlen($this->getIdThesaurus()); + $id_thesaurus = $loader->findNextThesaurusChildId($code, + $this->getIdThesaurus()); + $child_id_length = strlen($id_thesaurus); + return $child_id_length != $parent_id_length+self::ID_KEY_LENGTH + ? null + : $loader->newInstance(['code' => $code, + 'id_thesaurus' => $id_thesaurus]); } @@ -346,10 +352,11 @@ class Class_CodifThesaurus extends Storm_Model_Abstract { if ($entry) return $entry; - $entry = $this - ->newChildEntry() - ->updateAttributes(['id_origine' => $id_origine, - 'libelle' => $label]); + if (!$entry = $this->newChildEntry()) + return null; + + $entry->updateAttributes(['id_origine' => $id_origine, + 'libelle' => $label]); $entry->assertSave(); return $entry; diff --git a/library/Class/Indexation/PseudoNotice/FacettesVisitor.php b/library/Class/Indexation/PseudoNotice/FacettesVisitor.php index 18ace745434..b4ebb7757f6 100644 --- a/library/Class/Indexation/PseudoNotice/FacettesVisitor.php +++ b/library/Class/Indexation/PseudoNotice/FacettesVisitor.php @@ -154,9 +154,9 @@ class Class_Indexation_PseudoNotice_FacettesVisitor extends Class_Indexation_Pse return; foreach($value->getValueAsArray() as $one_value) { - $thesaurus = $this - ->_ensureThesaurusUnderWithValue($this->_ensureThesaurusFor($field), - $one_value); + if (!$thesaurus = $this->_ensureThesaurusUnderWithValue($this->_ensureThesaurusFor($field), + $one_value)) + continue; $this->_facettes[] = $thesaurus->getFacetteIndex(); } } diff --git a/tests/application/modules/admin/controllers/CmsControllerCustomFieldsTest.php b/tests/application/modules/admin/controllers/CmsControllerCustomFieldsTest.php index e71267254c8..342504fba65 100644 --- a/tests/application/modules/admin/controllers/CmsControllerCustomFieldsTest.php +++ b/tests/application/modules/admin/controllers/CmsControllerCustomFieldsTest.php @@ -173,6 +173,51 @@ class CmsControllerCustomFieldsAndIndexationPostEditActionTest + +class CmsControllerCustomFieldsAndIndexationPostEditWithTooManyValueActionTest + extends CmsControllerCustomFieldsTestCase { + protected + $_record, + $_field_thesaurus, + $_value_thesaurus; + + + public function setUp() { + parent::setUp(); + + $this->fixture('Class_CodifThesaurus', + ['id_thesaurus' => 'CFCF00019999', + 'id_origine' => 'newbies', + 'libelle' => 'newbies', + 'code' => Class_CodifThesaurus::CODE_CUSTOMFIELDS, + 'id' => 10722, + 'libelle_facette' => 'newbies', + 'rules' => null]); + + $this->postDispatch('admin/cms/edit/id/1', + ['titre' => 'News Article', + 'contenu' => 'Welcome', + 'indexation' => 1, + 'status' => Class_Article::STATUS_VALIDATED, + 'field_5' => 'Hardcore gamers', + 'debut' => '', + 'fin' => '', + 'events_debut' => '', + 'events_fin' => '']); + + $this->_value_thesaurus = Class_CodifThesaurus::findFirstBy(['libelle' => 'Hardcore gamers']); + } + + + /** @test */ + public function shouldNotCreateNewThesaurus() { + $this->assertNull($this->_value_thesaurus); + } +} + + + + class CmsControllerCustomFieldsNotIndexableMetaPostEditActionTest extends CmsControllerCustomFieldsTestCase { diff --git a/tests/library/Class/CodifThesaurusTest.php b/tests/library/Class/CodifThesaurusTest.php index f16b854ce2f..c3acb92bc7f 100644 --- a/tests/library/Class/CodifThesaurusTest.php +++ b/tests/library/Class/CodifThesaurusTest.php @@ -147,4 +147,42 @@ class CodifThesaurusDeleteTest extends ModelTestCase { public function recordShouldNotHaveFacetHCFCF() { $this->assertEquals('T8 A3452', $this->_record->getFacettes()); } +} + + + +/** @see http://forge.afi-sa.fr/issues/50500 */ +class CodifThesaurusTooManyValuesTest extends ModelTestCase { + protected $_storm_default_to_volatile = true; + + public function setUp() { + parent::setUp(); + + $this->fixture('Class_CodifThesaurus', + ['id_thesaurus' => 'TEST', + 'id_origine' => null, + 'libelle' => 'test', + 'code' => 'TEST', + 'id' => 723, + 'libelle_facette' => 'test', + 'rules' => '{"label":"610$a"}']); + + $this->fixture('Class_CodifThesaurus', + ['id_thesaurus' => 'TEST9999', + 'id_origine' => 'RIME', + 'libelle' => 'rime', + 'code' => 'TEST', + 'id' => 10722, + 'libelle_facette' => 'rime', + 'rules' => null]); + } + + + /** @test */ + public function shouldNotInsertMoreThan9999Child() { + Class_CodifThesaurus::find(723)->getOrCreateChild(strtoupper('métiers de la recherche'), 'métiers de la recherche'); + + $this + ->assertNull(Class_CodifThesaurus::findFirstBy(['id_thesaurus' => 'TEST00010000'])); + } } \ No newline at end of file -- GitLab