fun with AT

[Wichert Akkerman]

I've spend entirely too much time tracking down why I got a KeyError
when the catalog was updating the metadata for an object. After
suspecting bugs in the catalog, acquisition, persistent schema
attributes and several other interesting but innocent parts of out
stack I finally found the problem: good old class inheritence does
not mix well with Archetypes.

What happens that I have a archetype (lets call it Yacht)) which is
derived from another another archetype (called Boat):

    class Boat(BaseContent):
       schema = BoatSchema

    class Yacht(Boat):
       schema = BoatSchema.copy() + SailingSchema

for various historic reasons at some point an extra field 'motor' was
monkeypatched into Boat.schema, generateMethods was called to generate
an accessor and a catalog column for getMoter was added. The result is
quite interesting: the catalog aborts with KeyError when it tries to
index a yacht instance.

What happens is that Archetypes generates a getMotor accessor method
for the Boat type. Since Yacht is a derived class it inherits that
method. But since 'motor' is not in the schema for Yacht it accessor
can not find the correct field and generates a KeyError.

In other words there is an implicit rule for Archetypes: a derived
type must always have all fields its base classes have.

I can't decide if this is a bug in Archetypes or not. At least it
was unexpected for me. I can't come up with a change in AT behaviour
that would improve things though.

Wichert.

--
Wichert Akkerman <wichert@...>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Ricardo Alves-2]

Wichert Akkerman wrote:

> I've spend entirely too much time tracking down why I got a KeyError
> when the catalog was updating the metadata for an object. After
> suspecting bugs in the catalog, acquisition, persistent schema
> attributes and several other interesting but innocent parts of out
> stack I finally found the problem: good old class inheritence does
> not mix well with Archetypes.
>
> What happens that I have a archetype (lets call it Yacht)) which is
> derived from another another archetype (called Boat):
>
>     class Boat(BaseContent):
>        schema = BoatSchema
>
>     class Yacht(Boat):
>        schema = BoatSchema.copy() + SailingSchema
>
> for various historic reasons at some point an extra field 'motor' was
> monkeypatched into Boat.schema, generateMethods was called to generate
> an accessor and a catalog column for getMoter was added. The result is
> quite interesting: the catalog aborts with KeyError when it tries to
> index a yacht instance.
>
> What happens is that Archetypes generates a getMotor accessor method
> for the Boat type. Since Yacht is a derived class it inherits that
> method. But since 'motor' is not in the schema for Yacht it accessor
> can not find the correct field and generates a KeyError.
>
> In other words there is an implicit rule for Archetypes: a derived
> type must always have all fields its base classes have.
>
> I can't decide if this is a bug in Archetypes or not. At least it
> was unexpected for me. I can't come up with a change in AT behaviour
> that would improve things though.

IMHO, it should be considered a bug. I guess one possible fix would be
to add a condition in generatedAccessor, returning None if the field is
not available. Not sure if this won't potentially hide/swallow other
errors though...


Ricardo

--
Ricardo Alves <rsa@...>
Eurotux <http://www.eurotux.com>


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Michael Hierweck]

Wichert Akkerman wrote:

> What happens that I have a archetype (lets call it Yacht)) which is
> derived from another another archetype (called Boat):
>
>     class Boat(BaseContent):
>        schema = BoatSchema
>
>     class Yacht(Boat):
>        schema = BoatSchema.copy() + SailingSchema
>
> for various historic reasons at some point an extra field 'motor' was
> monkeypatched into Boat.schema, generateMethods was called to generate
> an accessor and a catalog column for getMoter was added. The result is
> quite interesting: the catalog aborts with KeyError when it tries to
> index a yacht instance.

That means either:

the monkey patch should extend BoatSchema instead of Boat.schema

or:

Yacht.schema should be defined as Boat.schema.copy() + Sailing.Schema


Michael

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Wichert Akkerman]

Previously Michael Hierweck wrote:

