DoBeforeRequest question about brookframework HOT 46 CLOSED

As its name says, DoBeforeRequest happens.. well, before request (is handled). So, the actual request is still processed. If you need to redirect at that time, override HandleRequest instead.

Sent from my Android phone with mail.com Mail. Please excuse my brevity.

tcaduto [email protected] wrote:

Hi,
I want to intercept before each request and if the session is expired show a session expired page, I sort of have this working by creating a new class where I override the DoBeforeRequest and then my new actions are created from the "base" session class.
This works and the DoBeforeRequest fires and I can check if the session is expired, the problem is when I do a redirect in DoBeforeRequest the actual dorequest still fires and it never redirects to the session expired page.

procedure TSessionAction.DoBeforeRequest(ARequest: TRequest;
AResponse: TResponse);
var
path:string;
begin
path:=ARequest.PathInfo;
if path = '/logon' then
exit;
fsession.Start(GetRequest);
//if the session is expired redirect to session timeout page
if (FSession.IsExpired) then
Redirect(UrlFor(TSessionExpired, []),301);
end;

—
Reply to this email directly or view it on GitHub.

from brookframework.

ok, I will check out HandleRequest.

from brookframework.

You mean DoRequest right?

from brookframework.

This should do the trick:
procedure TSessionAction.DoRequest(ARequest: TRequest; AResponse: TResponse);
begin
fsession.Start(ARequest);
if fsession.Expired then
Redirect('/sessiontimeout') else
inherited DoRequest(ARequest, AResponse);
end;

from brookframework.

Ah, yes. Sorry :">

Sent from my Android phone with mail.com Mail. Please excuse my brevity.

tcaduto [email protected] wrote:

You mean DoRequest right?

—
Reply to this email directly or view it on GitHub.

from brookframework.

Well, it kind of works, I am having a issue with doing the redirect.
Seems if I create my inherited class in another unit I can't access frequest and fresponse in my
overridden dorequests, so the redirect fails with a exception because frequest and fresponse are not set.
It works if I move Frequest and Fresponse to public instead of private or create a public property for each one in tbrooksession. I kind of like having my actions in separate units. Maybe a class helper would work?

fsession.Start(ARequest);
//if the session is expired redirect to session timeout page
if (FSession.IsExpired) then
begin
FRequest:=ARequest;
FResponse:=AResponse;
url:=UrlFor(TSessionExpired, []);
redirect(url);
end else
inherited DoRequest(ARequest,AResponse);

from brookframework.

Class helper or class cracker did not work. Is there anyway to get around this without modifying the brook source code?

from brookframework.

@tcaduto, can you send a small demo and a short explanation how to use your demo? I want to debug it here in my PC...

from brookframework.

Sure, if I move
FRequest: TRequest;
FResponse: TResponse;
in TbrookAction to Protected then I can use a class cracker to set them in my overridden DoAction;

unit session_action;

{$mode objfpc}{$H+}

interface

uses
BrookAction,BrookSession,HTTPDefs;

type

__cracker = class(TBrookAction);

{ TSessionAction }

TSessionAction = class(TBrookAction)
public
FSession: TBrookSession;
constructor Create; override;
destructor Destroy; override;
procedure DoRequest(ARequest: TRequest;AResponse: TResponse);override;
end;

implementation

uses session_expired, Unit1;

constructor TSessionAction.Create;
begin
inherited Create;
FSession := TBrookSession.Create;
FSession.TimeOut:=15;
fsession.FileName:='session.txt';
fsession.Directory:='C:\Users\20659\Documents\Lazarus Projects\BrookTest\sessions';
end;

destructor TSessionAction.Destroy;
begin
FSession.Finish(GetResponse);
FSession.free;
inherited Destroy;
end;

procedure TSessionAction.DoRequest(ARequest: TRequest;
AResponse: TResponse);
var
url:string;
begin
if self is TLogon then
begin
inherited DoRequest(ARequest,AResponse);
exit;
end;
fsession.Start(ARequest);
//if the session is expired redirect to session timeout page
if (FSession.IsExpired) then
begin
//This works if Frequest and Fresponse are protected vs private
__cracker(self).FRequest:=ARequest;
__cracker(self).FResponse:=AResponse;
url:=UrlFor(TSessionExpired, []);
redirect(url);
end else
inherited DoRequest(ARequest,AResponse);
end;

initialization

end.

from brookframework.

Going to email you the whole project, it's just a test thing, so it's messy.

Run it, then point browser to local host and put in tcaduto for username and 12345678 for password, then
the welcome page loads and shows some session vars, then if you reload the welcome page after 10 seconds
it should jump to the session timeout page if the frequest and fresponse are moved to protected vs private.

from brookframework.

I didn't understand your code seeing partially. See a small demo with session: https://github.com/silvioprog/brookframework/blob/master/demos/others/chat/src/auth.pas. It's working online here: http://brookframework.org/demos/others/chat/cgi1.fbf

from brookframework.

I received your email with the attachment. :) I'll put it here ...

from brookframework.

Just emailed you the whole project. I don't want to have to check if the session is expired in every single action I create, so I created a new class based on tbrookaction and overrode the DoRequest, that way I can check if the session is expired before the request is processed. I can then have full session support just be creating new actions using the derived one with the overridden DoRequest.

from brookframework.

Done. (http://www.sendspace.com/file/cs2jxj)

from brookframework.

The issue is I can't set Frequest or Fresponse in my overriden DoRequest method so the redirect works.

I initially tried to override DoBeforeRequest but there is no way to set a handled var so the Request which happens next is skipped.

maybe something like this:

procedure TBrookAction.DoBeforeRequest(ARequest: TRequest; AResponse: TResponse; var handled:boolean);
begin
end;

Then
procedure TBrookAction.DoRequest(ARequest: TRequest; AResponse: TResponse);
var
handled:boolean;
begin
FRequest := ARequest;
FResponse := AResponse;
DoBeforeRequest(ARequest, AResponse,handled);
if handled then
exit;
Request(ARequest, AResponse);
DoAfterRequest(ARequest, AResponse);
end;

from brookframework.

Ah ha... You want to implement it for all actions. You can implement it directly in router instead a class helper. Choose the best option. :)

from brookframework.

The issue is I can't set Frequest or Fresponse in my overriden DoRequest method so the redirect works.
I initially tried to override DoBeforeRequest but there is no way to set a handled var so the Request which happens next is skipped.
maybe something like this:

Hm... Please make a patch and send it via pull request.

from brookframework.

Not all actions, just certain ones, for example if I made a action to download a file I wouldn't want it to to session checking etc.

Do you think have a handled param in DoBeforeRequest would be feasible?

from brookframework.

I am not set up for patches, but all I did was this:

procedure TBrookAction.DoBeforeRequest(ARequest: TRequest; AResponse: TResponse;var handled:boolean = false);
begin
end;

procedure TBrookAction.DoRequest(ARequest: TRequest; AResponse: TResponse);
var
handled:boolean;
begin
FRequest := ARequest;
FResponse := AResponse;
DoBeforeRequest(ARequest, AResponse,handled);
if handled then
exit;
Request(ARequest, AResponse);
DoAfterRequest(ARequest, AResponse);
end;

from brookframework.

I understand, and I would to:

TAuthAction = class(TBrookAction) ...

TLogin = class(TAuthAction) ... // implementing session

TCPanel = class(TAuthAction) ...

TDownload = class(TBrookAction) ...

TAbout = class(TBrookAction) ...

But an AHandle param seems a good idea. @mario is the author of DoBeforeRequest/DoAfterRequest methods. So @mario, you think that this parameter would be a good idea?

I think that DoAfterRequest must be executed even if AHandler is true, e.g:

...
if not AHandled then
  Request(ARequest, AResponse);
DoAfterRequest(ARequest, AResponse);
...

from brookframework.

I just implemented the handled idea and it works great:
(I agree the DoAfterRequest should be fired)

