You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by Henri Dupre <he...@gmail.com> on 2006/07/24 20:43:28 UTC

IExternalPage and pageValidate

One thing that has been bugging me lately in Tapestry 4 is the order of
activateExternalPage and pageValidate...
I've been using pageValidate to verify that all data is correct (e.g. select
the default value...) but when the page is called with activateExternalPage,
pageValidate gets called before activateExternalPage, thus making this call
useless as the page is usually blank at this point.

Does anyone else think that life would be better if pageValidate was called
after activateExternalPage?

Thanks,

Henri.

Re: IExternalPage and pageValidate

Posted by Jesse Kuhnert <jk...@gmail.com>.
Hmm....I probably should have done more reading before implementing. Will
revert the changes either tomorrow or definitely the day after.

On 7/24/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
>
> Hi Henri.
>
> Although what you say makes sense in your case,
> validate/pageValidate/PageValidateListener
> were originally intended to validate that "the user is allowed to visit
> the page".
>
> http://tapestry.apache.org/tapestry4/tapestry/apidocs/org/apache/tapestry/IPage.html#validate(org.apache.tapestry.IRequestCycle)
>
> Traditionally confusing Tapestry naming, I know ;-) but I use it in this
> way,
> and also to throw redirects to https etc.
>
> If the order is changed then it will be a breaking change for me, and I
> suspect for
> others too, but more importantly we would need to keep in mind that
> activity in
> activateExternalPage would not be safe-guarded by pageValidate from
> malicious use.
>
> Security code intended for all pages should be executed before
> activateExternalPage.
> It could be placed in pageAttached, but initialization code you are
> putting in
> pageValidate has traditionally been put in pageBeginRender as suggested
> here:
>
> http://tapestry.apache.org/tapestry4/tapestry/apidocs/org/apache/tapestry/IPage.html#attach(org.apache.tapestry.IEngine,%20org.apache.tapestry.IRequestCycle)
>
> Is the stuff you put in pageValidate per-page? Why not pageBeginRender?
> A bit of discussion might be nice before shuffling page events around. ;-)
>
> Cheers,
> Nick.
>
>
> Henri Dupre wrote:
> > On 7/24/06, Jesse Kuhnert <jk...@gmail.com> wrote:
> >
> >>
> >> Sure, I can do that.
> >>
> >> 4.1 is still "unstable", so I guess if this breaks someone I can always
> >> revert pretty easily. I'll do it later today and reply back when it's
> >> deployed.
> >
> >
> >
> > Fantastic. I'm starting to really like the frequent releases.
> >
> > Thanks,
> >
> > Henri.
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>


-- 
Jesse Kuhnert
Tacos/Tapestry, team member/developer

Open source based consulting work centered around
dojo/tapestry/tacos/hivemind.

Re: IExternalPage and pageValidate

Posted by Jesse Kuhnert <jk...@gmail.com>.
4 classes? I was too lazy to do that. Instead I just defined one proxy that
universally handles all of the methods involved. (where applicable, there's
usually only one core method involved, IEngineService.service(IRequestCycle)
)

On 7/26/06, Henri Dupre <he...@gmail.com> wrote:
>
> Sounds good!
>
> I've done some hivemind interception but I find it quite heavyweight. It
> takes 4 classes just to intercept one method on one service (unless I
> missed
> something). I'll still need to look at the pageLoad hook that you showed
> me
> but that far pageValidate is the only location I know where you can throw
> a
> PageRedirectException.
>
>
>
> On 7/25/06, Jesse Kuhnert <jk...@gmail.com> wrote:
> >
> > I'll get to it tomorrow.
> >
> > I have to admit that I've never found a reason to use the majority of
> page
> > related methods. (pageBeginRender being the exception). Hivemind has
> > always
> > existed since I've known tapestry so intercepting services/etc has
> always
> > been my preferred approach for things like this.
> >
> > On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
> > >
> > > Guess I'm +0 this. I look forward to comments from the devs.
> > >
> > > Looking at the naming of things it's obvious (with hindsight)
> > > that the pageBeginRender name doesn't quite fit with the rewind.
> > > Sure it's a kind of virtual render, but the intended use of the
> > > event is not very clear.
> > >
> > > pageInitialise (actually, should be -ize) is probably not a good
> > > name either. Given activateExternalPage, perhaps pageActivate?
> > >
> > > Bah. The hardest part of programming. ;-)
> > > Hopefully Howard nails this in T5!
> > >
> > > Cheers,
> > > Nick.
> > >
> > >
> > > Henri Dupre wrote:
> > > > On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
> > > >
> > > >> The other option is another listener, but it seems a bit
> superfluous:
> > > >>
> > > >> pageAttached
> > > >> pageValidate - check user is allowed to access page
> > > >> activateExternalPage - setup here - but only for external pages
> > > >> +++ pageInitialise - setup here - not called for rewind?
> > > >
> > > >
> > > > I like this option... It makes sense to me and looks better than
> > > > pageBeginRender...
> > > >
> > > > pageBeginRender - setup here - but check for rewind
> > > >
> > > >> pageEndRender
> > > >> pageDetached
> > > >
> > > >
> > > > Cheers,
> > > >
> > > > Henri.
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> > > For additional commands, e-mail: dev-help@tapestry.apache.org
> > >
> > >
> >
> >
> > --
> > Jesse Kuhnert
> > Tacos/Tapestry, team member/developer
> >
> > Open source based consulting work centered around
> > dojo/tapestry/tacos/hivemind.
> >
> >
>
>
> --
> Thanks,
>
> Henri.
>
>


-- 
Jesse Kuhnert
Tacos/Tapestry, team member/developer

Open source based consulting work centered around
dojo/tapestry/tacos/hivemind.

Re: IExternalPage and pageValidate

Posted by Henri Dupre <he...@gmail.com>.
Sounds good!

I've done some hivemind interception but I find it quite heavyweight. It
takes 4 classes just to intercept one method on one service (unless I missed
something). I'll still need to look at the pageLoad hook that you showed me
but that far pageValidate is the only location I know where you can throw a
PageRedirectException.



On 7/25/06, Jesse Kuhnert <jk...@gmail.com> wrote:
>
> I'll get to it tomorrow.
>
> I have to admit that I've never found a reason to use the majority of page
> related methods. (pageBeginRender being the exception). Hivemind has
> always
> existed since I've known tapestry so intercepting services/etc has always
> been my preferred approach for things like this.
>
> On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
> >
> > Guess I'm +0 this. I look forward to comments from the devs.
> >
> > Looking at the naming of things it's obvious (with hindsight)
> > that the pageBeginRender name doesn't quite fit with the rewind.
> > Sure it's a kind of virtual render, but the intended use of the
> > event is not very clear.
> >
> > pageInitialise (actually, should be -ize) is probably not a good
> > name either. Given activateExternalPage, perhaps pageActivate?
> >
> > Bah. The hardest part of programming. ;-)
> > Hopefully Howard nails this in T5!
> >
> > Cheers,
> > Nick.
> >
> >
> > Henri Dupre wrote:
> > > On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
> > >
> > >> The other option is another listener, but it seems a bit superfluous:
> > >>
> > >> pageAttached
> > >> pageValidate - check user is allowed to access page
> > >> activateExternalPage - setup here - but only for external pages
> > >> +++ pageInitialise - setup here - not called for rewind?
> > >
> > >
> > > I like this option... It makes sense to me and looks better than
> > > pageBeginRender...
> > >
> > > pageBeginRender - setup here - but check for rewind
> > >
> > >> pageEndRender
> > >> pageDetached
> > >
> > >
> > > Cheers,
> > >
> > > Henri.
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> > For additional commands, e-mail: dev-help@tapestry.apache.org
> >
> >
>
>
> --
> Jesse Kuhnert
> Tacos/Tapestry, team member/developer
>
> Open source based consulting work centered around
> dojo/tapestry/tacos/hivemind.
>
>


-- 
Thanks,

Henri.

Re: IExternalPage and pageValidate

