Commit 4558c766 authored by Jérome Perrin's avatar Jérome Perrin

Fix preferences cache

* Fix the problem that preference cache was not reset when `setPreferredSomething` was called, only when `edit(preferred_something=)`
* Implement something more advanced than just `portal_cache.clearCache()` so that when a user change their preference, only preference of this user are affected.
* Increase default cache duration, now that we invalidate caches properly.

/reviewed-on !983
......@@ -232,8 +232,8 @@ class TestUpgrader(ERP5TypeTestCase):
alarm = getattr(self.portal.portal_alarms, 'upgrader_check_post_upgrade')
active_process = alarm.getLastActiveProcess()
detail_list = active_process.getResultList()[0].detail
message = 'Preference - Expected: edit_workflow, preference_workflow <> Found: (Default)'
self.assertTrue(message in detail_list, detail_list)
message = 'Preference - Expected: edit_workflow, preference_interaction_workflow, preference_workflow <> Found: (Default)'
self.assertIn(message, detail_list)
self.assertTrue(detail_list.count(message), 1)
def stepSetConstraintInPersonModulePortalType(self, sequence=None):
......
......@@ -133,7 +133,7 @@
</chain>
<chain>
<type>Preference</type>
<workflow>edit_workflow, preference_workflow</workflow>
<workflow>edit_workflow, preference_interaction_workflow, preference_workflow</workflow>
</chain>
<chain>
<type>Property Existence Constraint</type>
......@@ -169,7 +169,7 @@
</chain>
<chain>
<type>System Preference</type>
<workflow>edit_workflow, preference_workflow</workflow>
<workflow>edit_workflow, preference_interaction_workflow, preference_workflow</workflow>
</chain>
<chain>
<type>TALES Constraint</type>
......
......@@ -30,7 +30,7 @@ paste_info, = preference.manage_pasteObjects(cb_copy_data=cp, is_indexable=False
template = getattr(preference, paste_info['new_id'])
template.makeTemplate()
context.portal_caches.clearCacheFactory('erp5_ui_short')
portal.portal_preferences.clearCache(preference)
kw['keep_items'] = dict(portal_status_message=message)
return context.Base_redirect(form_id,
......
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="InteractionWorkflowDefinition" module="Products.ERP5.InteractionWorkflow"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_objects</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>groups</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>preference_interaction_workflow</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Interaction" module="Products.ERP5.Interaction"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>_objects</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>interactions</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="InteractionDefinition" module="Products.ERP5.Interaction"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>actbox_category</string> </key>
<value> <string>workflow</string> </value>
</item>
<item>
<key> <string>actbox_name</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>actbox_url</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>activate_script_name</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>after_script_name</string> </key>
<value>
<list>
<string>ERP5Site_clearPreferrenceCache</string>
</list>
</value>
</item>
<item>
<key> <string>before_commit_script_name</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>description</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>guard</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>edit</string> </value>
</item>
<item>
<key> <string>method_id</string> </key>
<value>
<list>
<string>_set.*</string>
<string>manage_beforeDelete</string>
<string>enable</string>
<string>disable</string>
</list>
</value>
</item>
<item>
<key> <string>once_per_transaction</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>portal_type_filter</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>portal_type_group_filter</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>script_name</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>temporary_document_disallowed</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>trigger_type</string> </key>
<value> <int>2</int> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Scripts" module="Products.DCWorkflow.Scripts"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>_objects</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>scripts</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
portal = sci['object'].getPortalObject()
portal.portal_preferences.clearCache(sci['object'])
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>sci</string> </value>
</item>
<item>
<key> <string>_proxy_roles</string> </key>
<value>
<tuple>
<string>Manager</string>
</tuple>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>ERP5Site_clearPreferrenceCache</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Variables" module="Products.DCWorkflow.Variables"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variables</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Worklists" module="Products.DCWorkflow.Worklists"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>worklists</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
......@@ -45,6 +45,7 @@ Module Component | dynamic_class_generation_interaction_workflow
Module Component | edit_workflow
Predicate | edit_workflow
Preference | edit_workflow
Preference | preference_interaction_workflow
Preference | preference_workflow
Property Existence Constraint | dynamic_class_generation_interaction_workflow
Property Sheet Tool | dynamic_class_generation_interaction_workflow
......@@ -55,6 +56,7 @@ SQL Method | edit_workflow
Standard Property | dynamic_class_generation_interaction_workflow
String Attribute Match Constraint | dynamic_class_generation_interaction_workflow
System Preference | edit_workflow
System Preference | preference_interaction_workflow
System Preference | preference_workflow
TALES Constraint | dynamic_class_generation_interaction_workflow
Test Component | component_validation_workflow
......
......@@ -6,6 +6,7 @@ distributed_ram_cache_interaction_workflow
dynamic_class_generation_interaction_workflow
edit_workflow
memcached_plugin_interaction_workflow
preference_interaction_workflow
preference_workflow
pricing_interaction_workflow
rule_validation_workflow
......
......@@ -100,8 +100,7 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
def _clearCache(self):
self.portal.portal_caches.clearCache(
cache_factory_list=('erp5_ui_short', # for preference cache
'erp5_content_short', # for authentication cache
cache_factory_list=('erp5_content_short', # for authentication cache
))
def _getPasswordEventList(self, login):
......
......@@ -49,6 +49,13 @@ class TestERP5Coordinate(ERP5TypeTestCase):
'erp5_full_text_mroonga_catalog',
'erp5_base',)
def afterSetUp(self):
ERP5TypeTestCase.afterSetUp(self)
self.default_site_preference = self.portal.portal_preferences.default_site_preference
if self.default_site_preference.getPreferenceState() != 'global':
self.default_site_preference.enable()
self.tic()
def beforeTearDown(self):
self.abort()
for module in ( self.portal.person_module,
......@@ -109,20 +116,20 @@ class TestERP5Coordinate(ERP5TypeTestCase):
def test_TelephonePreference(self):
pers = self.getPersonModule().newContent(portal_type='Person')
tel = pers.newContent(portal_type='Telephone')
pref = self.portal.portal_preferences.default_site_preference
pref.setPreferredTelephoneDefaultCountryNumber('33')
pref.setPreferredTelephoneDefaultAreaNumber('2')
pref.enable()
self.default_site_preference.setPreferredTelephoneDefaultCountryNumber('33')
self.default_site_preference.setPreferredTelephoneDefaultAreaNumber('2')
self.tic()
tel.fromText(coordinate_text='11111111')
self.assertEqual('+33(0)2-11111111',tel.asText())
def test_TelephoneCountryAndAreaCodeRemains(self):
pers = self.getPersonModule().newContent(portal_type='Person')
tel = pers.newContent(portal_type='Telephone')
pref = self.portal.portal_preferences.default_site_preference
pref.setPreferredTelephoneDefaultCountryNumber('')
pref.setPreferredTelephoneDefaultAreaNumber('')
pref.enable()
self.default_site_preference.setPreferredTelephoneDefaultCountryNumber('')
self.default_site_preference.setPreferredTelephoneDefaultAreaNumber('')
self.tic()
tel.fromText(coordinate_text='+11 1 11111111')
tel.fromText(coordinate_text='+22333445555')
self.assertEqual('+(0)-22333445555',tel.asText())
......@@ -312,10 +319,9 @@ class TestERP5Coordinate(ERP5TypeTestCase):
def test_TelephoneWhenTheDefaultCountryAndAreaPreferenceIsBlank(self):
pers = self.getPersonModule().newContent(portal_type='Person')
tel = pers.newContent(portal_type='Telephone')
pref = self.portal.portal_preferences.default_site_preference
pref.setPreferredTelephoneDefaultCountryNumber('')
pref.setPreferredTelephoneDefaultAreaNumber('')
pref.enable()
self.default_site_preference.setPreferredTelephoneDefaultCountryNumber('')
self.default_site_preference.setPreferredTelephoneDefaultAreaNumber('')
self.tic()
tel.fromText(coordinate_text='12345678')
self.assertEqual('+(0)-12345678',tel.asText())
......
......@@ -99,11 +99,10 @@ class TestEditorField(ERP5TypeTestCase, ZopeTestCase.Functional):
is using appropriate editor (editor) as defined
in preferences
"""
self.getDefaultSitePreference().setPreferredTextEditor(preferred_editor)
if self.getDefaultSitePreference().getPreferenceState() == 'global':
self.getDefaultSitePreference()._clearCache()
else:
self.getDefaultSitePreference().enable()
site_preference = self.getDefaultSitePreference()
site_preference.setPreferredTextEditor(preferred_editor)
if site_preference.getPreferenceState() != 'global':
site_preference.enable()
# commit transaction, are preferences are in transaction cache
self.commit()
......
......@@ -66,32 +66,3 @@ class Preference( Folder ):
security = ClassSecurityInfo()
security.declareObjectProtected(Permissions.AccessContentsInformation)
def _clearCache(self):
"""Clear caches used by methods of this preference
# TODO: clear different caches according to the preference priority
# TODO: (XXX) currently, if one use enables / disables a cache, caches
for all other users are reset. This is not good for a system
in which users do a lot of preference validation. A better solution
is needed for this. But it is not easy because the concept of
"per user cache" has been proven to be ambiguous or useless.
In theory, a solution could consist in using more keys to
select caches or to delete "manually" certain cache keys.
"""
portal_caches = getToolByName(self.getPortalObject(), 'portal_caches')
portal_caches.clearCache(cache_factory_list=('erp5_ui_short',))
def _edit(self, **kw):
"""edit and clear all caches"""
self._clearCache()
Folder._edit(self, **kw)
security.declareProtected(Permissions.ModifyPortalContent, 'enable')
def enable(self, **kw):
"""Workflow method"""
self._clearCache()
security.declareProtected(Permissions.ModifyPortalContent, 'disable')
def disable(self, **kw):
"""Workflow method"""
self._clearCache()
......@@ -41,6 +41,7 @@ from Products.ERP5Type.Cache import CachingMethod
from Products.ERP5Type.Utils import convertToUpperCase
from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
from Products.ERP5Form import _dtmldir
from BTrees.OIBTree import OIBTree
_marker = object()
......@@ -83,10 +84,11 @@ class PreferenceMethod(Method):
return default
_getPreference = CachingMethod(_getPreference,
id='%s.%s' % (self._preference_cache_id,
getSecurityManager().getUser().getId()),
cache_factory='erp5_ui_short')
instance.getPortalObject().portal_preferences._getCacheId()),
cache_factory='erp5_ui_long')
return _getPreference(default, *args, **kw)
class PreferenceTool(BaseTool):
"""
PreferenceTool manages User Preferences / User profiles.
......@@ -199,6 +201,35 @@ class PreferenceTool(BaseTool):
Note that this preference may be read only. """
return self._getActivePreferenceByPortalType('Preference')
security.declareProtected(Permissions.View, 'clearCache')
def clearCache(self, preference):
""" clear cache when a preference is modified.
This is called by an interaction workflow on preferences.
"""
self._getCacheId() # initialize _preference_cache if needed.
if preference.getPriority() == Priority.USER:
user_id = getSecurityManager().getUser().getId()
self._preference_cache[user_id] = \
self._preference_cache.get(user_id, 0) + 1
self._preference_cache[None] = self._preference_cache.get(None, 0) + 1
def _getCacheId(self):
"""Return a cache id for preferences.
We use:
- user_id: because preferences are always different by user
- self._preference_cache[user_id] which is increased everytime a user
preference is modified
- self._preference_cache[None] which is increased everytime a global
preference is modified
"""
user_id = getSecurityManager().getUser().getId()
try:
self._preference_cache
except AttributeError:
self._preference_cache = OIBTree()
return self._preference_cache.get(None), self._preference_cache.get(user_id), user_id
security.declareProtected(Permissions.View, 'getActiveUserPreference')
def getActiveUserPreference(self) :
""" returns the current user preference for the user.
......@@ -244,9 +275,10 @@ class PreferenceTool(BaseTool):
for doc in pref.contentValues(portal_type=portal_type) :
acceptable_template_list.append(doc.getRelativeUrl())
return acceptable_template_list
_getDocumentTemplateList = CachingMethod(_getDocumentTemplateList,
'portal_preferences.getDocumentTemplateList',
cache_factory='erp5_ui_short')
_getDocumentTemplateList = CachingMethod(
_getDocumentTemplateList,
'portal_preferences.getDocumentTemplateList.{}'.format(self._getCacheId()),
cache_factory='erp5_ui_long')
allowed_content_types = map(lambda pti: pti.id,
folder.allowedContentTypes())
......@@ -297,24 +329,12 @@ class PreferenceTool(BaseTool):
This method exists here due to bootstrap issues.
It should work even if erp5_authentication_policy bt5 is not installed.
"""
# XXX: define an interface
def _isAuthenticationPolicyEnabled():
portal_preferences = self.getPortalObject().portal_preferences
method_id = 'isPreferredAuthenticationPolicyEnabled'
method = getattr(self, method_id, None)
if method is not None and method():
return True
return False
tv = getTransactionalVariable()
tv_key = 'PreferenceTool._isAuthenticationPolicyEnabled.%s' % getSecurityManager().getUser().getId()
if tv.get(tv_key, None) is None:
_isAuthenticationPolicyEnabled = CachingMethod(_isAuthenticationPolicyEnabled,
id='PortalPreferences_isAuthenticationPolicyEnabled',
cache_factory='erp5_content_short')
tv[tv_key] = _isAuthenticationPolicyEnabled()
return tv[tv_key]
# isPreferredAuthenticationPolicyEnabled exisss if property sheets from
# erp5_authentication_policy are installed.
method = getattr(self, 'isPreferredAuthenticationPolicyEnabled', None)
if method is not None and method():
return True
# if it does not exist, for sure authentication policy is not enabled.
return False
InitializeClass(PreferenceTool)
......@@ -29,7 +29,7 @@
##############################################################################
import unittest
from unittest import expectedFailure
import mock
from AccessControl.SecurityManagement import noSecurityManager
from AccessControl.SecurityManagement import getSecurityManager
......@@ -217,6 +217,12 @@ class TestPreferences(PropertySheetTestCase):
self.assertEqual(list(pref_tool.getPreference(
'preferred_accounting_transaction_simulation_state_list')),
list(person1.getPreferredAccountingTransactionSimulationStateList()))
# edits can also be made with setters
person1.setPreferredAccountingTransactionSimulationStateList(['planned'])
self.assertEqual(list(pref_tool.getPreference(
'preferred_accounting_transaction_simulation_state_list')),
list(person1.getPreferredAccountingTransactionSimulationStateList()))
# disable person -> group is selected
self.getWorkflowTool().doActionFor(person1,
'disable_action', wf_id='preference_workflow')
......@@ -526,6 +532,8 @@ class TestPreferences(PropertySheetTestCase):
# But they can see others
system_pref.view()
# check accessors works
self.assertEqual(['http://127.0.0.1'],
preference_tool.getPreferredDocumentConversionServerUrlList())
system_pref.setPreferredDocumentConversionServerUrlList(['http://1.2.3.4'])
self.tic()
self.assertEqual(['http://1.2.3.4'],
......@@ -679,7 +687,6 @@ class TestPreferences(PropertySheetTestCase):
self.assertEqual(system_preference_string,
portal_preferences.getDummystring())
@expectedFailure
def test_system_preference_value_prefererred_clear_cache_disabled(self):
default_preference_string = 'Default Name'
normal_preference_string = 'Normal Preference'
......@@ -687,36 +694,50 @@ class TestPreferences(PropertySheetTestCase):
self._addProperty('Preference',
'test_system_preference_value_prefererred_clear_cache_disabled Preference',
portal_type='Standard Property',
property_id='dummystring',
property_id='dummystring_cache_disabled',
property_default='python: "%s"' % default_preference_string,
preference=True,
elementary_type='string')
portal_preferences = self.portal.portal_preferences
self.assertEqual(default_preference_string,
portal_preferences.getDummystring())
self.assertEqual(
default_preference_string,
portal_preferences.getDummystringCacheDisabled())
preference = portal_preferences.newContent(portal_type='Preference',
dummystring=normal_preference_string,
priority=Priority.SITE)
preference = portal_preferences.newContent(
portal_type='Preference',
dummystring_cache_disabled=normal_preference_string,
priority=Priority.SITE)
preference.enable()
self.tic()
self.assertEqual(normal_preference_string,
portal_preferences.getDummystring())
self.assertEqual(
normal_preference_string,
portal_preferences.getDummystringCacheDisabled())
# simulate situation when _clearCache does nothing, for example in case
# if memcached or any other non-deleteable cache is used
from Products.ERP5Form.Document.Preference import Preference
Preference._clearCache = lambda *args,**kwargs: None
system_preference = portal_preferences.newContent(portal_type='System Preference',
dummystring=system_preference_string)
system_preference.enable()
self.tic()
self.assertEqual(system_preference_string,
portal_preferences.getDummystring())
def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestPreferences))
return suite
with mock.patch.object(
self.portal.portal_caches.__class__,
'clearAllCache') as clearAllCache,\
mock.patch.object(
self.portal.portal_caches.__class__,
'clearCacheFactory') as clearCacheFactory:
system_preference = portal_preferences.newContent(
portal_type='System Preference',
dummystring_cache_disabled=system_preference_string)
system_preference.enable()
self.tic()
self.assertEqual(
system_preference_string,
portal_preferences.getDummystringCacheDisabled())
system_preference.setDummystringCacheDisabled("changed")
self.tic()
self.assertEqual(
"changed",
portal_preferences.getDummystringCacheDisabled())
clearAllCache.assert_not_called()
clearCacheFactory.assert_not_called()
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment