Principals vocabularies: Fixed handling of results from multiple PAS …

…plugins (like pasldap and mutable_properties).

Closes #60

news/60.bugfix

@@ -0,0 +1 @@
+Principals vocabularies: Fixed handling of results from multiple PAS plugins (like pasldap and mutable_properties).

plone/app/vocabularies/principals.py

@@ -59,6 +59,22 @@ def token_from_principal_info(info, prefix=False):
     return '{0}__{1}'.format(info['principal_type'], info['id'])
 
 
+def merge_principal_infos(infos, prefix=False):
+    info = infos[0]
+    if len(infos) > 1:
+        principal_types = set([
+            info['principal_type'] for info in infos if info['principal_type']]
+        )
+        if len(principal_types) > 1:
+            # Principals with the same ID but different types. Should not
+            # happen.
+            raise ValueError('Principal ID not unique: {}'.format(info['id']))
+        info['title'] = ';'.join(
+            [info['title'] for info in infos if info['title']]
+        )
+    return info
+
+
 def _get_acl_users():
     return getToolByName(getSite(), 'acl_users')
 
@@ -219,14 +235,26 @@ def term_triples():
                 search = getattr(acl_users, search_cfg['search'])
                 searchargs = search_cfg['searchargs'].copy()
                 searchargs[search_cfg['searchattr']] = query
+                infotree = {}
                 for info in search(**searchargs):
-                    value = info['id']
-                    if cfg['prefix']:
-                        value = '{0}:{1}'.format(info['principal_type'], value)
-                    token = token_from_principal_info(
-                        info, prefix=cfg['prefix']
-                    )
-                    yield (value, token, info['title'])
+                    infotree.setdefault(
+                        info['id'], {}).setdefault(
+                            info['principal_type'], []).append(
+                                info)
+                for principal_id, types_infos in infotree.items():
+                    if len(types_infos) > 1 and not cfg['prefix']:
+                        raise ValueError('Principal ID not unique: {}'.format(
+                            principal_id))
+                    for principal_type, principal_infos in types_infos.items():
+                        value = principal_id
+                        info = merge_principal_infos(principal_infos)
+                        if cfg['prefix']:
+                            value = '{0}:{1}'.format(
+                                info['principal_type'], value)
+                        token = token_from_principal_info(
+                            info, prefix=cfg['prefix']
+                        )
+                        yield (value, token, info['title'])
 
         vocabulary = PrincipalsVocabulary(
             [

plone/app/vocabularies/tests/test_principals.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from plone.app.vocabularies.testing import PAVocabularies_INTEGRATION_TESTING
 
+import mock
 import unittest
 
 
@@ -387,3 +388,103 @@ def test_factory_principal_query(self):
         self.assertTrue('user:usér2' in vocab)
         self.assertTrue('group:groüp0' in vocab)
         self.assertTrue('group:groüp2' in vocab)
+
+    def test_factory_user_duplicate(self):
+        """ For an LDAP user that has logged in at least once, we get one
+        result each from pasldap and from mutable_properties. This should be
+        treated as one user.
+        """
+        with mock.patch(
+            'plone.app.vocabularies.principals._get_acl_users',
+        ) as fake_get_acl_users:
+            fake_get_acl_users.return_value.searchUsers.return_value = (
+                    {'id': 'ldapusér',
+                     'login': 'ldapusér',
+                     'pluginid': 'pasldap',
+                     'userid': 'ldapusér',
+                     'principal_type': 'user',
+                     'title': 'LDAP Usér'},
+                    {'id': 'ldapusér',
+                     'login': 'ldapusér',
+                     'title': '',
+                     'description': '',
+                     'email': '',
+                     'pluginid': 'mutable_properties',
+                     'userid': 'ldapusér',
+                     'principal_type': 'user'},
+            )
+            from plone.app.vocabularies.principals import UsersFactory
+
+            factory = UsersFactory()
+            vocab = factory(self.portal)
+            self.assertEqual(vocab.getTerm('ldapusér').title, 'LDAP Usér')
+
+    def test_factory_user_conflict(self):
+        """ In a user vocabulary, multiple results for the same principal ID
+        but with different principal_type values indicate some problem. Raise
+        an error.
+        """
+        with mock.patch(
+            'plone.app.vocabularies.principals._get_acl_users',
+        ) as fake_get_acl_users:
+            fake_get_acl_users.return_value.searchUsers.return_value = (
+                    {'id': 'ldapusér',
+                     'login': 'ldapusér',
+                     'pluginid': 'pasldap',
+                     'userid': 'ldapusér',
+                     'principal_type': 'user',
+                     'title': 'LDAP Usér'},
+                    {'id': 'ldapusér',
+                     'login': 'ldapusér',
+                     'title': '',
+                     'description': '',
+                     'email': '',
+                     'pluginid': 'mutable_properties',
+                     'userid': 'ldapusér',
+                     'principal_type': 'unknown'},
+            )
+            from plone.app.vocabularies.principals import UsersFactory
+
+            factory = UsersFactory()
+            self.assertRaises(
+                ValueError,
+                factory,
+                self.portal,
+            )
+
+    def test_factory_principal_conflict(self):
+        """ In a principal vocabulary, multiple results for the same principal
+        ID but with different principal_type values can be handled because they
+        are prefixed.
+        """
+        with mock.patch(
+            'plone.app.vocabularies.principals._get_acl_users',
+        ) as fake_get_acl_users:
+            fake_get_acl_users.return_value.searchUsers.return_value = (
+                    {'id': 'duplicaté',
+                     'login': 'duplicaté',
+                     'pluginid': 'pasldap',
+                     'userid': 'duplicaté',
+                     'principal_type': 'user',
+                     'title': 'Duplicaté User'},
+                    {'id': 'duplicaté',
+                     'login': 'duplicaté',
+                     'title': 'Duplicaté Group',
+                     'description': '',
+                     'email': '',
+                     'pluginid': 'source_groups',
+                     'userid': 'duplicaté',
+                     'principal_type': 'group'},
+            )
+            from plone.app.vocabularies.principals import PrincipalsFactory
+
+            factory = PrincipalsFactory()
+            vocab = factory(self.portal)
+            self.assertEqual(
+                vocab.getTerm('user:duplicaté').title,
+                'Duplicaté User',
+            )
+            self.assertEqual(
+                vocab.getTerm('group:duplicaté').title,
+                'Duplicaté Group',
+            )