10 Oct
2010
10 Oct
'10
9:41 p.m.
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
11 Oct
11 Oct
9:32 a.m.
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
11:49 a.m.
HI Lukas,
On 11 October 2010 08:32, Lukas Renggli <renggli(a)gmail.com> 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 ]
11:53 a.m.
(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 <nick.ager(a)gmail.com> wrote:
> HI Lukas,
>
> On 11 October 2010 08:32, Lukas Renggli <renggli(a)gmail.com> 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
12:05 p.m.
On 11 October 2010 10:53, Lukas Renggli <renggli(a)gmail.com> wrote:
I understood that part but don't understand the intent of:
previous := object. object := anObject.
aBlock ensure: [ object := previous ]
(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.
12:11 p.m.
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 <nick.ager(a)gmail.com> wrote:
On 11 October 2010 10:53, Lukas Renggli <renggli(a)gmail.com> wrote:
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
(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. 
12 Oct
12 Oct
2:46 a.m.
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
<http://code.google.com/p/magritte-metamodel/issues/detail?id=7>John
On Mon, Oct 11, 2010 at 6:11 AM, Lukas Renggli <renggli(a)gmail.com> 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 <nick.ager(a)gmail.com> wrote:
--
http://john-mckeon.us
On 11 October 2010 10:53, Lukas Renggli <renggli(a)gmail.com> wrote:
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
(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.
8:14 a.m.
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 <p3anoman(a)gmail.com> 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 <renggli(a)gmail.com> wrote:
--
Lukas Renggli
www.lukas-renggli.ch
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 <nick.ager(a)gmail.com> wrote:
--
http://john-mckeon.us
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
On 11 October 2010 10:53, Lukas Renggli <renggli(a)gmail.com> wrote:
--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
> (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
8:58 a.m.
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 <p3anoman(a)gmail.com> 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 <renggli(a)gmail.com> wrote:
--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
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 <nick.ager(a)gmail.com> wrote:
--
http://john-mckeon.us
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
On 11 October 2010 10:53, Lukas Renggli <renggli(a)gmail.com> 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
9:14 a.m.
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:
--
Lukas Renggli
www.lukas-renggli.ch
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 <p3anoman(a)gmail.com> wrote:
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
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 <renggli(a)gmail.com> wrote:
--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
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 <nick.ager(a)gmail.com> wrote:
>
>
> On 11 October 2010 10:53, Lukas Renggli <renggli(a)gmail.com> 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
9:32 a.m.
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?
12:59 p.m.
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 <nick.ager(a)gmail.com> wrote:
--
Lukas Renggli
www.lukas-renggli.ch
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
3:14 p.m.
Thanks Lukas it solves my problem.
On 12 October 2010 11:59, Lukas Renggli <renggli(a)gmail.com> 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 <nick.ager(a)gmail.com> wrote:
--
Lukas Renggli
www.lukas-renggli.ch
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
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
4938
days inactive
4940
days old
12 comments
4 participants
participants (4)
-
John McKeon -
Lukas Renggli -
Nick Ager -
Norbert Hartl