Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

Ovid (2709)

Ovid
  (email not shown publicly)
http://publius-ovidius.livejournal.com/
AOL IM: ovidperl (Add Buddy, Send Message)

Stuff with the Perl Foundation. A couple of patches in the Perl core. A few CPAN modules. That about sums it up.

Journal of Ovid (2709)

Wednesday September 24, 2008
05:46 AM

Blocked in Refactoring Catalyst

[ #37525 ]

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.

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.
  • 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 wri

    • 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.

  • Hey Ovid, not to avoid the issue.. but is this really the forum? Assuming you're looking for actual help that is, and not some place to just gripe about catalyst ;) The Catalyst mailing list / irc channel likely have a lot more useful helpers..
    • 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)

      • 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!
  • 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 metad

    • 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 :(

      • 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 su