[PATCH 0/7] www: various feed/streaming related cleanups

[PATCH 0/7] www: various feed/streaming related cleanups

[Eric Wong]

Yet more progress towards converting our PSGI streaming
interfaces over to getline/close so it can be pull-based.

Eric Wong (7):
      MANIFEST: update with recent changes
      feed: avoid needless method dispatches on 404
      feed: remove dependence on fh->write for streaming
      mbox: remove feed dependency
      mbox: avoid write dependency for streaming
      feed: various object-orientation cleanups
      inbox: move field population logic to initializer

 .gitignore                    |  1 +
 MANIFEST                      |  2 +
 Makefile.PL                   |  5 ++-
 lib/PublicInbox/Config.pm     |  4 +-
 lib/PublicInbox/Feed.pm       | 99 ++++++++++++++++++-------------------------
 lib/PublicInbox/Inbox.pm      | 15 +++++++
 lib/PublicInbox/Mbox.pm       | 68 ++++++++++-------------------
 lib/PublicInbox/NNTP.pm       |  6 +--
 lib/PublicInbox/SearchView.pm |  3 +-
 lib/PublicInbox/View.pm       |  7 +--
 lib/PublicInbox/WWW.pm        |  4 +-
 lib/PublicInbox/WwwAttach.pm  |  5 +--
 t/feed.t                      | 44 ++++++-------------
 t/html_index.t                | 11 ++++-
 14 files changed, 119 insertions(+), 155 deletions(-)

[PATCH 1/7] MANIFEST: update with recent changes

[Eric Wong]

And add a check-manifest target to the Makefile to
ensure we're up-to-date with git (but do not depend on
git).
---
 .gitignore  | 1 +
 MANIFEST    | 2 ++
 Makefile.PL | 5 ++++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 18d0021..3b333a5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 /config.mak
+/MANIFEST.gen
 /Makefile.old
 /pm_to_blib
 /MYMETA.*
diff --git a/MANIFEST b/MANIFEST
index 9c8cc1c..17a2a31 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -76,6 +76,7 @@ lib/PublicInbox/View.pm
 lib/PublicInbox/WWW.pm
 lib/PublicInbox/WatchMaildir.pm
 lib/PublicInbox/WwwAttach.pm
+lib/PublicInbox/WwwStream.pm
 sa_config/Makefile
 sa_config/README
 sa_config/root/etc/spamassassin/public-inbox.pre
@@ -134,3 +135,4 @@ t/search.t
 t/spawn.t
 t/utf8.mbox
 t/view.t
+t/watch_maildir.t
diff --git a/Makefile.PL b/Makefile.PL
index 171712a..4a91103 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -47,7 +47,10 @@ my_syntax := \$(addsuffix .syntax, $PM_FILES \$(EXE_FILES) \$(SCRIPTS))
 
 syntax:: \$(my_syntax)
 
-check:: pure_all
+check-manifest :: MANIFEST
+	if git ls-files >\$<.gen 2>&1; then diff -u \$< \$<.gen; fi
+
+check:: pure_all check-manifest
 	\$(EATMYDATA) prove -lv -j\$(N)
 
 EOF

[PATCH 2/7] feed: avoid needless method dispatches on 404

[Eric Wong]

We overuse streaming, here.  Allow Content-Length to be
calculated in this case.
---
 lib/PublicInbox/Feed.pm | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 045e495..d88421b 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -79,9 +79,8 @@ sub emit_atom {
 
 sub _no_thread {
 	my ($cb) = @_;
-	my $fh = $cb->([404, ['Content-Type' => 'text/plain']]);
-	$fh->write("No feed found for thread\n");
-	$fh->close;
+	$cb->([404, ['Content-Type', 'text/plain'],
+		["No feed found for thread\n"]]);
 }
 
 sub end_feed {

[PATCH 3/7] feed: remove dependence on fh->write for streaming

[Eric Wong]

We'll be switching to a getline/close response body
to give the HTTP server more control when dealing
with slow clients.
---
 lib/PublicInbox/Feed.pm       | 44 ++++++++++++++++++++++---------------------
 lib/PublicInbox/SearchView.pm |  3 ++-
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index d88421b..07dce9e 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -72,7 +72,9 @@ sub emit_atom {
 			$fh->write($x . feed_updated(undef, $ts));
 			$x = undef;
 		}
-		add_to_feed($feed_opts, $fh, $path, $git);
+		my $s = feed_entry($feed_opts, $path, $git) or return 0;
+		$fh->write($s);
+		1;
 	});
 	end_feed($fh);
 }
@@ -103,7 +105,8 @@ sub emit_atom_thread {
 
 	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	foreach my $msg (@{$res->{msgs}}) {
-		add_to_feed($feed_opts, $fh, mid2path($msg->mid), $git);
+		my $s = feed_entry($feed_opts, mid2path($msg->mid), $git);
+		$fh->write($s) if defined $s;
 	}
 	end_feed($fh);
 }
@@ -306,52 +309,51 @@ sub feed_updated {
 	'<updated>' . strftime(DATEFMT, @t) . '</updated>';
 }
 
-# returns 0 (skipped) or 1 (added)
-sub add_to_feed {
-	my ($feed_opts, $fh, $add, $git) = @_;
+# returns undef or string
+sub feed_entry {
+	my ($feed_opts, $add, $git) = @_;
 
-	my $mime = do_cat_mail($git, $add) or return 0;
+	my $mime = do_cat_mail($git, $add) or return;
 	my $url = $feed_opts->{url};
 	my $midurl = $feed_opts->{midurl};
 
 	my $header_obj = $mime->header_obj;
 	my $mid = $header_obj->header_raw('Message-ID');
-	defined $mid or return 0;
+	defined $mid or return;
 	$mid = PublicInbox::Hval->new_msgid($mid);
 	my $href = $midurl.$mid->as_href;
 
-	my $content = qq(<pre\nstyle="white-space:pre-wrap">) .
-		PublicInbox::View::multipart_text_as_html($mime, $href) .
-		'</pre>';
 	my $date = $header_obj->header('Date');
 	my $updated = feed_updated($date);
 
 	my $title = $header_obj->header('Subject');
-	defined $title or return 0;
+	defined $title or return;
 	$title = title_tag($title);
 
-	my $from = $header_obj->header('From') or return 0;
+	my $from = $header_obj->header('From') or return;
 	my ($email) = PublicInbox::Address::emails($from);
 	my $name = PublicInbox::Address::from_name($from);
 	$name = ascii_html($name);
 	$email = ascii_html($email);
 
+	my $s = '';
 	if (delete $feed_opts->{emit_header}) {
-		$fh->write(atom_header($feed_opts, $title) . $updated);
+		$s .= atom_header($feed_opts, $title) . $updated;
 	}
-	$fh->write("<entry><author><name>$name</name><email>$email</email>" .
-		   "</author>$title$updated" .
-		   qq{<content\ntype="xhtml">} .
-		   qq{<div\nxmlns="http://www.w3.org/1999/xhtml">});
-	$fh->write($content);
+	$s .= "<entry><author><name>$name</name><email>$email</email>" .
+		"</author>$title$updated" .
+		qq{<content\ntype="xhtml">} .
+		qq{<div\nxmlns="http://www.w3.org/1999/xhtml">} .
+		qq(<pre\nstyle="white-space:pre-wrap">) .
+		PublicInbox::View::multipart_text_as_html($mime, $href) .
+		'</pre>';
 
 	$add =~ tr!/!!d;
 	my $h = '[a-f0-9]';
 	my (@uuid5) = ($add =~ m!\A($h{8})($h{4})($h{4})($h{4})($h{12})!o);
 	my $id = 'urn:uuid:' . join('-', @uuid5);
-	$fh->write(qq!</div></content><link\nhref="$href/"/>!.
-		   "<id>$id</id></entry>");
-	1;
+	$s .= qq!</div></content><link\nhref="$href/"/>!.
+		"<id>$id</id></entry>";
 }
 
 sub do_cat_mail {
diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 2ec7ddf..d4c44ba 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -249,7 +249,8 @@ sub adump {
 	for ($mset->items) {
 		$x = PublicInbox::SearchMsg->load_doc($_->get_document)->mid;
 		$x = mid2path($x);
-		PublicInbox::Feed::add_to_feed($feed_opts, $fh, $x, $git);
+		my $s = PublicInbox::Feed::feed_entry($feed_opts, $x, $git);
+		$fh->write($s) if defined $s;
 	}
 	PublicInbox::Feed::end_feed($fh);
 }

[PATCH 4/7] mbox: remove feed dependency

[Eric Wong]

We do not need feed options there (or anywhere, hopefully).
---
 lib/PublicInbox/Mbox.pm | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm
index 7934876..1c792c8 100644
--- a/lib/PublicInbox/Mbox.pm
+++ b/lib/PublicInbox/Mbox.pm
@@ -32,19 +32,14 @@ sub emit_msg {
 	foreach my $d (qw(Lines Bytes Content-Length Status)) {
 		$header_obj->header_set($d);
 	}
-	my $feed_opts = $ctx->{feed_opts};
-	unless ($feed_opts) {
-		require PublicInbox::Feed; # FIXME: gross
-		$feed_opts = PublicInbox::Feed::get_feedopts($ctx);
-		$ctx->{feed_opts} = $feed_opts;
-	}
-	my $base = $feed_opts->{url};
+	my $ibx = $ctx->{-inbox};
+	my $base = $ibx->base_url($ctx->{cgi});
 	my $mid = mid_clean($header_obj->header('Message-ID'));
 	$mid = uri_escape_utf8($mid);
 	my @append = (
 		'Archived-At', "<$base$mid/>",
 		'List-Archive', "<$base>",
-		'List-Post', "<mailto:$feed_opts->{id_addr}>",
+		'List-Post', "<mailto:$ibx->{-primary_address}>",
 	);
 	my $append = '';
 	my $crlf = $simple->crlf;

[PATCH 5/7] mbox: avoid write dependency for streaming

[Eric Wong]

Prefer to return strings instead, so Content-Length can be
calculated for caching and such.
---
 lib/PublicInbox/Mbox.pm | 47 +++++++++++++++++------------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm
index 1c792c8..0258d8c 100644
--- a/lib/PublicInbox/Mbox.pm
+++ b/lib/PublicInbox/Mbox.pm
@@ -12,19 +12,14 @@ use Plack::Util;
 require Email::Simple;
 
 sub emit1 {
-	my $simple = Email::Simple->new(pop);
-	my $ctx = pop;
-	sub {
-		my ($response) = @_;
-		# single message should be easily renderable in browsers
-		my $fh = $response->([200, ['Content-Type'=>'text/plain']]);
-		emit_msg($ctx, $fh, $simple);
-		$fh->close;
-	}
+	my ($ctx, $msg) = @_;
+	$msg = Email::Simple->new($msg);
+	# single message should be easily renderable in browsers
+	[200, ['Content-Type', 'text/plain'], [ msg_str($ctx, $msg)] ]
 }
 
-sub emit_msg {
-	my ($ctx, $fh, $simple) = @_; # Email::Simple object
+sub msg_str {
+	my ($ctx, $simple) = @_; # Email::Simple object
 	my $header_obj = $simple->header_obj;
 
 	# drop potentially confusing headers, ssoma already should've dropped
@@ -41,8 +36,9 @@ sub emit_msg {
 		'List-Archive', "<$base>",
 		'List-Post', "<mailto:$ibx->{-primary_address}>",
 	);
-	my $append = '';
 	my $crlf = $simple->crlf;
+	my $buf = "From mboxrd\@z Thu Jan  1 00:00:00 1970\n" .
+			$header_obj->as_string;
 	for (my $i = 0; $i < @append; $i += 2) {
 		my $k = $append[$i];
 		my $v = $append[$i + 1];
@@ -53,27 +49,18 @@ sub emit_msg {
 				last;
 			}
 		}
-		$append .= "$k: $v$crlf" if defined $v;
-	}
-	my $buf = $header_obj->as_string;
-	unless ($buf =~ /\AFrom /) {
-		$fh->write("From mboxrd\@z Thu Jan  1 00:00:00 1970\n");
+		$buf .= "$k: $v$crlf" if defined $v;
 	}
-	$append .= $crlf;
-	$fh->write($buf .= $append);
-
-	$buf = $simple->body;
-	$simple->body_set('');
+	$buf .= $crlf;
 
 	# mboxrd quoting style
 	# ref: http://www.qmail.org/man/man5/mbox.html
-	$buf =~ s/^(>*From )/>$1/gm;
-
-	$fh->write($buf .= "\n");
+	my $body = $simple->body;
+	$body =~ s/^(>*From )/>$1/gm;
+	$buf .= $body;
+	$buf .= "\n";
 }
 
-sub noop {}
-
 sub thread_mbox {
 	my ($ctx, $srch, $sfx) = @_;
 	eval { require IO::Compress::Gzip };
@@ -149,6 +136,7 @@ sub getline {
 	my $res;
 	my $ctx = $self->{ctx};
 	my $git = $ctx->{git};
+	my $gz = $self->{gz};
 	do {
 		while (defined(my $smsg = shift @{$self->{msgs}})) {
 			my $msg = eval {
@@ -156,8 +144,7 @@ sub getline {
 				Email::Simple->new($git->cat_file($p));
 			};
 			$msg or next;
-
-			PublicInbox::Mbox::emit_msg($ctx, $self->{gz}, $msg);
+			$gz->write(PublicInbox::Mbox::msg_str($ctx, $msg));
 			my $ret = _flush_buf($self);
 			return $ret if $ret;
 		}
@@ -166,7 +153,7 @@ sub getline {
 		$res = scalar @{$self->{msgs}};
 		$self->{opts}->{offset} += $res;
 	} while ($res);
-	$self->{gz}->close;
+	$gz->close;
 	_flush_buf($self);
 }
 

[PATCH 6/7] feed: various object-orientation cleanups

[Eric Wong]

Favor Inbox objects as our primary source of truth to simplify
our code.  This increases our coupling with PSGI to make it
easier to write tests in the future.

A lot of this code was originally designed to be usable
standalone without PSGI or CGI at all; but that might increase
development effort.
---
 lib/PublicInbox/Feed.pm      | 58 +++++++++++++++-----------------------------
 lib/PublicInbox/Inbox.pm     | 12 +++++++++
 lib/PublicInbox/Mbox.pm      | 10 +++-----
 lib/PublicInbox/NNTP.pm      |  6 ++---
 lib/PublicInbox/View.pm      |  7 +++---
 lib/PublicInbox/WWW.pm       |  4 +--
 lib/PublicInbox/WwwAttach.pm |  5 +---
 t/feed.t                     | 45 +++++++++++-----------------------
 t/html_index.t               | 12 +++++++--
 9 files changed, 67 insertions(+), 92 deletions(-)

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 07dce9e..455b8e2 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -65,14 +65,14 @@ sub emit_atom {
 	my $fh = $cb->([ 200, ['Content-Type' => 'application/atom+xml']]);
 	my $max = $ctx->{max} || MAX_PER_PAGE;
 	my $x = atom_header($feed_opts);
-	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
+	my $ibx = $ctx->{-inbox};
 	each_recent_blob($ctx, sub {
 		my ($path, undef, $ts) = @_;
 		if (defined $x) {
 			$fh->write($x . feed_updated(undef, $ts));
 			$x = undef;
 		}
-		my $s = feed_entry($feed_opts, $path, $git) or return 0;
+		my $s = feed_entry($feed_opts, $path, $ibx) or return 0;
 		$fh->write($s);
 		1;
 	});
@@ -103,9 +103,9 @@ sub emit_atom_thread {
 	$feed_opts->{url} = $html_url;
 	$feed_opts->{emit_header} = 1;
 
-	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
+	my $ibx = $ctx->{-inbox};
 	foreach my $msg (@{$res->{msgs}}) {
-		my $s = feed_entry($feed_opts, mid2path($msg->mid), $git);
+		my $s = feed_entry($feed_opts, mid2path($msg->mid), $ibx);
 		$fh->write($s) if defined $s;
 	}
 	end_feed($fh);
@@ -167,12 +167,12 @@ sub emit_html_index {
 
 sub emit_index_nosrch {
 	my ($ctx, $state) = @_;
-	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
+	my $ibx = $ctx->{-inbox};
 	my (undef, $last) = each_recent_blob($ctx, sub {
 		my ($path, $commit, $ts, $u, $subj) = @_;
 		$state->{first} ||= $commit;
 
-		my $mime = do_cat_mail($git, $path) or return 0;
+		my $mime = do_cat_mail($ibx, $path) or return 0;
 		PublicInbox::View::index_entry($mime, 0, $state);
 		1;
 	});
@@ -218,8 +218,8 @@ sub each_recent_blob {
 	# get recent messages
 	# we could use git log -z, but, we already know ssoma will not
 	# leave us with filenames with spaces in them..
-	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
-	my $log = $git->popen(qw/log --no-notes --no-color --raw -r
+	my $log = $ctx->{-inbox}->git->popen(qw/log
+				--no-notes --no-color --raw -r
 				--abbrev=16 --abbrev-commit/,
 				"--format=%h%x00%ct%x00%an%x00%s%x00",
 				$range);
@@ -269,31 +269,16 @@ sub get_feedopts {
 	my $inbox = $ctx->{inbox};
 	my $obj = $ctx->{-inbox};
 	my $cgi = $ctx->{cgi};
-	my %rv = ( description => $obj ? $obj->description : 'FIXME' );
-
-	if ($obj) {
-		$rv{address} = $obj->{address};
-		$rv{id_addr} = $obj->{-primary_address};
-	} elsif ($pi_config && defined $inbox && $inbox ne '') {
-		# TODO: remove
-		my $addr = $pi_config->get($inbox, 'address') || "";
-		$rv{address} = $addr;
-		$addr = $addr->[0] if ref($addr);
-		$rv{id_addr} = $addr;
-	}
-	$rv{id_addr} ||= 'public-inbox@example.com';
+	my %rv = ( description => $obj->description );
 
+	$rv{address} = $obj->{address};
+	$rv{id_addr} = $obj->{-primary_address};
 	my $url_base;
-	if ($obj) {
-		$url_base = $obj->base_url($cgi); # CGI may be undef
-		if (my $mid = $ctx->{mid}) { # per-thread feed:
-			$rv{atomurl} = "$url_base$mid/t.atom";
-		} else {
-			$rv{atomurl} = $url_base."new.atom";
-		}
+	$url_base = $obj->base_url($cgi); # CGI may be undef
+	if (my $mid = $ctx->{mid}) { # per-thread feed:
+		$rv{atomurl} = "$url_base$mid/t.atom";
 	} else {
-		$url_base = 'http://example.com/';
-		$rv{atomurl} = $url_base.'new.atom';
+		$rv{atomurl} = $url_base."new.atom";
 	}
 	$rv{url} ||= $url_base;
 	$rv{midurl} = $url_base;
@@ -311,9 +296,9 @@ sub feed_updated {
 
 # returns undef or string
 sub feed_entry {
-	my ($feed_opts, $add, $git) = @_;
+	my ($feed_opts, $add, $ibx) = @_;
 
-	my $mime = do_cat_mail($git, $add) or return;
+	my $mime = do_cat_mail($ibx, $add) or return;
 	my $url = $feed_opts->{url};
 	my $midurl = $feed_opts->{midurl};
 
@@ -357,12 +342,9 @@ sub feed_entry {
 }
 
 sub do_cat_mail {
-	my ($git, $path) = @_;
-	my $mime = eval {
-		my $str = $git->cat_file("HEAD:$path");
-		Email::MIME->new($str);
-	};
-	$@ ? undef : $mime;
+	my ($ibx, $path) = @_;
+	my $mime = eval { $ibx->msg_by_path($path) } or return;
+	Email::MIME->new($mime);
 }
 
 1;
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index c982d0b..faab03c 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -7,6 +7,7 @@ use strict;
 use warnings;
 use Scalar::Util qw(weaken);
 use PublicInbox::Git;
+use PublicInbox::MID qw(mid2path);
 
 sub new {
 	my ($class, $opts) = @_;
@@ -90,4 +91,15 @@ sub nntp_usable {
 	$ret;
 }
 
+sub msg_by_path ($$;$) {
+	my ($self, $path, $ref) = @_;
+	# TODO: allow other refs:
+	git($self)->cat_file('HEAD:'.$path, $ref);
+}
+
+sub msg_by_mid ($$;$) {
+	my ($self, $mid, $ref) = @_;
+	msg_by_path($self, mid2path($mid), $ref);
+}
+
 1;
diff --git a/lib/PublicInbox/Mbox.pm b/lib/PublicInbox/Mbox.pm
index 0258d8c..63ec605 100644
--- a/lib/PublicInbox/Mbox.pm
+++ b/lib/PublicInbox/Mbox.pm
@@ -107,7 +107,6 @@ EOF
 package PublicInbox::MboxGz;
 use strict;
 use warnings;
-use PublicInbox::MID qw(mid2path);
 
 sub new {
 	my ($class, $ctx, $cb) = @_;
@@ -135,15 +134,12 @@ sub getline {
 	my ($self) = @_;
 	my $res;
 	my $ctx = $self->{ctx};
-	my $git = $ctx->{git};
+	my $ibx = $ctx->{-inbox};
 	my $gz = $self->{gz};
 	do {
 		while (defined(my $smsg = shift @{$self->{msgs}})) {
-			my $msg = eval {
-				my $p = 'HEAD:'.mid2path($smsg->mid);
-				Email::Simple->new($git->cat_file($p));
-			};
-			$msg or next;
+			my $msg = eval { $ibx->msg_by_mid($smsg->mid) } or next;
+			$msg = Email::Simple->new($msg);
 			$gz->write(PublicInbox::Mbox::msg_str($ctx, $msg));
 			my $ret = _flush_buf($self);
 			return $ret if $ret;
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index e868321..93f654f 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -10,7 +10,6 @@ use fields qw(nntpd article rbuf ng long_res);
 use PublicInbox::Search;
 use PublicInbox::Msgmap;
 use PublicInbox::Git;
-use PublicInbox::MID qw(mid2path);
 require PublicInbox::EvCleanup;
 use Email::Simple;
 use POSIX qw(strftime);
@@ -481,10 +480,9 @@ find_mid:
 		defined $mid or return $err;
 	}
 found:
-	my $o = 'HEAD:' . mid2path($mid);
 	my $bytes;
-	my $s = eval { Email::Simple->new($ng->git->cat_file($o, \$bytes)) };
-	return $err unless $s;
+	my $s = eval { $ng->msg_by_mid($mid, \$bytes) } or return $err;
+	$s = Email::Simple->new($s);
 	my $lines;
 	if ($set_headers) {
 		set_nntp_headers($s->header_obj, $ng, $n, $mid);
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index e8ec0ed..006da8d 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -12,7 +12,7 @@ use Encode::MIME::Header;
 use Plack::Util;
 use PublicInbox::Hval qw/ascii_html/;
 use PublicInbox::Linkify;
-use PublicInbox::MID qw/mid_clean id_compress mid2path mid_mime/;
+use PublicInbox::MID qw/mid_clean id_compress mid_mime/;
 use PublicInbox::MsgIter;
 use PublicInbox::Address;
 use PublicInbox::WwwStream;
@@ -581,9 +581,10 @@ sub __thread_entry {
 
 	# lazy load the full message from mini_mime:
 	$mime = eval {
-		my $path = mid2path(mid_clean(mid_mime($mime)));
-		Email::MIME->new($state->{ctx}->{git}->cat_file('HEAD:'.$path));
+		my $mid = mid_clean(mid_mime($mime));
+		$state->{ctx}->{-inbox}->msg_by_mid($mid);
 	} or return;
+	$mime = Email::MIME->new($mime);
 
 	thread_html_head($mime, $state) if $state->{anchor_idx} == 0;
 	if (my $ghost = delete $state->{ghost}) {
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index f88894a..f1f4abd 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -206,9 +206,7 @@ sub get_index {
 # just returns a string ref for the blob in the current ctx
 sub mid2blob {
 	my ($ctx) = @_;
-	require PublicInbox::MID;
-	my $path = PublicInbox::MID::mid2path($ctx->{mid});
-	$ctx->{git}->cat_file("HEAD:$path");
+	$ctx->{-inbox}->msg_by_mid($ctx->{mid});
 }
 
 # /$INBOX/$MESSAGE_ID/raw                    -> raw mbox
diff --git a/lib/PublicInbox/WwwAttach.pm b/lib/PublicInbox/WwwAttach.pm
index 5cf56a8..33bfce2 100644
--- a/lib/PublicInbox/WwwAttach.pm
+++ b/lib/PublicInbox/WwwAttach.pm
@@ -8,16 +8,13 @@ use warnings;
 use Email::MIME;
 use Email::MIME::ContentType qw(parse_content_type);
 $Email::MIME::ContentType::STRICT_PARAMS = 0;
-use PublicInbox::MID qw(mid2path);
 use PublicInbox::MsgIter;
 
 # /$LISTNAME/$MESSAGE_ID/$IDX-$FILENAME
 sub get_attach ($$$) {
 	my ($ctx, $idx, $fn) = @_;
-	my $path = mid2path($ctx->{mid});
-
 	my $res = [ 404, [ 'Content-Type', 'text/plain' ], [ "Not found\n" ] ];
-	my $mime = $ctx->{git}->cat_file("HEAD:$path") or return $res;
+	my $mime = $ctx->{-inbox}->msg_by_mid($ctx->{mid}) or return $res;
 	$mime = Email::MIME->new($mime);
 	msg_iter($mime, sub {
 		my ($part, $depth, @idx) = @{$_[0]};
diff --git a/t/feed.t b/t/feed.t
index 26c4bce..5dd869a 100644
--- a/t/feed.t
+++ b/t/feed.t
@@ -8,6 +8,7 @@ use PublicInbox::Feed;
 use PublicInbox::Git;
 use PublicInbox::Import;
 use PublicInbox::Config;
+use PublicInbox::Inbox;
 use File::Temp qw/tempdir/;
 my $have_xml_feed = eval { require XML::Feed; 1 };
 require 't/common.perl';
@@ -40,8 +41,15 @@ sub rand_use ($) {
 
 my $tmpdir = tempdir('pi-feed-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 my $git_dir = "$tmpdir/gittest";
-my $git = PublicInbox::Git->new($git_dir);
-my $im = PublicInbox::Import->new($git, 'testbox', 'test@example');
+my $ibx = PublicInbox::Inbox->new({
+	address => 'test@example',
+	-primary_address => 'test@example',
+	name => 'testbox',
+	mainrepo => $git_dir,
+	url => 'http://example.com/test',
+});
+my $git = $ibx->git;
+my $im = PublicInbox::Import->new($git, $ibx->{name}, 'test@example');
 
 {
 	is(0, system(qw(git init -q --bare), $git_dir), "git init");
@@ -95,7 +103,7 @@ EOF
 	# check initial feed
 	{
 		my $feed = string_feed({
-			git_dir => $git_dir,
+			-inbox => $ibx,
 			max => 3
 		});
 		SKIP: {
@@ -103,7 +111,7 @@ EOF
 			my $p = XML::Feed->parse(\$feed);
 			is($p->format, "Atom", "parsed atom feed");
 			is(scalar $p->entries, 3, "parsed three entries");
-			is($p->id, 'mailto:public-inbox@example.com',
+			is($p->id, 'mailto:test@example',
 				"id is set to default");
 		}
 
@@ -136,7 +144,7 @@ EOF
 	# check spam shows up
 	{
 		my $spammy_feed = string_feed({
-			git_dir => $git_dir,
+			-inbox => $ibx,
 			max => 3
 		});
 		SKIP: {
@@ -160,10 +168,7 @@ EOF
 
 	# spam no longer shows up
 	{
-		my $feed = string_feed({
-			git_dir => $git_dir,
-			max => 3
-		});
+		my $feed = string_feed({ -inbox => $ibx, max => 3 });
 		SKIP: {
 			skip 'XML::Feed missing', 2 unless $have_xml_feed;
 			my $p = XML::Feed->parse(\$feed);
@@ -174,26 +179,4 @@ EOF
 	}
 }
 
-# check pi_config
-{
-	foreach my $addr (('a@example.com'), ['a@example.com','b@localhost']) {
-		my $feed = string_feed({
-			git_dir => $git_dir,
-			max => 3,
-			inbox => 'asdf',
-			pi_config => bless({
-				'publicinbox.asdf.address' => $addr,
-			}, 'PublicInbox::Config'),
-		});
-		SKIP: {
-			skip 'XML::Feed missing', 3 unless $have_xml_feed;
-			my $p = XML::Feed->parse(\$feed);
-			is($p->id, 'mailto:a@example.com',
-				"ID is set correctly");
-			is($p->format, "Atom", "parsed atom feed");
-			is(scalar $p->entries, 3, "parsed three entries");
-		}
-	}
-}
-
 done_testing();
diff --git a/t/html_index.t b/t/html_index.t
index 6896eb4..32d7b8d 100644
--- a/t/html_index.t
+++ b/t/html_index.t
@@ -7,10 +7,18 @@ use Email::MIME;
 use PublicInbox::Feed;
 use PublicInbox::Git;
 use PublicInbox::Import;
+use PublicInbox::Inbox;
 use File::Temp qw/tempdir/;
 my $tmpdir = tempdir('pi-http-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 my $git_dir = "$tmpdir/gittest";
-my $git = PublicInbox::Git->new($git_dir);
+my $ibx = PublicInbox::Inbox->new({
+	address => 'test@example',
+	-primary_address => 'test@example',
+	name => 'tester',
+	mainrepo => $git_dir,
+	url => 'http://example.com/test',
+});
+my $git = $ibx->git;
 my $im = PublicInbox::Import->new($git, 'tester', 'test@example');
 
 # setup
@@ -55,7 +63,7 @@ EOF
 {
 	use IO::File;
 	my $cb = PublicInbox::Feed::generate_html_index({
-		git_dir => $git_dir,
+		-inbox => $ibx,
 		max => 3
 	});
 	require 't/common.perl';

[PATCH 7/7] inbox: move field population logic to initializer

[Eric Wong]

Inboxes are normally created by Config, but having the
population logic in Inbox should make it easier to mock
for testing.
---
 lib/PublicInbox/Config.pm | 4 +---
 lib/PublicInbox/Inbox.pm  | 3 +++
 t/feed.t                  | 1 -
 t/html_index.t            | 1 -
 4 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 43ffba7..ea84da3 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -129,10 +129,8 @@ sub _fill {
 	my $name = $pfx;
 	$name =~ s/\Apublicinbox\.//;
 	$rv->{name} = $name;
-	my $v = $rv->{address} ||= 'public-inbox@example.com';
-	my $p = $rv->{-primary_address} = ref($v) eq 'ARRAY' ? $v->[0] : $v;
-	$rv->{domain} = ($p =~ /\@(\S+)\z/) ? $1 : 'localhost';
 	$rv = PublicInbox::Inbox->new($rv);
+	my $v = $rv->{address};
 	if (ref($v) eq 'ARRAY') {
 		$self->{-by_addr}->{lc($_)} = $rv foreach @$v;
 	} else {
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index faab03c..3f1b733 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -11,6 +11,9 @@ use PublicInbox::MID qw(mid2path);
 
 sub new {
 	my ($class, $opts) = @_;
+	my $v = $opts->{address} ||= 'public-inbox@example.com';
+	my $p = $opts->{-primary_address} = ref($v) eq 'ARRAY' ? $v->[0] : $v;
+	$opts->{domain} = ($p =~ /\@(\S+)\z/) ? $1 : 'localhost';
 	bless $opts, $class;
 }
 
diff --git a/t/feed.t b/t/feed.t
index 5dd869a..19a9ba0 100644
--- a/t/feed.t
+++ b/t/feed.t
@@ -43,7 +43,6 @@ my $tmpdir = tempdir('pi-feed-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 my $git_dir = "$tmpdir/gittest";
 my $ibx = PublicInbox::Inbox->new({
 	address => 'test@example',
-	-primary_address => 'test@example',
 	name => 'testbox',
 	mainrepo => $git_dir,
 	url => 'http://example.com/test',
diff --git a/t/html_index.t b/t/html_index.t
index 32d7b8d..100d21a 100644
--- a/t/html_index.t
+++ b/t/html_index.t
@@ -13,7 +13,6 @@ my $tmpdir = tempdir('pi-http-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 my $git_dir = "$tmpdir/gittest";
 my $ibx = PublicInbox::Inbox->new({
 	address => 'test@example',
-	-primary_address => 'test@example',
 	name => 'tester',
 	mainrepo => $git_dir,
 	url => 'http://example.com/test',