[Zope3-checkins] Re: [Checkins] SVN: Zope3/branches/3.3/ - fix for issue 517: intid might generate long instead of int which conflicts with btrees

[Tim Peters]

[Christian Theune]
> Log message for revision 68700:
>    - fix for issue 517: intid might generate long instead of int which conflicts with btrees
>
>
> Changed:
>   U   Zope3/branches/3.3/doc/CHANGES.txt
>   U   Zope3/branches/3.3/src/zope/app/intid/__init__.py
>   U   Zope3/branches/3.3/src/zope/app/intid/tests.py
>
> -=-
> Modified: Zope3/branches/3.3/doc/CHANGES.txt
> ===================================================================
> --- Zope3/branches/3.3/doc/CHANGES.txt  2006-06-17 03:29:43 UTC (rev 68699)
> +++ Zope3/branches/3.3/doc/CHANGES.txt  2006-06-17 04:10:29 UTC (rev 68700)
> @@ -10,6 +10,9 @@
>
>      Bugfixes
>
> +      - Fixed issue 517: Bug in intid that doesn't respect that BTrees can not
> +        handle long.
> +
>        - Fixed issue 399: traversal of missing skins now give 404 / NotFound
>          instead of 505 / ComponentLookupError
>
>
> Modified: Zope3/branches/3.3/src/zope/app/intid/__init__.py
> ===================================================================
> --- Zope3/branches/3.3/src/zope/app/intid/__init__.py   2006-06-17 03:29:43 UTC (rev 68699)
> +++ Zope3/branches/3.3/src/zope/app/intid/__init__.py   2006-06-17 04:10:29 UTC (rev 68700)
> @@ -47,7 +47,10 @@
>      """
>      implements(IIntIds)
>
> -    _v_nextid = None
> +    _v_nextid = None
> +
> +    # Used for testability of random function
> +    __randint__ = random.randint
>
>      def __init__(self):
>          self.ids = OIBTree.OIBTree()
> @@ -97,11 +100,16 @@
>          """
>          while True:
>              if self._v_nextid is None:
> -                self._v_nextid = random.randint(0, 2**31)
> +                self._v_nextid = self.__randint__(0, 2**31)
>              uid = self._v_nextid
>              self._v_nextid += 1
> -            if uid not in self.refs:
> -                return uid
> +            try:
> +                if uid not in self.refs:
> +                    return uid
> +            except TypeError:
> +                # uid was a long instead of int and btree complained
> +                # we just try again
> +                pass
>              self._v_nextid = None

Allow me to suggest that a simpler fix would have been to replace:

                self._v_nextid = random.randint(0, 2**31)

with:

                self._v_nextid = int(random.randrange(2**31))

and that's all.  Using randint() with an upper bound of 2**31 was
simply wrong.  Using randrange() with that upper bound corrects that
mistake.  Unfortunatey, the int() is needed if (and only if) randrange
happens to return 2**31-1, which _fits_ in an int but is returned as a
long by randrange() due to a miserable platform-from-Mars portability
hack in Python's internal float->int conversion.

As a bonus, the suggested way should run faster than the original way
too (randint() calls randrange() under the covers, so using
randrange() directly removes a layer of Python-level function  call).