Smalltalk › Frameworks & Tools › Magritte, Pier and Related Tools

Magritte validation problem with MABooleanDescription

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

13 messages Options

Nick

Magritte validation problem with MABooleanDescription

Hi,

I've been struggling with a Magritte validation problem. I find that a condition I add (using #addCondition:labelled:) to a MABooleanDescription is only intermittently called.

I've simplified the conditions to reproduce the problem to a component description comprising of two MABooleanDescription descriptions. The second description includes the condition which is intermittently called.

If the value of the first description is true (ie the checkbox is checked) the second description's condition will be executed, however if it is false it will never be executed.

I suspect the problems lies in the code in MAValidatorVisitor>>#visitContainer: which calls MAGraphVisitor>>#use:during: which is as follows:

MAGraphVisitor >>use: anObject during: aBlock

| previous |

(seen includes: anObject)

ifTrue: [ ^ self ].

anObject isNil

ifFalse: [ seen add: anObject ].

previous := object. object := anObject.

aBlock ensure: [ object := previous ]

If the first check box is false, and the second is also false then (seen includes: anObject) will return true and method returns early resulting in block associated with second description (self visit: description) never being executed.

I've attached a file out of a simple component TestMagritteValidation illustrating the problem that I tested in the latest build from Hudson.

Thanks

Nick

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

TestMagritteValidation.st (2K) Download Attachment

Lukas Renggli

Re: Magritte validation problem with MABooleanDescription

Wow, that's a very nasty bug. Amazing that this hasn't hit anybody
before. Looks like this could happen quite common in systems like
Pier, not only with MABooleanDescription instances.

http://code.google.com/p/magritte-metamodel/issues/detail?id=7

The validation code is really ugly. And likely this bug was introduced
when trying to support the validation for recursive structures. I
would be happy if someone could rewrite that code? (also get rid of
these nested, resumable exceptions).

Lukas

--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Nick

Re: Magritte validation problem with MABooleanDescription

HI Lukas,

On 11 October 2010 08:32, Lukas Renggli <[hidden email]> wrote:

Wow, that's a very nasty bug. Amazing that this hasn't hit anybody
before. Looks like this could happen quite common in systems like
Pier, not only with MABooleanDescription instances.

http://code.google.com/p/magritte-metamodel/issues/detail?id=7

The validation code is really ugly. And likely this bug was introduced
when trying to support the validation for recursive structures. I
would be happy if someone could rewrite that code? (also get rid of
these nested, resumable exceptions).

I'm happy to help, especially as I'd like a fix for the code I'm developing.

However I'm struggling to understand the intent of the some of the code for example I don't understand the intent behind the use of object and previous in MAGraphVisitor>>#use:during

MAGraphVisitor>>#use: anObject during: aBlock

| previous |

(seen includes: anObject)

ifTrue: [ ^ self ].

anObject isNil

ifFalse: [ seen add: anObject ].

previous := object. object := anObject.

aBlock ensure: [ object := previous ]

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Lukas Renggli

Re: Magritte validation problem with MABooleanDescription

> (seen includes: anObject)
> ifTrue: [ ^ self ].
> anObject isNil
> ifFalse: [ seen add: anObject ].

That part just tries to avoid to go into recursion if a described
object refers to another object that descriptions refer back to the
original object. That was requested at some point in the past, but I
never dependent on that myself.

Lukas

On 11 October 2010 11:49, Nick Ager <[hidden email]> wrote:

> HI Lukas,
>
> On 11 October 2010 08:32, Lukas Renggli <[hidden email]> wrote:
>>
>> Wow, that's a very nasty bug. Amazing that this hasn't hit anybody
>> before. Looks like this could happen quite common in systems like
>> Pier, not only with MABooleanDescription instances.
>>
>> http://code.google.com/p/magritte-metamodel/issues/detail?id=7
>>
>> The validation code is really ugly. And likely this bug was introduced
>> when trying to support the validation for recursive structures. I
>> would be happy if someone could rewrite that code? (also get rid of
>> these nested, resumable exceptions).
>
> I'm happy to help, especially as I'd like a fix for the code I'm
> developing.
> However I'm struggling to understand the intent of the some of the code for
> example I don't understand the intent behind the use of object and previous
> in MAGraphVisitor>>#use:during
> MAGraphVisitor>>#use: anObject during: aBlock
> | previous |
> (seen includes: anObject)
> ifTrue: [ ^ self ].
> anObject isNil
> ifFalse: [ seen add: anObject ].
> previous := object. object := anObject.
> aBlock ensure: [ object := previous ]
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>

