unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 6/7] feed: various object-orientation cleanups
Date: Mon, 20 Jun 2016 00:57:16 +0000	[thread overview]
Message-ID: <20160620005717.1482-7-e@80x24.org> (raw)
In-Reply-To: <20160620005717.1482-1-e@80x24.org>

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';

  parent reply	other threads:[~2016-06-20  0:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-20  0:57 [PATCH 0/7] www: various feed/streaming related cleanups Eric Wong
2016-06-20  0:57 ` [PATCH 1/7] MANIFEST: update with recent changes Eric Wong
2016-06-20  0:57 ` [PATCH 2/7] feed: avoid needless method dispatches on 404 Eric Wong
2016-06-20  0:57 ` [PATCH 3/7] feed: remove dependence on fh->write for streaming Eric Wong
2016-06-20  0:57 ` [PATCH 4/7] mbox: remove feed dependency Eric Wong
2016-06-20  0:57 ` [PATCH 5/7] mbox: avoid write dependency for streaming Eric Wong
2016-06-20  0:57 ` Eric Wong [this message]
2016-06-20  0:57 ` [PATCH 7/7] inbox: move field population logic to initializer Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160620005717.1482-7-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).