Merge pull request #115 from plone/maurits-schema-caching-fix-52

Fix user schema caching for 5.2

news/76.bugfix

@@ -0,0 +1,4 @@
+For user schemas use a volatile cache on the request instead of on the portal.
+This prevents seeing an empty user profile when you have custom user schemas.
+This fixes `issue 76 <https://github.com/plone/plone.app.users/issues/76>`_.
+[maurits]

plone/app/users/browser/account.py

@@ -1,7 +1,9 @@
 # -*- coding: utf-8 -*-
 from AccessControl import Unauthorized
 from Acquisition import aq_inner
+from plone.app.layout.navigation.interfaces import INavigationRoot
 from plone.app.users.browser.interfaces import IAccountPanelForm
+from plone.app.users.browser.schemaeditor import getFromBaseSchema
 from plone.app.users.utils import notifyWidgetActionExecutionError
 from plone.autoform.form import AutoExtensibleForm
 from plone.namedfile.file import NamedBlobImage
@@ -10,6 +12,7 @@
 from Products.CMFCore.utils import getToolByName
 from Products.CMFPlone import PloneMessageFactory as _
 from Products.CMFPlone.controlpanel.events import ConfigurationChangedEvent
+from Products.CMFPlone.interfaces import IPloneSiteRoot
 from Products.CMFPlone.interfaces import ISecuritySchema
 from Products.CMFPlone.utils import safe_unicode
 from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
@@ -21,7 +24,9 @@
 from zope.cachedescriptors.property import Lazy as lazy_property
 from zope.component import getMultiAdapter
 from zope.component import getUtility
+from zope.component import provideAdapter
 from zope.event import notify
+from zope.globalrequest import getRequest
 from zope.interface import implementer
 from ZTUtils import make_query
 
@@ -38,6 +43,44 @@
                u"name. Please choose another."))
 
 
+def getSchema(schema_interface, schema_adapter, form_name=None):
+    request = getRequest()
+    form_name_to_request_attr_name = {
+        "In User Profile": "_userdata_schema",
+        "On Registration": "_register_schema",
+        None: "_userdata_manager_schema",
+    }
+    request_attr_name = form_name_to_request_attr_name.pop(form_name, None)
+    if request_attr_name is not None:
+        schema = getattr(request, request_attr_name, None)
+    else:
+        schema = None
+    if schema is None:
+        schema = getFromBaseSchema(
+            schema_interface,
+            form_name=form_name
+        )
+        # Unset all request attr names.
+        # We do not want other caches to linger.
+        # See https://github.com/plone/plone.app.users/issues/76
+        # This is in the unlikely case that you visit both the add-user/register form
+        # and the user/personal-information form in one request,
+        # maybe during a migration.
+        for name in form_name_to_request_attr_name.values():
+            try:
+                delattr(request, name)
+            except AttributeError:
+                pass
+        if request_attr_name is not None:
+            setattr(request, request_attr_name, schema)
+        # As schema is a generated supermodel,
+        # needed adapters can only be registered at run time.
+        # Note that this overrides previous adapters for the same interfaces.
+        provideAdapter(schema_adapter, (IPloneSiteRoot,), schema)
+        provideAdapter(schema_adapter, (INavigationRoot,), schema)
+    return schema
+
+
 def isDefaultPortrait(value, portal):
     default_portrait_value = getattr(portal, default_portrait, None)
     return aq_inner(value) == aq_inner(default_portrait_value)

plone/app/users/browser/register.py

@@ -2,9 +2,9 @@
 from AccessControl import getSecurityManager
 from Acquisition import aq_inner
 from plone.app.users.browser.account import AccountPanelSchemaAdapter
+from plone.app.users.browser.account import getSchema
 from plone.app.users.browser.interfaces import ILoginNameGenerator
 from plone.app.users.browser.interfaces import IUserIdGenerator
-from plone.app.users.browser.schemaeditor import getFromBaseSchema
 from plone.app.users.schema import IAddUserSchema
 from plone.app.users.schema import ICombinedRegisterSchema
 from plone.app.users.schema import IRegisterSchema
@@ -17,10 +17,8 @@
 from Products.CMFCore.permissions import ManagePortal
 from Products.CMFCore.utils import getToolByName
 from Products.CMFPlone import PloneMessageFactory as _
-from Products.CMFPlone.interfaces import IPloneSiteRoot
 from Products.CMFPlone.interfaces import ISecuritySchema
 from Products.CMFPlone.interfaces import IUserGroupsSettingsSchema
-from Products.CMFPlone.utils import get_portal
 from Products.CMFPlone.utils import normalizeString
 from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
 from Products.statusmessages.interfaces import IStatusMessage
@@ -34,7 +32,6 @@
 from zope.component import getAdapter
 from zope.component import getMultiAdapter
 from zope.component import getUtility
-from zope.component import provideAdapter
 from zope.component import queryUtility
 from zope.schema import getFieldNames
 
@@ -47,16 +44,11 @@
 
 
 def getRegisterSchema():
-    portal = get_portal()
-    schema = getattr(portal, '_v_register_schema', None)
-    if schema is None:
-        portal._v_register_schema = schema = getFromBaseSchema(
-            ICombinedRegisterSchema,
-            form_name=u'On Registration'
-        )
-        # as schema is a generated supermodel,
-        # needed adapters can only be registered at run time
-        provideAdapter(AccountPanelSchemaAdapter, (IPloneSiteRoot,), schema)
+    schema = getSchema(
+        ICombinedRegisterSchema,
+        AccountPanelSchemaAdapter,
+        form_name='On Registration',
+    )
     return schema
 
 
@@ -720,4 +712,3 @@ def action_join(self, action):
         self.request.response.redirect(
             self.context.absolute_url() +
             '/@@usergroup-userprefs?searchstring=' + user_id)
-

plone/app/users/browser/userdatapanel.py

@@ -1,20 +1,18 @@
 # -*- coding: utf-8 -*-
-from plone.app.layout.navigation.interfaces import INavigationRoot
+from AccessControl.SecurityManagement import getSecurityManager
 from plone.app.users.browser.account import AccountPanelForm
 from plone.app.users.browser.account import AccountPanelSchemaAdapter
-from plone.app.users.browser.schemaeditor import getFromBaseSchema
+from plone.app.users.browser.account import getSchema
 from plone.app.users.schema import IUserDataSchema
 from plone.registry.interfaces import IRegistry
 from Products.CMFCore.utils import getToolByName
 from Products.CMFPlone import PloneMessageFactory as _
-from Products.CMFPlone.interfaces import IPloneSiteRoot
 from Products.CMFPlone.interfaces import ISecuritySchema
 from Products.CMFPlone.utils import get_portal
 from Products.CMFPlone.utils import set_own_login_name
 from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
 from zExceptions import NotFound
 from zope.component import getUtility
-from zope.component import provideAdapter
 
 try:
     from html import escape
@@ -99,19 +97,19 @@ def __call__(self):
 
 def getUserDataSchema():
     portal = get_portal()
-    schema = getattr(portal, '_v_userdata_schema', None)
-    if schema is None:
-        portal._v_userdata_schema = schema = getFromBaseSchema(
-            IUserDataSchema,
-            form_name=u'In User Profile'
-        )
-        # as schema is a generated supermodel,
-        # needed adapters can only be registered at run time
-        provideAdapter(UserDataPanelAdapter, (IPloneSiteRoot,), schema)
-        provideAdapter(UserDataPanelAdapter, (INavigationRoot,), schema)
+    form_name = u'In User Profile'
+    # This is needed on Plone 6, but has a bad side effect on Plone 5:
+    # as Manager you go to a member and then to your own personal-information
+    # form and you see the data of the member you just visited.
+    # I keep the code here commented out as warning in case someone compares
+    # the code.
+    # if getSecurityManager().checkPermission('Manage portal', portal):
+    #     form_name = None
+    schema = getSchema(IUserDataSchema, UserDataPanelAdapter, form_name=form_name)
     return schema
 
 
 class UserDataConfiglet(UserDataPanel):
     """Control panel version of the userdata panel"""
     template = ViewPageTemplateFile('account-configlet.pt')
+    tab = "userdata"

plone/app/users/tests/test_schema_types.py

@@ -1,10 +1,11 @@
 # -*- coding: utf-8 -*-
 from pkg_resources import resource_stream
+from plone.app.testing import SITE_OWNER_NAME
+from plone.app.testing import SITE_OWNER_PASSWORD
 from plone.app.testing import TEST_USER_ID
 from plone.app.testing import TEST_USER_NAME
 from plone.app.testing import TEST_USER_PASSWORD
 from plone.app.users.setuphandlers import import_schema
-from plone.app.users.testing import PLONE_APP_USERS_FUNCTIONAL_TESTING
 from plone.testing.z2 import Browser
 from Products.GenericSetup.tests.common import DummyImportContext
 from plone.app.users.tests.base import BaseTestCase
@@ -105,6 +106,7 @@ def setUp(self):
         transaction.commit()
 
         self.browser = Browser(self.layer['app'])
+        self.browser.handleErrors = False
         self.request = self.layer['request']
 
     def test_schema_types(self):
@@ -155,3 +157,119 @@ def test_schema_types(self):
         self.assertEqual(member.getProperty('pi'), 3.14)
         self.assertTrue(isinstance(member.getProperty('vegetarian'), bool))
         self.assertEqual(member.getProperty('vegetarian'), True)