--
Lukas Renggli
www.lukas-renggli.ch

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Nick

Re: Magritte validation problem with MABooleanDescription

On 11 October 2010 10:53, Lukas Renggli <[hidden email]> wrote:

> (seen includes: anObject)
> ifTrue: [ ^ self ].
> anObject isNil
> ifFalse: [ seen add: anObject ].

That part just tries to avoid to go into recursion if a described
object refers to another object that descriptions refer back to the
original object. That was requested at some point in the past, but I
never dependent on that myself.

I understood that part but don't understand the intent of:

previous := object. object := anObject.

aBlock ensure: [ object := previous ]

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Lukas Renggli

Re: Magritte validation problem with MABooleanDescription

That just remembers (in 'previous') and restores the currently
validated object while (in 'object') recursively walking through the
graph (which is a tree in 99% of the cases).

Lukas

On 11 October 2010 12:05, Nick Ager <[hidden email]> wrote:

>
>
> On 11 October 2010 10:53, Lukas Renggli <[hidden email]> wrote:
>>
>> > (seen includes: anObject)
>> > ifTrue: [ ^ self ].
>> > anObject isNil
>> > ifFalse: [ seen add: anObject ].
>>
>> That part just tries to avoid to go into recursion if a described
>> object refers to another object that descriptions refer back to the
>> original object. That was requested at some point in the past, but I
>> never dependent on that myself.
>
> I understood that part but don't understand the intent of:
> previous := object. object := anObject.
> aBlock ensure: [ object := previous ]
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>

--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

John McKeon

Re: Magritte validation problem with MABooleanDescription

I've posted a mcz to http://code.google.com/p/magritte-metamodel/issues/detail?id=7

It uses the description instead of the object in the collection of descriptions already visited

John

On Mon, Oct 11, 2010 at 6:11 AM, Lukas Renggli <[hidden email]> wrote:

That just remembers (in 'previous') and restores the currently
validated object while (in 'object') recursively walking through the
graph (which is a tree in 99% of the cases).

Lukas

On 11 October 2010 12:05, Nick Ager <[hidden email]> wrote:
>
>
> On 11 October 2010 10:53, Lukas Renggli <[hidden email]> wrote:
>>
>> > (seen includes: anObject)
>> > ifTrue: [ ^ self ].
>> > anObject isNil
>> > ifFalse: [ seen add: anObject ].
>>
>> That part just tries to avoid to go into recursion if a described
>> object refers to another object that descriptions refer back to the
>> original object. That was requested at some point in the past, but I
>> never dependent on that myself.
>
> I understood that part but don't understand the intent of:
> previous := object. object := anObject.
> aBlock ensure: [ object := previous ]

> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>

--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

--
http://john-mckeon.us

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Lukas Renggli

Re: Magritte validation problem with MABooleanDescription

I thought about that too, but this is not a solution: It can be that
different objects are described with the same description instances.

Lukas

On 12 October 2010 02:46, John McKeon <[hidden email]> wrote:

> I've posted a mcz
> to http://code.google.com/p/magritte-metamodel/issues/detail?id=7
> It uses the description instead of the object in the collection of
> descriptions already visited
> John
>
> On Mon, Oct 11, 2010 at 6:11 AM, Lukas Renggli <[hidden email]> wrote:
>>
>> That just remembers (in 'previous') and restores the currently
>> validated object while (in 'object') recursively walking through the
>> graph (which is a tree in 99% of the cases).
>>
>> Lukas
>>
>> On 11 October 2010 12:05, Nick Ager <[hidden email]> wrote:
>> >
>> >
>> > On 11 October 2010 10:53, Lukas Renggli <[hidden email]> wrote:
>> >>
>> >> > (seen includes: anObject)
>> >> > ifTrue: [ ^ self ].
>> >> > anObject isNil
>> >> > ifFalse: [ seen add: anObject ].
>> >>
>> >> That part just tries to avoid to go into recursion if a described
>> >> object refers to another object that descriptions refer back to the
>> >> original object. That was requested at some point in the past, but I
>> >> never dependent on that myself.
>> >
>> > I understood that part but don't understand the intent of:
>> > previous := object. object := anObject.
>> > aBlock ensure: [ object := previous ]
>> > _______________________________________________
>> > Magritte, Pier and Related Tools ...
>> > https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>> >
>>
>>
>>
>> --
>> Lukas Renggli
>> www.lukas-renggli.ch
>> _______________________________________________
>> Magritte, Pier and Related Tools ...
>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>
>
>
> --
> http://john-mckeon.us
>
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>

