]> Sergey Matveev's repositories - public-inbox.git/commitdiff
search: get rid of most lookup_* subroutines
authorEric Wong (Contractor, The Linux Foundation) <e@80x24.org>
Thu, 29 Mar 2018 09:57:52 +0000 (09:57 +0000)
committerEric Wong (Contractor, The Linux Foundation) <e@80x24.org>
Thu, 29 Mar 2018 10:00:04 +0000 (10:00 +0000)
Too many similar functions doing the same basic thing was
redundant and misleading, especially since Message-ID is
no longer treated as a truly unique identifier.

For displaying threads in the HTML, this makes it clear
that we favor the primary Message-ID mapped to an NNTP
article number if a message cannot be found.

lib/PublicInbox/Inbox.pm
lib/PublicInbox/Search.pm
lib/PublicInbox/SearchThread.pm
lib/PublicInbox/SearchView.pm
lib/PublicInbox/View.pm
t/search-thr-index.t
t/search.t

index 4c7305f96dfa041710987697fc4a2dc76eba8549..01aa500c35123debd5ce948c77904f9e07a4f152 100644 (file)
@@ -293,20 +293,20 @@ sub path_check {
        git($self)->check('HEAD:'.$path);
 }
 
+sub smsg_by_mid ($$) {
+       my ($self, $mid) = @_;
+       my $srch = search($self) or return;
+       # favor the Message-ID we used for the NNTP article number:
+       my $mm = mm($self) or return;
+       my $num = $mm->num_for($mid);
+       $srch->lookup_article($num);
+}
+
 sub msg_by_mid ($$;$) {
        my ($self, $mid, $ref) = @_;
        my $srch = search($self) or
-                       return msg_by_path($self, mid2path($mid), $ref);
-       my $smsg;
-       # favor the Message-ID we used for the NNTP article number:
-       if (my $mm = mm($self)) {
-               my $num = $mm->num_for($mid);
-               $smsg = $srch->lookup_article($num);
-       } else {
-               $smsg = $srch->retry_reopen(sub {
-                       $srch->lookup_skeleton($mid) and $smsg->load_expand;
-               });
-       }
+               return msg_by_path($self, mid2path($mid), $ref);
+       my $smsg = smsg_by_mid($self, $mid);
        $smsg ? msg_by_smsg($self, $smsg, $ref) : undef;
 }
 
index 584a508e45f15d6c80cbdaa46ff8fa466cbc8de9..7d42aaad40d45676e2601d32d52a4d5918856e8d 100644 (file)
@@ -18,7 +18,7 @@ use constant YYYYMMDD => 5; # for searching in the WWW UI
 use Search::Xapian qw/:standard/;
 use PublicInbox::SearchMsg;
 use PublicInbox::MIME;
-use PublicInbox::MID qw/mid_clean id_compress/;
+use PublicInbox::MID qw/id_compress/;
 
 # This is English-only, everything else is non-standard and may be confused as
 # a prefix common in patch emails