procedure TSessionAction.DoBeforeRequest(ARequest: TRequest;
AResponse: TResponse; var handled: boolean);
var
url:string;
begin
if self is TLogon then
exit;
fsession.Start(ARequest);
//if the session is expired redirect to session timeout page
if (FSession.IsExpired) then
begin
handled:=true;
url:=UrlFor(TSessionExpired, []);
redirect(url);
end;
end;

from brookframework.

I changed my copy to:

procedure TBrookAction.DoBeforeRequest(ARequest: TRequest; AResponse: TResponse;var Ahandled:boolean = false);
begin
end;

procedure TBrookAction.DoRequest(ARequest: TRequest; AResponse: TResponse);
var
Ahandled:boolean;
begin
FRequest := ARequest;
FResponse := AResponse;
DoBeforeRequest(ARequest, AResponse,Ahandled);
if not Ahandled then
Request(ARequest, AResponse);
DoAfterRequest(ARequest, AResponse);
end;

from brookframework.

Just as a background I currently have a app I did with CherryPy which has a DoBeforeRequest method that you can redirect if the session has expired and want to use Brook to replace it.

from brookframework.

With that little change you can base all actions you want to have session support on a child class and never have to worry about it in the actions at all. Works super nice and handy :-)

from brookframework.

@leledumbo , news, questions? :-)

from brookframework.

What if DoRequest checks for FResponse.HasContent? I don't like changing APIs in the name of backward compatibility

from brookframework.

Can you send the full implementation?

To avoid inconveniences, we could put the word 'deprecated' in the current implementation.

from brookframework.

Forget about it, I think too fast. There's a possibility that one wants to output something in DoBeforeRequest AND the main request, even also on DoAfterRequest. The Handled parameter could indeed be put as the last parameter and given default value for backward compatibility.

Now on to the logic, the above implementation flow is:

DoBeforeRequest(Request,Response,Handled)
if not Handled then Request(Request,Response)
DoAfterRequest(Request,Response)

should it be made:

DoBeforeRequest(Request,Response,Handled)
if not Handled then Request(Request,Response,Handled)
if not Handled then DoAfterRequest(Request,Response)

or maybe the first one is already OK?

from brookframework.

I think the first one would be good because you may want to do additional
processing in do after request.
On Apr 26, 2013 11:28 PM, "Mario Ray Mahardhika" [email protected]
wrote:

Forget about it, I think too fast. There's a possibility that one wants to
output something in DoBeforeRequest AND the main request, even also on
DoAfterRequest. The Handled parameter could indeed be given default value
for backward compatibility.

Now on to the logic, the above implementation flow is:

DoBeforeRequest(Request,Response,Handled)if not Handled then Request(Request,Response)DoAfterRequest(Request,Response)

should it be made:

DoBeforeRequest(Request,Response,Handled)if not Handled then Request(Request,Response,Handled)if not Handled then DoAfterRequest(Request,Response)

or maybe the first one is already OK?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-17109635
.

from brookframework.

I think that you don't need to change the current implementation. See example below:

procedure Request(ARequest: TRequest; AResponse: TResponse); override;
...
procedure TSessionAction.Request(ARequest: TRequest; AResponse: TResponse);
begin
  // do nothing
end;

I.e, if you need to 'kill' the "Request" method, just doesn't implement their inheritance. :)

from brookframework.

I don't think that would work where you create a single descendent to
handle your sessions
On Apr 27, 2013 2:46 PM, "Silvio Clecio" [email protected] wrote:

I think that you don't need to change the current implementation. See
example below:

procedure Request(ARequest: TRequest; AResponse: TResponse); override;
...
procedure TSessionAction.Request(ARequest: TRequest; AResponse: TResponse);
begin
// do nothing
end;

I.e, if you need to 'kill' the "Request" method, just doesn't implement
their inheritance. :)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-17121167
.

from brookframework.

But there is no differences between this sequence and the sequence of your code. The difference is only that one have a param and other not. :) E.g:

procedure TSessionAction.Request(ARequest: TRequest; AResponse: TResponse);
begin
  // do nothing
