]> Sergey Matveev's repositories - public-inbox.git/commitdiff
remove redundant NewsGroup class
authorEric Wong <e@80x24.org>
Sat, 28 May 2016 01:57:14 +0000 (01:57 +0000)
committerEric Wong <e@80x24.org>
Sat, 28 May 2016 01:57:39 +0000 (01:57 +0000)
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
lib/PublicInbox/Config.pm
lib/PublicInbox/Inbox.pm
lib/PublicInbox/NNTP.pm
lib/PublicInbox/NNTPD.pm
lib/PublicInbox/NewsGroup.pm [deleted file]
lib/PublicInbox/NewsWWW.pm
lib/PublicInbox/WWW.pm
t/config.t
t/nntp.t
t/nntpd.t

index 259f42ce0a936fadcde800f0dc8d8cc80a8d9651..370eac9be797a22f3e75694486b1a3b04ad4a940 100644 (file)
--- 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
index 317d290a4c36954c0688609dac91694832526d25..a8c5105e636b50a2866860d686d7e4bb1eab16da 100644 (file)
@@ -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;
index c07aaa9f7056509c16008ea1e301ba74e647a7d3..d050dc86b0ad677c289e8b171bd2bcf191aa3878 100644 (file)
@@ -83,4 +83,11 @@ sub base_url {
        }
 }
 
+sub nntp_usable {
+       my ($self) = @_;
+       my $ret = $self->mm && $self->search;
+       weaken_all();
+       $ret;
+}
+
 1;
index f3de4b1cd5722b603f5aa2f1d016ee8e7fb8572c..58b86a8297a5ba5e4a60941c5533277530e9235d 100644 (file)
@@ -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>");
index fc26c5c0b812c18875ae2370b3efbb701b31dc98..50d022be4ae2142ca3bcb39c378c70c8ab349f04 100644 (file)
@@ -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 (file)
index 500f61e..0000000
+++ /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;
index 5357059dee21733a4d9f537f591fd0376ea5fda8..908c43510dbd8f41bf6ce48dfb2e6212e771fcff 100644 (file)
@@ -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;
index ab3cd5d63e9794a71f2f08ff6c42efb157c3ef11..cf370afa750fd5c832ab29a661f1d58bc762252d 100644 (file)
@@ -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 $_;";
        }
 }
index 78971a2f75726a78096883c72bb5e439836e2f4e..dc448cdffff6b7f92574a4a7903323ccd9e769b3 100644 (file)
@@ -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");
index 5513c7bcd58a5be43e02876f374157d2707ad0b4..de07abb0f38ea9a82b8560329dd9d17a703cdb16 100644 (file)
--- 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');
index 837b9d465ba67a44c783839139caaa3269f1d45c..c5bc0b54fb76bdbbd941c10f1da94a4cda485afc 100644 (file)
--- 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