> Wichert Akkerman wrote:
> > What happens that I have a archetype (lets call it Yacht)) which is
> > derived from another another archetype (called Boat):
> >
> >     class Boat(BaseContent):
> >        schema = BoatSchema
> >
> >     class Yacht(Boat):
> >        schema = BoatSchema.copy() + SailingSchema
> >
> > for various historic reasons at some point an extra field 'motor' was
> > monkeypatched into Boat.schema, generateMethods was called to generate
> > an accessor and a catalog column for getMoter was added. The result is
> > quite interesting: the catalog aborts with KeyError when it tries to
> > index a yacht instance.
>
> That means either:
>
> the monkey patch should extend BoatSchema instead of Boat.schema
>
> or:
>
> Yacht.schema should be defined as Boat.schema.copy() + Sailing.Schema

BoatSchema and Boat.schema are the exact same object, so those
suggestions won't make a difference.

Wichert.

--
Wichert Akkerman <wichert@...>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Daniel Nouri]

Wichert Akkerman <wichert@...>
writes:

> Previously Michael Hierweck wrote:
>> Wichert Akkerman wrote:
>> > What happens that I have a archetype (lets call it Yacht)) which is
>> > derived from another another archetype (called Boat):
>> >
>> >     class Boat(BaseContent):
>> >        schema = BoatSchema
>> >
>> >     class Yacht(Boat):
>> >        schema = BoatSchema.copy() + SailingSchema
>> >
>> > for various historic reasons at some point an extra field 'motor' was
>> > monkeypatched into Boat.schema, generateMethods was called to generate
>> > an accessor and a catalog column for getMoter was added. The result is
>> > quite interesting: the catalog aborts with KeyError when it tries to
>> > index a yacht instance.
>>
>> That means either:
>>
>> the monkey patch should extend BoatSchema instead of Boat.schema
>>
>> or:
>>
>> Yacht.schema should be defined as Boat.schema.copy() + Sailing.Schema
>
> BoatSchema and Boat.schema are the exact same object, so those
> suggestions won't make a difference.

What *would* make a difference would be to define Boat as:

    class Boat(BaseContent):
        schema = BoatSchema.copy()

or have the patch do it.


Daniel


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Hedley Roos-2]

I don't consider it a bug. This late in the game I'll call it a pattern.

ArchgenXML for example takes great care to copy the schema from a base
class and appends to it. That's not too much of a pain for me.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Daniel Nouri]

Daniel Nouri <daniel.nouri@...>
writes:

> Wichert Akkerman <wichert@...>
> writes:
>
>> Previously Michael Hierweck wrote:
>>> Wichert Akkerman wrote:
>>> > What happens that I have a archetype (lets call it Yacht)) which is
>>> > derived from another another archetype (called Boat):
>>> >
>>> >     class Boat(BaseContent):
>>> >        schema = BoatSchema
>>> >
>>> >     class Yacht(Boat):
>>> >        schema = BoatSchema.copy() + SailingSchema
>>> >
>>> > for various historic reasons at some point an extra field 'motor' was
>>> > monkeypatched into Boat.schema, generateMethods was called to generate
>>> > an accessor and a catalog column for getMoter was added. The result is
>>> > quite interesting: the catalog aborts with KeyError when it tries to
>>> > index a yacht instance.
>>>
>>> That means either:
>>>
>>> the monkey patch should extend BoatSchema instead of Boat.schema
>>>
>>> or:
>>>
>>> Yacht.schema should be defined as Boat.schema.copy() + Sailing.Schema
>>
>> BoatSchema and Boat.schema are the exact same object, so those
>> suggestions won't make a difference.
>
> What *would* make a difference would be to define Boat as:
>
>     class Boat(BaseContent):
>         schema = BoatSchema.copy()
>
> or have the patch do it.

Duh, forget about this.  This has no effect on how generateMethods works
of course.

I don't think having the automatic accessor return None in that case is
any better.  I guess the catalog index should really use an adapter to
get to the data, instead of blindly calling methods, but that's another
story.


Daniel


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Andreas Zeidler]

Wichert Akkerman wrote:

> What happens that I have a archetype (lets call it Yacht)) which is
> derived from another another archetype (called Boat):
>
>     class Boat(BaseContent):
>        schema = BoatSchema
>
>     class Yacht(Boat):
>        schema = BoatSchema.copy() + SailingSchema
>
> for various historic reasons at some point an extra field 'motor' was
> monkeypatched into Boat.schema, generateMethods was called to generate
> an accessor and a catalog column for getMoter was added. The result is
> quite interesting: the catalog aborts with KeyError when it tries to
> index a yacht instance.
>
> What happens is that Archetypes generates a getMotor accessor method
> for the Boat type. Since Yacht is a derived class it inherits that
> method. But since 'motor' is not in the schema for Yacht it accessor
> can not find the correct field and generates a KeyError.

well, if 'motor' is not part of Yacht's schema, then apparently the
monkey patch was only applied after the `BoatSchema.copy() + ...` in
Yacht, right?  if that's the case, wasn't that error to be expected really?

> I can't come up with a change in AT behaviour
> that would improve things though.

how about if `generateMethods` would somehow keep track of or mark the
accessors/mutators it did generate?  this way it could compare them with
the schema of the derived class and actually remove the ones for missing
fields...

however, imho this would only work around an issue that hasn't got to do
with neither archetypes nor class inheritance, but is solely due to
monkey patching the schema "too late".  btw, how does this relate to
at.schemaextender?  it should break the same way, no?

cheers,


andi

--
zeidler it consulting - http://zitc.de/ - info@...
friedelstraße 31 - 12047 berlin - telefon [phone]
pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/
plone 3.1.3 released! -- http://plone.org/products/plone/


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Ricardo Alves-2]

Andreas Zeidler wrote:

> Wichert Akkerman wrote:
>> What happens that I have a archetype (lets call it Yacht)) which is
>> derived from another another archetype (called Boat):
>>
>>     class Boat(BaseContent):
>>        schema = BoatSchema
>>
>>     class Yacht(Boat):
>>        schema = BoatSchema.copy() + SailingSchema
>>
>> for various historic reasons at some point an extra field 'motor' was
>> monkeypatched into Boat.schema, generateMethods was called to generate
>> an accessor and a catalog column for getMoter was added. The result is
>> quite interesting: the catalog aborts with KeyError when it tries to
>> index a yacht instance.
>>
>> What happens is that Archetypes generates a getMotor accessor method
>> for the Boat type. Since Yacht is a derived class it inherits that
>> method. But since 'motor' is not in the schema for Yacht it accessor
>> can not find the correct field and generates a KeyError.
>
> well, if 'motor' is not part of Yacht's schema, then apparently the
> monkey patch was only applied after the `BoatSchema.copy() + ...` in
> Yacht, right?  if that's the case, wasn't that error to be expected really?
>
>> I can't come up with a change in AT behaviour
>> that would improve things though.
>
> how about if `generateMethods` would somehow keep track of or mark the
> accessors/mutators it did generate?  this way it could compare them with
> the schema of the derived class and actually remove the ones for missing
> fields...
>
> however, imho this would only work around an issue that hasn't got to do
> with neither archetypes nor class inheritance, but is solely due to
> monkey patching the schema "too late".  

I think the point is that you can't have a derived class that doesn't
include all fields from the base class. Here, the monkey patch motivated
the problem. But, if I understand it correctly, even without a monkey
patch, you can't still do the following:


BoatSchema = Schema((StringField('motor'),))

class Boat(BaseContent):
    schema = BaseSchema + BoatSchema


SailingSchema = Schema(())

class Yacht(Boat):
    schema = BaseSchema + SailingSchema


Yatch doesn't include field 'motor', but will inherit the generated
methods, and if the catalog tries to indexes 'getMotor', we will get the
KeyError.

If this is not a bug, probably AT should somehow complain/warn about it...


Ricardo

--
Ricardo Alves <rsa@...>
Eurotux <http://www.eurotux.com>


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Laurence Rowe]

Andreas Zeidler wrote:

Wichert Akkerman wrote:
> What happens that I have a archetype (lets call it Yacht)) which is
> derived from another another archetype (called Boat):
>
>     class Boat(BaseContent):
>        schema = BoatSchema
>
>     class Yacht(Boat):
>        schema = BoatSchema.copy() + SailingSchema
>
> for various historic reasons at some point an extra field 'motor' was
> monkeypatched into Boat.schema, generateMethods was called to generate
> an accessor and a catalog column for getMoter was added. The result is
> quite interesting: the catalog aborts with KeyError when it tries to
> index a yacht instance.
>
> What happens is that Archetypes generates a getMotor accessor method
> for the Boat type. Since Yacht is a derived class it inherits that
> method. But since 'motor' is not in the schema for Yacht it accessor
> can not find the correct field and generates a KeyError.

well, if 'motor' is not part of Yacht's schema, then apparently the
monkey patch was only applied after the `BoatSchema.copy() + ...` in
Yacht, right?  if that's the case, wasn't that error to be expected really?

> I can't come up with a change in AT behaviour
> that would improve things though.

how about if `generateMethods` would somehow keep track of or mark the
accessors/mutators it did generate?  this way it could compare them with
the schema of the derived class and actually remove the ones for missing
fields...

however, imho this would only work around an issue that hasn't got to do
with neither archetypes nor class inheritance, but is solely due to
monkey patching the schema "too late".  btw, how does this relate to
at.schemaextender?  it should break the same way, no?

I wander if renaming the monkey patch product to 'AAproduct' fixes the issue?

Laurence

[Martin Aspeli-2]

Wichert Akkerman wrote:

> In other words there is an implicit rule for Archetypes: a derived
> type must always have all fields its base classes have.
>
> I can't decide if this is a bug in Archetypes or not. At least it
> was unexpected for me. I can't come up with a change in AT behaviour
> that would improve things though.

I don't think it's a bug. It's not exactly different from the Zope 3
type scenario, where the base class implements IBoat and its fields,
which are inherited in the subclass that may add more fields.

Martin

--
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Andreas Zeidler]

Laurence Rowe wrote:
> I wander if renaming the monkey patch product to 'AAproduct' fixes the
> issue?

according to hanno it would very likely fix it on windows, and perhaps
also on other platforms if you're lucky (apparently `os.listdir` returns
things in alphabetical order on windows).


andi

--
zeidler it consulting - http://zitc.de/ - info@...
friedelstraße 31 - 12047 berlin - telefon [phone]
pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/
plone 3.1.3 released! -- http://plone.org/products/plone/


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers

[Daniel Nouri]

Andreas Zeidler writes:

> Laurence Rowe wrote:
>> I wander if renaming the monkey patch product to 'AAproduct' fixes the
>> issue?
>
> according to hanno it would very likely fix it on windows, and perhaps
> also on other platforms if you're lucky (apparently `os.listdir` returns
> things in alphabetical order on windows).

Wichert does not want the Yacht to have the monkey-patched 'motor'
field.  Still, his Yacht inherits the method 'getMotor'.  You can't
avoid this with changing the order of imports.  Consider this:

  >>> class A(object): schema = {}
  >>> class B(A): schema = {}

  >>> A.schema['motor'] = 'A motor'  # monkey-patch schema
  >>> A.getMotor = lambda self: self.schema['motor']  # monkey-patch method

  >>> A().getMotor()
  'A motor'
  >>> B().getMotor()
  KeyError: 'motor'


An AAproduct is useful for situations where you'd be otherwise patching
a schema too late.  Consider this simplified example:

in AUsefulProduct/content.py:

  >>> class Yacht(BaseContent):
  ...     schema = WaterborneSchema.copy()

in ZZPatch/__init__.py:

  >>> from AUsefulProduct.schema import WaterborneSchema
  >>> monkey_with(WaterborneSchema)

In this case, you might be monkey patching WaterborneSchema *after*
Yacht's class makes a copy of it, which probably isn't what you want.

Using AAPatch as ZZPatch's product name instead will help with this;
consistently across Linux and OSX last time I checked.


--
Daniel Nouri
http://danielnouri.org


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Plone-developers mailing list
Plone-developers@...
https://lists.sourceforge.net/lists/listinfo/plone-developers