collective / collective.elephantvocabulary

zope vocabulary with possibility to hide/show terms
http://pypi.python.org/pypi/collective.elephantvocabulary
GNU General Public License v2.0
1 stars 0 forks source link

Internal terms lists grow indefinitely when populated from registry #4

Closed lukasgraf closed 10 years ago

lukasgraf commented 10 years ago

In vocabulary.py:56 and vocabulary.py:76 the internal lists VocabularyFactory.visible_terms and VocabularyFactory.hidden_terms get updated with values from the registry.

However, terms are only ever appended to those lists, and no check for duplicates is being done. This will cause those lists to grow indefinitely, because on ever __call__() the terms from the registry will be appended again. In production sites running for a couple weeks we had vocabularies with those lists having 28'000 items, most of them duplicates.

Proposed fix: Either

or

I slightly prefer the first solution because it preserves order (shouldn't matter for the implementation, but makes for writing nicer tests).

garbas commented 10 years ago

yup ... first one it is, will look into it

phgross commented 10 years ago

:+1: looks good for me.

lukasgraf commented 10 years ago

Rebased on master after merging #2

phgross commented 10 years ago

:+1: