[PATCH 7/7] nntp: simplify the long_response API

["Eric Wong (Contractor, The Linux Foundation)" <e@80x24.org>]

We we worked around the default range/termination conditions of
long_response in many cases to reduce calls to SQLite or Xapian.
So continue that trend and become more like the PSGI API
which doesn't force callers to specify an article range or
work inside a loop.
---
 lib/PublicInbox/Msgmap.pm | 12 +++++++
 lib/PublicInbox/NNTP.pm   | 83 ++++++++++++++++++++++-------------------------
 t/v2writable.t            | 10 ++++++
 3 files changed, 61 insertions(+), 44 deletions(-)

diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm
index 26565d4..c6a7315 100644
--- a/lib/PublicInbox/Msgmap.pm
+++ b/lib/PublicInbox/Msgmap.pm
@@ -196,6 +196,18 @@ ORDER BY num ASC LIMIT 1000
 	$ids;
 }
 
+sub msg_range {
+	my ($self, $beg, $end) = @_;
+	my $dbh = $self->{dbh};
+	my $attr = { Columns => [] };
+	my $mids = $dbh->selectall_arrayref(<<'', $attr, $$beg, $end);
+SELECT num,mid FROM msgmap WHERE num >= ? AND num <= ?
+ORDER BY num ASC
+
+	$$beg = $mids->[-1]->[0] + 1 if @$mids;
+	$mids
+}
+
 # only used for mapping external serial numbers (e.g. articles from gmane)
 # see scripts/xhdr-num2mid or PublicInbox::Filter::RubyLang for usage
 sub mid_set {
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index b91cda1..e517935 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -225,11 +225,11 @@ sub cmd_listgroup ($;$) {
 
 	$self->{ng} or return '412 no newsgroup selected';
 	my $n = 0;
-	long_response($self, 0, long_response_limit, sub {
-		my ($i) = @_;
+	long_response($self, sub {
 		my $ary = $self->{ng}->mm->ids_after(\$n);
-		scalar @$ary or return ($$i = long_response_limit);
+		scalar @$ary or return;
 		more($self, join("\r\n", @$ary));
+		1;
 	});
 }
 
@@ -328,8 +328,7 @@ sub cmd_newnews ($$$$;$$) {
 	return '.' unless @srch;
 
 	my $prev = 0;
-	long_response($self, 0, long_response_limit, sub {
-		my ($i) = @_;
+	long_response($self, sub {
 		my $srch = $srch[0];
 		my $msgs = $srch->query_ts($ts, $prev);
 		if (scalar @$msgs) {
@@ -341,8 +340,9 @@ sub cmd_newnews ($$$$;$$) {
 			shift @srch;
 			if (@srch) { # continue onto next newsgroup
 				$prev = 0;
+				return 1;
 			} else { # break out of the long response.
-				$$i = long_response_limit;
+				return;
 			}
 		}
 	});
@@ -564,8 +564,8 @@ sub get_range ($$) {
 	[ $beg, $end ];
 }
 
-sub long_response ($$$$) {
-	my ($self, $beg, $end, $cb) = @_;
+sub long_response ($$) {
+	my ($self, $cb) = @_;
 	die "BUG: nested long response" if $self->{long_res};
 
 	my $fd = $self->{fd};
@@ -576,23 +576,14 @@ sub long_response ($$$$) {
 	$self->watch_read(0);
 	my $t0 = now();
 	$self->{long_res} = sub {
-		# limit our own running time for fairness with other
-		# clients and to avoid buffering too much:
-		my $lim = $end == long_response_limit ? 1 : 100;
-
-		my $err;
-		do {
-			eval { $cb->(\$beg) };
-		} until (($err = $@) || $self->{closed} ||
-			 ++$beg > $end || !--$lim || $self->{write_buf_size});
-
-		if ($err || $self->{closed}) {
+		my $more = eval { $cb->() };
+		if ($@ || $self->{closed}) {
 			$self->{long_res} = undef;
 
-			if ($err) {
+			if ($@) {
 				err($self,
 				    "%s during long response[$fd] - %0.6f",
-				    $err, now() - $t0);
+				    $@, now() - $t0);
 			}
 			if ($self->{closed}) {
 				out($self, " deferred[$fd] aborted - %0.6f",
@@ -601,7 +592,7 @@ sub long_response ($$$$) {
 				update_idle_time($self);
 				$self->watch_read(1);
 			}
-		} elsif (!$lim || $self->{write_buf_size}) {
+		} elsif ($more) { # $self->{write_buf_size}:
 			# no recursion, schedule another call ASAP
 			# but only after all pending writes are done
 			update_idle_time($self);
@@ -633,10 +624,13 @@ sub hdr_message_id ($$$) { # optimize XHDR Message-ID [range] for slrnpull.
 		my $mm = $self->{ng}->mm;
 		my ($beg, $end) = @$r;
 		more($self, $xhdr ? r221 : r225);
-		long_response($self, $beg, $end, sub {
-			my ($i) = @_;
-			my $mid = $mm->mid_for($$i);
-			more($self, "$$i <$mid>") if defined $mid;
+		long_response($self, sub {
+			my $r = $mm->msg_range(\$beg, $end);
+			@$r or return;
+			more($self, join("\r\n", map {
+				"$_->[0] <$_->[1]>"
+			} @$r));
+			1;
 		});
 	}
 }
@@ -676,10 +670,16 @@ sub hdr_xref ($$$) { # optimize XHDR Xref [range] for rtin
 		my $mm = $ng->mm;
 		my ($beg, $end) = @$r;
 		more($self, $xhdr ? r221 : r225);
-		long_response($self, $beg, $end, sub {
-			my ($i) = @_;
-			my $mid = $mm->mid_for($$i);
-			more($self, "$$i ".xref($ng, $$i)) if defined $mid;
+		long_response($self, sub {
+			my $r = $mm->msg_range(\$beg, $end);
+			@$r or return;
+			more($self, join("\r\n", map {
+				# TODO: use $_->[1] (mid) to fill
+				# Xref: from other inboxes
+				my $num = $_->[0];
+				"$num ".xref($ng, $num);
+			} @$r));
+			1;
 		});
 	}
 }
@@ -707,11 +707,9 @@ sub hdr_searchmsg ($$$$) {
 		my ($beg, $end) = @$r;
 		more($self, $xhdr ? r221 : r225);
 		my $cur = $beg;
-		long_response($self, 0, long_response_limit, sub {
-			my ($i) = @_;
+		long_response($self, sub {
 			my $msgs = $srch->query_xover($cur, $end);
-			my $nr = scalar @$msgs or
-					return ($$i = long_response_limit);
+			my $nr = scalar @$msgs or return;
 			my $tmp = '';
 			foreach my $s (@$msgs) {
 				$tmp .= $s->num . ' ' . $s->$field . "\r\n";
@@ -792,12 +790,11 @@ sub cmd_xrover ($;$) {
 	my $mm = $ng->mm;
 	my $srch = $ng->search;
 	more($self, '224 Overview information follows');
-	long_response($self, $beg, $end, sub {
-		my ($i) = @_;
-		my $num = $$i;
-		my $h = search_header_for($srch, $num, 'references');
-		defined $h or return;
-		more($self, "$num $h");
+
+	long_response($self, sub {
+		my $h = search_header_for($srch, $beg, 'references');
+		more($self, "$beg $h") if defined($h);
+		$beg++ < $end;
 	});
 }
 
@@ -844,17 +841,15 @@ sub cmd_xover ($;$) {
 	more($self, "224 Overview information follows for $beg to $end");
 	my $srch = $self->{ng}->search;
 	my $cur = $beg;
-	long_response($self, 0, long_response_limit, sub {
-		my ($i) = @_;
+	long_response($self, sub {
 		my $msgs = $srch->query_xover($cur, $end);
-		my $nr = scalar @$msgs or return ($$i = long_response_limit);
+		my $nr = scalar @$msgs or return;
 
 		# OVERVIEW.FMT
 		more($self, join("\r\n", map {
 			over_line($_->{num}, $_);
 			} @$msgs));
 		$cur = $msgs->[-1]->{num} + 1;
-		1;
 	});
 }
 
diff --git a/t/v2writable.t b/t/v2writable.t
index 6294735..2f83977 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -105,6 +105,7 @@ if ('ensure git configs are correct') {
 
 {
 	$mime->header_set('Message-Id', '<abcde@1>', '<abcde@2>');
+	$mime->header_set('References', '<zz-mid@b>');
 	ok($im->add($mime), 'message with multiple Message-ID');
 	$im->done;
 	my @found;
@@ -196,6 +197,15 @@ EOF
 	}
 	is_deeply([sort keys %lg], [sort keys %$x],
 		'XOVER and LISTGROUPS return the same article numbers');
+
+	my $xref = $n->xhdr('Xref', '1-');
+	is_deeply([sort keys %lg], [sort keys %$xref], 'Xref range OK');
+
+	my $mids = $n->xhdr('Message-ID', '1-');
+	is_deeply([sort keys %lg], [sort keys %$xref], 'Message-ID range OK');
+
+	my $rover = $n->xrover('1-');
+	is_deeply([sort keys %lg], [sort keys %$rover], 'XROVER range OK');
 };
 {
 	local $ENV{NPROC} = 2;
-- 
EW