[PATCH 0/6] another round of search updates

[PATCH 0/6] another round of search updates

[Eric Wong]

Gah, so much code, now :x

 lib/PublicInbox/ExtMsg.pm     | 45 ++++++++++++++++++++++++++-----
 lib/PublicInbox/Search.pm     | 30 +++++++++++++++------
 lib/PublicInbox/SearchView.pm | 63 ++++++++++++++++++++++++++-----------------
 lib/PublicInbox/WWW.pm        |  1 +
 4 files changed, 100 insertions(+), 39 deletions(-)

[PATCH 1/6] searchview: factor out dump_mset subroutine

[Eric Wong]

We'll be moving atom and thread display support inline
and reducing endpoints.  Maybe it makes sense, maybe not.
---
 lib/PublicInbox/SearchView.pm | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 4d49d25..960049f 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -49,23 +49,7 @@ sub sres_top_html {
 		}
 		$res .= "]\n\n";
 
-		my $pad = length("$total");
-		my $pfx = ' ' x $pad;
-		foreach my $m ($mset->items) {
-			my $rank = sprintf("%${pad}d", $m->get_rank + 1);
-			my $pct = $m->get_percent;
-			my $smsg = $m->get_document;
-			$smsg = PublicInbox::SearchMsg->load_doc($smsg);
-			my $s = PublicInbox::Hval->new_oneline($smsg->subject);
-			my $f = $smsg->from_name;
-			$f = PublicInbox::Hval->new_oneline($f)->as_html;
-			my $d = strftime('%Y-%m-%d %H:%M', gmtime($smsg->ts));
-			my $mid = $smsg->mid;
-			$mid = PublicInbox::Hval->new_msgid($mid)->as_href;
-			$res .= qq{$rank. <b><a\nhref="$mid/t/#u">}.
-				$s->as_html . "</a></b>\n";
-			$res .= "$pfx  - by $f @ $d UTC [$pct%]\n\n";
-		}
+		dump_mset(\$res, $mset);
 		my $nr = scalar $mset->items;
 		my $end = $o + $nr;
 		my $beg = $o + 1;
@@ -86,17 +70,32 @@ sub sres_top_html {
 			$qp .= "&amp;r" if $r;
 			$res .= qq{<a\nhref="?$qp">prev</a>};
 		}
-		$res .= "\n\n".$foot;
+		$res .= "\n\n" . $foot;
 	}
 
 	$res .= "</pre></body></html>";
 	[200, ['Content-Type'=>'text/html; charset=UTF-8'], [$res]];
 }
 