Posted by Jesse Kuhnert <jk...@gmail.com>.
I'll get to it tomorrow.

I have to admit that I've never found a reason to use the majority of page
related methods. (pageBeginRender being the exception). Hivemind has always
existed since I've known tapestry so intercepting services/etc has always
been my preferred approach for things like this.

On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
>
> Guess I'm +0 this. I look forward to comments from the devs.
>
> Looking at the naming of things it's obvious (with hindsight)
> that the pageBeginRender name doesn't quite fit with the rewind.
> Sure it's a kind of virtual render, but the intended use of the
> event is not very clear.
>
> pageInitialise (actually, should be -ize) is probably not a good
> name either. Given activateExternalPage, perhaps pageActivate?
>
> Bah. The hardest part of programming. ;-)
> Hopefully Howard nails this in T5!
>
> Cheers,
> Nick.
>
>
> Henri Dupre wrote:
> > On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
> >
> >> The other option is another listener, but it seems a bit superfluous:
> >>
> >> pageAttached
> >> pageValidate - check user is allowed to access page
> >> activateExternalPage - setup here - but only for external pages
> >> +++ pageInitialise - setup here - not called for rewind?
> >
> >
> > I like this option... It makes sense to me and looks better than
> > pageBeginRender...
> >
> > pageBeginRender - setup here - but check for rewind
> >
> >> pageEndRender
> >> pageDetached
> >
> >
> > Cheers,
> >
> > Henri.
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>


-- 
Jesse Kuhnert
Tacos/Tapestry, team member/developer

Open source based consulting work centered around
dojo/tapestry/tacos/hivemind.

Re: IExternalPage and pageValidate

Posted by Nick Westgate <ni...@key-planning.co.jp>.
Guess I'm +0 this. I look forward to comments from the devs.

Looking at the naming of things it's obvious (with hindsight)
that the pageBeginRender name doesn't quite fit with the rewind.
Sure it's a kind of virtual render, but the intended use of the
event is not very clear.

pageInitialise (actually, should be -ize) is probably not a good
name either. Given activateExternalPage, perhaps pageActivate?

Bah. The hardest part of programming. ;-)
Hopefully Howard nails this in T5!

Cheers,
Nick.


Henri Dupre wrote:
> On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
> 
>> The other option is another listener, but it seems a bit superfluous:
>>
>> pageAttached
>> pageValidate - check user is allowed to access page
>> activateExternalPage - setup here - but only for external pages
>> +++ pageInitialise - setup here - not called for rewind?
> 
> 
> I like this option... It makes sense to me and looks better than
> pageBeginRender...
> 
> pageBeginRender - setup here - but check for rewind
> 
>> pageEndRender
>> pageDetached
> 
> 
> Cheers,
> 
> Henri.
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: IExternalPage and pageValidate

Posted by Henri Dupre <he...@gmail.com>.
On 7/25/06, Nick Westgate <ni...@key-planning.co.jp> wrote:
>
> The other option is another listener, but it seems a bit superfluous:
>
> pageAttached
> pageValidate - check user is allowed to access page
> activateExternalPage - setup here - but only for external pages
> +++ pageInitialise - setup here - not called for rewind?


I like this option... It makes sense to me and looks better than
pageBeginRender...

pageBeginRender - setup here - but check for rewind
> pageEndRender
> pageDetached


Cheers,

Henri.

Re: IExternalPage and pageValidate

Posted by Nick Westgate <ni...@key-planning.co.jp>.
Hi Henri, Jesse.

> That makes sense too but in this scenario, if you check before the
> activateExternalPage is called and lets imagine the user is not allowed, 
> you
> throw a PageRedirectException => the parameters passed to
> activateExternalPage become lost. So I really do have a problem with the
> logic of this method...

Luckily I haven't had to worry about that yet, but I agree it's a problem.
Still, I'm really just pointing out that this is the way things were done
(mainly by Howard, in his infinite and exponentially increasing wisdom!)
and if the events can be improved without breaking things, I'm all for it!