+
+    def test_regression_76_user_information(self):
+        # Test that issue 76 does not return: user info sometimes appears empty.
+        # https://github.com/plone/plone.app.users/issues/76
+        # Here we test as admin.
+        portal_url = self.portal.absolute_url()
+        self.browser.open(portal_url)
+        self.browser.getLink('Log in').click()
+        self.browser.getControl('Login Name').value = SITE_OWNER_NAME
+        self.browser.getControl('Password').value = SITE_OWNER_PASSWORD
+        self.browser.getControl('Log in').click()
+
+        # Set information for the test user.
+        info_page = "{}/@@user-information?userid={}".format(
+            portal_url, TEST_USER_ID,
+        )
+        self.browser.open(info_page)
+        self.browser.getControl('Full Name').value = 'Isaac Newton'
+        self.browser.getControl('Email').value = 'isaac@cambridge.com'
+        self.browser.getControl('Age').value = '40'
+        self.browser.getControl('Save').click()
+
+        # Open the page again, check that the information is set.
+        self.browser.open(info_page)
+        self.assertEqual(self.browser.getControl('Full Name').value, 'Isaac Newton')
+        self.assertEqual(self.browser.getControl('Email').value, 'isaac@cambridge.com')
+        self.assertEqual(self.browser.getControl('Age').value, '40')
+
+        # Opening the new-user/register page used to be enough to trigger the problem.
+        self.browser.open("{}/@@new-user".format(portal_url))
+
+        # Any next calls to the user or personal information pages would show empty.
+        self.browser.open(info_page)
+        self.assertEqual(self.browser.getControl('Full Name').value, 'Isaac Newton')
+        self.assertEqual(self.browser.getControl('Email').value, 'isaac@cambridge.com')
+        self.assertEqual(self.browser.getControl('Age').value, '40')
+
+    def _enable_self_registration(self):
+        from plone.registry.interfaces import IRegistry
+        from Products.CMFPlone.interfaces import ISecuritySchema
+        from zope.component import getUtility
+
+        self.portal.manage_permission('Add portal member', roles=['Anonymous'])
+        registry = getUtility(IRegistry)
+        security_settings = registry.forInterface(ISecuritySchema, prefix="plone")
+        security_settings.enable_user_pwd_choice = True
+        transaction.commit()
+
+    def test_regression_76_personal_information(self):
+        # Test that issue 76 does not return: personal info sometimes appears empty.
+        # https://github.com/plone/plone.app.users/issues/76
+        # Here we test as user.
+        portal_url = self.portal.absolute_url()
+        self.browser.open(portal_url)
+        self.browser.getLink('Log in').click()
+        self.browser.getControl('Login Name').value = TEST_USER_NAME
+        self.browser.getControl('Password').value = TEST_USER_PASSWORD
+        self.browser.getControl('Log in').click()
+
+        # Set information for the test user.
+        info_page = "{}/@@personal-information".format(portal_url)
+        self.browser.open(info_page)
+        self.browser.getControl('Full Name').value = 'Isaac Newton'
+        self.browser.getControl('Email').value = 'isaac@cambridge.com'
+        self.browser.getControl('Age').value = '40'
+        self.browser.getControl('Save').click()
+
+        # Open the page again, check that the information is set.
+        self.browser.open(info_page)
+        self.assertEqual(self.browser.getControl('Full Name').value, 'Isaac Newton')
+        self.assertEqual(self.browser.getControl('Email').value, 'isaac@cambridge.com')
+        self.assertEqual(self.browser.getControl('Age').value, '40')
+
+        # Enable self registration.
+        self._enable_self_registration()
+
+        # Opening the new-user/register page used to be enough to trigger the problem.
+        # Logout, try it, and login again.
+        self.browser.open("{}/@@logout".format(portal_url))
+        self.browser.open("{}/@@register".format(portal_url))
+        # Check that the registration page is loading correctly.
+        self.assertNotIn("This site doesn't have a valid email setup", self.browser.contents)
+        self.assertIn("Enter your new password.", self.browser.contents)
+
+        self.browser.open("{}/@@login".format(portal_url))
+        self.browser.getControl('Login Name').value = TEST_USER_NAME
+        self.browser.getControl('Password').value = TEST_USER_PASSWORD
+        self.browser.getControl('Log in').click()
+
+        # Any next calls to the user or personal information pages would show empty.
+        self.browser.open(info_page)
+        self.assertEqual(self.browser.getControl('Full Name').value, 'Isaac Newton')
+        self.assertEqual(self.browser.getControl('Email').value, 'isaac@cambridge.com')
+        self.assertEqual(self.browser.getControl('Age').value, '40')
+
+        # Now login as Manager.
+        self.browser.getLink('Log out').click()
+        self.browser.getLink('Log in').click()
+        self.browser.getControl('Login Name').value = SITE_OWNER_NAME
+        self.browser.getControl('Password').value = SITE_OWNER_PASSWORD
+        self.browser.getControl('Log in').click()
+
+        # Check the information page of the user.
+        self.browser.open("{}/@@user-information?userid={}".format(
+            portal_url, TEST_USER_ID
+        ))
+        self.assertEqual(self.browser.getControl('Full Name').value, 'Isaac Newton')
+        self.assertEqual(self.browser.getControl('Email').value, 'isaac@cambridge.com')
+        self.assertEqual(self.browser.getControl('Age').value, '40')
+
+        # Check the personal information page of the manager.
+        # Nothing should be visible here.
+        self.browser.open(info_page)
+        self.assertEqual(self.browser.getControl('Full Name').value, '')
+        self.assertEqual(self.browser.getControl('Email').value, '')
+        self.assertEqual(self.browser.getControl('Age').value, '0')