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 shortcircuit=no autolearn=ham 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 47BA2209BB for ; Sat, 10 Dec 2016 03:43:07 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 6/7] thread: last Reference always wins Date: Sat, 10 Dec 2016 03:43:04 +0000 Message-Id: <20161210034305.2654-7-e@80x24.org> In-Reply-To: <20161210034305.2654-1-e@80x24.org> References: <20161210034305.2654-1-e@80x24.org> List-Id: Since we use SearchMsg from Xapian data, we can be assured we do not get self-referential {references} field. However, we may need to be more careful when checking has_descendent for loops, as blindly calling add_child could open us up to that possibility... --- lib/PublicInbox/SearchThread.pm | 13 ++++++++----- t/thread-cycle.t | 8 -------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/PublicInbox/SearchThread.pm b/lib/PublicInbox/SearchThread.pm index ee35f0d..601a84b 100644 --- a/lib/PublicInbox/SearchThread.pm +++ b/lib/PublicInbox/SearchThread.pm @@ -52,6 +52,10 @@ sub _add_message ($$) { # B. For each element in the message's References field: defined(my $refs = $smsg->{references}) or return; + # This loop exists to help fill in gaps left from missing + # messages. It is not needed in a perfect world where + # everything is perfectly referenced, only the last ref + # matters. my $prev; foreach my $ref ($refs =~ m/<([^>]+)>/g) { # Find a Container object for the given Message-ID @@ -74,10 +78,8 @@ sub _add_message ($$) { } # C. Set the parent of this message to be the last element in - # References... - if ($prev && !$this->has_descendent($prev)) { # would loop - $prev->add_child($this) - } + # References. + $prev->add_child($this) if defined $prev; } sub order { @@ -127,8 +129,9 @@ sub add_child { sub has_descendent { my ($self, $child) = @_; + my %seen; # loop prevention XXX may not be necessary while ($child) { - return 1 if $self == $child; + return 1 if $self == $child || $seen{$child}++; $child = $child->{parent}; } 0; diff --git a/t/thread-cycle.t b/t/thread-cycle.t index 0e1ecfe..b084449 100644 --- a/t/thread-cycle.t +++ b/t/thread-cycle.t @@ -70,14 +70,6 @@ SKIP: { is($check, $st, 'Mail::Thread output matches'); } -@msgs = map { bless $_, 'PublicInbox::SearchMsg' } ( - { mid => 'a@b' }, - { mid => 'b@c', references => ' ' }, - { mid => 'd@e', references => '' }, -); - -is(thread_to_s(\@msgs), "a\@b\n b\@c\nd\@e\n", 'ok with self-references'); - done_testing(); sub thread_to_s { -- EW