> I can imagine that there is a security risk if you do actions in the
> activateExternalPage... Is that what you are doing? I usually only load 
> page
> properties from the parameters so my code there is harmless and in the 
> worst case it just throws an exception.

This why I mentioned the change in mentality required if the event order
is switched. Obviously the way pageValidate was intended to be used placed
a natural order on the events. My code is harmless too - it decodes params
and might just use an ASO ... Oh! But the ASO might not be valid if the
pageValidate doesn't enforce it, which mine does. We redefine harmless. ;-)

> Isn't pageBeginRender called twice with a form? One before rewinding and
> once on the rendering?
> My code in there is really per page... stuff like set the default 
> selection.
> I think this is the main reason why I moved all my code to 
> pageValidate... I
> got lots of tortured code that with the rewiding checks in my forms so I
> moved away from this method.

Sure, this is part of the, ahem, *charm* of pageBeginRender. But it's not
too bad - just an if statement present in every pageBeginRender I write! ;-)

     public void pageBeginRender(PageEvent event)
     {
         // initialize properties etc
         if (!event.getRequestCycle().isRewinding())
         {
            // code almost always goes here ...
         }
     }

> At least with frequent releases this can be solved fast ;-)

Yes indeed - no complaints here with respect to dev activity! ;-)
Some amazing work being done by you guys. Still, the cast of characters
in the dev list seems to have thinned out recently, which is a shame.
Topics like this usually benefit from a few different points of view.

The other option is another listener, but it seems a bit superfluous:

pageAttached
pageValidate - check user is allowed to access page
activateExternalPage - setup here - but only for external pages
+++ pageInitialise - setup here - not called for rewind?
pageBeginRender - setup here - but check for rewind
pageEndRender
pageDetached

Cheers,
Nick.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: IExternalPage and pageValidate

Posted by Henri Dupre <he...@gmail.com>.
Hi Nick

On 7/24/06, Nick Westgate <ni...@key-planning.co.jp> wrote:

>
> Although what you say makes sense in your case,
> validate/pageValidate/PageValidateListener
> were originally intended to validate that "the user is allowed to visit
> the page".
>
> http://tapestry.apache.org/tapestry4/tapestry/apidocs/org/apache/tapestry/IPage.html#validate(org.apache.tapestry.IRequestCycle)
>
> Traditionally confusing Tapestry naming, I know ;-) but I use it in this
> way,
> and also to throw redirects to https etc.
>
> If the order is changed then it will be a breaking change for me, and I
> suspect for
> others too, but more importantly we would need to keep in mind that
> activity in
> activateExternalPage would not be safe-guarded by pageValidate from
> malicious use.


That makes sense too but in this scenario, if you check before the
activateExternalPage is called and lets imagine the user is not allowed, you
throw a PageRedirectException => the parameters passed to
activateExternalPage become lost. So I really do have a problem with the
logic of this method...

I can imagine that there is a security risk if you do actions in the
activateExternalPage... Is that what you are doing? I usually only load page
properties from the parameters so my code there is harmless and in the worst
case it just throws an exception.



> Security code intended for all pages should be executed before
> activateExternalPage.
> It could be placed in pageAttached, but initialization code you are
> putting in
> pageValidate has traditionally been put in pageBeginRender as suggested
> here:
>
> http://tapestry.apache.org/tapestry4/tapestry/apidocs/org/apache/tapestry/IPage.html#attach(org.apache.tapestry.IEngine,%20org.apache.tapestry.IRequestCycle)
>
> Is the stuff you put in pageValidate per-page? Why not pageBeginRender?


Isn't pageBeginRender called twice with a form? One before rewinding and
once on the rendering?
My code in there is really per page... stuff like set the default selection.
I think this is the main reason why I moved all my code to pageValidate... I
got lots of tortured code that with the rewiding checks in my forms so I
moved away from this method.


> A bit of discussion might be nice before shuffling page events around. ;-)


At least with frequent releases this can be solved fast ;-)

Cheers,

Henri.

Re: IExternalPage and pageValidate