@@ -193,9 +193,8 @@ sub query {
 
 sub get_thread {
        my ($self, $mid, $opts) = @_;
-       my $smsg = retry_reopen($self, sub { lookup_skeleton($self, $mid) });
-
-       return { total => 0, msgs => [] } unless $smsg;
+       my $smsg = first_smsg_by_mid($self, $mid) or
+                       return { total => 0, msgs => [] };
        my $qtid = Search::Xapian::Query->new('G' . $smsg->thread_id);
        my $path = $smsg->path;
        if (defined $path && $path ne '') {
@@ -346,48 +345,13 @@ sub query_ts {
        _do_enquire($self, $query, $opts);
 }
 
-sub lookup_skeleton {
+sub first_smsg_by_mid {
        my ($self, $mid) = @_;
-       my $skel = $self->{skel} or return lookup_message($self, $mid);
-       $mid = mid_clean($mid);
-       my $term = 'Q' . $mid;
        my $smsg;
-       my $beg = $skel->postlist_begin($term);
-       if ($beg != $skel->postlist_end($term)) {
-               my $doc_id = $beg->get_docid;
-               if (defined $doc_id) {
-                       # raises on error:
-                       my $doc = $skel->get_document($doc_id);
-                       $smsg = PublicInbox::SearchMsg->wrap($doc, $mid);
-                       $smsg->{doc_id} = $doc_id;
-               }
-       }
+       each_smsg_by_mid($self, $mid, sub { $smsg = $_[0]; undef });
        $smsg;
 }
 
-sub lookup_message {
-       my ($self, $mid) = @_;
-       $mid = mid_clean($mid);
-
-       my $doc_id = $self->find_first_doc_id('Q' . $mid);
-       my $smsg;
-       if (defined $doc_id) {
-               # raises on error:
-               my $doc = $self->{xdb}->get_document($doc_id);
-               $smsg = PublicInbox::SearchMsg->wrap($doc, $mid);
-               $smsg->{doc_id} = $doc_id;
-       }
-       $smsg;
-}
-
-sub lookup_mail { # no ghosts!
-       my ($self, $mid) = @_;
-       retry_reopen($self, sub {
-               my $smsg = lookup_skeleton($self, $mid) or return;
-               $smsg->load_expand;
-       });
-}
-
 sub lookup_article {
        my ($self, $num) = @_;
        my $term = 'XNUM'.$num;
@@ -447,16 +411,6 @@ sub find_doc_ids {
        ($db->postlist_begin($termval), $db->postlist_end($termval));
 }
 
-sub find_first_doc_id {
-       my ($self, $termval) = @_;
-
-       my ($begin, $end) = $self->find_doc_ids($termval);
-
-       return undef if $begin->equal($end); # not found
-
-       $begin->get_docid;
-}
-
 # normalize subjects so they are suitable as pathnames for URLs
 # XXX: consider for removal
 sub subject_path {
index 6fbce15c86e8a4b573f0ebe48a4f24fa13209415..1d250b4672f0f674a4290136fe519b4cc83ff0a8 100644 (file)
@@ -22,15 +22,15 @@ use strict;
 use warnings;
 
 sub thread {
-       my ($messages, $ordersub, $srch) = @_;
+       my ($messages, $ordersub, $ibx) = @_;
        my $id_table = {};
        _add_message($id_table, $_) foreach @$messages;
        my $rootset = [ grep {
-                       !delete($_->{parent}) && $_->visible($srch)
+                       !delete($_->{parent}) && $_->visible($ibx)
                } values %$id_table ];
        $id_table = undef;
        $rootset = $ordersub->($rootset);
-       $_->order_children($ordersub, $srch) for @$rootset;
+       $_->order_children($ordersub, $ibx) for @$rootset;
        $rootset;
 }
 
@@ -131,20 +131,20 @@ sub has_descendent {
 # a ghost Message-ID is the result of a long header line
 # being folded/mangled by a MUA, and not a missing message.
 sub visible ($$) {
-       my ($self, $srch) = @_;
-       ($self->{smsg} ||= eval { $srch->lookup_mail($self->{id}) }) ||
+       my ($self, $ibx) = @_;
+       ($self->{smsg} ||= eval { $ibx->smsg_by_mid($self->{id}) }) ||
         (scalar values %{$self->{children}});
 }
 
 sub order_children {
-       my ($cur, $ordersub, $srch) = @_;
+       my ($cur, $ordersub, $ibx) = @_;
 
        my %seen = ($cur => 1); # self-referential loop prevention
        my @q = ($cur);
        while (defined($cur = shift @q)) {
                my $c = $cur->{children}; # The hashref here...
 
-               $c = [ grep { !$seen{$_}++ && visible($_, $srch) } values %$c ];
+               $c = [ grep { !$seen{$_}++ && visible($_, $ibx) } values %$c ];
                $c = $ordersub->($c) if scalar @$c > 1;
                $cur->{children} = $c; # ...becomes an arrayref
                push @q, @$c;
index 1a8fe7f7f1681eb8b1a7fe459b5c28192ccd4225..c78979582432d97856a9d5690d7a98c90646732d 100644 (file)
@@ -228,7 +228,7 @@ sub mset_thread {
        my $r = $q->{r};
        my $rootset = PublicInbox::SearchThread::thread($msgs,
                $r ? sort_relevance(\%pct) : *PublicInbox::View::sort_ds,
-               $srch);
+               $ctx);
        my $skel = search_nav_bot($mset, $q). "<pre>";
        my $inbox = $ctx->{-inbox};
        $ctx->{-upfx} = '';
index aad860e996f0d12c1a33de74035c0f0b976fc672..f5b278c297ee59a5e9496a05736aa9cdccaa7997 100644 (file)
@@ -430,7 +430,7 @@ sub thread_html {
        $ctx->{mapping} = {};
        $ctx->{s_nr} = "$nr+ messages in thread";
 
-       my $rootset = thread_results($msgs, $srch);
+       my $rootset = thread_results($ctx, $msgs);
 
        # reduce hash lookups in pre_thread->skel_dump
        my $inbox = $ctx->{-inbox};
@@ -686,7 +686,7 @@ sub thread_skel {
        # reduce hash lookups in skel_dump
        my $ibx = $ctx->{-inbox};
        $ctx->{-obfs_ibx} = $ibx->{obfuscate} ? $ibx : undef;
-       walk_thread(thread_results($sres, $srch), $ctx, *skel_dump);
+       walk_thread(thread_results($ctx, $sres), $ctx, *skel_dump);
 
        $ctx->{parent_msg} = $parent;
 }
@@ -809,9 +809,9 @@ sub load_results {
 }
 
 sub thread_results {
-       my ($msgs, $srch) = @_;
+       my ($ctx, $msgs) = @_;
        require PublicInbox::SearchThread;
-       PublicInbox::SearchThread::thread($msgs, *sort_ds, $srch);
+       PublicInbox::SearchThread::thread($msgs, *sort_ds, $ctx->{-inbox});
 }
 
 sub missing_thread {
@@ -952,7 +952,7 @@ sub acc_topic {
        my ($ctx, $level, $node) = @_;
        my $srch = $ctx->{srch};
        my $mid = $node->{id};
-       my $x = $node->{smsg} || $srch->lookup_mail($mid);
+       my $x = $node->{smsg} || $ctx->{-inbox}->smsg_by_mid($mid);
        my ($subj, $ds);
        my $topic;
        if ($x) {
@@ -1078,7 +1078,7 @@ sub index_topics {
        my $nr = scalar @{$sres->{msgs}};
        if ($nr) {
                $sres = load_results($srch, $sres);
-               walk_thread(thread_results($sres, $srch), $ctx, *acc_topic);
+               walk_thread(thread_results($ctx, $sres), $ctx, *acc_topic);
        }
        $ctx->{-next_o} = $off+ $nr;
        $ctx->{-cur_o} = $off;
index 6c6e4c57639dab2b63dc27d7d4e2ade0f8bc9143..9549976d6c1fb80144ccf97179eb00bbeac2fd4a 100644 (file)
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use Test::More;
 use File::Temp qw/tempdir/;
+use PublicInbox::MID qw(mids);
 use Email::MIME;
 eval { require PublicInbox::SearchIdx; };
 plan skip_all => "Xapian missing for search" if $@;
@@ -41,7 +42,7 @@ foreach (reverse split(/\n\n/, $data)) {
        $mime->header_set('From' => 'bw@g');
        $mime->header_set('To' => 'git@vger.kernel.org');
        my $bytes = bytes::length($mime->as_string);
-       my $mid = $mime->header('Message-Id');
+       my $mid = mids($mime->header_obj)->[0];
        my $doc_id = $rw->add_message($mime, $bytes, ++$num, 'ignored', $mid);
        push @mids, $mid;
        ok($doc_id, 'message added: '. $mid);
index 6b1aa2a31120e1d43dd11f718dcd7400c1b8d119..ccf0f7463588eb6a68870c5cd67b049588a695b7 100644 (file)
@@ -89,7 +89,7 @@ sub filter_mids {
 {
        $rw_commit->();
        $ro->reopen;
-       my $found = $ro->lookup_message('<root@s>');
+       my $found = $ro->first_smsg_by_mid('root@s');
        ok($found, "message found");
        is($root_id, $found->{doc_id}, 'doc_id set correctly');
        is($found->mid, 'root@s', 'mid set correctly');
@@ -264,7 +264,7 @@ sub filter_mids {
                ],
                body => "LOOP!\n"));
        ok($doc_id > 0, "doc_id defined with circular reference");
-       my $smsg = $rw->lookup_message('circle@a');
+       my $smsg = $rw->first_smsg_by_mid('circle@a');
        is($smsg->references, '', "no references created");
        my $msg = PublicInbox::SearchMsg->load_doc($smsg->{doc});
        is($s, $msg->subject, 'long subject not rewritten');
@@ -281,7 +281,7 @@ sub filter_mids {
        my $mime = Email::MIME->new($str);
        my $doc_id = $rw->add_message($mime);
        ok($doc_id > 0, 'message indexed doc_id with UTF-8');
-       my $smsg = $rw->lookup_message('testmessage@example.com');
+       my $smsg = $rw->first_smsg_by_mid('testmessage@example.com');
        my $msg = PublicInbox::SearchMsg->load_doc($smsg->{doc});
 
        is($mime->header('Subject'), $msg->subject, 'UTF-8 subject preserved');