We're doing a lot of work in refactoring Catalyst "Regex" attributes into proper Chained methods. So let's say I have a URL like this:
http://www.example.com/api/v1/episode/$id/bundle/
We used to have horrible regexes for that, but now we do something like this:
sub episode_chain :
Local :
ActionClass('REST') :
PathPart('episode') Chained('/api/v1/v1_chain') CaptureArgs(1) {}
sub episode_chain_GET {}
sub bundle :
Local :
ActionClass('REST') :
Chained('episode_chain') Args(0) {}
We're having serious trouble refactoring a lot of that because we also have things like this in our broadcast class (and in several other classes):
sub broadcast_chain :
Local :
ActionClass('REST') :
PathPart('broadcast') Chained('/api/v1/v1_chain') CaptureArgs(1) {}
sub broadcast_chain_GET {}
sub bundle :
Local :
ActionClass('REST') :
Chained('broadcast_chain') Args(0) {}
The problem about refactoring is that much of the data is embedded in attributes and attributes are not Perl code, they're merely text data which is fed to your attribute handler. The following illustrates.
#!/usr/bin/env perl
use strict;
use warnings;
my $novoid;
BEGIN { $novoid = 'NOVOID' }
use base 'Attribute::Context';
sub foo : Arrayref($novoid) {
return;
}
Now if you run deparse on this (perl -MO=Deparse attribute.pl):
use warnings;
use strict 'refs';
my $novoid;
sub BEGIN {
use warnings;
use strict 'refs';
$novoid = 'NOVOID';
}
use base ('Attribute::Context');
use attributes ('main', sub {
use warnings;
use strict 'refs';
return;
}
, 'Arrayref($novoid)');
As you can see, the raw text "$novoid" is passed through. It's not interpolated. This means that much of our refactoring work is severely hampered. We'll either have to rethink design decisions to work around a serious limitation with attributes or just live with the cut-n-paste pain.
I understand that Matt Trout was working on a solution to this, but I've no idea where he is with this.
Dispatch has to be a tree, not graph, if uri_for is supposed to be possible. And I like uri_for too much to consider any proposal that breaks it.
Instead, I would like to see a way to constrain the possible values of CaptureArgs. That way you could say something like this:
PathPart("") Chained('/api/v1/v1_chain') CaptureArgs(2) ArgMatch(0, 'episode|broadcast')
Since uri_for already has a mechanism for specifying captures, it would work out of the box. And in fact (I think) it’s already possible to write ArgMatch yourself!
I don’t know if a subroutine can have several instances of the same attribute. If not, the interface would have to be pretty clumsy. Otherwise, you could just repeat it to constrain multiple captures. So with that in hand you could throw in this for good measure: ArgMatch(1,'\d+')
Re:That's not what's missing
Ovid on 2008-09-24T12:54:01
Yeah, I've thought that matching args against regular expressions would make this grief go away. I was whining about this yesterday to colleagues (I've found myself supremely dissatisfied with the amount of cut-n-paste that Chained actions entail which the Regex actions didn't, even though our regexes were too unwieldy).
<rafl> uhm.. Chained + Local? sounds line an unintended feature to me.
You want one of them, not both. They are different dispatch types. The fact that your code works, if it does, is an accident.
Re:Btw
Ovid on 2008-09-24T13:20:50
Oops. That was cargo-cult on my part. Thanks.
Re:You'd be luckier on the catalyst ML..
Ovid on 2008-09-24T13:19:38
This is true. My main concern here was attributes and since this is so core in Catalyst, I really didn't see a major architectural change there. I should sign up for the Catalyst list again, but like the DBIx list, I rarely read the thing
:) (and I am very infrequently on IRC) Re:You'd be luckier on the catalyst ML..
castaway on 2008-09-24T13:35:17
Me either.. but I pick one of the two if I have a problem I want solved or need help figuring out how the thing works (or should work;)
Coding in isolation is out!Re:You'd be luckier on the catalyst ML..
Ovid on 2008-09-24T14:08:41
Done. I've resubscribed to the Catalyst list.
This is the thing about sub attributes: they turn into a mess as soon as you try to do anything complicated with them. Java has tried to solve this with annotations. Those don't look so hot to me, but they do work better for this amount of metadata. Mason has a special markup for adding attributes to components. It's somewhat easier to read than this, but still not actual perl code.
The simple approach is to have some kind of explicit configuration instead, like CGI::Application does. Then your metadata is real perl code that anyone can work with, and the standard toolchain works on it. Unfortunately, sub attributes look so appealing when you use them for simple cases that people who have never seen them go bad often get sucked in.
Re:alternative metadata systems
Ovid on 2008-09-24T22:09:55
The problem with "configuration" is that you often have significant effects being generated by something which is not visible in the code. As a result, yes, attributes look appealing. Unfortunately, I don't see this being easily solved in a "clean" manner in Perl 5
:( Re:alternative metadata systems
mpeters on 2008-09-25T14:33:46
The "configuration" that perrin was talking about in CGI::Application modules *is* visible in the code because it's just in a
setup()method. It's not magical, it's all localized (no need to guess where the dispatch rules are scattered throughout the application) and it's just plain code.I'm not saying that it's perfect or can't be improved, but sometimes we try to be too magical just to save some minor style nits. But I have to admit that I'm really excited that Perl 6 is going to do it better by making sub attributes real Perl 6 syntax.