Posted by Nick Westgate <ni...@key-planning.co.jp>.
Hi Henri.

Although what you say makes sense in your case, validate/pageValidate/PageValidateListener
were originally intended to validate that "the user is allowed to visit the page".
http://tapestry.apache.org/tapestry4/tapestry/apidocs/org/apache/tapestry/IPage.html#validate(org.apache.tapestry.IRequestCycle)

Traditionally confusing Tapestry naming, I know ;-) but I use it in this way,
and also to throw redirects to https etc.

If the order is changed then it will be a breaking change for me, and I suspect for
others too, but more importantly we would need to keep in mind that activity in
activateExternalPage would not be safe-guarded by pageValidate from malicious use.

Security code intended for all pages should be executed before activateExternalPage.
It could be placed in pageAttached, but initialization code you are putting in
pageValidate has traditionally been put in pageBeginRender as suggested here:
http://tapestry.apache.org/tapestry4/tapestry/apidocs/org/apache/tapestry/IPage.html#attach(org.apache.tapestry.IEngine,%20org.apache.tapestry.IRequestCycle)

Is the stuff you put in pageValidate per-page? Why not pageBeginRender?
A bit of discussion might be nice before shuffling page events around. ;-)

Cheers,
Nick.


Henri Dupre wrote:
> On 7/24/06, Jesse Kuhnert <jk...@gmail.com> wrote:
> 
>>
>> Sure, I can do that.
>>
>> 4.1 is still "unstable", so I guess if this breaks someone I can always
>> revert pretty easily. I'll do it later today and reply back when it's
>> deployed.
> 
> 
> 
> Fantastic. I'm starting to really like the frequent releases.
> 
> Thanks,
> 
> Henri.
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: IExternalPage and pageValidate

Posted by Jesse Kuhnert <jk...@gmail.com>.
Me too. (or more specifically, I'm starting to really like the move to
maven2 )

Ok, this has been implemented/commited. Should be available in a few minutes
when maven is done packaging/deploying.

On 7/24/06, Henri Dupre <he...@gmail.com> wrote:
>
> On 7/24/06, Jesse Kuhnert <jk...@gmail.com> wrote:
> >
> > Sure, I can do that.
> >
> > 4.1 is still "unstable", so I guess if this breaks someone I can always
> > revert pretty easily. I'll do it later today and reply back when it's
> > deployed.
>
>
> Fantastic. I'm starting to really like the frequent releases.
>
> Thanks,
>
> Henri.
>
>


-- 
Jesse Kuhnert
Tacos/Tapestry, team member/developer

Open source based consulting work centered around
dojo/tapestry/tacos/hivemind.

Re: IExternalPage and pageValidate

Posted by Henri Dupre <he...@gmail.com>.
On 7/24/06, Jesse Kuhnert <jk...@gmail.com> wrote:
>
> Sure, I can do that.
>
> 4.1 is still "unstable", so I guess if this breaks someone I can always
> revert pretty easily. I'll do it later today and reply back when it's
> deployed.


Fantastic. I'm starting to really like the frequent releases.

Thanks,

Henri.

Re: IExternalPage and pageValidate

Posted by Jesse Kuhnert <jk...@gmail.com>.
Sure, I can do that.

4.1 is still "unstable", so I guess if this breaks someone I can always
revert pretty easily. I'll do it later today and reply back when it's
deployed.

On 7/24/06, Henri Dupre <he...@gmail.com> wrote:
>
> One thing that has been bugging me lately in Tapestry 4 is the order of
> activateExternalPage and pageValidate...
> I've been using pageValidate to verify that all data is correct (e.g.
> select
> the default value...) but when the page is called with
> activateExternalPage,
> pageValidate gets called before activateExternalPage, thus making this
> call
> useless as the page is usually blank at this point.
>
> Does anyone else think that life would be better if pageValidate was
> called
> after activateExternalPage?
>
> Thanks,
>
> Henri.
>
>


-- 
Jesse Kuhnert
Tacos/Tapestry, team member/developer

Open source based consulting work centered around
dojo/tapestry/tacos/hivemind.