* [PATCH 3/4] inbox: cleanup and consolidate object weakening
2016-07-07 1:50 [PATCH 0/4] more tweaks and finalize WwwStream conversion Eric Wong
2016-07-07 1:50 ` [PATCH 1/4] githttpbackend: avoid intermediate array creation from stat Eric Wong
2016-07-07 1:50 ` [PATCH 2/4] t/git-http-backend: check BSD::Resource availability Eric Wong
@ 2016-07-07 1:50 ` Eric Wong
2016-07-07 1:50 ` [PATCH 4/4] www: remove old footer generation code and normalize new.html Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-07-07 1:50 UTC (permalink / raw)
To: meta
This fixes some layering violations and consolidates
the cleanup into the inbox object itself. Keeping in
mind weakening does not work at all without our PSGI
server.
---
lib/PublicInbox/HTTP.pm | 16 ----------------
lib/PublicInbox/Inbox.pm | 37 +++++++++++++++++++++++++++++++++----
lib/PublicInbox/NNTP.pm | 17 -----------------
lib/PublicInbox/WWW.pm | 2 --
4 files changed, 33 insertions(+), 39 deletions(-)
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index e19c592..abf648f 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -25,15 +25,6 @@ use constant {
CHUNK_MAX_HDR => 256,
};
-# FIXME: duplicated code with NNTP.pm, layering violation
-my $WEAKEN = {}; # string(inbox) -> inbox
-my $weakt;
-sub weaken_task () {
- $weakt = undef;
- $_->weaken_all for values %$WEAKEN;
- $WEAKEN = {};
-}
-
my $pipelineq = [];
my $pipet;
sub process_pipelineq () {
@@ -252,13 +243,6 @@ sub response_done ($$) {
$self->{env} = undef;
$self->write("0\r\n\r\n") if $alive == 2;
$self->write(sub { $alive ? next_request($self) : $self->close });
-
- # FIXME: layering violation
- if (my $obj = $env->{'pi-httpd.inbox'}) {
- # grace period for reaping resources
- $WEAKEN->{"$obj"} = $obj;
- PublicInbox::EvCleanup::later(*weaken_task);
- }
}
sub getline_response {
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 728caa0..dc9980b 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -9,6 +9,26 @@ use Scalar::Util qw(weaken isweak);
use PublicInbox::Git;
use PublicInbox::MID qw(mid2path);
+my $weakt;
+eval {
+ $weakt = 'disabled';
+ require PublicInbox::EvCleanup;
+ $weakt = undef; # OK if we get here
+};
+
+my $WEAKEN = {}; # string(inbox) -> inbox
+sub weaken_task () {
+ $weakt = undef;
+ _weaken_fields($_) for values %$WEAKEN;
+ $WEAKEN = {};
+}
+
+sub _weaken_later ($) {
+ my ($self) = @_;
+ $weakt ||= PublicInbox::EvCleanup::later(*weaken_task);
+ $WEAKEN->{"$self"} = $self;
+}
+
sub new {
my ($class, $opts) = @_;
my $v = $opts->{address} ||= 'public-inbox@example.com';
@@ -17,7 +37,7 @@ sub new {
bless $opts, $class;
}
-sub weaken_all {
+sub _weaken_fields {
my ($self) = @_;
foreach my $f (qw(git mm search)) {
isweak($self->{$f}) or weaken($self->{$f});
@@ -26,17 +46,26 @@ sub weaken_all {
sub git {
my ($self) = @_;
- $self->{git} ||= eval { PublicInbox::Git->new($self->{mainrepo}) };
+ $self->{git} ||= eval {
+ _weaken_later($self);
+ PublicInbox::Git->new($self->{mainrepo});
+ };
}
sub mm {
my ($self) = @_;
- $self->{mm} ||= eval { PublicInbox::Msgmap->new($self->{mainrepo}) };
+ $self->{mm} ||= eval {
+ _weaken_later($self);
+ PublicInbox::Msgmap->new($self->{mainrepo});
+ };
}
sub search {
my ($self) = @_;
- $self->{search} ||= eval { PublicInbox::Search->new($self->{mainrepo}) };
+ $self->{search} ||= eval {
+ _weaken_later($self);
+ PublicInbox::Search->new($self->{mainrepo});
+ };
}
sub try_cat {
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 56d0838..0b30d43 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -38,8 +38,6 @@ my %DISABLED; # = map { $_ => 1 } qw(xover list_overview_fmt newnews xhdr);
my $EXPMAP; # fd -> [ idle_time, $self ]
my $expt;
our $EXPTIME = 180; # 3 minutes
-my $WEAKEN = {}; # string(nntpd) -> nntpd
-my $weakt;
my $nextt;
my $nextq = [];
@@ -64,16 +62,6 @@ sub update_idle_time ($) {
defined $fd and $EXPMAP->{$fd} = [ now(), $self ];
}
-# reduce FD pressure by closing some "git cat-file --batch" processes
-# and unused FDs for msgmap and Xapian indices
-sub weaken_groups () {
- $weakt = undef;
- foreach my $nntpd (values %$WEAKEN) {
- $_->weaken_all foreach (@{$nntpd->{grouplist}});
- }
- $WEAKEN = {};
-}
-
sub expire_old () {
my $now = now();
my $exp = $EXPTIME;
@@ -92,15 +80,11 @@ sub expire_old () {
$EXPMAP = \%new;
if ($nr) {
$expt = PublicInbox::EvCleanup::later(*expire_old);
- weaken_groups();
} else {
$expt = undef;
# noop to kick outselves out of the loop ASAP so descriptors
# really get closed
PublicInbox::EvCleanup::asap(sub {});
-
- # grace period for reaping resources
- $weakt ||= PublicInbox::EvCleanup::later(*weaken_groups);
}
}
@@ -113,7 +97,6 @@ sub new ($$$) {
$self->{rbuf} = '';
$self->watch_read(1);
update_idle_time($self);
- $WEAKEN->{"$nntpd"} = $nntpd;
$expt ||= PublicInbox::EvCleanup::later(*expire_old);
$self;
}
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 5425308..5de5ffe 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -148,8 +148,6 @@ sub invalid_inbox {
if (defined $obj) {
$ctx->{git_dir} = $obj->{mainrepo};
$ctx->{git} = $obj->git;
- # for PublicInbox::HTTP::weaken_task:
- $ctx->{env}->{'pi-httpd.inbox'} = $obj;
$ctx->{-inbox} = $obj;
$ctx->{inbox} = $inbox;
return;
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 4/4] www: remove old footer generation code and normalize new.html
2016-07-07 1:50 [PATCH 0/4] more tweaks and finalize WwwStream conversion Eric Wong
` (2 preceding siblings ...)
2016-07-07 1:50 ` [PATCH 3/4] inbox: cleanup and consolidate object weakening Eric Wong
@ 2016-07-07 1:50 ` Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-07-07 1:50 UTC (permalink / raw)
To: meta
We now generate all of our HTML using WwwStream which
forces us to have consistent headers and footers in
the HTML itself.
This also makes the search-capable vs search-less installs
go to the new.html endpoint to maintain consistency
(in case an admin decides to enable Xapian).
---
lib/PublicInbox/Feed.pm | 86 +++++++++++++------------------------------------
lib/PublicInbox/View.pm | 47 +++++++++++++++++++++------
lib/PublicInbox/WWW.pm | 42 +-----------------------
t/cgi.t | 17 +---------
t/html_index.t | 16 ---------
t/plack.t | 4 +--
6 files changed, 63 insertions(+), 149 deletions(-)
diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 026a069..65adf37 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -31,7 +31,19 @@ sub generate_thread_atom {
sub generate_html_index {
my ($ctx) = @_;
- sub { emit_html_index($_[0], $ctx) };
+ # if the 'r' query parameter is given, it is a legacy permalink
+ # which we must continue supporting:
+ my $qp = $ctx->{qp};
+ if ($qp && !$qp->{r} && $ctx->{srch}) {
+ return PublicInbox::View::index_topics($ctx);
+ }
+
+ my $env = $ctx->{env};
+ my $url = $ctx->{-inbox}->base_url($env) . 'new.html';
+ my $qs = $env->{QUERY_STRING};
+ $url .= "?$qs" if $qs ne '';
+ [302, [ 'Location', $url, 'Content-Type', 'text/plain'],
+ [ "Redirecting to $url\n" ] ];
}
sub new_html {
@@ -56,7 +68,7 @@ sub new_html {
$s .= '</pre>' unless $more;
return $s;
}
- undef;
+ new_html_footer($ctx, $last);
});
}
@@ -159,74 +171,20 @@ sub _html_index_top {
"</head><body>$top";
}
-sub emit_html_index {
- my ($res, $ctx) = @_;
- my $feed_opts = get_feedopts($ctx);
- my $fh = $res->([200,['Content-Type'=>'text/html; charset=UTF-8']]);
-
- my $max = $ctx->{max} || MAX_PER_PAGE;
- $ctx->{-upfx} = '';
-
- my ($footer, $param, $last);
- $ctx->{seen} = {};
- $ctx->{anchor_idx} = 0;
- $ctx->{fh} = $fh;
- my $srch = $ctx->{srch};
- $fh->write(_html_index_top($feed_opts, $srch));
-
- # if the 'r' query parameter is given, it is a legacy permalink
- # which we must continue supporting:
- my $qp = $ctx->{qp};
- if ($qp && !$qp->{r} && $srch) {
- $last = PublicInbox::View::emit_index_topics($ctx);
- $param = 'o';
- } else {
- $last = emit_index_nosrch($ctx);
- $param = 'r';
- }
- $footer = nav_footer($ctx, $last, $feed_opts, $param);
- if ($footer) {
- my $list_footer = $ctx->{footer};
- $footer .= "\n\n" . $list_footer if $list_footer;
- $footer = "<hr><pre>$footer</pre>";
- }
- $fh->write("$footer</body></html>");
- $fh->close;
-}
-
-sub emit_index_nosrch {
- my ($ctx) = @_;
- my $ibx = $ctx->{-inbox};
- my $fh = $ctx->{fh};
- my (undef, $last) = each_recent_blob($ctx, sub {
- my ($path, $commit, $ts, $u, $subj) = @_;
- $ctx->{first} ||= $commit;
-
- my $mime = do_cat_mail($ibx, $path) or return 0;
- $fh->write(PublicInbox::View::index_entry($mime, $ctx, 1));
- 1;
- });
- $last;
-}
-
-sub nav_footer {
- my ($ctx, $last, $feed_opts, $param) = @_;
- my $qp = $ctx->{qp} or return '';
- my $old_r = $qp->{$param};
- my $head = ' ';
+sub new_html_footer {
+ my ($ctx, $last) = @_;
+ my $qp = delete $ctx->{qp} or return;
+ my $old_r = $qp->{r};
+ my $latest = '';
my $next = ' ';
- my $first = $ctx->{first};
- my $anchor = $ctx->{anchor_idx};
if ($last) {
- $next = qq!<a\nhref="?$param=$last"\nrel=next>next</a>!;
+ $next = qq!<a\nhref="?r=$last"\nrel=next>next</a>!;
}
if ($old_r) {
- $head = $ctx->{env}->{PATH_INFO};
- $head = qq!<a\nhref="$head">head</a>!;
+ $latest = qq! <a\nhref='./new.html'>latest</a>!;
}
- my $atom = "<a\nhref=\"$feed_opts->{atomurl}\">Atom feed</a>";
- "<a\nname=\"s$anchor\">page:</a> $next $head $atom";
+ "<hr><pre>page: $next$latest</pre>";
}
sub each_recent_blob {
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index 7ff3b40..0903550 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -24,7 +24,7 @@ sub th_pfx ($) { $_[0] == 0 ? '' : TCHILD };
# public functions: (unstable)
sub msg_html {
- my ($ctx, $mime, $footer) = @_;
+ my ($ctx, $mime) = @_;
my $hdr = $mime->header_obj;
my $tip = _msg_html_prepare($hdr, $ctx);
PublicInbox::WwwStream->response($ctx, 200, sub {
@@ -853,13 +853,15 @@ sub add_topic {
}
}
-sub emit_topics {
+sub topics {
my ($ctx) = @_;
my $order = $ctx->{order};
my $subjs = $ctx->{subjs};
my $latest = $ctx->{latest};
- my $fh = $ctx->{fh};
- return $fh->write("\n[No topics in range]</pre>") unless scalar @$order;
+ if (!@$order) {
+ $ctx->{-html_tip} = '<pre>[No topics in range]</pre>';
+ return 404;
+ }
my $pfx;
my $prev = 0;
my $prev_attr = '';
@@ -903,13 +905,38 @@ sub emit_topics {
}
push @recent, $cur if $cur;
@recent = map { $_->[1] } sort { $b->[0] <=> $a->[0] } @recent;
- $fh->write(join('', @recent) . '</pre>');
+ $ctx->{-html_tip} = join('', '<pre>', @recent, '</pre>');
+ 200;
}
-sub emit_index_topics {
+sub index_nav { # callback for WwwStream
+ my (undef, $ctx) = @_;
+ delete $ctx->{qp} or return;
+ my ($next, $prev);
+ $next = $prev = ' ';
+ my $latest = '';
+
+ my $next_o = $ctx->{-next_o};
+ if ($next_o) {
+ $next = qq!<a\nhref="?o=$next_o"\nrel=next>next</a>!;
+ }
+ if (my $cur_o = $ctx->{-cur_o}) {
+ $latest = qq! <a\nhref=.>latest</a>!;
+
+ my $o = $cur_o - ($next_o - $cur_o);
+ if ($o > 0) {
+ $prev = qq!<a\nhref="?o=$o"\nrel=prev>prev</a>!;
+ } elsif ($o == 0) {
+ $prev = qq!<a\nhref=.\nrel=prev>prev</a>!;
+ }
+ }
+ "<hr><pre>page: $next $prev$latest</pre>";
+}
+
+sub index_topics {
my ($ctx) = @_;
my ($off) = (($ctx->{qp}->{o} || '0') =~ /(\d+)/);
- $ctx->{order} = [];
+ my $order = $ctx->{order} = [];
$ctx->{subjs} = {};
$ctx->{latest} = {};
my $max = 25;
@@ -921,9 +948,9 @@ sub emit_index_topics {
walk_thread(thread_results($sres), $ctx, *add_topic);
$opts{offset} += $nr;
}
-
- emit_topics($ctx);
- $opts{offset};
+ $ctx->{-next_o} = $opts{offset};
+ $ctx->{-cur_o} = $off;
+ PublicInbox::WwwStream->response($ctx, topics($ctx), *index_nav);
}
sub thread_adj_level {
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 5de5ffe..2c60d59 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -198,7 +198,6 @@ sub get_index {
my ($ctx) = @_;
require PublicInbox::Feed;
my $srch = searcher($ctx);
- footer($ctx);
if ($ctx->{env}->{QUERY_STRING} =~ /(?:\A|[&;])q=/) {
require PublicInbox::SearchView;
PublicInbox::SearchView::sres_top_html($ctx);
@@ -227,11 +226,10 @@ sub get_mid_html {
my $x = mid2blob($ctx) or return r404($ctx);
require PublicInbox::View;
- my $foot = footer($ctx);
require Email::MIME;
my $mime = Email::MIME->new($x);
searcher($ctx);
- PublicInbox::View::msg_html($ctx, $mime, $foot);
+ PublicInbox::View::msg_html($ctx, $mime);
}
# /$INBOX/$MESSAGE_ID/t/
@@ -250,44 +248,6 @@ sub ctx_get {
$val;
}
-sub footer {
- my ($ctx) = @_;
- return '' unless $ctx;
- my $obj = $ctx->{-inbox} or return '';
-
- # auto-generate a footer
- chomp(my $desc = $obj->description);
- $desc = PublicInbox::Hval::ascii_html($desc);
-
- my $urls;
- my @urls = @{$obj->cloneurl};
- my %seen = map { $_ => 1 } @urls;
- my $env = $ctx->{env};
- my $http = $obj->base_url($env);
- chop $http;
- $seen{$http} or unshift @urls, $http;
- my $ssoma_url = PublicInbox::Hval::prurl($env, SSOMA_URL);
- if (scalar(@urls) == 1) {
- $urls = "URL for <a\nhref=\"" . $ssoma_url .
- qq(">ssoma</a> or <b>git clone --mirror $urls[0]</b>);
- } else {
- $urls = "URLs for <a\nhref=\"" . $ssoma_url .
- qq(">ssoma</a> or <b>git clone --mirror</b>\n) .
- join("\n", map { "\tgit clone --mirror $_" } @urls);
- }
-
- my $addr = $obj->{-primary_address};
- $ctx->{footer} = join("\n",
- '- ' . $desc,
- "A <a\nhref=\"" .
- PublicInbox::Hval::prurl($ctx->{env}, PI_URL) .
- '">public-inbox</a>, ' .
- 'anybody may post in plain-text (not HTML):',
- $addr,
- $urls
- );
-}
-
# search support is optional, returns undef if Xapian is not installed
# or not configured for the given GIT_DIR
sub searcher {
diff --git a/t/cgi.t b/t/cgi.t
index 720925a..a0f09c5 100644
--- a/t/cgi.t
+++ b/t/cgi.t
@@ -131,21 +131,6 @@ EOF
like($res->{body}, qr/what\?/, "reply included");
}
-# indices
-{
- local $ENV{HOME} = $home;
- my $res = cgi_run("/test/");
- like($res->{head}, qr/Status: 200 OK/, "index returns 200");
-
- my $idx = cgi_run("/test/index.html");
- $idx->{body} =~ s!/index.html(\?r=)!/$1!g; # dirty...
- $idx->{body} = [ split(/\n/, $idx->{body}) ];
- $res->{body} = [ split(/\n/, $res->{body}) ];
- is_deeply($res, $idx,
- '/$INBOX/ and /$INBOX/index.html are nearly identical');
- # more checks in t/feed.t
-}
-
# message-id pages
{
local $ENV{HOME} = $home;
@@ -187,7 +172,7 @@ EOF
$res = cgi_run("/test/blahblah\@example.con/");
like($res->{head}, qr/Status: 300 Multiple Choices/, "mid html miss");
- $res = cgi_run("/test/");
+ $res = cgi_run("/test/new.html");
like($res->{body}, qr/slashy%2Fasdf%40example\.com/,
"slashy URL generated correctly");
}
diff --git a/t/html_index.t b/t/html_index.t
index 100d21a..f29b442 100644
--- a/t/html_index.t
+++ b/t/html_index.t
@@ -26,11 +26,6 @@ my $im = PublicInbox::Import->new($git, 'tester', 'test@example');
my $prev = "";
foreach my $i (1..6) {
- # my $pid = open(my $pipe, "|-");
- # defined $pid or die "fork/pipe failed: $!\n";
- # if ($pid == 0) {
- # exec("ssoma-mda", $git_dir);
- # }
my $mid = "<$i\@example.com>";
my $mid_line = "Message-ID: $mid";
if ($prev) {
@@ -58,15 +53,4 @@ EOF
$im->done;
}
-# check HTML index
-{
- use IO::File;
- my $cb = PublicInbox::Feed::generate_html_index({
- -inbox => $ibx,
- max => 3
- });
- require 't/common.perl';
- like(stream_to_string($cb), qr/html/, "feed is valid HTML :)");
-}
-
done_testing();
diff --git a/t/plack.t b/t/plack.t
index 40298e5..db3a9b2 100644
--- a/t/plack.t
+++ b/t/plack.t
@@ -120,9 +120,9 @@ EOF
test_psgi($app, sub {
my ($cb) = @_;
my $atomurl = 'http://example.com/test/new.atom';
- my $res = $cb->(GET('http://example.com/test/'));
+ my $res = $cb->(GET('http://example.com/test/new.html'));
is(200, $res->code, 'success response received');
- like($res->content, qr!href="\Q$atomurl\E"!,
+ like($res->content, qr!href="new\.atom"!,
'atom URL generated');
like($res->content, qr!href="blah%40example\.com/"!,
'index generated');
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread