[fpc-devel] Questions and suggestions to fpWeb

[Michael Van Canneyt]

On Tue, 3 Aug 2010, Luiz Americo Pereira Camara wrote:

> 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.

But that can be a perfectly valid result ?

>>> - 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.

The question is: which one ?

>
>>> 
>>> - 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)

I have seen it.

>
>> 
>>> 
>>> - 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.

There is a simple reason: if you want to have a service application, there
will be 2 application objects. Now you can have only 1 application object
(service application) which uses the webhandler to process requests.

You can now also produce 1 binary which can be configured to run as a
CGI/FCGI/Service/standalone server, and which will have 1 single application
instance for all 4 cases.

>>> Follows patch with my suggestions.
>> 
>> Can you please create bug reports for the remaining issues ? 
>
> Done

Thank you.

Michael.