[fpc-devel] Questions and suggestions to fpWeb

[Luiz Americo Pereira Camara]

Michael Van Canneyt escreveu:
>
>
> On Fri, 30 Jul 2010, Luiz Americo Pereira Camara wrote:
>
>> Hi,
>>
>> I've playing a bit with fpWeb and have some suggestions and questions:
>>
>> - In TFPWebAction.DoHandleRequest if request is not handled by 
>> OnRequest and inherited, the content is copied to the response and 
>> handled is checked by the response content.
>> - FContensts will be always created even if is not created previously
>> - At this point AResponse.Contents can have some content?
>> - If so the handled checking is wrong. See these conditions
>>    AResponse.Contents.Text <> '';
>>    Self.Contents.Text = '';
>>    Handled will be true. Should be false.
>
> Why ? It is handled ?

No. Because TWebAction.Contents is empty.

>> - Content and Contents properties are redundant. Also can lead to 
>> some performance issues. See the code of DoGetContent:
>>
>>   If (Self.Content<>'') then
>>     Content.Write(Self.Content[1],Length(Self.Content));
>>
>> Content is computed (Contents items concatenated) three times.
>>
>> Don't be surprised if users do things like
>>
>> while  not ready do
>> Content := Content + 'xxx';
>
> I agree that the contents/content properties should be reduced to 1
> property. But this is a change which will break backwards compatibility.

AFAIK the fpWeb interface is still in progress. Anyway, better change 
now than later.

>>
>> - TFPWebAction.GetContent is empty. It seems that should call 
>> TFPWebAction.DoGetContent.
>> DoGetContent is inneficient. Cached FContents.Text and and isolated 
>> string references
>
> Suggestions for improvements are welcome.

See patch below (in previous mail)

>
>>
>> - In TCustomFPWebModule.HandleRequest assigned(Session) will always 
>> evaluate to true (if Session is nil it will be created and then freed 
>> again).
>
> I will look into this.
>
>>
>> - It's not necessary to check for nil in SetTemplate
>> - In the other side a check to nil should be added in SetContents to 
>> avoid creating FContents
>>
>> - CGI apps enters in a infinite loop.
>> Adding
>>
>> if FWebHandler.FTerminated then
>>   Terminate;
>>
>> to TCustomWebApplication.DoRun fixes it.
>
> This is not sufficient, but I have committed a fix in revision 15698.
> Please test it.

OK. BTW what's the intention of adding TWebHandler AFAIK only add 
another layer.
>
>>
>> Follows patch with my suggestions.
>
> Can you please create bug reports for the remaining issues ? 

Done

Luiz