Le 30/03/2012 22:54, Marius Gedminas a écrit :

On Fri, Mar 30, 2012 at 06:36:13PM +0200, Alexandre Garel wrote:

...

Hello,

I fill a bug report for zope.schema :

"zope.schema does not handle well nested object fields" https://bugs.launchpad.net/zope.schema/+bug/969350

my patch change a particular behaviour (binding of Choice field nested in Object field)

I explain why on the ticket.

Any comment would be appreciated.

Here's the diff attached to that bug:

Thanks for taking time.

As you can see below I'm ok for all your comment but one, which needs further discussion.

...

Index: src/zope/schema/tests/test_objectfield_nested.py

--- src/zope/schema/tests/test_objectfield_nested.py (révision 0) +++ src/zope/schema/tests/test_objectfield_nested.py (révision 0) @@ -0,0 +1,74 @@ +############################################################################## +# +# Copyright (c) 2001, 2002 Zope Foundation and Contributors.

Probably should be 2012 ;)

Hum copy/paste killed me :-)

...

+# All Rights Reserved. +# +# This software is subject to the provisions of the Zope Public License, +# Version 2.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. +# +############################################################################## +"""This test exercises Object fields in the case of nestment inside list

s/netsment/nesting/, although the sentence could use some rewriting.

ok for nesting

...

+to verify binding correcly happens at validation time +"""

+from unittest import TestCase, TestSuite, main, makeSuite

+from zope.interface import Interface, implementer +from zope.schema import Choice, Object, Set +from zope.schema.interfaces import IContextSourceBinder, WrongContainedType +from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm +from zope.testing.cleanup import CleanUp

+@implementer(IContextSourceBinder) +class EnumContext(object):

• def __call__(self, context):

•    return SimpleVocabulary([
  

•        SimpleTerm(value=v, token=v, title='Letter %s' % v)
  

•        for v in context])
  

+class IMultipleChoice(Interface):

• choices = Set(value_type=Choice(source=EnumContext()))

+@implementer(IMultipleChoice) +class Choices(object):

• def __iter__(self):

•    return iter(range(5))  # to have EnumSource work
  

EnumSource? Do you mean EnumContext?

Yes, I'll correct

...

• def __init__(self, choices):

•    self.choices = choices
  

+class IFavorites(Interface):

• fav = Object(title=u"Favorites number", schema=IMultipleChoice)

+class ObjectNestedTest(CleanUp, TestCase):

• """Test the Object Field in cases of nested fields"""
• def test_validate_nested(self):

•    context = range(5)
  

This is a bit confusing. IField.bind() is usually passed a context that is an object that has an attribute with that field. You're passing a list of numbers. Would the test also work if you had context = object()?

In fact as I just wan't to test validation, I did use a range. I would of course work with an object which is iterable (because EnumContext needs it)

...

•    # must not raise
  

•    IFavorites['fav'].bind(context).validate(Choices(set([1, 3])))
  

•    # checking against dictionnary do work
  

I don't see any dictionaries... Perhaps you meant 'checking of set values against the source works'?

Hum sorry, that may be a copy/paste remainder there again :-/

...

•    self.assertRaises(
  

•        WrongContainedType,
  

•        IFavorites['fav'].bind(context).validate, Choices(set([1, 8])))
  

+def test_suite():

• suite = TestSuite()
• suite.addTest(makeSuite(ObjectNestedTest))
• return suite

+if __name__ == '__main__':

• main(defaultTest='test_suite')

Index: src/zope/schema/_field.py

--- src/zope/schema/_field.py (révision 124803) +++ src/zope/schema/_field.py (copie de travail) @@ -492,12 +492,7 @@ if not IMethod.providedBy(schema[name]): try: attribute = schema[name]

•                if IChoice.providedBy(attribute):
  

•                    # Choice must be bound before validation otherwise
  

•                    # IContextSourceBinder is not iterable in validation
  

•                    bound = attribute.bind(value)
  

•                    bound.validate(getattr(value, name))
  

•                elif IField.providedBy(attribute):
  

•                if IField.providedBy(attribute):
                      # validate attributes that are fields
                      attribute.validate(getattr(value, name))
              except ValidationError as error:
  

@@ -510,6 +505,24 @@ return errors

+class BindedSchema(object):

BoundSchema would be a more grammatically correct name.

Ok I'll change that

...

• """This class proxies a schema to get its fields binded to a context

s/binded/bound/

ok

...

• """
• def __init__(self, schema, context):

•    self.schema = schema
  

•    self.context = context
  

• # we redefine getitem
• def __getitem__(self, name):

•    attr = self.schema[name]
  

•    return attr.bind(self.context)
  

Shouldn't this be

        return attr.bind(getattr(self.context, name))

That's what I discuss on the ticket. For me binding have to give a context for vocabulary. The problem is that validation has to take place at initialization time. The object may have no attribute 'name' at this time.

For example in cromlech.forms, or zeam.forms in the case of adding an object context is an Adder instance, not an object of the expected type.

Thus I prefer to give the upper context for binding of vocabularies, but I maybe wrong.

...

• # but let all the rest slip to schema
• def __getattr__(self, name):

•    return getattr(self.schema, name)
  

The comments don't really add anything to the code.

Ok.

...

• @implementer(IObject) class Object(Field): __doc__ = IObject.__doc__

@@ -521,6 +534,11 @@ self.schema = schema super(Object, self).__init__(**kw)

• def bind(self, context=None):

•    binded = super(Object, self).bind(context)
  

•    binded.schema = BindedSchema(self.schema, context)
  

s/binded/bound/

...

•    return binded
  

• def _validate(self, value):
      super(Object, self)._validate(value)
  

Marius Gedminas