Merge pull request #965 from CPAN-API/pod-urls

New pod urls
metacpan · Oct 23, 2013 · 08f3d42 · 08f3d42
2 parents 0103d7c + 4713561
commit 08f3d42
Show file tree

Hide file tree

Showing 16 changed files with 201 additions and 124 deletions.
diff --git a/lib/MetaCPAN/Web/Controller/Changes.pm b/lib/MetaCPAN/Web/Controller/Changes.pm
@@ -52,8 +52,8 @@ sub get : Private {
     }
     # display as pod if it is a pod file (perldelta.pod and some other dists)
     elsif( $file->{documentation} ){
-        # FIXME: the 'pod' view needs to be reusable (new pod controller #176)
-        $c->forward('/module/path', [@$file{qw( author release path )}]);
+        # Is there a better way to reuse the pod view?
+        $c->forward('/pod/release', [@$file{qw( author release path )}]);
     }
     else {
         $c->stash({ file => $file });

diff --git a/lib/MetaCPAN/Web/Controller/Module.pm b/lib/MetaCPAN/Web/Controller/Module.pm
@@ -2,107 +2,28 @@ package MetaCPAN::Web::Controller::Module;
 
 use Moose;
 use namespace::autoclean;
-use HTML::Restrict;
 
 BEGIN { extends 'MetaCPAN::Web::Controller' }
 
-with qw(
-    MetaCPAN::Web::Role::ReleaseInfo
-);
+# NOTE: We may (be able to) put these redirects into nginx
+# but it's nice to have them here (additionally) for development.
 
-sub path : PathPart('module') : Chained('/') : Args {
+sub redirect_to_pod : PathPart('module') : Chained('/') : Args {
     my ( $self, $c, @path ) = @_;
 
-    # force consistent casing in URLs
-    if ( @path > 2 && $path[0] ne uc($path[0]) ) {
-        $c->res->redirect( '/module/' . join( '/', uc(shift @path), @path ), 301 );
-        $c->detach();
-    }
-
-    my $model = $c->model('API::Module');
-    my $data = @path > 2 ? $model->get(@path)->recv : $model->find(@path)->recv;
+    # Forward old '/module/' links to the new '/pod/' controller.
 
-    if($data->{directory}) {
-        $c->res->redirect( '/source/' . join( '/', @path ), 301 );
-        $c->detach;
+    # /module/AUTHOR/Release-0.0/lib/Foo/Bar.pm
+    if ( @path > 1 ) {
+        # Force the author arg to uppercase to avoid another redirect.
+        $c->res->redirect( '/pod/release/' . join( '/', uc(shift @path), @path ), 301 );
     }
-
-    my ( $documentation, $pod )
-        = map { $_->{name}, $_->{associated_pod} }
-        grep { @path > 1 || $path[0] eq $_->{name} }
-        grep { !$data->{documentation} || $data->{documentation} eq $_->{name} }
-        grep { $_->{associated_pod} } @{ $data->{module} || [] };
-    $data->{documentation} = $documentation if $documentation;
-
-    $c->detach('/not_found') unless ( $data->{name} );
-    my $reqs = $self->api_requests(
-        $c,
-        {   pod => $c->model('API')
-                ->request( '/pod/' . ( $pod || join( '/', @path ) ) . '?show_errors=1' ),
-            release => $c->model('API::Release')
-                ->get( @{$data}{qw(author release)} ),
-        },
-        $data,
-    );
-    $reqs = $self->recv_all($reqs);
-    $self->stash_api_results( $c, $reqs, $data );
-    $self->add_favorites_data( $data, $reqs->{favorites}, $data );
-
-    my $hr = HTML::Restrict->new;
-    $hr->set_rules(
-        {   a       => [qw( href target )],
-            b       => [],
-            br      => [],
-            caption => [],
-            center  => [],
-            code    => [],
-            dd      => ['id'],
-            div     => [qw(id style)],
-            dl      => ['id'],
-            dt      => ['id'],
-            em      => [],
-            h1      => ['id'],
-            h2      => ['id'],
-            h3      => ['id'],
-            h4      => ['id'],
-            h5      => ['id'],
-            h6      => ['id'],
-            i       => [],
-            img     => [qw( alt border height width src style title / )],
-            li      => ['id'],
-            ol      => [],
-            p       => [qw(class style)],
-            pre     => [qw(id class style)],
-            span    => [qw(style)],
-            strong  => [],
-            sub     => [],
-            sup     => [],
-            table => [qw( style class border cellspacing cellpadding align )],
-            tbody => [],
-            td    => [qw(style class)],
-            tr    => [qw(style class)],
-            u     => [],
-            ul    => ['id'],
-        }
-    );
-
-    # ensure page is not cached when latest release is a trial
-    $c->res->last_modified(
-               $reqs->{versions}->{hits}->{hits}->[0]->{fields}->{date}
-            || $data->{date} );
-
-    $c->stash(
-        {   module   => $data,
-            pod      => $hr->process( $reqs->{pod}->{raw} ),
-            release  => $reqs->{release}->{hits}->{hits}->[0]->{_source},
-            template => 'module.html',
-        }
-    );
-    unless ($c->stash->{pod}) {
-        $c->stash(
-            pod_error => $reqs->{pod}->{message},
-        );
+    # /module/Foo::Bar
+    else {
+        $c->res->redirect( '/pod/' . join( '/', @path ), 301 );
     }
+
+    $c->detach();
 }
 
 1;
diff --git a/lib/MetaCPAN/Web/Controller/Pod.pm b/lib/MetaCPAN/Web/Controller/Pod.pm
@@ -0,0 +1,153 @@
+package MetaCPAN::Web::Controller::Pod;
+
+use Moose;
+use namespace::autoclean;
+use Try::Tiny;
+use HTML::Restrict;
+
+BEGIN { extends 'MetaCPAN::Web::Controller' }
+
+with qw(
+    MetaCPAN::Web::Role::ReleaseInfo
+);
+
+sub root : Chained('/') PathPart('pod') CaptureArgs(0) {
+}
+
+# /pod/$name
+sub find : Chained('root') PathPart('') Args(1) {
+    my ( $self, $c, @path ) = @_;
+
+    # TODO: Pass size param so we can disambiguate?
+    $c->stash->{pod_file} = $c->model('API::Module')->find(@path)->recv;
+
+    # TODO: Disambiguate if there's more than once match. #176
+
+    $c->forward('view', [@path]);
+}
+
+# /pod/release/$AUTHOR/$release/@path
+sub release : Chained('root') Local Args {
+    my ( $self, $c, @path ) = @_;
+
+    # force consistent casing in URLs
+    if ( @path > 2 && $path[0] ne uc($path[0]) ) {
+        $c->res->redirect( '/pod/release/' . join( '/', uc(shift @path), @path ), 301 );
+        $c->detach();
+    }
+
+    $c->stash->{pod_file} = $c->model('API::Module')->get(@path)->recv;
+    $c->forward('view', [@path]);
+}
+
+# /pod/distribution/$name/@path
+sub distribution : Chained('root') Local Args {
+    my ( $self, $c, $dist, @path ) = @_;
+
+    # TODO: Could we do this with one query?
+    # filter => { path => join('/', @path), distribution => $dist, status => latest }
+
+    # Get latest "author/release" of dist so we can use it to find the file.
+    # TODO: Pass size param so we can disambiguate?
+    my $release = try {
+        $c->model('API::Release')->find($dist)->recv->{hits}{hits}->[0]->{_source}
+    } or $c->detach('/not_found');
+
+    # TODO: Disambiguate if there's more than once match. #176
+
+    unshift @path, @$release{qw( author name )};
+
+    $c->stash->{pod_file} = $c->model('API::Module')->get(@path)->recv;
+
+    $c->forward('view', [@path]);
+}
+
+sub view : Private {
+    my ( $self, $c, @path ) = @_;
+
+    my $data = $c->stash->{pod_file};
+
+    if($data->{directory}) {
+        $c->res->redirect( '/source/' . join( '/', @path ), 301 );
+        $c->detach;
+    }
+
+    my ( $documentation, $pod )
+        = map { $_->{name}, $_->{associated_pod} }
+        grep { @path > 1 || $path[0] eq $_->{name} }
+        grep { !$data->{documentation} || $data->{documentation} eq $_->{name} }
+        grep { $_->{associated_pod} } @{ $data->{module} || [] };
+    $data->{documentation} = $documentation if $documentation;
+
+    $c->detach('/not_found') unless ( $data->{name} );
+    my $reqs = $self->api_requests(
+        $c,
+        {   pod => $c->model('API')
+                ->request( '/pod/' . ( $pod || join( '/', @path ) ) . '?show_errors=1' ),
+            release => $c->model('API::Release')
+                ->get( @{$data}{qw(author release)} ),
+        },
+        $data,
+    );
+    $reqs = $self->recv_all($reqs);
+    $self->stash_api_results( $c, $reqs, $data );
+    $self->add_favorites_data( $data, $reqs->{favorites}, $data );
+
+    my $hr = HTML::Restrict->new;
+    $hr->set_rules(
+        {   a       => [qw( href target )],
+            b       => [],
+            br      => [],
+            caption => [],
+            center  => [],
+            code    => [],
+            dd      => ['id'],
+            div     => [qw(id style)],
+            dl      => ['id'],
+            dt      => ['id'],
+            em      => [],
+            h1      => ['id'],
+            h2      => ['id'],
+            h3      => ['id'],
+            h4      => ['id'],
+            h5      => ['id'],
+            h6      => ['id'],
+            i       => [],
+            img     => [qw( alt border height width src style title / )],
+            li      => ['id'],
+            ol      => [],
+            p       => [qw(class style)],
+            pre     => [qw(id class style)],
+            span    => [qw(style)],
+            strong  => [],
+            sub     => [],
+            sup     => [],
+            table => [qw( style class border cellspacing cellpadding align )],
+            tbody => [],
+            td    => [qw(style class)],
+            tr    => [qw(style class)],
+            u     => [],
+            ul    => ['id'],
+        }
+    );
+
+    # ensure page is not cached when latest release is a trial
+    $c->res->last_modified(
+               $reqs->{versions}->{hits}->{hits}->[0]->{fields}->{date}
+            || $data->{date} );
+
+    $c->stash(
+        {   module   => $data,
+            pod      => $hr->process( $reqs->{pod}->{raw} ),
+            release  => $reqs->{release}->{hits}->{hits}->[0]->{_source},
+            template => 'pod.html',
+        }
+    );
+    unless ($c->stash->{pod}) {
+        $c->stash(
+            pod_error => $reqs->{pod}->{message},
+        );
+    }
+}
+
+1;
diff --git a/lib/MetaCPAN/Web/Controller/Search.pm b/lib/MetaCPAN/Web/Controller/Search.pm
@@ -31,7 +31,7 @@ sub index : Path {
          $query =~ s[^ (?: \\ | ! ) ][]x) {
         my $module = $model->first($query)->recv;
         $c->detach('/not_found') unless ($module);
-        $c->res->redirect("/module/$module");
+        $c->res->redirect("/pod/$module");
         $c->detach;
     }
     else {

diff --git a/root/browse.html b/root/browse.html
@@ -42,7 +42,7 @@
   : file.mime.match("x-c") ? 'silk-page-white-c'
   : 'silk-page-white'
   -%>" title="<% file.path %>"><% file.name %></a></strong></td>
-  <td class="documentation" class="ellipsis"><strong><a href="/module/<% [author, release, file.path].join("/") %>" title="<% file.path %>"><% file.slop ? file.documentation ? file.documentation : file.name : "" %></a></strong></td>
+  <td class="documentation" class="ellipsis"><strong><a href="/pod/release/<% [author, release, file.path].join("/") %>" title="<% file.path %>"><% file.slop ? file.documentation ? file.documentation : file.name : "" %></a></strong></td>
   <td class="size"><span sort="<% file.directory == "false" ? file.${"stat.size"} : 0 %>"><% file.directory == "false" ? file.${"stat.size"} | format_bytes : "" %></td>
   <td class="mtime" nowrap="nowrap"><span class="relatize" sort="<% date = file.${"stat.mtime"}.dt_http; date %>"><% date %></span></td>
 </tr>

diff --git a/root/inc/dependencies.html b/root/inc/dependencies.html
@@ -12,7 +12,7 @@
 deps.push(dep.module);
 END;
 FOREACH dep IN deps.sort %>
-<a href="/module/<% dep %>" title="<% dep %>" class="ellipsis"><% dep %></a>
+<a href="/pod/<% dep %>" title="<% dep %>" class="ellipsis"><% dep %></a>
 <% END %>
 <%- IF deps.size == 0 && !release.metadata  %>
 <i title="no META file provided">unknown</i>

diff --git a/root/module.html → root/pod.html b/root/module.html → root/pod.html
@@ -1,5 +1,6 @@
 <% twitter_card_inc = 'inc/twitter/module.html' %>
-<% canonical = "/module/" _ module.documentation %>
+<%# FIXME: Check if this is PAUSE indexed, otherwise use /pod/distribution/ url? %>
+<% canonical = "/pod/" _ module.documentation %>
 <% meta_description = module.abstract %>
 <% title = module.documentation _ (module.abstract ? ' - ' _ module.abstract : ''); rss = 'distribution/' _ module.distribution %>
 <div itemscope itemtype="http://schema.org/SoftwareApplication">
@@ -17,7 +18,7 @@
   <span class="select-text" itemprop="name"><% module.documentation %></span>
   </big></strong>
   <% INCLUDE inc/favorite.html module = module %>
-  <% IF release.status != 'latest' %><div style="float: right"><strong><big><% IF release.maturity == 'developer'; 'dev release, '; END %></big><a href="/module/<% module.documentation %>"><big>go to latest</big></a></strong></div><% END %><br><br>
+  <% IF release.status != 'latest' %><div style="float: right"><strong><big><% IF release.maturity == 'developer'; 'dev release, '; END %></big><a href="<% canonical %>"><big>go to latest</big></a></strong></div><% END %><br><br>
 
 <div class="search-bar">
   <strong></strong>
@@ -61,9 +62,9 @@
   <hr>
   <strong>Permalinks</strong>
   <ul>
-    <li><a href="/module/<% module.author %>/<% module.release %>/<% module.path %>">This version</a></li>
+    <li><a href="/pod/release/<% module.author %>/<% module.release %>/<% module.path %>">This version</a></li>
     <% IF module.documentation %>
-    <li><a href="/module/<% module.documentation %>" itemprop="url">Latest version</a></li>
+    <li><a href="/pod/<% module.documentation %>" itemprop="url">Latest version</a></li>
     <% END %>
   </ul>
 </div>

diff --git a/root/preprocess.html b/root/preprocess.html
@@ -60,10 +60,10 @@
     && file.module.grep(->(module){ module.authorized && module.indexed });
     path = file.documentation;
   ELSE;
-    path = [file.author, file.release, file.path].join("/");
+    path = ['release', file.author, file.release, file.path].join("/");
   END;
 %>
-<a href="/module/<% path %>"><%- title %></a>
+<a href="/pod/<% path %>"><%- title %></a>
 <%- END
   # protocols = {
   #   "bzr" = 10,

diff --git a/root/release.html b/root/release.html
@@ -123,15 +123,15 @@
   <div>
     <% IF file.path.match('\.pod$') %>
       <strong>
-        <a href="/module/<% release.author; '/'; release.name; '/'; file.path %>"><% file.path %></a>
+        <a href="/pod/release/<% release.author; '/'; release.name; '/'; file.path %>"><% file.path %></a>
       </strong>
     <% ELSE %>
       <strong>
         <a href="/source/<% release.author; '/'; release.name; '/'; file.path %>"><% file.path %></a>
       </strong>
         <% IF file.pod_lines %>
           [<strong>
-            <a href="/module/<% release.author; '/'; release.name; '/'; file.path %>">pod</a>
+            <a href="/pod/release/<% release.author; '/'; release.name; '/'; file.path %>">pod</a>
           </strong>]
         <% END %>
     <% END %>

diff --git a/root/source.html b/root/source.html
@@ -14,9 +14,10 @@
   <ul>
     <li><a href="/release/<% module.author %>/<% module.release %>/">Release Info</a></li>
     <% IF module.documentation %>
-      <li><a href="/module/<% module.documentation %>">Module Documentation</a></li>
+      <%# FIXME: Can we tell if this is PAUSE indexed? %>
+      <li><a href="/pod/<% module.documentation %>">Module Documentation</a></li>
     <% ELSIF module.slop %>
-      <li><a href="/module/<% doc_view_url %>">Documentation View</a></li>
+      <li><a href="/pod/release/<% doc_view_url %>">Documentation View</a></li>
     <% END %>
     <li><a href="/author/<% module.author %>/">Author</a></li>
     <li>&nbsp;</li>

diff --git a/root/static/js/cpan.js b/root/static/js/cpan.js
@@ -174,7 +174,7 @@ $(document).ready(function() {
             return item.documentation;
         }
     }).result(function(e, item) {
-        document.location.href = '/module/'+ item.documentation;
+        document.location.href = '/pod/'+ item.documentation;
     });
 
     $('#search-input.autofocus').focus();