[Catalyst] Handling arguments with Reges vs the meaning of paths

[Matt S Trout]

On Thu, Jan 05, 2006 at 11:39:51PM -0800, apv wrote:
> On Thursday, January 5, 2006, at 10:39  PM, Bill Moseley wrote: 
> 
> > On Thu, Jan 05, 2006 at 02:18:44PM -0800, apv wrote: 
> > 
> > > Now, I am arguing that the Regex argument (not snippet) handling 
> > > is 
> > > conceptually broken and should be changed. 
> > > 
> > >  http://host.xyz/this/is/a/path 
> > >  http://host.xyz/?these=1;are=1;arguments=1 
> > > 
> >  
> > Those are parameters.  Arguments are what follow the action. 
> > 
> Yes, as far as Catalyst is concerned. A parsed query string becomes the 
> arguments for standard CGI. And a good standard for URIs is 
> that what appears to be a literal "/this/is/something" should 
> really map to something directly and with full meaning; no matter 
> what is happening in the background (file system, dispatch, 302s, ...). 

Which is why, for example, one might have a URI like

 /<category>/item/<id>

However, in Catalyst you'd do this by registering a Regex of ^/(.*)/item$
and grabbing the id out the args. If the object/record/whatever corresponding
to that id doesn't exist, sure, 404 it - but Catalyst's default actions
explicitly provide longest-match dispatch, and for a majority of situations
that's incredibly useful.

> > > If I have a Regex ('whatever/(STOP-HERE)$'), I really mean the 
> > > '$'. I   
> > > want it to stop. 
> > > I don't want to slurp up more path into arguments because it's 
> > > not   
> > > mapping to anything the application does. 
> > > 
> > > 
> > > http://host.xyz/whatever/STOP-HERE/meaningless/misleading/nonsense/ 
> > >  
> > > should/404 
> > > 
> >  
> > I'm not sure I'm clear what you are saying.  Why not just ignore 
> > the 
> > arguments? 
> > 
> Would we ignore the middle argument? Or the first? Of course not. 
> Ignoring 
> trailing ones is possible, because we happen to read left to right, but 
> I 
> think goes against a best practice mindset. 
> 
> And *all* input should be dealt with by solid code. The more of that 
> the 
> framework takes off the hacker, the better the framework is. 
> 
> > It's how the CGI standard works and all the other actions work.  
> > Why 
> > should the Regex matching be different? 
> > 
> > 
> > > So, I'm in the position where I'm about to start adding: 
> > >   my ( $self, $c, $path_args ) = @_; 
> > >   die 404 if $path_args; 
> > > into most all my Regex controllers. 
> > > 
> >  
> > But, you wouldn't do that for Local and Path actions? 
> > 
> In those cases I am often going to check @path_args for size and 
> content and die if they don't meet expectations. In those cases 
> it's not an extra chore to do so; DRY. Though it would be nice to 
> have a sub xyz : Literal {} to also circumvent needing to do that 
> when it's not already needed. 

Look at the DispatchType system and the way actions are registered; it
should be easy enough to implement.

> I'll add that to the wish/fix/discussion list: a "Literal" dispatch 
> type that accepts no path arguments. Would also work as 
> sub index : Literal {} to catch on the local "/." 

That's how index already works.
 
> > If $ instead matches the end of the path then someone that wants 
> > arguments cannot as easily get them.  As it is now if you really 
> > don't 
> > want to accept requests with extra PATH_INFO you can just check 
> > $path_args, as in your example above. 
> > 
>  
> If you want more arguments you could easily get them by leaving off the 
> '$' 
> and signifying the end of your desired string with other regex 
> techniques. 

However that would be a substantial backwards-compatibility breakage because
$ already means something else currently.
 
> I know I can check the args but as I see it, the args are wrong to be 
> taken 
> up when they're to be ignored in many cases, and any framework 
> which requires code to circumvent default behavior a good chunk of the 
> time 
> is broken, as I'm seeing this. 
> 
> It's not like it's a huge headache to work around but I do believe it 
> would benefit from the tweaks I'm suggesting. And of course, 
> as Catalyst is gaining steam, there may not be a chance to 
> revisit conceptual design choices like this a year from now. 

Read 'sub register_actions' in Catalyst::Base, then look at
setup_actions and prepare_action in Catalyst::Dispatcher and how the
Catalyst::DispatchType::* and Catalyst::Action objects interact; I'd
be very interested to see an implementation of :Literal done this way;
much of my work on the 5.5 dispatcher was to make the sort of extensions
you're talking about easy to do.

-- 
     Matt S Trout       Offering custom development, consultancy and support
  Technical Director    contracts for Catalyst, DBIx::Class and BAST. Contact
Shadowcat Systems Ltd.  mst (at) shadowcatsystems.co.uk for more information

 + Help us build a better perl ORM: http://dbix-class.shadowcatsystems.co.uk/ +