* [PATCH 7/7] remove redundant NewsGroup class
2016-05-28 1:57 [PATCH 0/7] miscellaneous cleanups Eric Wong
` (5 preceding siblings ...)
2016-05-28 1:57 ` [PATCH 6/7] config: remove try_cat Eric Wong
@ 2016-05-28 1:57 ` Eric Wong
6 siblings, 0 replies; 8+ messages in thread
From: Eric Wong @ 2016-05-28 1:57 UTC (permalink / raw)
To: meta
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', "<mailto:$ng->{address}>");
- if (my $url = $ng->{url}) {
+ header_append($hdr, 'List-Post', "<mailto:$ng->{-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 <meta@public-inbox.org>
-# 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
^ permalink raw reply related [flat|nested] 8+ messages in thread