end;

procedure TSessionAction.DoBeforeRequest(ARequest: TRequest; AResponse: TResponse);
var
  url:string;
begin
  if self is TLogon then
    exit;
  fsession.Start(ARequest);
  if (FSession.IsExpired) then
  begin
    url:=UrlFor(TSessionExpired, []);
    redirect(url);
  end;
end;

Please, try run this implementation above (I don't tested it).

from brookframework.

Yes, but wouldn't that permanently disable the request? You only want to
kill the original request if the session has expired. I tried to override
do request and then skip the inherited if the session was expired but you
need access to frequest and fresponse, if you move fresponse and frequest
to protected you can access them via a class cracker.
On Apr 27, 2013 3:11 PM, "Silvio Clecio" [email protected] wrote:

But there is no differences between this sequence and the sequence of your
code. The difference is only that one have a param and other not. :) E.g:

procedure TSessionAction.Request(ARequest: TRequest; AResponse: TResponse);
begin
// do nothing
end;

procedure TSessionAction.DoBeforeRequest(ARequest: TRequest; AResponse: TResponse; var handled: boolean);
var
url:string;
begin
if self is TLogon then
exit;
fsession.Start(ARequest);
if (FSession.IsExpired) then
begin
url:=UrlFor(TSessionExpired, []);
redirect(url);
end;
end;

Please, try run this implementation above (I don't tested it).

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-17121558
.

from brookframework.

OK. So, please try this change implementing on your copy:

  TBrookAction = class(TBrookObject)
...
  protected
    property Request: TRequest read FResponse;
    property Response: TResponse read FResponse;
...

It adds two new protected properties on TBrookAction, If it work fine, IMHO it can be added on Brook without problems.

from brookframework.

Sorry, change to:

TBrookAction = class(TBrookObject)
...
  protected
    property Request: TRequest read FRequest;
    property Response: TResponse read FResponse;
...

from brookframework.

@tcaduto , it worked for you?

from brookframework.

Sorry, been out of town with only my phone.
On Apr 29, 2013 2:27 PM, "Silvio Clecio" [email protected] wrote:

@tcaduto https://github.com/tcaduto , it worked for you?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-17188476
.

from brookframework.

Hello @tcaduto , you could test if it worked? If yes, I'll apply it in trunk. :)

A small change is:

  protected
    property RequestProp: TRequest read FRequest;
    property ResponseProp: TResponse read FResponse;

from brookframework.

It works but requires a cracker class to work properly, the handled param
was a much more elegant solution.

Why was the reason that was frowned upon? Its a minor change to the API
On May 20, 2013 3:49 PM, "Silvio Clecio" [email protected] wrote:

Hello @tcaduto https://github.com/tcaduto , you could test if it
worked? If yes, I'll apply it on trunk. :)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-18172534
.

from brookframework.

If the first change doesn't compromise old codes and not decrease the performance of the class, it can be applied without problems. I can add a overloaded method with "AHandled" param.

from brookframework.

I don't have any old code to test it with but it did have a default param
Which should have made it transparent to any existing code.
On May 20, 2013 4:03 PM, "Silvio Clecio" [email protected] wrote:

If the first change doesn't compromise old codes and not decrease the
performance of the class, it can be applied without problems.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-18174029
.

from brookframework.

Please, can you test this new implementation in attached?: http://www.sendspace.com/file/ke0jwo (this is a GIT patch). It implements a overloaded method. Seems better two methods, because the compiler will choose the implementation according the number of params.

from brookframework.

OK. If the last patch that I sent work fine (http://www.sendspace.com/file/ke0jwo), I'll apply it on working branche.

I don't have any old code to test it with but it did have a default param Which should have made it transparent to any existing code.

from brookframework.

I'll commit it. Can I close this issue?

from brookframework.

Yes sounds good
On May 26, 2013 4:28 PM, "Silvio Clecio" [email protected] wrote:

I'll commit it. Can I close this issue?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-18470035
.

from brookframework.

Thank you very much! :)

from brookframework.