[PATCH 0/5] a few more minor fixups

[PATCH 0/5] a few more minor fixups

[Eric Wong]

Nothing earth shattering, serving $INBOX_URL/description
properly for v2 inboxes should make it easier to mirror
while preserving descriptions (only per-epoch descriptions
are propagated in manifest.js.gz).

Eric Wong (5):
  www: stricter regexp for 405 errors
  www: serve $INBOX_DIR/description as $INBOX_URL/description
  inbox: simplify ->description and ->cloneurl
  inbox: remove TODO item for msg_by_path
  over: simplify read-only vs read-write checking

 lib/PublicInbox/Inbox.pm   | 26 ++++++++++----------------
 lib/PublicInbox/Over.pm    |  7 +++----
 lib/PublicInbox/OverIdx.pm |  2 +-
 lib/PublicInbox/WWW.pm     | 13 ++++++++++++-
 t/httpd.t                  |  5 +++++
 t/over.t                   |  7 ++++++-
 t/plack.t                  |  2 ++
 t/psgi_v2.t                |  3 +++
 8 files changed, 42 insertions(+), 23 deletions(-)

[PATCH 1/5] www: stricter regexp for 405 errors

[Eric Wong]

We want to match "GET" and "HEAD" exactly, not requests which
start with "GET" or end with "HEAD".  This doesn't seem like
a real problem for public-inboxes which are actually public
data anyways.
---
 lib/PublicInbox/WWW.pm | 2 +-
 t/httpd.t              | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index efe7c8ca..3ce7cc2a 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -70,7 +70,7 @@ sub call {
 			return invalid_inbox($ctx, $1) || mbox_results($ctx);
 		}
 	}
-	elsif ($method !~ /\AGET|HEAD\z/) {
+	elsif ($method !~ /\A(?:GET|HEAD)\z/) {
 		return r(405);
 	}
 
diff --git a/t/httpd.t b/t/httpd.t
index 2972afb2..c9756a70 100644
--- a/t/httpd.t
+++ b/t/httpd.t
@@ -49,6 +49,11 @@ EOF
 	$td = start_script($cmd, undef, { 3 => $sock });
 	my $host = $sock->sockhost;
 	my $port = $sock->sockport;
+	{
+		my $bad = tcp_connect($sock);
+		print $bad "GETT / HTTP/1.0\r\n\r\n" or die;
+		like(<$bad>, qr!\AHTTP/1\.[01] 405\b!, 'got 405 on bad req');
+	}
 	my $conn = tcp_connect($sock);
 	ok($conn, 'connected');
 	ok($conn->write("GET / HTTP/1.0\r\n\r\n"), 'wrote data to socket');

[PATCH 2/5] www: serve $INBOX_DIR/description as $INBOX_URL/description

[Eric Wong]

Instead of serving $INBOX_DIR/all.git/description, since
$INBOX_DIR/all.git/description is not described in the
default message when it's missing.
---
 lib/PublicInbox/WWW.pm | 11 +++++++++++
 t/plack.t              |  2 ++
 t/psgi_v2.t            |  3 +++
 3 files changed, 16 insertions(+)

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 3ce7cc2a..02b5ded3 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -85,6 +85,8 @@ sub call {
 		invalid_inbox($ctx, $1) || get_atom($ctx);
 	} elsif ($path_info =~ m!$INBOX_RE/new\.html\z!o) {
 		invalid_inbox($ctx, $1) || get_new($ctx);
+	} elsif ($path_info =~ m!$INBOX_RE/description\z!o) {
+		get_description($ctx, $1);
 	} elsif ($path_info =~ m!$INBOX_RE/(?:(?:git/)?([0-9]+)(?:\.git)?/)?
 				($PublicInbox::GitHTTPBackend::ANY)\z!ox) {
 		my ($epoch, $path) = ($2, $3);
@@ -621,4 +623,13 @@ sub get_css ($$$) {
 	[ 200, $h, [ $css ] ];
 }
 
+sub get_description {
+	my ($ctx, $inbox) = @_;
+	invalid_inbox($ctx, $inbox) || do {
+		my $d = $ctx->{-inbox}->description . "\n";
+		[ 200, [ 'Content-Length', bytes::length($d),
+			'Content-Type', 'text/plain' ], [ $d ] ];
+	};
+}
+
 1;
diff --git a/t/plack.t b/t/plack.t
index a9a053ed..e23658fd 100644
--- a/t/plack.t
+++ b/t/plack.t
@@ -189,6 +189,8 @@ test_psgi($app, sub {
 	like($body, qr/<title>test for public-inbox/,
 		"set title in XML feed");
 	like($body, qr/zzzzzz/, 'body included');
+	$res = $cb->(GET($pfx . '/description'));
+	like($res->content, qr/test for public-inbox/, 'got description');
 });
 
 test_psgi($app, sub {
diff --git a/t/psgi_v2.t b/t/psgi_v2.t
index 2ecd7458..fdd6969f 100644
--- a/t/psgi_v2.t
+++ b/t/psgi_v2.t
@@ -58,6 +58,9 @@ my $www = PublicInbox::WWW->new($config);
 my ($res, $raw, @from_);
 test_psgi(sub { $www->call(@_) }, sub {
 	my ($cb) = @_;
+	$res = $cb->(GET('/v2test/description'));
+	like($res->content, qr!\$INBOX_DIR/description missing!,
+		'got v2 description missing message');
 	$res = $cb->(GET('/v2test/a-mid@b/raw'));
 	$raw = $res->content;
 	like($raw, qr/^hello world$/m, 'got first message');

[PATCH 3/5] inbox: simplify ->description and ->cloneurl

[Eric Wong]

We can use "//=" from Perl 5.10 to simplify the logic for these
methods.  The use of chomp() in ->cloneurl was also unnecessary
since split(/\s+/s,...) already removes newlines.
---
 lib/PublicInbox/Inbox.pm | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index b76d4e5a..accfd974 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -219,25 +219,19 @@ sub try_cat {
 
 sub description {
 	my ($self) = @_;
-	my $desc = $self->{description};
-	return $desc if defined $desc;
-	$desc = try_cat("$self->{inboxdir}/description");
-	local $/ = "\n";
-	chomp $desc;
-	$desc =~ s/\s+/ /smg;
-	$desc = '($INBOX_DIR/description missing)' if $desc eq '';
-	$self->{description} = $desc;
+	$self->{description} //= do {
+		my $desc = try_cat("$self->{inboxdir}/description");
+		local $/ = "\n";
+		chomp $desc;
+		$desc =~ s/\s+/ /smg;
+		$desc eq '' ? '($INBOX_DIR/description missing)' : $desc;
+	};
 }
 
 sub cloneurl {
 	my ($self) = @_;
-	my $url = $self->{cloneurl};
-	return $url if $url;
-	$url = try_cat("$self->{inboxdir}/cloneurl");
-	my @url = split(/\s+/s, $url);
-	local $/ = "\n";
-	chomp @url;
-	$self->{cloneurl} = \@url;
+	$self->{cloneurl} //=
+		[ split(/\s+/s, try_cat("$self->{inboxdir}/cloneurl")) ];
 }
 
 sub base_url {

[PATCH 4/5] inbox: remove TODO item for msg_by_path

[Eric Wong]

It's an old function which only gets called by inboxes w/o
SQLite indices.
---
 lib/PublicInbox/Inbox.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index accfd974..a5849a61 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -305,9 +305,9 @@ sub nntp_usable {
 	$ret;
 }
 
+# for v1 users w/o SQLite only
 sub msg_by_path ($$;$) {
 	my ($self, $path, $ref) = @_;
-	# TODO: allow other refs:
 	my $str = git($self)->cat_file('HEAD:'.$path, $ref);
 	$$str =~ s/\A[\r\n]*From [^\r\n]*\r?\n//s if $str;
 	$str;

[PATCH 5/5] over: simplify read-only vs read-write checking

[Eric Wong]

No need to call ref() and do a string comparison.  Add some
extra tests using the {ReadOnly} attribute in DBI.pm.
---
 lib/PublicInbox/Over.pm    | 7 +++----
 lib/PublicInbox/OverIdx.pm | 2 +-
 t/over.t                   | 7 ++++++-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm
index 0f8f433a..57c82bfc 100644
--- a/lib/PublicInbox/Over.pm
+++ b/lib/PublicInbox/Over.pm
@@ -14,17 +14,16 @@ use Compress::Zlib qw(uncompress);
 use constant DEFAULT_LIMIT => 1000;
 
 sub dbh_new {
-	my ($self) = @_;
-	my $ro = ref($self) eq 'PublicInbox::Over';
+	my ($self, $rw) = @_;
 	my $f = $self->{filename};
-	if (!$ro && !-f $f) { # SQLite defaults mode to 0644, we want 0666
+	if ($rw && !-f $f) { # SQLite defaults mode to 0644, we want 0666
 		open my $fh, '+>>', $f or die "failed to open $f: $!";
 	}
 	my $dbh = DBI->connect("dbi:SQLite:dbname=$f",'','', {
 		AutoCommit => 1,
 		RaiseError => 1,
 		PrintError => 0,
-		ReadOnly => $ro,
+		ReadOnly => !$rw,
 		sqlite_use_immediate_transaction => 1,
 	});
 	$dbh->{sqlite_unicode} = 1;
diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm
index 5f1007aa..a966a710 100644
--- a/lib/PublicInbox/OverIdx.pm
+++ b/lib/PublicInbox/OverIdx.pm
@@ -20,7 +20,7 @@ use PublicInbox::Search;
 
 sub dbh_new {
 	my ($self) = @_;
-	my $dbh = $self->SUPER::dbh_new;
+	my $dbh = $self->SUPER::dbh_new(1);
 	$dbh->do('PRAGMA journal_mode = TRUNCATE');
 	$dbh->do('PRAGMA cache_size = 80000');
 	create_tables($dbh);
diff --git a/t/over.t b/t/over.t
index 4e630bcd..daa7176f 100644
--- a/t/over.t
+++ b/t/over.t
@@ -18,10 +18,15 @@ is($y, $x+1, 'tid increases');
 $x = $over->sid('hello-world');
 is(int($x), $x, 'integer sid');
 $y = $over->sid('hello-WORLD');
-is($y, $x+1, 'sid ncreases');
+is($y, $x+1, 'sid increases');
 is($over->sid('hello-world'), $x, 'idempotent');
+ok(!$over->{dbh}->{ReadOnly}, 'OverIdx is not ReadOnly');
 $over->disconnect;
 
+$over = PublicInbox::Over->new("$tmpdir/over.sqlite3");
+$over->connect;
+ok($over->{dbh}->{ReadOnly}, 'Over is ReadOnly');
+
 $over = PublicInbox::OverIdx->new("$tmpdir/over.sqlite3");
 $over->connect;
 is($over->sid('hello-world'), $x, 'idempotent across reopen');