Seaside 3.0.4 ... Issue 482

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Seaside 3.0.4 ... Issue 482

Dale Henrichs

I think Issue482 requires a post load doit when loaded into an existing
Seaside3.0.x installation.

The contents of the table class instance variable were changed to effect
the speedup, but if you are updating an existing installation the class
initialize methods won't be re-executed automatically, so the new code
will be running with the old table contents....

Now it's true that I've seen the tests pass when run in an updated
issue, but I am wondering if there might be trouble lurking somewhere...

A naive comparison of the WAEncoder code in 3.0.3:

   encoded isString
     ifTrue: [ stream nextPutAll: encoded ]
     ifFalse: [ stream nextPut: encoded ]

and in 3.0.4:

   encoded notNil
     ifTrue: [ stream nextPutAll: encoded ]
     ifFalse: [ stream nextPut: encoded ]

makes me think that if the encode variable is a Character then it will
be passed into the #nextPutAll:, which is expecting a String ... now it
is possible that the Pharo implementation doesn't care whether the arg
to nextPutAll: is a Character or a String, or maybe I'm missing
something ...

Anyway, I guess this is a long winded way of saying that I think an
explicit post load doit is _required_ that explicitly rebuilds the
tables with new contents:

   WAUrlEncoder initialize.
   WAXmlEncoder initialize.

I expect to add this to the Metacello config.

I ran into this in GemStone, because we have a primitive that encodes
the string based on the encoding table and of course the primitive
choked when the table contents changed.

This happens to be one of the reasons that I've branched Seaside-Core
for GemStone ... I don't expect to be able to eliminate all of the
places where I've found it necessary to create a GemStone branch of the
common Seaside code, but perhaps this _is_ an instance where some
creative application of Grease would improve the portability?

Dale
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
Reply | Threaded
Open this post in threaded view
|

Re: Seaside 3.0.4 ... Issue 482

Philippe Marschall
2011/2/15 Dale Henrichs <[hidden email]>:

>
> I think Issue482 requires a post load doit when loaded into an existing
> Seaside3.0.x installation.
>
> The contents of the table class instance variable were changed to effect the
> speedup, but if you are updating an existing installation the class
> initialize methods won't be re-executed automatically, so the new code will
> be running with the old table contents....
>
> Now it's true that I've seen the tests pass when run in an updated issue,
> but I am wondering if there might be trouble lurking somewhere...
>
> A naive comparison of the WAEncoder code in 3.0.3:
>
>  encoded isString
>    ifTrue: [ stream nextPutAll: encoded ]
>    ifFalse: [ stream nextPut: encoded ]
>
> and in 3.0.4:
>
>  encoded notNil
>    ifTrue: [ stream nextPutAll: encoded ]
>    ifFalse: [ stream nextPut: encoded ]
>
> makes me think that if the encode variable is a Character then it will be
> passed into the #nextPutAll:, which is expecting a String ... now it is
> possible that the Pharo implementation doesn't care whether the arg to
> nextPutAll: is a Character or a String, or maybe I'm missing something ...
>
> Anyway, I guess this is a long winded way of saying that I think an explicit
> post load doit is _required_ that explicitly rebuilds the tables with new
> contents:
>
>  WAUrlEncoder initialize.
>  WAXmlEncoder initialize.
>
> I expect to add this to the Metacello config.

Hmm. I changed the class side #initialize methods of WAUrlEncoder and
WAXmlEncoder in the fix. The way I understand how Monticello works is
that in this case it should send these messages to the class again. At
least on Pharo/Squeak.

Cheers
Philippe
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
Reply | Threaded
Open this post in threaded view
|

Re: Seaside 3.0.4 ... Issue 482

Dale Henrichs
On 02/15/2011 10:01 PM, Philippe Marschall wrote:

> 2011/2/15 Dale Henrichs<[hidden email]>:
>>
>> I think Issue482 requires a post load doit when loaded into an existing
>> Seaside3.0.x installation.
>>
>> The contents of the table class instance variable were changed to effect the
>> speedup, but if you are updating an existing installation the class
>> initialize methods won't be re-executed automatically, so the new code will
>> be running with the old table contents....
>>
>> Now it's true that I've seen the tests pass when run in an updated issue,
>> but I am wondering if there might be trouble lurking somewhere...
>>
>> A naive comparison of the WAEncoder code in 3.0.3:
>>
>>   encoded isString
>>     ifTrue: [ stream nextPutAll: encoded ]
>>     ifFalse: [ stream nextPut: encoded ]
>>
>> and in 3.0.4:
>>
>>   encoded notNil
>>     ifTrue: [ stream nextPutAll: encoded ]
>>     ifFalse: [ stream nextPut: encoded ]
>>
>> makes me think that if the encode variable is a Character then it will be
>> passed into the #nextPutAll:, which is expecting a String ... now it is
>> possible that the Pharo implementation doesn't care whether the arg to
>> nextPutAll: is a Character or a String, or maybe I'm missing something ...
>>
>> Anyway, I guess this is a long winded way of saying that I think an explicit
>> post load doit is _required_ that explicitly rebuilds the tables with new
>> contents:
>>
>>   WAUrlEncoder initialize.
>>   WAXmlEncoder initialize.
>>
>> I expect to add this to the Metacello config.
>
> Hmm. I changed the class side #initialize methods of WAUrlEncoder and
> WAXmlEncoder in the fix. The way I understand how Monticello works is
> that in this case it should send these messages to the class again. At
> least on Pharo/Squeak.


Ahh, you are correct! Sorry about that. It works that way in GemStone,
too....the primitive was blowing up precisely because the new initialize
methods _had_ been run...

As Emily Littella used to say: "Never mind"

Dale

_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev