From 528230a0411897a14cbb79d0dae02ea89827bf1e Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 28 May 2016 01:57:14 +0000 Subject: remove redundant NewsGroup class 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. --- 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 +- 7 files changed, 52 insertions(+), 152 deletions(-) delete mode 100644 lib/PublicInbox/NewsGroup.pm (limited to 'lib') diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm index 317d290a..a8c5105e 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 c07aaa9f..d050dc86 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 f3de4b1c..58b86a82 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 fc26c5c0..50d022be 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 500f61e3..00000000 --- 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 5357059d..908c4351 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 ab3cd5d6..cf370afa 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 $_;"; } } -- cgit v1.2.3-24-ge0c7