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,BAYES_00, URIBL_BLOCKED shortcircuit=no autolearn=unavailable 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 B38C71FCFF for ; Sat, 28 May 2016 01:57:19 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 7/7] remove redundant NewsGroup class Date: Sat, 28 May 2016 01:57:14 +0000 Message-Id: <20160528015714.1325-8-e@80x24.org> In-Reply-To: <20160528015714.1325-1-e@80x24.org> References: <20160528015714.1325-1-e@80x24.org> List-Id: Most of its functionality is in the PublicInbox::Inbox class. While we're at it, we no longer auto-create newsgroup names based on the inbox name, since newsgroup names probably deserve some thought when it comes to hierarchy. --- MANIFEST | 1 - lib/PublicInbox/Config.pm | 37 ++++++++++++++----- lib/PublicInbox/Inbox.pm | 7 ++++ lib/PublicInbox/NNTP.pm | 9 ++--- lib/PublicInbox/NNTPD.pm | 27 ++++---------- lib/PublicInbox/NewsGroup.pm | 84 -------------------------------------------- lib/PublicInbox/NewsWWW.pm | 38 ++------------------ lib/PublicInbox/WWW.pm | 2 +- t/config.t | 2 ++ t/nntp.t | 15 +++++--- t/nntpd.t | 4 ++- 11 files changed, 67 insertions(+), 159 deletions(-) delete mode 100644 lib/PublicInbox/NewsGroup.pm diff --git a/MANIFEST b/MANIFEST index 259f42c..370eac9 100644 --- a/MANIFEST +++ b/MANIFEST @@ -37,7 +37,6 @@ lib/PublicInbox/MID.pm lib/PublicInbox/Mbox.pm lib/PublicInbox/Msgmap.pm lib/PublicInbox/NNTP.pm -lib/PublicInbox/NewsGroup.pm lib/PublicInbox/NewsWWW.pm lib/PublicInbox/ProcessPipe.pm lib/PublicInbox/Search.pm diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm index 317d290..a8c5105 100644 --- a/lib/PublicInbox/Config.pm +++ b/lib/PublicInbox/Config.pm @@ -20,6 +20,7 @@ sub new { # caches $self->{-by_addr} ||= {}; $self->{-by_name} ||= {}; + $self->{-by_newsgroup} ||= {}; $self; } @@ -55,7 +56,24 @@ sub lookup_name { my $rv = $self->{-by_name}->{$name}; return $rv if $rv; $rv = _fill($self, "publicinbox.$name") or return; - $self->{-by_name}->{$name} = $rv; +} + +sub lookup_newsgroup { + my ($self, $ng) = @_; + $ng = lc($ng); + my $rv = $self->{-by_newsgroup}->{$ng}; + return $rv if $rv; + + foreach my $k (keys %$self) { + $k =~ /\A(publicinbox\.[\w-]+)\.newsgroup\z/ or next; + my $v = $self->{$k}; + my $pfx = $1; + if ($v eq $ng) { + $rv = _fill($self, $pfx); + return $rv; + } + } + undef; } sub get { @@ -103,24 +121,27 @@ sub _fill { my ($self, $pfx) = @_; my $rv = {}; - foreach my $k (qw(mainrepo address filter url)) { + foreach my $k (qw(mainrepo address filter url newsgroup)) { my $v = $self->{"$pfx.$k"}; $rv->{$k} = $v if defined $v; } return unless $rv->{mainrepo}; - my $inbox = $pfx; - $inbox =~ s/\Apublicinbox\.//; - $rv->{name} = $inbox; + my $name = $pfx; + $name =~ s/\Apublicinbox\.//; + $rv->{name} = $name; my $v = $rv->{address} ||= 'public-inbox@example.com'; - $rv->{-primary_address} = ref($v) eq 'ARRAY' ? $v->[0] : $v; + my $p = $rv->{-primary_address} = ref($v) eq 'ARRAY' ? $v->[0] : $v; + $rv->{domain} = ($p =~ /\@(\S+)\z/) ? $1 : 'localhost'; $rv = PublicInbox::Inbox->new($rv); if (ref($v) eq 'ARRAY') { $self->{-by_addr}->{lc($_)} = $rv foreach @$v; } else { $self->{-by_addr}->{lc($v)} = $rv; } - $rv; + if (my $ng = $rv->{newsgroup}) { + $self->{-by_newsgroup}->{$ng} = $rv; + } + $self->{-by_name}->{$name} = $rv; } - 1; diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm index c07aaa9..d050dc8 100644 --- a/lib/PublicInbox/Inbox.pm +++ b/lib/PublicInbox/Inbox.pm @@ -83,4 +83,11 @@ sub base_url { } } +sub nntp_usable { + my ($self) = @_; + my $ret = $self->mm && $self->search; + weaken_all(); + $ret; +} + 1; diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index f3de4b1..58b86a8 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -195,7 +195,7 @@ sub list_active_times ($;$) { foreach my $ng (@{$self->{nntpd}->{grouplist}}) { $ng->{newsgroup} =~ $wildmat or next; my $c = eval { $ng->mm->created_at } || time; - more($self, "$ng->{newsgroup} $c $ng->{address}"); + more($self, "$ng->{newsgroup} $c $ng->{-primary_address}"); } } @@ -413,7 +413,8 @@ sub cmd_last ($) { article_adj($_[0], -1) } sub cmd_post ($) { my ($self) = @_; my $ng = $self->{ng}; - $ng ? "440 mailto:$ng->{address} to post" : '440 posting not allowed' + $ng ? "440 mailto:$ng->{-primary_address} to post" + : '440 posting not allowed' } sub cmd_quit ($) { @@ -438,8 +439,8 @@ sub set_nntp_headers { # clobber some $hdr->header_set('Newsgroups', $ng->{newsgroup}); $hdr->header_set('Xref', xref($ng, $n)); - header_append($hdr, 'List-Post', "{address}>"); - if (my $url = $ng->{url}) { + header_append($hdr, 'List-Post', "{-primary_address}>"); + if (my $url = $ng->base_url) { $mid = uri_escape_utf8($mid); header_append($hdr, 'Archived-At', "<$url$mid/>"); header_append($hdr, 'List-Archive', "<$url>"); diff --git a/lib/PublicInbox/NNTPD.pm b/lib/PublicInbox/NNTPD.pm index fc26c5c..50d022b 100644 --- a/lib/PublicInbox/NNTPD.pm +++ b/lib/PublicInbox/NNTPD.pm @@ -6,7 +6,6 @@ package PublicInbox::NNTPD; use strict; use warnings; -require PublicInbox::NewsGroup; require PublicInbox::Config; sub new { @@ -26,28 +25,16 @@ sub refresh_groups () { my @list; foreach my $k (keys %$pi_config) { $k =~ /\Apublicinbox\.([^\.]+)\.mainrepo\z/ or next; - my $g = $1; + my $name = $1; my $git_dir = $pi_config->{$k}; - my $addr = $pi_config->{"publicinbox.$g.address"}; - my $ngname = $pi_config->{"publicinbox.$g.newsgroup"}; - my $url = $pi_config->{"publicinbox.$g.url"}; - if (defined $ngname) { - next if ($ngname eq ''); # disabled - $g = $ngname; - } - my $ng = PublicInbox::NewsGroup->new($g, $git_dir, $addr, $url); - my $old_ng = $self->{groups}->{$g}; - - # Reuse the old one if possible since it can hold - # references to valid mm and gcf objects - if ($old_ng) { - $old_ng->update($ng); - $ng = $old_ng; - } + my $ngname = $pi_config->{"publicinbox.$name.newsgroup"}; + next unless defined $ngname; + next if ($ngname eq ''); # disabled + my $ng = $pi_config->lookup_newsgroup($ngname) or next; # Only valid if msgmap and search works - if ($ng->usable) { - $new->{$g} = $ng; + if ($ng->nntp_usable) { + $new->{$ngname} = $ng; push @list, $ng; } } diff --git a/lib/PublicInbox/NewsGroup.pm b/lib/PublicInbox/NewsGroup.pm deleted file mode 100644 index 500f61e..0000000 --- a/lib/PublicInbox/NewsGroup.pm +++ /dev/null @@ -1,84 +0,0 @@ -# Copyright (C) 2015 all contributors -# License: AGPLv3 or later (https://www.gnu.org/licenses/agpl-3.0.txt) -# -# Used only by the NNTP server to represent a public-inbox git repository -# as a newsgroup -package PublicInbox::NewsGroup; -use strict; -use warnings; -use Scalar::Util qw(weaken); -require Danga::Socket; -require PublicInbox::Msgmap; -require PublicInbox::Search; -require PublicInbox::Git; - -sub new { - my ($class, $newsgroup, $git_dir, $address, $url) = @_; - - # first email address is preferred - $address = $address->[0] if ref($address); - if ($url) { - # assume protocol-relative URLs which start with '//' means - # the server supports both HTTP and HTTPS, favor HTTPS. - $url = "https:$url" if $url =~ m!\A//!; - $url .= '/' if $url !~ m!/\z!; - } - my $self = bless { - newsgroup => $newsgroup, - git_dir => $git_dir, - address => $address, - url => $url, - }, $class; - $self->{domain} = ($address =~ /\@(\S+)\z/) ? $1 : 'localhost'; - $self; -} - -sub weaken_all { - my ($self) = @_; - weaken($self->{$_}) foreach qw(gcf mm search); -} - -sub gcf { - my ($self) = @_; - $self->{gcf} ||= eval { PublicInbox::Git->new($self->{git_dir}) }; -} - -sub usable { - my ($self) = @_; - eval { - PublicInbox::Msgmap->new($self->{git_dir}); - PublicInbox::Search->new($self->{git_dir}); - }; -} - -sub mm { - my ($self) = @_; - $self->{mm} ||= eval { PublicInbox::Msgmap->new($self->{git_dir}) }; -} - -sub search { - my ($self) = @_; - $self->{search} ||= eval { PublicInbox::Search->new($self->{git_dir}) }; -} - -sub description { - my ($self) = @_; - open my $fh, '<', "$self->{git_dir}/description" or return ''; - my $desc = eval { local $/; <$fh> }; - chomp $desc; - $desc =~ s/\s+/ /smg; - $desc; -} - -sub update { - my ($self, $new) = @_; - $self->{address} = $new->{address}; - $self->{domain} = $new->{domain}; - if ($self->{git_dir} ne $new->{git_dir}) { - # new git_dir requires a new mm and gcf - $self->{mm} = $self->{gcf} = undef; - $self->{git_dir} = $new->{git_dir}; - } -} - -1; diff --git a/lib/PublicInbox/NewsWWW.pm b/lib/PublicInbox/NewsWWW.pm index 5357059..908c435 100644 --- a/lib/PublicInbox/NewsWWW.pm +++ b/lib/PublicInbox/NewsWWW.pm @@ -19,7 +19,6 @@ sub new { sub call { my ($self, $env) = @_; - my $ng_map = $self->newsgroup_map; my $path = $env->{PATH_INFO}; $path =~ s!\A/+!!; $path =~ s!/+\z!!; @@ -27,11 +26,11 @@ sub call { # some links may have the article number in them: # /inbox.foo.bar/123456 my ($ng, $article) = split(m!/+!, $path, 2); - if (my $info = $ng_map->{$ng}) { - my $url = PublicInbox::Hval::prurl($env, $info->{url}); + if (my $inbox = $self->{pi_config}->lookup_newsgroup($ng)) { + my $url = PublicInbox::Hval::prurl($env, $inbox->{url}); my $code = 301; if (defined $article && $article =~ /\A\d+\z/) { - my $mid = eval { ng_mid_for($ng, $info, $article) }; + my $mid = eval { $inbox->mm->mid_for($article) }; if (defined $mid) { # article IDs are not stable across clones, # do not encourage caching/bookmarking them @@ -47,35 +46,4 @@ sub call { [ 404, [ 'Content-Type' => 'text/plain' ], [] ]; } -sub ng_mid_for { - my ($ng, $info, $article) = @_; - # may fail due to lack of Danga::Socket - # for defer_weaken: - require PublicInbox::NewsGroup; - $ng = $info->{ng} ||= - PublicInbox::NewsGroup->new($ng, $info->{git_dir}, ''); - $ng->mm->mid_for($article); -} - -sub newsgroup_map { - my ($self) = @_; - my $rv; - $rv = $self->{ng_map} and return $rv; - my $pi_config = $self->{pi_config}; - my %ng_map; - foreach my $k (keys %$pi_config) { - $k =~ /\Apublicinbox\.([^\.]+)\.mainrepo\z/ or next; - my $inbox = $1; - my $git_dir = $pi_config->{"publicinbox.$inbox.mainrepo"}; - my $url = $pi_config->{"publicinbox.$inbox.url"}; - defined $url or next; - my $ng = $pi_config->{"publicinbox.$inbox.newsgroup"}; - next if (!defined $ng) || ($ng eq ''); # disabled - - $url =~ m!/\z! or $url .= '/'; - $ng_map{$ng} = { url => $url, git_dir => $git_dir }; - } - $self->{ng_map} = \%ng_map; -} - 1; diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm index ab3cd5d..cf370af 100644 --- a/lib/PublicInbox/WWW.pm +++ b/lib/PublicInbox/WWW.pm @@ -107,7 +107,7 @@ sub preload { foreach (qw(PublicInbox::Search PublicInbox::SearchView PublicInbox::Mbox IO::Compress::Gzip - PublicInbox::NewsWWW PublicInbox::NewsGroup)) { + PublicInbox::NewsWWW)) { eval "require $_;"; } } diff --git a/t/config.t b/t/config.t index 78971a2..dc448cd 100644 --- a/t/config.t +++ b/t/config.t @@ -26,6 +26,7 @@ my $tmpdir = tempdir('pi-config-XXXXXX', TMPDIR => 1, CLEANUP => 1); is_deeply($cfg->lookup('meta@public-inbox.org'), { 'mainrepo' => '/home/pi/meta-main.git', 'address' => 'meta@public-inbox.org', + 'domain' => 'public-inbox.org', 'url' => 'http://example.com/meta', -primary_address => 'meta@public-inbox.org', 'name' => 'meta', @@ -41,6 +42,7 @@ my $tmpdir = tempdir('pi-config-XXXXXX', TMPDIR => 1, CLEANUP => 1); 'test@public-inbox.org'], -primary_address => 'try@public-inbox.org', 'mainrepo' => '/home/pi/test-main.git', + 'domain' => 'public-inbox.org', 'name' => 'test', 'url' => 'http://example.com/test', }, "lookup matches expected output for test"); diff --git a/t/nntp.t b/t/nntp.t index 5513c7b..de07abb 100644 --- a/t/nntp.t +++ b/t/nntp.t @@ -11,7 +11,7 @@ foreach my $mod (qw(DBD::SQLite Search::Xapian Danga::Socket)) { } use_ok 'PublicInbox::NNTP'; -use_ok 'PublicInbox::NewsGroup'; +use_ok 'PublicInbox::Inbox'; { sub quote_str { @@ -99,9 +99,14 @@ use_ok 'PublicInbox::NewsGroup'; { # test setting NNTP headers in HEAD and ARTICLE requests require Email::MIME; my $u = 'https://example.com/a/'; - my $ng = PublicInbox::NewsGroup->new('test', 'test.git', - 'a@example.com', '//example.com/a'); - is($ng->{url}, $u, 'URL expanded'); + my $ng = PublicInbox::Inbox->new({ name => 'test', + mainrepo => 'test.git', + address => 'a@example.com', + -primary_address => 'a@example.com', + newsgroup => 'test', + domain => 'example.com', + url => '//example.com/a'}); + is($ng->base_url, $u, 'URL expanded'); my $mid = 'a@b'; my $mime = Email::MIME->new("Message-ID: <$mid>\r\n\r\n"); PublicInbox::NNTP::set_nntp_headers($mime->header_obj, $ng, 1, $mid); @@ -118,7 +123,7 @@ use_ok 'PublicInbox::NewsGroup'; is_deeply([ $mime->header('Xref') ], [ 'example.com test:1' ], 'Xref: set'); - $ng->{url} = 'http://mirror.example.com/m/'; + $ng->{-base_url} = 'http://mirror.example.com/m/'; PublicInbox::NNTP::set_nntp_headers($mime->header_obj, $ng, 2, $mid); is_deeply([ $mime->header('Message-ID') ], [ "<$mid>" ], 'Message-ID unchanged'); diff --git a/t/nntpd.t b/t/nntpd.t index 837b9d4..c5bc0b5 100644 --- a/t/nntpd.t +++ b/t/nntpd.t @@ -24,7 +24,6 @@ my $out = "$tmpdir/stdout.log"; my $maindir = "$tmpdir/main.git"; my $group = 'test-nntpd'; my $addr = $group . '@example.com'; -my $cfgpfx = "publicinbox.$group"; my $nntpd = 'blib/script/public-inbox-nntpd'; my $init = 'blib/script/public-inbox-init'; use_ok 'PublicInbox::Import'; @@ -44,6 +43,9 @@ END { kill 'TERM', $pid if defined $pid }; { local $ENV{HOME} = $home; system($init, $group, $maindir, 'http://example.com/', $addr); + is(system(qw(git config), "--file=$home/.public-inbox/config", + "publicinbox.$group.newsgroup", $group), + 0, 'enabled newsgroup'); my $len; # ensure successful message delivery -- EW