OpenNTI / nti.schema

Enhancements for zope.schema
Other
0 stars 0 forks source link

BeforeSchemaFieldAssignedEvent uses FieldPropertyStoredThroughField mangled name #10

Closed cutz closed 7 years ago

cutz commented 7 years ago

It is useful for subscribers on IBeforeSchemaFieldAssignedEvent to have access to the field name being set on the context. The event object has a name field but in the case of a FieldPropertyStoredThroughField this is a mangled name of the form__st_TermsAgreement_st not the attribute name for the field TermsAgreement.

I'm not sure what the best solution here is, we could:

a) unmangle the name when we create the event. This seems cleanest but there may be subscribers assuming something about the existing mangled names? b) expose the guts of FieldValidationMixin.__fixup_name__ that subscribers of the event could use to cleanup the name before using it. c) stash the unmangled name on the event as a new attribute.

@jamadden Thoughts? I lean towards option a or b.

jamadden commented 7 years ago

Unmangling the name on the event is not an option. There are reasons for subscribers to want to do [gs]etattr(evt.object, evt.name) or even evt.object.__dict__[evt.name]. In fact, I think in some cases this is necessary to avoid infinite recursion when using a FieldPropertyStoredThroughField. That may be why the mangling was introduced.

The unmangling done by FieldValidationMixin is intended for UI purposes, not programatic ones, IIRC (where UI includes uses of the JSON schema). It's just to make things look pretty. What's your use case?

That said, the mangling is stable and cannot change (without breaking persistent objects), so it's safe to rely on, especially if the use case is narrow.

As for a new attribute, and in general actually: We're doing some monkey-patching shenanigans here. zope.schema defines an IBeforeObjectAssignedEvent that (after the monkey-patching) is a subinterface of IBeforeSchemaFieldAssignedEvent, but not a subclass of BeforeSchemaFieldAssignedEvent. So there are compatibility issues there. We'd have to get a PR accepted to zope.schema, and it would need a good rationale and clean implementation (which I can work on providing, we just have to have a good story).

cutz commented 7 years ago

What's your use case?

Following the comment here I was attempting to locate the value being set to a parent using the unmangled name.

I ended up with a different solution involving a slightly customized FieldPropertyStoredThroughField which feels cleaner than what I was trying to do in the first place here. We can see if something like this comes up again in the future.