user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* [PATCH 0/3] inbox objectification cleanups + fixes
@ 2016-05-16  7:56 Eric Wong
  2016-05-16  7:56 ` [PATCH 1/3] declare Inbox object for reusability Eric Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eric Wong @ 2016-05-16  7:56 UTC (permalink / raw)
  To: meta

This series is mainly paving the way for future cleanups
and marking some old code paths as deprecated. There's
also a legitimate bugfix for redirecting under "mount"
directives in Plack::Builder.

Eric Wong (3):
      declare Inbox object for reusability
      config: allow taking an existing reference
      www: fix for running under mount paths

 lib/PublicInbox/Config.pm | 54 +++++++++++++++++++-------
 lib/PublicInbox/Feed.pm   | 36 +++++++++---------
 lib/PublicInbox/Inbox.pm  | 96 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/PublicInbox/WWW.pm    | 57 ++++++++++++----------------
 script/public-inbox-init  |  4 +-
 script/public-inbox-mda   |  4 +-
 t/config.t                |  6 ++-
 t/inbox.t                 | 12 ++++++
 t/psgi_mount.t            | 78 ++++++++++++++++++++++++++++++++++++++
 9 files changed, 277 insertions(+), 70 deletions(-)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] declare Inbox object for reusability
  2016-05-16  7:56 [PATCH 0/3] inbox objectification cleanups + fixes Eric Wong
@ 2016-05-16  7:56 ` Eric Wong
  2016-05-16  7:56 ` [PATCH 2/3] config: allow taking an existing reference Eric Wong
  2016-05-16  7:56 ` [PATCH 3/3] www: fix for running under mount paths Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2016-05-16  7:56 UTC (permalink / raw)
  To: meta

From the beginning, we've avoided objects here in favor
of faster startup time; but it may not be worth it
since a persistent httpd/nntpd is faster and -mda
isn't hit as often.
---
 lib/PublicInbox/Config.pm | 51 +++++++++++++++++++++++--------
 lib/PublicInbox/Inbox.pm  | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/PublicInbox/WWW.pm    | 42 +++++++++-----------------
 script/public-inbox-init  |  4 +--
 script/public-inbox-mda   |  4 +--
 t/config.t                |  6 ++--
 6 files changed, 136 insertions(+), 47 deletions(-)
 create mode 100644 lib/PublicInbox/Inbox.pm

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 331d25c..3f3707e 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -7,6 +7,7 @@ use strict;
 use warnings;
 use base qw/Exporter/;
 our @EXPORT_OK = qw/try_cat/;
+require PublicInbox::Inbox;
 use File::Path::Expand qw/expand_filename/;
 
 # returns key-value pairs of config directives in a hash
@@ -14,12 +15,18 @@ use File::Path::Expand qw/expand_filename/;
 sub new {
 	my ($class, $file) = @_;
 	$file = default_file() unless defined($file);
-	bless git_config_dump($file), $class;
+	my $self = bless git_config_dump($file), $class;
+	$self->{-by_addr} = {};
+	$self->{-by_name} = {};
+	$self;
 }
 
 sub lookup {
 	my ($self, $recipient) = @_;
 	my $addr = lc($recipient);
+	my $inbox = $self->{-by_addr}->{$addr};
+	return $inbox if $inbox;
+
 	my $pfx;
 
 	foreach my $k (keys %$self) {
@@ -37,20 +44,15 @@ sub lookup {
 			last;
 		}
 	}
-
 	defined $pfx or return;
+	_fill($self, $pfx);
+}
 
