[Zope3-dev] Symbolic Constants for security settings

[Steve Alexander]

Hi folks,

You may recall that a few weeks ago, I found a bug where strings were 
being used as symbolic constants in App/Security/Settings.py, and these 
lost identity when pickled and unpickled.

This was a problem because the security checking system, and the 
security UI, is written assuming that you can compare these symbolic 
constants by identity.

I fixed the problem by having each symbolic constant as its own class 
(that can't be instantiated), and implementing useful __str__, etc 
methods in a meta-class.

Well, I've revisited this code, and I'm thinking of checking in the 
following to replace it:

----
# Settings.py
#
# Copyright (c) 2001 Zope Coporation and Contributors.  All Rights Reserved.
#
# This software is subject to the provisions of the Zope Public License,
# Version 1.1 (ZPL).  A copy of the ZPL should accompany this distribution.
# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
# FOR A PARTICULAR PURPOSE.

""" Security setting constants """


class PermissionSetting(object):
     """PermissionSettings should be considered as immutable.
     They can be compared by identity. They are identified by
     their name.
     """

     def __new__(cls, name, description=None):
         """Keep a dict of PermissionSetting instances, indexed by
         name. If the name already exists in the dict, return that
         instance rather than creating a new one.
         """
         instances = cls.__dict__.get('__instances__')
         if instances is None:
             cls.__instances__ = instances = {}
         it = instances.get(name)
         if it is None:
             instances[name] = it = object.__new__(cls)
             it._init(name, description)
         return it

     def _init(self, name, description):
         self.__name = name
         self.__description = description

     def getDescription(self):
         return self.__description

     def getName(self):
         return self.__name

     def __str__(self):
         return "PermissionSetting: %s" % self.__name

# register PermissionSettings to be symbolic constants by identity,
# even when pickled and unpickled.
import copy_reg
copy_reg.constructor(PermissionSetting)
copy_reg.pickle(PermissionSetting, lambda ps: ps.getName(), 
PermissionSetting)



Allow = PermissionSetting('Allow',
     'Explicit allow setting for permissions')

Deny = PermissionSetting('Deny',
     'Explicit deny setting for permissions')

Unset = PermissionSetting('Unset',
     'Unset constant that denotes no setting for permission and role')

Assign = PermissionSetting('Assign',
     'Explicit assign setting for roles')

Remove = PermissionSetting('Remove',
     'Explicit remove setting for roles')

----

I've exchanged a fancy meta-class + class arrangement for a fancy 
__new__ + copy_reg arrangement.

I think this version is clearer, and it has fewer lines of code.
However, it is marginally less efficient when pickling and unpickling; 
although I think the difference really is marginal.

It is no different in terms of how fast these constants can be processed 
by the security machinery.

I've also noticed that there are no unit test checking that these 
symbolic constants retain identity when pickled and unpickled. I'll add 
a test for that in any case.

This change will cause any existing Zope 3 TTW security mappings to be 
ignored, as if they are not set.


Any comments?

If no-one screams, I'll check this in in a few hours.

--
Steve Alexander