Re: [Zope-dev] [Checkins] SVN: z3c.form/trunk/ merged branch icemac_validate_NOT_CHANGED
Hello Michael, After some simple thinking there are some fears that this wrecks other basic fields validation. Something like the field is required, has a default value but is omitted on input. What will be the result? Accepting the default value instead of raising an error is definitely a problem. Also, values in the system might not be needed to re-validate? Image passing around a 100mb uploaded file. If this is meant only for a file upload field, then I think there should be a special validator registered for those. Would be nice to see some (functional) tests closing out the above fears. Functional, because on the unittest level you can force the widget to a lot of unexpected things, but that might not be inline with what happens when it's really done by the framework. Or better said, revert the changes locally, add those tests, apply the changes back and see whether the tests still pass. Sunday, May 17, 2009, 3:21:19 PM, you wrote: MH> Log message for revision 100028: MH> merged branch icemac_validate_NOT_CHANGED MH> MH> Changed: MH> U z3c.form/trunk/AUTHOR.txt MH> U z3c.form/trunk/CHANGES.txt MH> U z3c.form/trunk/src/z3c/form/validator.py MH> U z3c.form/trunk/src/z3c/form/validator.txt MH> -=- MH> Modified: z3c.form/trunk/AUTHOR.txt MH> =================================================================== MH> --- z3c.form/trunk/AUTHOR.txt 2009-05-17 13:18:11 UTC (rev 100027) MH> +++ z3c.form/trunk/AUTHOR.txt 2009-05-17 13:21:18 UTC (rev 100028) MH> @@ -11,10 +11,11 @@ MH> Daniel Nouri MH> Darryl Cousins MH> Herman Himmelbauer MH> +Jacob Holm MH> Laurent Mignon MH> Malthe Borch MH> Marius Gedminas MH> Martijn Faassen MH> Michael Howitz MH> Michael Kerrin MH> -Paul Carduner MH> +Paul Carduner MH> \ No newline at end of file MH> Modified: z3c.form/trunk/CHANGES.txt MH> =================================================================== MH> --- z3c.form/trunk/CHANGES.txt 2009-05-17 13:18:11 UTC (rev 100027) MH> +++ z3c.form/trunk/CHANGES.txt 2009-05-17 13:21:18 UTC (rev 100028) MH> @@ -150,7 +150,11 @@ MH> MH> - Bug: Don't cause warnings in Python 2.6. MH> MH> +- Bug: `validator.SimpleFieldValidator` is now able to handle MH> + `interfaces.NOT_CHANGED`. This value is set for file uploads when MH> + the user does not choose a file for upload. MH> MH> + MH> Version 1.9.0 (2008-08-26) MH> -------------------------- MH> MH> Modified: z3c.form/trunk/src/z3c/form/validator.py MH> =================================================================== MH> --- z3c.form/trunk/src/z3c/form/validator.py 2009-05-17 13:18:11 UTC (rev 100027) MH> +++ z3c.form/trunk/src/z3c/form/validator.py 2009-05-17 13:21:18 UTC (rev 100028) MH> @@ -42,9 +42,28 @@ MH> MH> def validate(self, value): MH> """See interfaces.IValidator""" MH> + context = self.context MH> field = self.field MH> - if self.context is not None: MH> - field = field.bind(self.context) MH> + widget = self.widget MH> + if context is not None: MH> + field = field.bind(context) MH> + if value is interfaces.NOT_CHANGED: MH> + if (interfaces.IContextAware.providedBy(widget) and MH> + not widget.ignoreContext): MH> + # get value from context MH> + value = zope.component.getMultiAdapter( MH> + (context, field), MH> + interfaces.IDataManager).query() MH> + else: MH> + value = interfaces.NO_VALUE MH> + if value is interfaces.NO_VALUE: MH> + # look up default value MH> + value = field.default MH> + adapter = zope.component.queryMultiAdapter( MH> + (context, self.request, self.view, field, widget), MH> + interfaces.IValue, name='default') MH> + if adapter: MH> + value = adapter.get() MH> return field.validate(value) MH> MH> def __repr__(self): MH> Modified: z3c.form/trunk/src/z3c/form/validator.txt MH> =================================================================== MH> --- z3c.form/trunk/src/z3c/form/validator.txt 2009-05-17 13:18:11 UTC (rev 100027) MH> +++ z3c.form/trunk/src/z3c/form/validator.txt 2009-05-17 13:21:18 UTC (rev 100028) MH> @@ -117,7 +117,6 @@ MH> in the login name: MH> MH> >>> import re MH> - MH> >>> class LoginValidator(validator.SimpleFieldValidator): MH> ... MH> ... def validate(self, value): MH> @@ -182,7 +181,125 @@ MH> ... (None, None, None, IPerson['email'], None), MH> ... interfaces.IValidator) MH> MH> +Widget Validators and File-Uploads MH> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ MH> MH> +File-Uploads behave a bit different than the other form MH> +elements. Whether the user did not choose a file to upload MH> +``interfaces.NOT_CHANGED`` is set as value. But the validator knows MH> +how to handle this. MH> + MH> +The example has two bytes fields where File-Uploads are possible, one MH> +field is required the other one not: MH> + + >>>> class IPhoto(zope.interface.Interface): MH> + ... data = zope.schema.Bytes( MH> + ... title=u'Photo') MH> + ... MH> + ... thumb = zope.schema.Bytes( MH> + ... title=u'Thumbnail', MH> + ... required=False) MH> + MH> +There are several possible cases to differentiate between: MH> + MH> +No widget MH> ++++++++++ MH> + MH> +If there is no widget or the widget does not provide MH> +``interfaces.IContextAware``, no value is looked up from the MH> +context. So the not required field validates successfully but the MH> +required one has an required missing error, as the default value of MH> +the field is looked up on the field: MH> + + >>>> simple_thumb = validator.SimpleFieldValidator( MH> + ... None, None, None, IPhoto['thumb'], None) + >>>> simple_thumb.validate(interfaces.NOT_CHANGED) MH> + + >>>> simple_data = validator.SimpleFieldValidator( MH> + ... None, None, None, IPhoto['data'], None) + >>>> simple_data.validate(interfaces.NOT_CHANGED) MH> + Traceback (most recent call last): MH> + RequiredMissing MH> + MH> +Widget which ignores context MH> +++++++++++++++++++++++++++++ MH> + MH> +If the context is ignored in the widget - as in the add form - the MH> +behavior is the same as if there was no widget: MH> + + >>>> import z3c.form.widget + >>>> widget = z3c.form.widget.Widget(None) + >>>> zope.interface.alsoProvides(widget, interfaces.IContextAware) + >>>> widget.ignoreContext = True + >>>> simple_thumb = validator.SimpleFieldValidator( MH> + ... None, None, None, IPhoto['thumb'], widget) + >>>> simple_thumb.validate(interfaces.NOT_CHANGED) MH> + + >>>> simple_data = validator.SimpleFieldValidator( MH> + ... None, None, None, IPhoto['data'], widget) + >>>> simple_data.validate(interfaces.NOT_CHANGED) MH> + Traceback (most recent call last): MH> + RequiredMissing MH> + MH> +Look up value from default adapter MH> +++++++++++++++++++++++++++++++++++ MH> + MH> +When the value is ``interfaces.NOT_CHANGED`` the validator tries to MH> +look up the default value using a ``interfaces.IValue`` MH> +adapter. Whether the adapter is found, its value is used as default, MH> +so the validation of the required field is successful here: MH> + + >>>> data_default = z3c.form.widget.StaticWidgetAttribute( MH> + ... 'data', context=None, request=None, view=None, MH> + ... field=IPhoto['data'], widget=widget) + >>>> zope.component.provideAdapter(data_default, name='default') + >>>> simple_data.validate(interfaces.NOT_CHANGED) MH> + MH> + MH> +Look up value from context MH> +++++++++++++++++++++++++++ MH> + MH> +If there is a context aware widget which does not ignore its context, MH> +the value is looked up on the context using a data manager: MH> + + >>>> class Photo(object): MH> + ... zope.interface.implements(IPhoto) MH> + ... MH> + ... data = None MH> + ... thumb = None + >>>> photo = Photo() + >>>> widget.ignoreContext = False + >>>> zope.component.provideAdapter(z3c.form.datamanager.AttributeField) MH> + + >>>> simple_thumb = validator.SimpleFieldValidator( MH> + ... photo, None, None, IPhoto['thumb'], widget) + >>>> simple_thumb.validate(interfaces.NOT_CHANGED) MH> + MH> +If the value is not set on the context it is a required missing as MH> +neither context nor input have a valid value: MH> + + >>>> simple_data = validator.SimpleFieldValidator( MH> + ... photo, None, None, IPhoto['data'], widget) + >>>> simple_data.validate(interfaces.NOT_CHANGED) MH> + Traceback (most recent call last): MH> + RequiredMissing MH> + MH> +After setting the value validation is successful: MH> + + >>>> photo.data = 'data' + >>>> simple_data.validate(interfaces.NOT_CHANGED) MH> + MH> + MH> +Clean-up MH> +++++++++ MH> + + >>>> gsm = zope.component.getGlobalSiteManager() + >>>> gsm.unregisterAdapter(z3c.form.datamanager.AttributeField) MH> + True + >>>> gsm.unregisterAdapter(data_default, name='default') MH> + True MH> + MH> + MH> Widget Manager Validators MH> ------------------------- MH> MH> _______________________________________________ MH> Checkins mailing list MH> Checkins@zope.org MH> http://mail.zope.org/mailman/listinfo/checkins -- Best regards, Adam GROSZER mailto:agroszer@gmail.com -- Quote of the day: Let him who takes the Plunge remember to return it by Tuesday.
Am 24.05.2009 um 10:46 schrieb Adam GROSZER:
Hello Michael,
After some simple thinking there are some fears that this wrecks other basic fields validation.
Something like the field is required, has a default value but is omitted on input. What will be the result?
When the field is omitted on input it should not have the value interfaces.NOT_CHANGED but no value, so special handling for NOT_CHANGED does not apply.
Accepting the default value instead of raising an error is definitely a problem.
Also, values in the system might not be needed to re-validate? Image passing around a 100mb uploaded file.
z3c.form has definitely a problem with this use-case. Not only the validator but also the edit form reads the whole file into RAM.
If this is meant only for a file upload field, then I think there should be a special validator registered for those.
interfaces.NOT_CHANGED is currently only used for file uploads, but it might be used for other fields as well, so I see no reason to put the handling into a special validator.
Would be nice to see some (functional) tests closing out the above fears. Functional, because on the unittest level you can force the widget to a lot of unexpected things, but that might not be inline with what happens when it's really done by the framework. Or better said, revert the changes locally, add those tests, apply the changes back and see whether the tests still pass.
File upload fields did not work at all after someone introduced interfaces.NOT_CHANGED and before adding my changes. Jacob Holm, who suggested the patch, wrote in http://mail.zope.org/pipermail/zope-dev/2009-April/036258.html The "code should compute the same value as z3c.form.widget.Widget.update would when ignoreRequest is True. Thus effectively converting NOT_CHANGED into the "existing" value before validating."
MH> Modified: z3c.form/trunk/src/z3c/form/validator.py MH> =================================================================== MH> --- z3c.form/trunk/src/z3c/form/validator.py 2009-05-17 13:18:11 UTC (rev 100027) MH> +++ z3c.form/trunk/src/z3c/form/validator.py 2009-05-17 13:21:18 UTC (rev 100028) MH> @@ -42,9 +42,28 @@ MH> MH> def validate(self, value): MH> """See interfaces.IValidator""" MH> + context = self.context MH> field = self.field MH> - if self.context is not None: MH> - field = field.bind(self.context) MH> + widget = self.widget MH> + if context is not None: MH> + field = field.bind(context) MH> + if value is interfaces.NOT_CHANGED: MH> + if (interfaces.IContextAware.providedBy(widget) and MH> + not widget.ignoreContext): MH> + # get value from context MH> + value = zope.component.getMultiAdapter( MH> + (context, field), MH> + interfaces.IDataManager).query() MH> + else: MH> + value = interfaces.NO_VALUE MH> + if value is interfaces.NO_VALUE: MH> + # look up default value MH> + value = field.default MH> + adapter = zope.component.queryMultiAdapter( MH> + (context, self.request, self.view, field, widget), MH> + interfaces.IValue, name='default') MH> + if adapter: MH> + value = adapter.get() MH> return field.validate(value) MH> MH> def __repr__(self):
Yours sincerely, -- Michael Howitz · mh@gocept.com · software developer gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 8 · fax +49 345 1229889 1 Zope and Plone consulting and development
participants (2)
-
Adam GROSZER -
Michael Howitz