-sub sres_top_atom {
-}
+sub dump_mset {
+	my ($res, $mset) = @_;
 
-sub sres_top_thread {
+	my $total = $mset->get_matches_estimated;
+	my $pad = length("$total");
+	my $pfx = ' ' x $pad;
+	foreach my $m ($mset->items) {
+		my $rank = sprintf("%${pad}d", $m->get_rank + 1);
+		my $pct = $m->get_percent;
+		my $smsg = PublicInbox::SearchMsg->load_doc($m->get_document);
+		my $s = PublicInbox::Hval->new_oneline($smsg->subject);
+		my $f = $smsg->from_name;
+		$f = PublicInbox::Hval->new_oneline($f)->as_html;
+		my $d = strftime('%Y-%m-%d %H:%M', gmtime($smsg->ts));
+		my $mid = PublicInbox::Hval->new_msgid($smsg->mid)->as_href;
+		$$res .= qq{$rank. <b><a\nhref="$mid/t/#u">}.
+			$s->as_html . "</a></b>\n";
+		$$res .= "$pfx  - by $f @ $d UTC [$pct%]\n\n";
+	}
 }
 
 1;
-- 
EW

[PATCH 2/6] search: use relevance as secondary sort by default

[Eric Wong]

Might as well give relevance some weight if the timestamp is tied.
---
 lib/PublicInbox/Search.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index fd79b63..1874ee9 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -118,7 +118,7 @@ sub do_enquire {
 	if ($opts->{relevance}) {
 		$enquire->set_sort_by_relevance_then_value(TS, 1);
 	} else {
-		$enquire->set_sort_by_value(TS, 1);
+		$enquire->set_sort_by_value_then_relevance(TS, 1);
 	}
 	$opts ||= {};
 	my $offset = $opts->{offset} || 0;
-- 
EW

[PATCH 3/6] searchview: error description for invalid queries

[Eric Wong]

Xapian may raise exceptions on some queries.  Pass the error
along to the user so they can read Xapian documentation.
---
 lib/PublicInbox/SearchView.pm | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 960049f..96b691d 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -12,13 +12,18 @@ our $LIM = 25;
 sub sres_top_html {
 	my ($ctx, $q) = @_;
 	my $cgi = $ctx->{cgi};
+	my $code = 200;
 	# $q ||= $cgi->param('q');
 	my $o = int($cgi->param('o') || 0);
 	my $r = $cgi->param('r');
 	$r = (defined $r && $r ne '0');
 	my $opts = { limit => $LIM, offset => $o, mset => 1, relevance => $r };
-	my $mset = $ctx->{srch}->query($q, $opts);
-	my $total = $mset->get_matches_estimated;
+	my ($mset, $total);
+	eval {
+		$mset = $ctx->{srch}->query($q, $opts);
+		$total = $mset->get_matches_estimated;
+	};
+	my $err = $@;
 	my $query = PublicInbox::Hval->new_oneline($q);
 	my $qh = $query->as_html;
 	my $res = "<html><head><title>$qh - search results</title></head>" .
@@ -32,7 +37,16 @@ sub sres_top_html {
 
 	my $foot = $ctx->{footer} || '';
 	$foot = qq{Back to <a\nhref=".">index</a>.};
-	if ($total == 0) {
+	if ($err) {
+		my $u = 'http://xapian.org/docs/queryparser.html';
+		$code = 400;
+		$err =~ s/^\s*Exception:\s*//; # bad word to show users :P
+		$err = PublicInbox::Hval->new_oneline($err)->as_html;
+		$res .= "\n\nBad query: <b>$err</b>\n";
+		$res .= qq{See <a\nhref="$u">$u</a> for Xapian query syntax};
+		$res .= "</pre><hr /><pre>$foot";
+	} elsif ($total == 0) {
+		$code = 404;
 		$res .= "\n\n[No results found]</pre><hr /><pre>$foot";
 	} else {
 		$q = $query->as_href;
@@ -74,7 +88,7 @@ sub sres_top_html {
 	}
 
 	$res .= "</pre></body></html>";
-	[200, ['Content-Type'=>'text/html; charset=UTF-8'], [$res]];
+	[$code, ['Content-Type'=>'text/html; charset=UTF-8'], [$res]];
 }
 
 sub dump_mset {
-- 
EW

[PATCH 4/6] search: note why we do not support FLAG_PURE_NOT

[Eric Wong]

Perhaps this can be optionally enabled in the future for smaller
sites.
---
 lib/PublicInbox/Search.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index 1874ee9..b6e71da 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -28,6 +28,9 @@ use constant {
 	# 8 - remove redundant/unneeded document data
 	# 9 - disable Message-ID compression
 	SCHEMA_VERSION => 9,
+
+	# n.b. FLAG_PURE_NOT is expensive not suitable for a public website
+	# as it could become a denial-of-service vector
 	QP_FLAGS => FLAG_PHRASE|FLAG_BOOLEAN|FLAG_LOVEHATE|FLAG_WILDCARD,
 };
 
-- 
EW

[PATCH 5/6] search: tweak parsing for internal queries

[Eric Wong]

We should not need to use QueryParser for internal queries,
but rather for external ones.

We'll also be exposing searching Message-IDs with the "mid:" prefix
for broken mids on some servers, and enabling partial searching
with 'm' to help with URL truncations.

Since thread IDs may be volatile, they cannot be exposed to the
public, there's no reason to expose them to the query parser,
either.

Also, add 's:' as an alternative probabilistic prefix to 'subject'
as it is shorter.
---
 lib/PublicInbox/Search.pm | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index b6e71da..2065055 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -37,16 +37,18 @@ use constant {
 # setup prefixes
 my %bool_pfx_internal = (
 	type => 'T', # "mail" or "ghost"
-	mid => 'Q', # uniQue id (Message-ID)
+	thread => 'G', # newsGroup (or similar entity - e.g. a web forum name)
 );
 
 my %bool_pfx_external = (
 	path => 'XPATH',
-	thread => 'G', # newsGroup (or similar entity - e.g. a web forum name)
+	mid => 'Q', # uniQue id (Message-ID)
 );
 
 my %prob_prefix = (
 	subject => 'S',
+	s => 'S', # for mairix compatibility
+	m => 'Q', # 'mid' is exact, 'm' can do partial
 );
 
 my %all_pfx = (%bool_pfx_internal, %bool_pfx_external, %prob_prefix);
@@ -91,8 +93,8 @@ sub query {
 
 sub get_subject_path {
 	my ($self, $path, $opts) = @_;
-	my $query = $self->qp->parse_query("path:".mid_compress($path), 0);
-	$self->do_enquire($query, $opts);
+	my $q = Search::Xapian::Query->new(xpfx("path").mid_compress($path));
+	$self->do_enquire($q, $opts);
 }
 
 sub get_thread {
@@ -100,9 +102,9 @@ sub get_thread {
 	my $smsg = eval { $self->lookup_message($mid) };
 
 	return { total => 0, msgs => [] } unless $smsg;
-	my $qp = $self->qp;
-	my $qtid = $qp->parse_query('thread:'.$smsg->thread_id, 0);
-	my $qsub = $qp->parse_query('path:'.mid_compress($smsg->path), 0);
+	my $qtid = Search::Xapian::Query->new(xpfx('thread').$smsg->thread_id);
+	my $path = mid_compress($smsg->path);
+	my $qsub = Search::Xapian::Query->new(xpfx('path').$path);
 	my $query = Search::Xapian::Query->new(OP_OR, $qtid, $qsub);
 	$self->do_enquire($query, $opts);
 }
-- 
EW

[PATCH 6/6] extmsg: fall back to partial Message-ID matching

[Eric Wong]

In case a URL gets truncated (as is common with long URLs),
we can rely on Xapian for partial matches and bring the user
to their destination.
---
 lib/PublicInbox/ExtMsg.pm | 45 +++++++++++++++++++++++++++++++++++++++------
 lib/PublicInbox/Search.pm |  9 +++++++++
 lib/PublicInbox/WWW.pm    |  1 +
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 7cf696d..243b6ba 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -23,7 +23,7 @@ sub ext_msg {
 
 	eval { require PublicInbox::Search };
 	my $have_xap = $@ ? 0 : 1;
-	my @nox;
+	my (@nox, @pfx);
 
 	foreach my $k (keys %$pi_config) {
 		$k =~ /\Apublicinbox\.([A-Z0-9a-z-]+)\.url\z/ or next;
@@ -40,8 +40,9 @@ sub ext_msg {
 
 		# try to find the URL with Xapian to avoid forking
 		if ($have_xap) {
+			my $s;
 			my $doc_id = eval {
-				my $s = PublicInbox::Search->new($git_dir);
+				$s = PublicInbox::Search->new($git_dir);
 				$s->find_unique_doc_id('mid', $mid);
 			};
 			if ($@) {
@@ -53,6 +54,7 @@ sub ext_msg {
 				# no point in trying the fork fallback if we
 				# know Xapian is up-to-date but missing the
 				# message in the current repo
+				push @pfx, { srch => $s, url => $url };
 				next;
 			}
 		}
@@ -82,19 +84,50 @@ sub ext_msg {
 		}
 	}
 
+	# fall back to partial MID matching
+	my $n_partial = 0;
+	my @partial;
+	if ($have_xap) {
+		my $cgi = $ctx->{cgi};
+		my $url = ref($cgi) eq 'CGI' ? $cgi->url(-base) . '/'
+					: $cgi->base->as_string;
+		$url .= $listname;
+		unshift @pfx, { srch => $ctx->{srch}, url => $url };
+		foreach my $pfx (@pfx) {
+			my $srch = delete $pfx->{srch} or next;
+			if (my $res = $srch->mid_prefix($mid)) {
+				$n_partial += scalar(@$res);
+				$pfx->{res} = $res;
+				push @partial, $pfx;
+			}
+		}
+	}
 	my $code = 404;
 	my $h = PublicInbox::Hval->new_msgid($mid, 1);
 	my $href = $h->as_href;
 	my $html = $h->as_html;
 	my $title = "Message-ID <$html> not found";
-
-	# Fall back to external repos if configured
 	my $s = "<html><head><title>$title</title>" .
-		"</head><body><pre><b>$title</b>";
+		"</head><body><pre><b>$title</b>\n";
 
+	if ($n_partial) {
+		$code = 300;
+		$s.= "\nPartial matches found:\n\n";
+		foreach my $pfx (@partial) {
+			my $u = $pfx->{url};
+			foreach my $m (@{$pfx->{res}}) {
+				$h = PublicInbox::Hval->new($m);
+				$href = $h->as_href;
+				$html = $h->as_html;
+				$s .= qq{<a\nhref="$u/$href/">$u/$html/</a>\n};
+			}
+		}
+	}
+
+	# Fall back to external repos if configured
 	if (@EXT_URL) {
 		$code = 300;
-		$s .= "\n\nPerhaps try an external site:\n\n";
+		$s .= "\nPerhaps try an external site:\n\n";
 		foreach my $u (@EXT_URL) {
 			my $r = sprintf($u, $href);
 			my $t = sprintf($u, $html);
diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index 2065055..e7ea96c 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -269,4 +269,13 @@ sub enquire {
 	$self->{enquire} ||= Search::Xapian::Enquire->new($self->{xdb});
 }
 
+sub mid_prefix {
+	my ($self, $mpfx) = @_;
+	my $query = eval { $self->qp->parse_query("m:$mpfx", FLAG_PARTIAL) };
+	return if $@;
+	my $res = $self->do_enquire($query, { relevance => 1 });
+	return unless $res->{total};
+	[ map { $_->mid } @{$res->{msgs}} ];
+}
+
 1;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 2718854..8f15506 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -79,6 +79,7 @@ sub r404 {
 	my ($ctx) = @_;
 	if ($ctx && $ctx->{mid}) {
 		require PublicInbox::ExtMsg;
+		searcher($ctx);
 		return PublicInbox::ExtMsg::ext_msg($ctx);
 	}
 	r(404, 'Not Found');
-- 
EW

Re: [PATCH 6/6] extmsg: fall back to partial Message-ID matching

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> In case a URL gets truncated (as is common with long URLs),
> we can rely on Xapian for partial matches and bring the user
> to their destination.

Note: this is a bit half-assed and does not work when Message-IDs
is broken into multiple terms (common).

Perhaps a prefix trie is necessary; but a good on-disk one does
not seem to readily exist in Debian (or anywhere) for Perl?
Oh well, this is a rare feature.

diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 243b6ba..77537c2 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -95,6 +95,8 @@ sub ext_msg {
 		unshift @pfx, { srch => $ctx->{srch}, url => $url };
 		foreach my $pfx (@pfx) {
 			my $srch = delete $pfx->{srch} or next;
+
+			# FIXME we may need a proper prefix trie here...
 			if (my $res = $srch->mid_prefix($mid)) {
 				$n_partial += scalar(@$res);
 				$pfx->{res} = $res;

[PATCH] extmsg: add note about the deficiency of the implementation

[Eric Wong]

ref: http://public-inbox.org/meta/20150905091457.GA27857@dcvr.yhbt.net/
---
 lib/PublicInbox/ExtMsg.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index 243b6ba..77537c2 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -95,6 +95,8 @@ sub ext_msg {
 		unshift @pfx, { srch => $ctx->{srch}, url => $url };
 		foreach my $pfx (@pfx) {
 			my $srch = delete $pfx->{srch} or next;
+
+			# FIXME we may need a proper prefix trie here...
 			if (my $res = $srch->mid_prefix($mid)) {
 				$n_partial += scalar(@$res);
 				$pfx->{res} = $res;
-- 
EW