Re: Fix, GDL2, EORelationship (-validateName:)

[Matt Rice]

On Mon, Apr 28, 2008 at 11:56 PM, David Ayers <ayers@fsfe.org> wrote:
> Hello Georg,
>
>  Thanks for the report!
>
>  Georg Fleischmann schrieb:
>
>  > here is a patch for EORelationship, proposing a fix for awakening of
>  > attributes that have a definition.
>  >
>  > When relationships are constructed they are validated against
>  > attributes. That in turn awakens the attributes, being dependent on
>  > relationships that are not there at this time. The definitions will fail
>  > to be set.
>  > Here is the part of the stack of that problematic dependency:
>
>  OK, we have a bug.  We are working out a test case for the test suite.

i haven't got a test case for it yet, but i see from the backtrace
that things would go awry after
frame #5,

>
>  > The attached patch (-c) is fixing the issue by simply not validating the
>  > relationships against attributes.
>  > This seems to be correct behaviour, since the class documentation
>  > doesn't mention a test against attributes, only against relationships
>  > and stored procedures. Quote from the EOAccess docu:
>  >
>  > validateName:
>  > "Validates name and returns nil if its a valid name, or an exception if
>  > it isn't. A name is invalid if it has zero
>  > length; starts with a character other than a letter, a number, or "@",
>  > "#", or "_"; or contains a character other
>  > than a letter, a number, "@", "#", "_", or "$". A name is also invalid
>  > if the receiver's EOEntity already has
>  > an EORelationship with the same name, or if the model has a stored
>  > procedure that has an argument with
>  > the same name."
>

that documentation is actually incomplete

however, IIRC it is a gdl2 extension that we do any validation of
model objects loaded from plists.
I only recently changed this area to using validateName:

previously it was manually doing duplicate name checks, but lacked any
checks for invalid names.
and I thought it was safe but appears not to be in the case of definitions.

though, we can get a list of names for dupe checking without
triggering the loading code,
attached is a patch which implements this, we should probably do a
similar thing for relationships/etc
if it becomes an issue there as well...

seems to pass the testsuite,
let me know if this fixes your bug; and if you have time to come up
with a test for it, that'd be helpful

i haven't tried it yet but
it might perform better if we use _attributesByName if it exists, and
then falling back on the [_attributes valueForKey:] stuff

though we can't currently use -attributesByName: to create
_attributesByName without triggering loading...
making it possible is another idea, but a bit more complicated,
(-_attributesByName:loadFlag:)?

for loadFlag:
NO would potentially return objects with which are not yet awakened
(dictionaries or EOAttributes depending on if loading has occured
previously) and YES would cause the loading code to occur if it hasn't
already and initialize/awaken attributes and all objects would be
EOAttribute objects.

anyhow sounds like quite a bit of work i'm not sure how critical
performance is for the methods involved :)

matt

foo.diff
Description: Text Data