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.
- 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';
- TFPWebAction.GetContent is empty. It seems that should call
TFPWebAction.DoGetContent.
DoGetContent is inneficient. Cached FContents.Text and and isolated
string references
- In TCustomFPWebModule.HandleRequest assigned(Session) will always
evaluate to true (if Session is nil it will be created and then freed
again).
- 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.
Follows patch with my suggestions.
Luiz
Index: packages/fcl-web/src/base/fpweb.pp
===================================================================
--- packages/fcl-web/src/base/fpweb.pp (revision 15669)
+++ packages/fcl-web/src/base/fpweb.pp (working copy)
@@ -29,9 +29,7 @@
FOnrequest: TWebActionEvent;
FContents : TStrings;
FTemplate : TFPTemplate;
- function GetStringContent: String;
function GetContents: TStrings;
- procedure SetContent(const AValue: String);
procedure SetContents(const AValue: TStrings);
Procedure SetTemplate(const AValue : TFPTemplate);
Protected
@@ -43,7 +41,6 @@
Destructor destroy; override;
Procedure Assign(Source : TPersistent); override;
published
- Property Content : String Read GetStringContent Write SetContent;
Property Contents : TStrings Read GetContents Write SetContents;
Property OnRequest: TWebActionEvent Read FOnrequest Write FOnrequest;
Property Template : TFPTemplate Read FTemplate Write SetTemplate;
@@ -174,7 +171,7 @@
procedure TFPWebAction.GetContent(ARequest: TRequest; Content: TStream; Var
Handled : Boolean);
begin
-
+ DoGetContent(ARequest, Content, Handled);
end;
procedure TFPWebAction.Assign(Source: TPersistent);
@@ -185,12 +182,12 @@
begin
If (Source is TFPWebAction) then
begin
- A:=Source as TFPWebAction;
+ A:=TFPWebAction(Source);
Name:=A.Name;
- Content:=A.Content;
AfterResponse:=A.AfterResponse;
BeforeRequest:=A.BeforeRequest;
Default:=A.default;
+ Contents:=A.FContents;
ContentProducer:=A.ContentProducer;
OnRequest:=A.OnRequest;
FTemplate.Assign(A.Template);
@@ -211,11 +208,6 @@
inherited destroy;
end;
-function TFPWebAction.GetStringContent: String;
-begin
- Result:=Contents.Text;
-end;
-
function TFPWebAction.GetContents: TStrings;
begin
If Not Assigned(FContents) then
@@ -223,23 +215,17 @@
Result:=FContents;
end;
-procedure TFPWebAction.SetContent(const AValue: String);
+procedure TFPWebAction.SetContents(const AValue: TStrings);
begin
- If (AValue='') then
+ if AValue = nil then
FreeAndNil(FContents)
else
- Contents.Text:=AValue;
-end;
-
-procedure TFPWebAction.SetContents(const AValue: TStrings);
-begin
- Contents.Assign(AValue);
+ Contents.Assign(AValue);
end;
procedure TFPWebAction.SetTemplate(const AValue: TFPTemplate);
begin
- If Assigned(AValue) then
- FTemplate.Assign(AValue);
+ FTemplate.Assign(AValue);
end;
@@ -268,8 +254,9 @@
Inherited DoHandleRequest(ARequest,AResponse,Handled);
If not Handled then
begin
- AResponse.Contents.AddStrings(Self.Contents);
- Handled:=(AResponse.Content<>'');
+ Handled := (FContents <> nil) and (FContents.Count > 0);
+ if Handled then
+ AResponse.Contents.AddStrings(FContents);
end;
end;
{$ifdef cgidebug}
@@ -279,12 +266,24 @@
procedure TFPWebAction.DoGetContent(ARequest: TRequest; Content: TStream; Var
Handled : Boolean);
+ //isolate string references in a subprocedure to avoid implicit exceptions
in main procedure
+ procedure CopyContent;
+ var
+ ContentStr: String;
+ begin
+ if FContents <> nil then
+ begin
+ ContentStr := FContents.Text;
+ If ContentStr<>'' then
+ Content.Write(ContentStr[1],Length(ContentStr));
+ end;
+ end;
+
begin
If Assigned(ContentProducer) then
ContentProducer.GetContent(ARequest,Content,Handled)
else
- If (Self.Content<>'') then
- Content.Write(Self.Content[1],Length(Self.Content));
+ CopyContent;
end;
_______________________________________________
fpc-devel maillist - fpc-devel@lists.freepascal.org
http://lists.freepascal.org/mailman/listinfo/fpc-devel
