From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 2BFAE20193 for ; Mon, 20 Jun 2016 00:57:20 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 6/7] feed: various object-orientation cleanups Date: Mon, 20 Jun 2016 00:57:16 +0000 Message-Id: <20160620005717.1482-7-e@80x24.org> In-Reply-To: <20160620005717.1482-1-e@80x24.org> References: <20160620005717.1482-1-e@80x24.org> List-Id: 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';