--
Lukas Renggli
www.lukas-renggli.ch

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

NorbertHartl

Re: Magritte validation problem with MABooleanDescription

Wow, that's a hairy problem. I just read the thread a few minutes ago. I agree that using a description does not solve the problem. But if I'm not wrong then the problem exists for every object where

anObject copy == anObject

right? (Btw. is there a selector in Smalltalk that can test this?) On the other hand we are talking about preventing infinite loops, right? Is there a chance that any of the objects that meet the condition above can trigger an infinite recursion? I don't think so. So it should be sufficient to manage the seen list only for the object that do no meet the condition. Although the above is neither nice nor fast.

Norbert

On 12.10.2010, at 08:14, Lukas Renggli wrote:

> I thought about that too, but this is not a solution: It can be that
> different objects are described with the same description instances.
>
> Lukas
>
> On 12 October 2010 02:46, John McKeon <[hidden email]> wrote:
>> I've posted a mcz
>> to http://code.google.com/p/magritte-metamodel/issues/detail?id=7
>> It uses the description instead of the object in the collection of
>> descriptions already visited
>> John
>>
>> On Mon, Oct 11, 2010 at 6:11 AM, Lukas Renggli <[hidden email]> wrote:
>>>
>>> That just remembers (in 'previous') and restores the currently
>>> validated object while (in 'object') recursively walking through the
>>> graph (which is a tree in 99% of the cases).
>>>
>>> Lukas
>>>
>>> On 11 October 2010 12:05, Nick Ager <[hidden email]> wrote:
>>>>
>>>>
>>>> On 11 October 2010 10:53, Lukas Renggli <[hidden email]> wrote:
>>>>>
>>>>>> (seen includes: anObject)
>>>>>> ifTrue: [ ^ self ].
>>>>>> anObject isNil
>>>>>> ifFalse: [ seen add: anObject ].
>>>>>
>>>>> That part just tries to avoid to go into recursion if a described
>>>>> object refers to another object that descriptions refer back to the
>>>>> original object. That was requested at some point in the past, but I
>>>>> never dependent on that myself.
>>>>
>>>> I understood that part but don't understand the intent of:
>>>> previous := object. object := anObject.
>>>> aBlock ensure: [ object := previous ]
>>>> _______________________________________________
>>>> Magritte, Pier and Related Tools ...
>>>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>>>>
>>>
>>>
>>>
>>> --
>>> Lukas Renggli
>>> www.lukas-renggli.ch
>>> _______________________________________________
>>> Magritte, Pier and Related Tools ...
>>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>>
>>
>>
>> --
>> http://john-mckeon.us
>>
>> _______________________________________________
>> Magritte, Pier and Related Tools ...
>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>>
>
>
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Lukas Renggli

Re: Magritte validation problem with MABooleanDescription

> Wow, that's a hairy problem. I just read the thread a few minutes ago. I agree that using a description does not solve the problem. But if I'm not wrong then the problem exists for every object where
>
> anObject copy == anObject
>
> right?

Yeah, the problem is complex.

And I am tempted to avoid it by not going deeply into the graph, but
by only validating what would be visible (if for example rendered by
Seaside). This was the case a few years back, but then somebody
complained about incomplete validation of 1:1 and 1:n relationships.
Now I do not remember exactly why we changed this, but it seems to me
the cause of all problems :-)

> (Btw. is there a selector in Smalltalk that can test this?)

I don't think so. The code you wrote is probably the only test.

> On the other hand we are talking about preventing infinite loops, right? Is there a chance that any of the objects that meet the condition above can trigger an infinite recursion? I don't think so.

Correct, but there can be other cases.

Imagine multiple 1:1 relationships where the same some of them refer
to the same object, but have different validation conditions.
Similarly, as we saw before, we can have different objects with the
same description instances.

Lukas

> So it should be sufficient to manage the seen list only for the object that do no meet the condition. Although the above is neither nice nor fast.
>
> Norbert
>
> On 12.10.2010, at 08:14, Lukas Renggli wrote:
>
>> I thought about that too, but this is not a solution: It can be that
>> different objects are described with the same description instances.
>>
>> Lukas
>>
>> On 12 October 2010 02:46, John McKeon <[hidden email]> wrote:
>>> I've posted a mcz
>>> to http://code.google.com/p/magritte-metamodel/issues/detail?id=7
>>> It uses the description instead of the object in the collection of
>>> descriptions already visited
>>> John
>>>
>>> On Mon, Oct 11, 2010 at 6:11 AM, Lukas Renggli <[hidden email]> wrote:
>>>>
>>>> That just remembers (in 'previous') and restores the currently
>>>> validated object while (in 'object') recursively walking through the
>>>> graph (which is a tree in 99% of the cases).
>>>>
>>>> Lukas
>>>>
>>>> On 11 October 2010 12:05, Nick Ager <[hidden email]> wrote:
>>>>>
>>>>>
>>>>> On 11 October 2010 10:53, Lukas Renggli <[hidden email]> wrote:
>>>>>>
>>>>>>> (seen includes: anObject)
>>>>>>> ifTrue: [ ^ self ].
>>>>>>> anObject isNil
>>>>>>> ifFalse: [ seen add: anObject ].
>>>>>>
>>>>>> That part just tries to avoid to go into recursion if a described
>>>>>> object refers to another object that descriptions refer back to the
>>>>>> original object. That was requested at some point in the past, but I
>>>>>> never dependent on that myself.
>>>>>
>>>>> I understood that part but don't understand the intent of:
>>>>> previous := object. object := anObject.
>>>>> aBlock ensure: [ object := previous ]
>>>>> _______________________________________________
>>>>> Magritte, Pier and Related Tools ...
>>>>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Lukas Renggli
>>>> www.lukas-renggli.ch
>>>> _______________________________________________
>>>> Magritte, Pier and Related Tools ...
>>>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>>>
>>>
>>>
>>> --
>>> http://john-mckeon.us
>>>
>>> _______________________________________________
>>> Magritte, Pier and Related Tools ...
>>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>>>
>>
>>
>>
>> --
>> Lukas Renggli
>> www.lukas-renggli.ch
>>
>> _______________________________________________
>> Magritte, Pier and Related Tools ...
>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>
>
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>

--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Nick

Re: Magritte validation problem with MABooleanDescription

Imagine multiple 1:1 relationships where the same some of them refer
to the same object, but have different validation conditions.
Similarly, as we saw before, we can have different objects with the

same description instances.

Would a hash that combined the description's hash with the container description's hash work?

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Lukas Renggli

Re: Magritte validation problem with MABooleanDescription

I submitted an update to Magritte-Model that attempts to fix the
problem, by not walking through the complete graph but only through
the immediate references. Let's see if this solves more problems than
it introduces?

Lukas

On 12 October 2010 09:32, Nick Ager <[hidden email]> wrote:

>>
>> Imagine multiple 1:1 relationships where the same some of them refer
>> to the same object, but have different validation conditions.
>> Similarly, as we saw before, we can have different objects with the
>> same description instances.
>>
> Would a hash that combined the description's hash with the container
> description's hash work?
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>

--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Nick

Re: Magritte validation problem with MABooleanDescription

Thanks Lukas it solves my problem.

On 12 October 2010 11:59, Lukas Renggli <[hidden email]> wrote:

I submitted an update to Magritte-Model that attempts to fix the
problem, by not walking through the complete graph but only through
the immediate references. Let's see if this solves more problems than
it introduces?

Lukas

On 12 October 2010 09:32, Nick Ager <[hidden email]> wrote:
>>
>> Imagine multiple 1:1 relationships where the same some of them refer
>> to the same object, but have different validation conditions.
>> Similarly, as we saw before, we can have different objects with the
>> same description instances.
>>
> Would a hash that combined the description's hash with the container
> description's hash work?

> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>

--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki