[ckan-changes] commit/ckan: 7 new changesets

• Previous message: [ckan-changes] commit/ckanext-harvest: amercader: Set a flag to force harvesters to import objects
• Next message: [ckan-changes] commit/ckanext-inspire: amercader: Better message for metadata date exception
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

7 new changesets in ckan:

http://bitbucket.org/okfn/ckan/changeset/08cd38e6573b/
changeset:   r3070:08cd38e6573b
branch:      bug-1083-userobjectroles
user:        John Lawrence Aspden
date:        2011-05-09 21:47:51
summary:     [authz][s]: added test for multiple userobjectroles add/remove behaviour
affected #:  1 file (1.0 KB)

--- a/ckan/tests/models/test_authz.py	Mon May 09 18:30:31 2011 +0100
+++ b/ckan/tests/models/test_authz.py	Mon May 09 20:47:51 2011 +0100
@@ -293,6 +293,27 @@
                                              action=model.Action.READ,
                                              domain_object=self.anna)
 
+    def test_3_add_twice_remove_twice(self):
+        tester = model.User.by_name(u'tester')
+        war = model.Package.by_name(u'warandpeace')
+
+        def tester_roles():
+            return [x.role \
+             for x in model.Session.query(model.PackageRole).all() \
+             if x.user and x.user.name=='tester' and x.package.name==u'warandpeace']
+          
+        print tester_roles()
+        assert len(tester_roles()) == 0, "wrong number of roles for tester"
+        model.add_user_to_role(tester, model.Role.ADMIN, war)
+        assert len(tester_roles()) == 1, "wrong number of roles for tester"
+        model.add_user_to_role(tester, model.Role.ADMIN, war)
+        assert len(tester_roles()) == 1, "wrong number of roles for tester"
+        model.remove_user_from_role(tester, model.Role.ADMIN, war)
+        assert len(tester_roles()) == 0, "wrong number of roles for tester"
+        model.remove_user_from_role(tester, model.Role.ADMIN, war)
+        assert len(tester_roles()) == 0, "wrong number of roles for tester"
+
+
 class TestMigrate:
     @classmethod
     def setup_class(self):


http://bitbucket.org/okfn/ckan/changeset/2122990d9c4e/
changeset:   r3071:2122990d9c4e
branch:      bug-1083-userobjectroles
user:        John Lawrence Aspden
date:        2011-05-09 22:11:45
summary:     [authz][s]: Resign self to having to call commit_and_remove whenever adding roles.
It appears that it's by design that add_user_to_role doesn't commit, since changing it breaks many things.
affected #:  1 file (79 bytes)

--- a/ckan/tests/models/test_authz.py	Mon May 09 20:47:51 2011 +0100
+++ b/ckan/tests/models/test_authz.py	Mon May 09 21:11:45 2011 +0100
@@ -305,8 +305,11 @@
         print tester_roles()
         assert len(tester_roles()) == 0, "wrong number of roles for tester"
         model.add_user_to_role(tester, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
         assert len(tester_roles()) == 1, "wrong number of roles for tester"
         model.add_user_to_role(tester, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
+
         assert len(tester_roles()) == 1, "wrong number of roles for tester"
         model.remove_user_from_role(tester, model.Role.ADMIN, war)
         assert len(tester_roles()) == 0, "wrong number of roles for tester"


http://bitbucket.org/okfn/ckan/changeset/f26028f75918/
changeset:   r3072:f26028f75918
branch:      bug-1083-userobjectroles
user:        John Lawrence Aspden
date:        2011-05-09 23:06:51
summary:     [authz][s]: if we've somehow got several identical userobjectroles in our database, remove them all rather than bombing
affected #:  1 file (526 bytes)

--- a/ckan/model/authz.py	Mon May 09 21:11:45 2011 +0100
+++ b/ckan/model/authz.py	Mon May 09 22:06:51 2011 +0100
@@ -187,15 +187,21 @@
 
     @classmethod
     def add_user_to_role(cls, user, role, domain_obj):
-        '''role assignment already exists
-        NB: leaves caller to commit change'''
+        '''NB: Leaves the caller to commit the change. If called twice without a
+        commit, will add the role to the database twice. Since some other
+        functions count the number of occurrences, that leaves a fairly obvious
+        bug. But adding a commit here seems to break various tests.
+        So don't call this twice without committing, I guess...
+        '''
+        # Here we're trying to guard against adding the same role twice, but
+        # that won't work if the transaction hasn't been committed yet, which allows a role to be added twice (you can do this from the interface)
         if cls.user_has_role(user, role, domain_obj):
             return
         objectrole = cls(role=role, user=user)
         if cls.name is not None:
             setattr(objectrole, cls.name, domain_obj)
         Session.add(objectrole)
-        
+         
     @classmethod
     def add_authorization_group_to_role(cls, authorization_group, role, domain_obj):
         # role assignment already exists
@@ -209,8 +215,8 @@
     @classmethod
     def remove_user_from_role(cls, user, role, domain_obj):
         q = cls._user_query(user, role, domain_obj)
-        uo_role = q.one()
-        Session.delete(uo_role)
+        for uo_role in q.all():
+            Session.delete(uo_role)
         Session.commit()
         Session.remove()
 


http://bitbucket.org/okfn/ckan/changeset/e5df7023cff5/
changeset:   r3073:e5df7023cff5
branch:      bug-1083-userobjectroles
user:        John Lawrence Aspden
date:        2011-05-10 21:24:58
summary:     [authz][s]: add failing test for authzgroups
affected #:  1 file (1.2 KB)

--- a/ckan/tests/models/test_authz.py	Mon May 09 22:06:51 2011 +0100
+++ b/ckan/tests/models/test_authz.py	Tue May 10 20:24:58 2011 +0100
@@ -214,7 +214,8 @@
         mreditor = model.User(name=u'mreditor')
         mrreader = model.User(name=u'mrreader')
         tester = model.User(name=u'tester')
-        for obj in [anna, war, mradmin, mreditor, mrreader, tester]:
+        anauthzgroup = model.AuthorizationGroup(name=u'anauthzgroup')
+        for obj in [anna, war, mradmin, mreditor, mrreader, tester, anauthzgroup]:
             model.Session.add(obj)
         model.repo.commit_and_remove()
 
@@ -302,7 +303,6 @@
              for x in model.Session.query(model.PackageRole).all() \
              if x.user and x.user.name=='tester' and x.package.name==u'warandpeace']
           
-        print tester_roles()
         assert len(tester_roles()) == 0, "wrong number of roles for tester"
         model.add_user_to_role(tester, model.Role.ADMIN, war)
         model.repo.commit_and_remove()
@@ -316,6 +316,30 @@
         model.remove_user_from_role(tester, model.Role.ADMIN, war)
         assert len(tester_roles()) == 0, "wrong number of roles for tester"
 
+    def test_4_add_twice_remove_twice_for_authzgroups(self):
+        aag = model.AuthorizationGroup.by_name(u'anauthzgroup')
+        war = model.Package.by_name(u'warandpeace')
+
+        def aag_roles():
+            return [x.role \
+             for x in model.Session.query(model.PackageRole).all() \
+             if x.authorized_group and x.authorized_group.name=='anauthzgroup' and x.package.name==u'warandpeace']
+          
+        assert len(aag_roles()) == 0, "wrong number of roles for anauthzgroup"
+        model.add_authorization_group_to_role(aag, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
+        assert len(aag_roles()) == 1, "wrong number of roles for anauthzgroup"
+        model.add_authorization_group_to_role(aag, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
+
+        assert len(aag_roles()) == 1, "wrong number of roles for anauthzgroup"
+        model.remove_authorization_group_from_role(aag, model.Role.ADMIN, war)
+        assert len(aag_roles()) == 0, "wrong number of roles for anauthzgroup"
+        model.remove_authorization_group_from_role(aag, model.Role.ADMIN, war)
+        assert len(aag_roles()) == 0, "wrong number of roles for anauthzgroup"
+
+
+
 
 class TestMigrate:
     @classmethod


http://bitbucket.org/okfn/ckan/changeset/b56aec870933/
changeset:   r3074:b56aec870933
branch:      bug-1083-userobjectroles
user:        John Lawrence Aspden
date:        2011-05-10 21:27:38
summary:     [authz][s]: corresponding fix for authzgroup removal
affected #:  1 file (348 bytes)

--- a/ckan/model/authz.py	Tue May 10 20:24:58 2011 +0100
+++ b/ckan/model/authz.py	Tue May 10 20:27:38 2011 +0100
@@ -204,7 +204,12 @@
          
     @classmethod
     def add_authorization_group_to_role(cls, authorization_group, role, domain_obj):
-        # role assignment already exists
+        '''NB: Leaves the caller to commit the change. If called twice without a
+        commit, will add the role to the database twice. Since some other
+        functions count the number of occurrences, that leaves a fairly obvious
+        bug. But adding a commit here seems to break various tests.
+        So don't call this twice without committing, I guess...
+        '''
         if cls.authorization_group_has_role(authorization_group, role, domain_obj):
             return
         objectrole = cls(role=role, authorized_group=authorization_group)
@@ -223,8 +228,8 @@
     @classmethod
     def remove_authorization_group_from_role(cls, authorization_group, role, domain_obj):
         q = cls._authorized_group_query(authorization_group, role, domain_obj)
-        ago_role = q.one()
-        Session.delete(ago_role)
+        for ago_role in q.all():
+            Session.delete(ago_role)
         Session.commit()
         Session.remove()
 


http://bitbucket.org/okfn/ckan/changeset/bad4a415ba75/
changeset:   r3075:bad4a415ba75
branch:      bug-1083-userobjectroles
user:        John Lawrence Aspden
date:        2011-05-10 21:50:59
summary:     close branch after full test run
affected #:  0 files (0 bytes)

http://bitbucket.org/okfn/ckan/changeset/0bc1ed5ea0c3/
changeset:   r3076:0bc1ed5ea0c3
user:        John Lawrence Aspden
date:        2011-05-10 22:07:00
summary:     Merge
affected #:  2 files (3.2 KB)

--- a/ckan/model/authz.py	Mon May 09 18:30:31 2011 +0100
+++ b/ckan/model/authz.py	Tue May 10 21:07:00 2011 +0100
@@ -187,18 +187,29 @@
 
     @classmethod
     def add_user_to_role(cls, user, role, domain_obj):
-        '''role assignment already exists
-        NB: leaves caller to commit change'''
+        '''NB: Leaves the caller to commit the change. If called twice without a
+        commit, will add the role to the database twice. Since some other
+        functions count the number of occurrences, that leaves a fairly obvious
+        bug. But adding a commit here seems to break various tests.
+        So don't call this twice without committing, I guess...
+        '''
+        # Here we're trying to guard against adding the same role twice, but
+        # that won't work if the transaction hasn't been committed yet, which allows a role to be added twice (you can do this from the interface)
         if cls.user_has_role(user, role, domain_obj):
             return
         objectrole = cls(role=role, user=user)
         if cls.name is not None:
             setattr(objectrole, cls.name, domain_obj)
         Session.add(objectrole)
-        
+         
     @classmethod
     def add_authorization_group_to_role(cls, authorization_group, role, domain_obj):
-        # role assignment already exists
+        '''NB: Leaves the caller to commit the change. If called twice without a
+        commit, will add the role to the database twice. Since some other
+        functions count the number of occurrences, that leaves a fairly obvious
+        bug. But adding a commit here seems to break various tests.
+        So don't call this twice without committing, I guess...
+        '''
         if cls.authorization_group_has_role(authorization_group, role, domain_obj):
             return
         objectrole = cls(role=role, authorized_group=authorization_group)
@@ -209,16 +220,16 @@
     @classmethod
     def remove_user_from_role(cls, user, role, domain_obj):
         q = cls._user_query(user, role, domain_obj)
-        uo_role = q.one()
-        Session.delete(uo_role)
+        for uo_role in q.all():
+            Session.delete(uo_role)
         Session.commit()
         Session.remove()
 
     @classmethod
     def remove_authorization_group_from_role(cls, authorization_group, role, domain_obj):
         q = cls._authorized_group_query(authorization_group, role, domain_obj)
-        ago_role = q.one()
-        Session.delete(ago_role)
+        for ago_role in q.all():
+            Session.delete(ago_role)
         Session.commit()
         Session.remove()
 


--- a/ckan/tests/models/test_authz.py	Mon May 09 18:30:31 2011 +0100
+++ b/ckan/tests/models/test_authz.py	Tue May 10 21:07:00 2011 +0100
@@ -214,7 +214,8 @@
         mreditor = model.User(name=u'mreditor')
         mrreader = model.User(name=u'mrreader')
         tester = model.User(name=u'tester')
-        for obj in [anna, war, mradmin, mreditor, mrreader, tester]:
+        anauthzgroup = model.AuthorizationGroup(name=u'anauthzgroup')
+        for obj in [anna, war, mradmin, mreditor, mrreader, tester, anauthzgroup]:
             model.Session.add(obj)
         model.repo.commit_and_remove()
 
@@ -293,6 +294,53 @@
                                              action=model.Action.READ,
                                              domain_object=self.anna)
 
+    def test_3_add_twice_remove_twice(self):
+        tester = model.User.by_name(u'tester')
+        war = model.Package.by_name(u'warandpeace')
+
+        def tester_roles():
+            return [x.role \
+             for x in model.Session.query(model.PackageRole).all() \
+             if x.user and x.user.name=='tester' and x.package.name==u'warandpeace']
+          
+        assert len(tester_roles()) == 0, "wrong number of roles for tester"
+        model.add_user_to_role(tester, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
+        assert len(tester_roles()) == 1, "wrong number of roles for tester"
+        model.add_user_to_role(tester, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
+
+        assert len(tester_roles()) == 1, "wrong number of roles for tester"
+        model.remove_user_from_role(tester, model.Role.ADMIN, war)
+        assert len(tester_roles()) == 0, "wrong number of roles for tester"
+        model.remove_user_from_role(tester, model.Role.ADMIN, war)
+        assert len(tester_roles()) == 0, "wrong number of roles for tester"
+
+    def test_4_add_twice_remove_twice_for_authzgroups(self):
+        aag = model.AuthorizationGroup.by_name(u'anauthzgroup')
+        war = model.Package.by_name(u'warandpeace')
+
+        def aag_roles():
+            return [x.role \
+             for x in model.Session.query(model.PackageRole).all() \
+             if x.authorized_group and x.authorized_group.name=='anauthzgroup' and x.package.name==u'warandpeace']
+          
+        assert len(aag_roles()) == 0, "wrong number of roles for anauthzgroup"
+        model.add_authorization_group_to_role(aag, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
+        assert len(aag_roles()) == 1, "wrong number of roles for anauthzgroup"
+        model.add_authorization_group_to_role(aag, model.Role.ADMIN, war)
+        model.repo.commit_and_remove()
+
+        assert len(aag_roles()) == 1, "wrong number of roles for anauthzgroup"
+        model.remove_authorization_group_from_role(aag, model.Role.ADMIN, war)
+        assert len(aag_roles()) == 0, "wrong number of roles for anauthzgroup"
+        model.remove_authorization_group_from_role(aag, model.Role.ADMIN, war)
+        assert len(aag_roles()) == 0, "wrong number of roles for anauthzgroup"
+
+
+
+
 class TestMigrate:
     @classmethod
     def setup_class(self):

Repository URL: https://bitbucket.org/okfn/ckan/

--

This is a commit notification from bitbucket.org. You are receiving
this because you have the service enabled, addressing the recipient of
this email.

• Previous message: [ckan-changes] commit/ckanext-harvest: amercader: Set a flag to force harvesters to import objects
• Next message: [ckan-changes] commit/ckanext-inspire: amercader: Better message for metadata date exception
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]