-	my %rv;
-	foreach my $k (qw(mainrepo address filter)) {
-		my $v = $self->{"$pfx.$k"};
-		$rv{$k} = $v if defined $v;
-	}
-	my $inbox = $pfx;
-	$inbox =~ s/\Apublicinbox\.//;
-	$rv{inbox} = $inbox;
-	my $v = $rv{address};
-	$rv{-primary_address} = ref($v) eq 'ARRAY' ? $v->[0] : $v;
-	\%rv;
+sub lookup_name {
+	my ($self, $name) = @_;
+	my $rv = $self->{-by_name}->{$name};
+	return $rv if $rv;
+	$self->{-by_name}->{$name} = _fill($self, "publicinbox.$name");
 }
 
 sub get {
@@ -105,4 +107,27 @@ sub try_cat {
 	$rv;
 }
 
+sub _fill {
+	my ($self, $pfx) = @_;
+	my $rv = {};
+
+	foreach my $k (qw(mainrepo address filter url)) {
+		my $v = $self->{"$pfx.$k"};
+		$rv->{$k} = $v if defined $v;
+	}
+	my $inbox = $pfx;
+	$inbox =~ s/\Apublicinbox\.//;
+	$rv->{name} = $inbox;
+	my $v = $rv->{address} ||= 'public-inbox@example.com';
+	$rv->{-primary_address} = ref($v) eq 'ARRAY' ? $v->[0] : $v;
+	$rv = PublicInbox::Inbox->new($rv);
+	if (ref($v) eq 'ARRAY') {
+		$self->{-by_addr}->{lc($_)} = $rv foreach @$v;
+	} else {
+		$self->{-by_addr}->{lc($v)} = $rv;
+	}
+	$rv;
+}
+
+
 1;
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
new file mode 100644
index 0000000..5d9fdb3
--- /dev/null
+++ b/lib/PublicInbox/Inbox.pm
@@ -0,0 +1,76 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+#
+# Represents a public-inbox (which may have multiple mailing addresses)
+package PublicInbox::Inbox;
+use strict;
+use warnings;
+use Scalar::Util qw(weaken);
+use PublicInbox::Git;
+
+sub new {
+	my ($class, $opts) = @_;
+	bless $opts, $class;
+}
+
+sub weaken_all {
+	my ($self) = @_;
+	weaken($self->{$_}) foreach qw(git mm search);
+}
+
+sub git {
+	my ($self) = @_;
+	$self->{git} ||= eval { PublicInbox::Git->new($self->{mainrepo}) };
+}
+
+sub mm {
+	my ($self) = @_;
+	$self->{mm} ||= eval { PublicInbox::Msgmap->new($self->{mainrepo}) };
+}
+
+sub search {
+	my ($self) = @_;
+	$self->{search} ||= eval { PublicInbox::Search->new($self->{mainrepo}) };
+}
+
+sub try_cat {
+	my ($path) = @_;
+	my $rv = '';
+	if (open(my $fh, '<', $path)) {
+		local $/;
+		$rv = <$fh>;
+	}
+	$rv;
+}
+
+sub description {
+	my ($self) = @_;
+	my $desc = $self->{description};
+	return $desc if defined $desc;
+	$desc = try_cat("$self->{mainrepo}/description");
+	chomp $desc;
+	$desc =~ s/\s+/ /smg;
+	$desc = '($GIT_DIR/description missing)' if $desc eq '';
+	$self->{description} = $desc;
+}
+
+sub cloneurl {
+	my ($self) = @_;
+	my $url = $self->{cloneurl};
+	return $url if $url;
+	$url = try_cat("$self->{mainrepo}/cloneurl");
+	my @url = split(/\s+/s, $url);
+	chomp @url;
+	$self->{cloneurl} = \@url;
+}
+
+sub footer_html {
+	my ($self) = @_;
+	my $footer = $self->{footer};
+	return $footer if defined $footer;
+	$footer = try_cat("$self->{mainrepo}/public-inbox/footer.html");
+	chomp $footer;
+	$self->{footer} = $footer;
+}
+
+1;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 51dc3da..94ab032 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -125,10 +125,11 @@ sub r { [ $_[0], ['Content-Type' => 'text/plain'], [ join(' ', @_, "\n") ] ] }
 # returns undef if valid, array ref response if invalid
 sub invalid_inbox {
 	my ($self, $ctx, $inbox, $mid) = @_;
-	my $git_dir = $ctx->{pi_config}->get($inbox, "mainrepo");
-	if (defined $git_dir) {
-		$ctx->{git_dir} = $git_dir;
-		$ctx->{git} = PublicInbox::Git->new($git_dir);
+	my $obj = $ctx->{pi_config}->lookup_name($inbox);
+	if (defined $obj) {
+		$ctx->{git_dir} = $obj->{mainrepo};
+		$ctx->{git} = $obj->git;
+		$ctx->{-inbox} = $obj;
 		$ctx->{inbox} = $inbox;
 		return;
 	}
@@ -243,27 +244,18 @@ sub ctx_get {
 sub footer {
 	my ($ctx) = @_;
 	return '' unless $ctx;
-	my $git_dir = ctx_get($ctx, 'git_dir');
-
-	# favor user-supplied footer
-	my $footer = try_cat("$git_dir/public-inbox/footer.html");
-	if (defined $footer) {
-		chomp $footer;
-		$ctx->{footer} = $footer;
-		return $footer;
-	}
+	my $obj = $ctx->{-inbox} or return '';
+	my $footer = $obj->footer_html;
+	return $ctx->{footer} = $footer if $footer;
 
 	# auto-generate a footer
-	my $inbox = ctx_get($ctx, 'inbox');
-	my $desc = try_cat("$git_dir/description");
-	$desc = '$GIT_DIR/description missing' unless defined $desc;
-	chomp $desc;
+	chomp(my $desc = $obj->description);
 
-	my $urls = try_cat("$git_dir/cloneurl");
-	my @urls = split(/\r?\n/, $urls || '');
+	my $urls;
+	my @urls = @{$obj->cloneurl};
 	my %seen = map { $_ => 1 } @urls;
 	my $cgi = $ctx->{cgi};
-	my $http = $cgi->base->as_string . $inbox;
+	my $http = $cgi->base->as_string . $obj->{name};
 	$seen{$http} or unshift @urls, $http;
 	my $ssoma_url = PublicInbox::Hval::prurl($cgi->{env}, SSOMA_URL);
 	if (scalar(@urls) == 1) {
@@ -275,13 +267,7 @@ sub footer {
 			join("\n", map { "\tgit clone --mirror $_" } @urls);
 	}
 
-	my $addr = $ctx->{pi_config}->get($inbox, 'address');
-	if (ref($addr) eq 'ARRAY') {
-		$addr = $addr->[0]; # first address is primary
-	}
-
-	$addr = "<a\nhref=\"mailto:$addr\">$addr</a>";
-
+	my $addr = $obj->{-primary_address};
 	$ctx->{footer} = join("\n",
 		'- ' . $desc,
 		"A <a\nhref=\"" .
@@ -299,7 +285,7 @@ sub searcher {
 	my ($ctx) = @_;
 	eval {
 		require PublicInbox::Search;
-		$ctx->{srch} = PublicInbox::Search->new($ctx->{git_dir});
+		$ctx->{srch} = $ctx->{-inbox}->search;
 	};
 }
 
diff --git a/script/public-inbox-init b/script/public-inbox-init
index ca0f9dd..54d6ce4 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -43,10 +43,10 @@ if (-e $pi_config) {
 	foreach my $addr (@address) {
 		my $found = $cfg->lookup($addr);
 		if ($found) {
-			if ($found->{inbox} ne $name) {
+			if ($found->{name} ne $name) {
 				print STDERR
 					"`$addr' already defined for ",
-					"`$found->{inbox}',\n",
+					"`$found->{name}',\n",
 					"does not match intend `$name'\n";
 				$conflict = 1;
 			} else {
diff --git a/script/public-inbox-mda b/script/public-inbox-mda
index 8e224a5..b606c59 100755
--- a/script/public-inbox-mda
+++ b/script/public-inbox-mda
@@ -49,7 +49,7 @@ if (PublicInbox::MDA->precheck($filter, $dst->{address}) &&
 	} elsif ($fcfg eq 'scrub') {
 		$filter_arg = undef; # the default for legacy versions
 	} else {
-		warn "publicinbox.$dst->{inbox}.filter=$fcfg invalid\n";
+		warn "publicinbox.$dst->{name}.filter=$fcfg invalid\n";
 		warn "must be either 'scrub' or 'reject' (the default)\n";
 	}
 
@@ -65,7 +65,7 @@ if (PublicInbox::MDA->precheck($filter, $dst->{address}) &&
 			};
 			my $git = PublicInbox::Git->new($main_repo);
 			my $im = PublicInbox::Import->new($git,
-						$dst->{inbox}, $recipient);
+						$dst->{name}, $recipient);
 			if (defined $im->add($msg)) {
 				$im->done;
 				$filter->ignore; # exits
diff --git a/t/config.t b/t/config.t
index 7a63aa7..76f6065 100644
--- a/t/config.t
+++ b/t/config.t
@@ -28,8 +28,9 @@ my $tmpdir = tempdir('pi-config-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 	is_deeply($cfg->lookup('meta@public-inbox.org'), {
 		'mainrepo' => '/home/pi/meta-main.git',
 		'address' => 'meta@public-inbox.org',
+		'url' => 'http://example.com/meta',
 		-primary_address => 'meta@public-inbox.org',
-		'inbox' => 'meta',
+		'name' => 'meta',
 	}, "lookup matches expected output");
 
 	is($cfg->lookup('blah@example.com'), undef,
@@ -42,7 +43,8 @@ my $tmpdir = tempdir('pi-config-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 			      'test@public-inbox.org'],
 		-primary_address => 'try@public-inbox.org',
 		'mainrepo' => '/home/pi/test-main.git',
-		'inbox' => 'test',
+		'name' => 'test',
+		'url' => 'http://example.com/test',
 	}, "lookup matches expected output for test");
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] config: allow taking an existing reference
  2016-05-16  7:56 [PATCH 0/3] inbox objectification cleanups + fixes Eric Wong
  2016-05-16  7:56 ` [PATCH 1/3] declare Inbox object for reusability Eric Wong
@ 2016-05-16  7:56 ` Eric Wong
  2016-05-16  7:56 ` [PATCH 3/3] www: fix for running under mount paths Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2016-05-16  7:56 UTC (permalink / raw)
  To: meta

This should make creating test cases easier and faster.
---
 lib/PublicInbox/Config.pm | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 3f3707e..b5f0fcb 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -15,9 +15,12 @@ use File::Path::Expand qw/expand_filename/;
 sub new {
 	my ($class, $file) = @_;
 	$file = default_file() unless defined($file);
-	my $self = bless git_config_dump($file), $class;
-	$self->{-by_addr} = {};
-	$self->{-by_name} = {};
+	$file = ref $file ? $file : git_config_dump($file);
+	my $self = bless $file, $class;
+
+	# caches
+	$self->{-by_addr} ||= {};
+	$self->{-by_name} ||= {};
 	$self;
 }
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] www: fix for running under mount paths
  2016-05-16  7:56 [PATCH 0/3] inbox objectification cleanups + fixes Eric Wong
  2016-05-16  7:56 ` [PATCH 1/3] declare Inbox object for reusability Eric Wong
  2016-05-16  7:56 ` [PATCH 2/3] config: allow taking an existing reference Eric Wong
@ 2016-05-16  7:56 ` Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2016-05-16  7:56 UTC (permalink / raw)
  To: meta

We try to avoid issues like these by using relative URLs
in hrefs, but we can't avoid the problem with Location:
for redirects and Atom feeds which are likely to be
rehosted elsewhere.

We also reorder some of the code to work around a weird
issue on the psgi-plack mailing list:
<20160516073750.GA11931@dcvr.yhbt.net>
(Somewhere on https://groups.google.com/group/psgi-plack
 but it's probably not bookmarkable)
---
 lib/PublicInbox/Feed.pm  | 36 +++++++++++-----------
 lib/PublicInbox/Inbox.pm | 20 +++++++++++++
 lib/PublicInbox/WWW.pm   | 15 ++++++----
 t/inbox.t                | 12 ++++++++
 t/psgi_mount.t           | 78 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 138 insertions(+), 23 deletions(-)
 create mode 100644 t/inbox.t
 create mode 100644 t/psgi_mount.t

diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index 0d3bc81..52fe0db 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -61,9 +61,9 @@ sub atom_header {
 
 sub emit_atom {
 	my ($cb, $ctx) = @_;
+	my $feed_opts = get_feedopts($ctx);
 	my $fh = $cb->([ 200, ['Content-Type' => 'application/atom+xml']]);
 	my $max = $ctx->{max} || MAX_PER_PAGE;
-	my $feed_opts = get_feedopts($ctx);
 	my $x = atom_header($feed_opts);
 	my $git = $ctx->{git} ||= PublicInbox::Git->new($ctx->{git_dir});
 	each_recent_blob($ctx, sub {
@@ -95,8 +95,8 @@ sub emit_atom_thread {
 	my ($cb, $ctx) = @_;
 	my $res = $ctx->{srch}->get_thread($ctx->{mid});
 	return _no_thread($cb) unless $res->{total};
-	my $fh = $cb->([200, ['Content-Type' => 'application/atom+xml']]);
 	my $feed_opts = get_feedopts($ctx);
+	my $fh = $cb->([200, ['Content-Type' => 'application/atom+xml']]);
 
 	my $html_url = $feed_opts->{atomurl} = $ctx->{self_url};
 	$html_url =~ s!/t\.atom\z!/!;
@@ -112,10 +112,10 @@ sub emit_atom_thread {
 
 sub emit_html_index {
 	my ($res, $ctx) = @_;
+	my $feed_opts = get_feedopts($ctx);
 	my $fh = $res->([200,['Content-Type'=>'text/html; charset=UTF-8']]);
 
 	my $max = $ctx->{max} || MAX_PER_PAGE;
-	my $feed_opts = get_feedopts($ctx);
 
 	my $title = ascii_html($feed_opts->{description} || '');
 	my ($footer, $param, $last);
@@ -261,15 +261,15 @@ sub get_feedopts {
 	my ($ctx) = @_;
 	my $pi_config = $ctx->{pi_config};
 	my $inbox = $ctx->{inbox};
+	my $obj = $ctx->{-inbox};
 	my $cgi = $ctx->{cgi};
-	my %rv;
-	if (open my $fh, '<', "$ctx->{git_dir}/description") {
-		chomp($rv{description} = <$fh>);
-	} else {
-		$rv{description} = '($GIT_DIR/description missing)';
-	}
+	my %rv = ( description => $obj ? $obj->description : 'FIXME' );
 
-	if ($pi_config && defined $inbox && $inbox ne '') {
+	if ($obj) {
+		$rv{address} = $obj->{address};
+		$rv{id_addr} = $obj->{-primary_address};
+	} elsif ($pi_config && defined $inbox && $inbox ne '') {
+		# TODO: remove
 		my $addr = $pi_config->get($inbox, 'address') || "";
 		$rv{address} = $addr;
 		$addr = $addr->[0] if ref($addr);
@@ -278,19 +278,19 @@ sub get_feedopts {
 	$rv{id_addr} ||= 'public-inbox@example.com';
 
 	my $url_base;
-	if ($cgi) {
-		$url_base = $cgi->base->as_string . $inbox;
+	if ($obj) {
+		$url_base = $obj->base_url($cgi); # CGI may be undef
 		if (my $mid = $ctx->{mid}) { # per-thread feed:
-			$rv{atomurl} = "$url_base/$mid/t.atom";
+			$rv{atomurl} = "$url_base$mid/t.atom";
 		} else {
-			$rv{atomurl} = "$url_base/new.atom";
+			$rv{atomurl} = $url_base."new.atom";
 		}
 	} else {
-		$url_base = "http://example.com";
-		$rv{atomurl} = "$url_base/new.atom";
+		$url_base = 'http://example.com/';
+		$rv{atomurl} = $url_base.'new.atom';
 	}
-	$rv{url} ||= "$url_base/";
-	$rv{midurl} = "$url_base/";
+	$rv{url} ||= $url_base;
+	$rv{midurl} = $url_base;
 
 	\%rv;
 }
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 5d9fdb3..4bcab96 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -64,6 +64,7 @@ sub cloneurl {
 	$self->{cloneurl} = \@url;
 }
 
+# TODO: can we remove this?
 sub footer_html {
 	my ($self) = @_;
 	my $footer = $self->{footer};
@@ -73,4 +74,23 @@ sub footer_html {
 	$self->{footer} = $footer;
 }
 
+sub base_url {
+	my ($self, $prq) = @_; # Plack::Request
+	if (defined $prq) {
+		my $url = $prq->base->as_string;
+		$url .= '/' if $url !~ m!/\z!; # for mount in Plack::Builder
+		$url .= $self->{name} . '/';
+	} else {
+		# either called from a non-PSGI environment (e.g. NNTP/POP3)
+		$self->{-base_url} ||= do {
+			my $url = $self->{url};
+			# expand protocol-relative URLs to HTTPS if we're
+			# not inside a web server
+			$url = "https:$url" if $url =~ m!\A//!;
+			$url .= '/' if $url !~ m!/\z!;
+			$url;
+		};
+	}
+}
+
 1;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 94ab032..95288a7 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -39,7 +39,7 @@ sub run {
 sub call {
 	my ($self, $env) = @_;
 	my $cgi = Plack::Request->new($env);
-	my $ctx = { cgi => $cgi, pi_config => $self->{pi_config} };
+	my $ctx = {cgi => $cgi, pi_config => $self->{pi_config}, www => $self};
 	my $path_info = $cgi->path_info;
 
 	my $method = $cgi->method;
@@ -124,7 +124,7 @@ sub r { [ $_[0], ['Content-Type' => 'text/plain'], [ join(' ', @_, "\n") ] ] }
 
 # returns undef if valid, array ref response if invalid
 sub invalid_inbox {
-	my ($self, $ctx, $inbox, $mid) = @_;
+	my ($self, $ctx, $inbox) = @_;
 	my $obj = $ctx->{pi_config}->lookup_name($inbox);
 	if (defined $obj) {
 		$ctx->{git_dir} = $obj->{mainrepo};
@@ -144,7 +144,7 @@ sub invalid_inbox {
 # returns undef if valid, array ref response if invalid
 sub invalid_inbox_mid {
 	my ($self, $ctx, $inbox, $mid) = @_;
-	my $ret = invalid_inbox($self, $ctx, $inbox, $mid);
+	my $ret = invalid_inbox($self, $ctx, $inbox);
 	return $ret if $ret;
 
 	$ctx->{mid} = $mid = uri_unescape($mid);
@@ -382,9 +382,14 @@ sub legacy_redirects {
 sub r301 {
 	my ($ctx, $inbox, $mid, $suffix) = @_;
 	my $cgi = $ctx->{cgi};
-	my $url;
+	my $obj = $ctx->{-inbox};
+	unless ($obj) {
+		my $r404 = invalid_inbox($ctx->{www}, $ctx, $inbox);
+		return $r404 if $r404;
+		$obj = $ctx->{-inbox};
+	}
+	my $url = $obj->base_url($cgi);
 	my $qs = $cgi->env->{QUERY_STRING};
-	$url = $cgi->base->as_string . $inbox . '/';
 	$url .= (uri_escape_utf8($mid) . '/') if (defined $mid);
 	$url .= $suffix if (defined $suffix);
 	$url .= "?$qs" if $qs ne '';
diff --git a/t/inbox.t b/t/inbox.t
new file mode 100644
index 0000000..45ba1df
--- /dev/null
+++ b/t/inbox.t
@@ -0,0 +1,12 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use strict;
+use warnings;
+use Test::More;
+use_ok 'PublicInbox::Inbox';
+my $x = PublicInbox::Inbox->new({url => '//example.com/test/'});
+is($x->base_url, 'https://example.com/test/', 'expanded protocol-relative');
+$x = PublicInbox::Inbox->new({url => 'http://example.com/test'});
+is($x->base_url, 'http://example.com/test/', 'added trailing slash');
+
+done_testing();
diff --git a/t/psgi_mount.t b/t/psgi_mount.t
new file mode 100644
index 0000000..c1c1b0c
--- /dev/null
+++ b/t/psgi_mount.t
@@ -0,0 +1,78 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use strict;
+use warnings;
+use Test::More;
+use Email::MIME;
+use File::Temp qw/tempdir/;
+my $tmpdir = tempdir('psgi-path-XXXXXX', TMPDIR => 1, CLEANUP => 1);
+my $maindir = "$tmpdir/main.git";
+my $addr = 'test-public@example.com';
+my $cfgpfx = "publicinbox.test";
+my @mods = qw(HTTP::Request::Common Plack::Request Plack::Test URI::Escape);
+foreach my $mod (@mods) {
+	eval "require $mod";
+	plan skip_all => "$mod missing for plack.t" if $@;
+}
+use_ok $_ foreach @mods;
+use PublicInbox::Import;
+use PublicInbox::Git;
+use PublicInbox::Config;
+use PublicInbox::WWW;
+use Plack::Builder;
+use Plack::App::URLMap;
+my $config = PublicInbox::Config->new({
+	"$cfgpfx.address" => $addr,
+	"$cfgpfx.mainrepo" => $maindir,
+});
+is(0, system(qw(git init -q --bare), $maindir), "git init (main)");
+my $git = PublicInbox::Git->new($maindir);
+my $im = PublicInbox::Import->new($git, 'test', $addr);
+{
+	my $mime = Email::MIME->new(<<EOF);
+From: Me <me\@example.com>
+To: You <you\@example.com>
+Cc: $addr
+Message-Id: <blah\@example.com>
+Subject: hihi
+Date: Thu, 01 Jan 1970 00:00:00 +0000
+
+zzzzzz
+EOF
+	$im->add($mime);
+	$im->done;
+}
+
+my $www = PublicInbox::WWW->new($config);
+my $app = builder {
+	enable 'Head';
+	mount '/a' => builder { sub { $www->call(@_) } };
+	mount '/b' => builder { sub { $www->call(@_) } };
+};
+
+test_psgi($app, sub {
+	my ($cb) = @_;
+	my $res;
+	# Atom feed:
+	$res = $cb->(GET('/a/test/new.atom'));
+	like($res->content, qr!\bhttp://[^/]+/a/test/!,
+		'URLs which exist in Atom feed are mount-aware');
+	unlike($res->content, qr!\b\Qhttp://[^/]+/test/\E!,
+		'No URLs which are not mount-aware');
+
+	# redirects
+	$res = $cb->(GET('/a/test/blah%40example.com/'));
+	is($res->code, 200, 'OK with URLMap mount');
+	$res = $cb->(GET('/a/test/blah%40example.com/raw'));
+	is($res->code, 200, 'OK with URLMap mount');
+	$res = $cb->(GET('/a/test/m/blah%40example.com.html'));
+	is($res->header('Location'),
+		'http://localhost/a/test/blah%40example.com/',
+		'redirect functions properly under mount');
+
+	$res = $cb->(GET('/test/blah%40example.com/'));
+	is($res->code, 404, 'intentional 404 with URLMap mount');
+
+});
+
+done_testing();

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-05-16  7:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-16  7:56 [PATCH 0/3] inbox objectification cleanups + fixes Eric Wong
2016-05-16  7:56 ` [PATCH 1/3] declare Inbox object for reusability Eric Wong
2016-05-16  7:56 ` [PATCH 2/3] config: allow taking an existing reference Eric Wong
2016-05-16  7:56 ` [PATCH 3/3] www: fix for running under mount paths Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).