]> Sergey Matveev's repositories - public-inbox.git/commitdiff
wwwlisting: avoid hogging event loop
authorEric Wong <e@80x24.org>
Wed, 9 Sep 2020 06:26:16 +0000 (06:26 +0000)
committerEric Wong <e@80x24.org>
Thu, 10 Sep 2020 19:45:18 +0000 (19:45 +0000)
By using the just-introduced ConfigIter class.
And make ManifestJsGz a subclass of it to reduce duplication.

lib/PublicInbox/ConfigIter.pm
lib/PublicInbox/ManifestJsGz.pm
lib/PublicInbox/WWW.pm
lib/PublicInbox/WwwListing.pm

index 26cc70e28ce5094254a0df4c1e518fe603cb1c04..e6fa81728a1afda8293b58fc92b33fa199aa2150 100644 (file)
@@ -25,4 +25,16 @@ sub event_step {
        PublicInbox::DS::requeue($self) if defined($section);
 }
 
+# for generic PSGI servers
+sub each_section {
+       my $self = shift;
+       my ($pi_cfg, $i, $cb, @arg) = @$self;
+       while (defined(my $section = $pi_cfg->{-section_order}->[$$i++])) {
+               eval { $cb->($pi_cfg, $section, @arg) };
+               warn "E: $@ in ${self}::each_section" if $@;
+       }
+       eval { $cb->($pi_cfg, undef, @arg) };
+       warn "E: $@ in ${self}::each_section" if $@;
+}
+
 1;
index 328303ceb097abbc9558b0c62fbf19176cded78a..f98d9d010925dcf1a6d6a1ff8fca89f7ad65eec5 100644 (file)
@@ -1,8 +1,11 @@
 # Copyright (C) 2020 all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+
+# generates manifest.js.gz for grokmirror(1)
 package PublicInbox::ManifestJsGz;
 use strict;
 use v5.10.1;
+use parent qw(PublicInbox::WwwListing);
 use Digest::SHA ();
 use File::Spec ();
 use bytes (); # length
@@ -19,22 +22,12 @@ for my $mod (qw(JSON::MaybeXS JSON JSON::PP)) {
        $json = $mod->new->ascii(1) and last;
 }
 
-sub response {
-       my ($env, $list) = @_;
-       $json or return [ 404, [], [] ];
-       my $self = bless {
-               -abs2urlpath => {},
-               -mtime => 0,
-               manifest => {},
-               -list => $list,
-               psgi_env => $env,
-       }, __PACKAGE__;
-
-       # PSGI server will call this immediately and give us a callback (-wcb)
-       sub {
-               $self->{-wcb} = $_[0]; # HTTP write callback
-               iterate_start($self);
-       };
+# called by WwwListing
+sub url_regexp {
+       my ($ctx) = @_;
+       # grokmirror uses relative paths, so it's domain-dependent
+       # SUPER calls PublicInbox::WwwListing::url_regexp
+       $ctx->SUPER::url_regexp('publicInbox.grokManifest', 'match=domain');
 }
 
 sub fingerprint ($) {
@@ -53,7 +46,7 @@ sub fingerprint ($) {
 }
 
 sub manifest_add ($$;$$) {
-       my ($self, $ibx, $epoch, $default_desc) = @_;
+       my ($ctx, $ibx, $epoch, $default_desc) = @_;
        my $url_path = "/$ibx->{name}";
        my $git_dir = $ibx->{inboxdir};
        if (defined $epoch) {
@@ -92,12 +85,12 @@ sub manifest_add ($$;$$) {
                        $reference =~ s!/[^/]+/?\z!!; # basename
                }
        }
-       $self->{-abs2urlpath}->{$git_dir} = $url_path;
+       $ctx->{-abs2urlpath}->{$git_dir} = $url_path;
        my $modified = $git->modified;
-       if ($modified > $self->{-mtime}) {
-               $self->{-mtime} = $modified;
+       if ($modified > ($ctx->{-mtime} // 0)) {
+               $ctx->{-mtime} = $modified;
        }
-       $self->{manifest}->{$url_path} = {
+       $ctx->{manifest}->{$url_path} = {
                owner => $owner,
                reference => $reference,
                description => $desc,
@@ -106,48 +99,37 @@ sub manifest_add ($$;$$) {
        };
 }
 
-sub iterate_start {
-       my ($self) = @_;
-       if (my $async = $self->{psgi_env}->{'pi-httpd.async'}) {
-               # PublicInbox::HTTPD::Async->new
-               $async->(undef, undef, $self);
-       } else {
-               event_step($self) while $self->{-wcb};
-       }
-}
-
-sub event_step {
-       my ($self) = @_;
-       while (my $ibx = shift(@{$self->{-list}})) {
-               eval {
-                       if (defined(my $max = $ibx->max_git_epoch)) {
-                               my $desc = $ibx->description;
-                               for my $epoch (0..$max) {
-                                       manifest_add($self, $ibx, $epoch, $desc)
-                               }
-                       } else {
-                               manifest_add($self, $ibx);
+sub ibx_entry {
+       my ($ctx, $ibx) = @_;
+       eval {
+               if (defined(my $max = $ibx->max_git_epoch)) {
+                       my $desc = $ibx->description;
+                       for my $epoch (0..$max) {
+                               manifest_add($ctx, $ibx, $epoch, $desc);
                        }
-               };
-               warn "E: $@" if $@;
-               if (my $async = $self->{psgi_env}->{'pi-httpd.async'}) {
-                       # PublicInbox::HTTPD::Async->new
-                       $async->(undef, undef, $self);
+               } else {
+                       manifest_add($ctx, $ibx);
                }
-               return; # more steps needed
-       }
-       my $abs2urlpath = delete $self->{-abs2urlpath};
-       my $wcb = delete $self->{-wcb};
-       my $manifest = delete $self->{manifest};
+       };
+       warn "E: $@" if $@;
+}
+
+sub hide_key { 'manifest' }
+
+# overrides WwwListing->psgi_triple
+sub psgi_triple {
+       my ($ctx) = @_;
+       my $abs2urlpath = delete($ctx->{-abs2urlpath}) // {};
+       my $manifest = delete($ctx->{manifest}) // {};
        while (my ($url_path, $repo) = each %$manifest) {
                defined(my $abs = $repo->{reference}) or next;
                $repo->{reference} = $abs2urlpath->{$abs};
        }
        $manifest = $json->encode($manifest);
        gzip(\$manifest => \(my $out));
-       $wcb->([ 200, [ qw(Content-Type application/gzip),
-                'Last-Modified', time2str($self->{-mtime}),
-                'Content-Length', bytes::length($out) ], [ $out ] ]);
+       [ 200, [ qw(Content-Type application/gzip),
+                'Last-Modified', time2str($ctx->{-mtime}),
+                'Content-Length', bytes::length($out) ], [ $out ] ]
 }
 
 1;
index 93ab3c9d82c7ee12b8139420f07dee6d2cd13396..85abf327de884140aac246c5407229b135817433 100644 (file)
@@ -77,8 +77,12 @@ sub call {
        }
 
        # top-level indices and feeds
-       if ($path_info eq '/' || $path_info eq '/manifest.js.gz') {
-               www_listing($self)->call($env);
+       if ($path_info eq '/') {
+               require PublicInbox::WwwListing;
+               PublicInbox::WwwListing->response($ctx);
+       } elsif ($path_info eq '/manifest.js.gz') {
+               require PublicInbox::ManifestJsGz;
+               PublicInbox::ManifestJsGz->response($ctx);
        } elsif ($path_info =~ m!$INBOX_RE\z!o) {
                invalid_inbox($ctx, $1) || r301($ctx, $1);
        } elsif ($path_info =~ m!$INBOX_RE(?:/|/index\.html)?\z!o) {
@@ -171,7 +175,6 @@ sub preload {
                }
                $self->cgit;
                $self->stylesheets_prepare($_) for ('', '../', '../../');
-               $self->www_listing;
                $self->news_www;
                $pi_config->each_inbox(\&preload_inbox);
        }
@@ -496,21 +499,13 @@ sub cgit {
        }
 }
 
-sub www_listing {
-       my ($self) = @_;
-       $self->{www_listing} ||= do {
-               require PublicInbox::WwwListing;
-               PublicInbox::WwwListing->new($self);
-       }
-}
-
 # GET $INBOX/manifest.js.gz
 sub get_inbox_manifest ($$$) {
        my ($ctx, $inbox, $key) = @_;
        my $r404 = invalid_inbox($ctx, $inbox);
        return $r404 if $r404;
        require PublicInbox::ManifestJsGz;
-       PublicInbox::ManifestJsGz::response($ctx->{env}, [$ctx->{-inbox}]);
+       PublicInbox::ManifestJsGz->response($ctx);
 }
 
 sub get_attach {
index a7c7cbc1239e6025e6594524a68ed803263dfcb1..bda2761c63d93091b4680065757a3b241a6eb895 100644 (file)
 # Used by PublicInbox::WWW
 package PublicInbox::WwwListing;
 use strict;
-use PublicInbox::Hval qw(ascii_html prurl fmt_ts);
+use PublicInbox::Hval qw(prurl fmt_ts);
 use PublicInbox::Linkify;
 use PublicInbox::GzipFilter qw(gzf_maybe);
-use PublicInbox::ManifestJsGz;
+use PublicInbox::ConfigIter;
 use bytes (); # bytes::length
 
-sub list_all_i {
-       my ($ibx, $list, $hide_key) = @_;
-       push @$list, $ibx unless $ibx->{-hide}->{$hide_key};
-}
-
-sub list_all ($$$) {
-       my ($self, $env, $hide_key) = @_;
-       my $list = [];
-       $self->{pi_config}->each_inbox(\&list_all_i, $list, $hide_key);
-       $list;
-}
+sub ibx_entry {
+       my ($ctx, $ibx) = @_;
+       my $mtime = $ibx->modified;
+       my $ts = fmt_ts($mtime);
+       my $url = prurl($ctx->{env}, $ibx->{url});
+       my $tmp = <<"";
+* $ts - $url
+  ${\$ibx->description}
 
-sub list_match_domain_i {
-       my ($ibx, $list, $hide_key, $re) = @_;
-       if (!$ibx->{-hide}->{$hide_key} && grep(/$re/, @{$ibx->{url}})) {
-               push @$list, $ibx;
+       if (defined(my $info_url = $ibx->{infourl})) {
+               $tmp .= '  ' . prurl($ctx->{env}, $info_url) . "\n";
        }
+       push @{$ctx->{-list}}, [ $mtime, $tmp ];
 }
 
-sub list_match_domain ($$$) {
-       my ($self, $env, $hide_key) = @_;
-       my $list = [];
-       my $host = $env->{HTTP_HOST} // $env->{SERVER_NAME};
-       $host =~ s/:[0-9]+\z//;
-       $self->{pi_config}->each_inbox(\&list_match_domain_i, $list, $hide_key,
-                               qr!\A(?:https?:)?//\Q$host\E(?::[0-9]+)?/!i);
-       $list;
-}
-
-sub list_404 ($$) { [] }
-
-# TODO: +cgit
-my %VALID = (
-       all => \&list_all,
-       'match=domain' => \&list_match_domain,
-       404 => \&list_404,
-);
-
-sub set_cb ($$$) {
-       my ($pi_config, $k, $default) = @_;
-       my $v = $pi_config->{lc $k} // $default;
-       $VALID{$v} || do {
-               warn <<"";
-`$v' is not a valid value for `$k'
-$k be one of `all', `match=domain', or `404'
-
-               $VALID{$default};
-       };
+sub list_match_i { # ConfigIter callback
+       my ($cfg, $section, $re, $ctx) = @_;
+       if (defined($section)) {
+               return if $section !~ m!\Apublicinbox\.([^/]+)\z!;
+               my $ibx = $cfg->lookup_name($1) or return;
+               if (!$ibx->{-hide}->{$ctx->hide_key} &&
+                                       grep(/$re/, @{$ibx->{url}})) {
+                       $ctx->ibx_entry($ibx);
+               }
+       } else { # undef == "EOF"
+               $ctx->{-wcb}->($ctx->psgi_triple);
+       }
 }
 
-sub new {
-       my ($class, $www) = @_;
-       my $pi_config = $www->{pi_config};
-       bless {
-               pi_config => $pi_config,
-               style => $www->style("\0"),
-               www_cb => set_cb($pi_config, 'publicInbox.wwwListing', 404),
-               manifest_cb => set_cb($pi_config, 'publicInbox.grokManifest',
-                                       'match=domain'),
-       }, $class;
+sub url_regexp {
+       my ($ctx, $key, $default) = @_;
+       $key //= 'publicInbox.wwwListing';
+       $default //= '404';
+       my $v = $ctx->{www}->{pi_config}->{lc $key} // $default;
+again:
+       if ($v eq 'match=domain') {
+               my $h = $ctx->{env}->{HTTP_HOST} // $ctx->{env}->{SERVER_NAME};
+               $h =~ s/:[0-9]+\z//;
+               qr!\A(?:https?:)?//\Q$h\E(?::[0-9]+)?/!i;
+       } elsif ($v eq 'all') {
+               qr/./;
+       } elsif ($v eq '404') {
+               undef;
+       } else {
+               warn <<EOF;
+`$v' is not a valid value for `$key'
+$key be one of `all', `match=domain', or `404'
+EOF
+               $v = $default; # 'match=domain' or 'all'
+               goto again;
+       }
 }
 
-sub ibx_entry {
-       my ($mtime, $ibx, $env) = @_;
-       my $ts = fmt_ts($mtime);
-       my $url = prurl($env, $ibx->{url});
-       my $tmp = <<"";
-* $ts - $url
-  ${\$ibx->description}
-
-       if (defined(my $info_url = $ibx->{infourl})) {
-               $tmp .= '  ' . prurl($env, $info_url) . "\n";
+sub hide_key { 'www' }
+
+sub response {
+       my ($class, $ctx) = @_;
+       bless $ctx, $class;
+       my $re = $ctx->url_regexp or return $ctx->psgi_triple;
+       my $iter = PublicInbox::ConfigIter->new($ctx->{www}->{pi_config},
+                                               \&list_match_i, $re, $ctx);
+       sub {
+               $ctx->{-wcb} = $_[0]; # HTTP server callback
+               $ctx->{env}->{'pi-httpd.async'} ?
+                               $iter->event_step : $iter->each_section;
        }
-       $tmp;
 }
 
-sub html ($$) {
-       my ($env, $list) = @_;
+sub psgi_triple {
+       my ($ctx) = @_;
        my $h = [ 'Content-Type', 'text/html; charset=UTF-8',
                        'Content-Length', undef ];
-       my $gzf = gzf_maybe($h, $env);
+       my $gzf = gzf_maybe($h, $ctx->{env});
        $gzf->zmore('<html><head><title>' .
                                'public-inbox listing</title>' .
                                '</head><body><pre>');
        my $code = 404;
-       if (@$list) {
+       if (my $list = $ctx->{-list}) {
                $code = 200;
-               # Schwartzian transform since Inbox->modified is expensive
-               @$list = sort {
-                       $b->[0] <=> $a->[0]
-               } map { [ $_->modified, $_ ] } @$list;
-
-               my $tmp = join("\n", map { ibx_entry(@$_, $env) } @$list);
+               # sort by ->modified
+               @$list = map { $_->[1] } sort { $b->[0] <=> $a->[0] } @$list;
+               $list = join("\n", @$list);
                my $l = PublicInbox::Linkify->new;
-               $gzf->zmore($l->to_html($tmp));
+               $gzf->zmore($l->to_html($list));
        } else {
                $gzf->zmore('no inboxes, yet');
        }
        my $out = $gzf->zflush('</pre><hr><pre>'.
-                               PublicInbox::WwwStream::code_footer($env) .
-                               '</pre></body></html>');
+                       PublicInbox::WwwStream::code_footer($ctx->{env}) .
+                       '</pre></body></html>');
        $h->[3] = bytes::length($out);
        [ $code, $h, [ $out ] ];
 }
 
-# not really a stand-alone PSGI app, but maybe it could be...
-sub call {
-       my ($self, $env) = @_;
-
-       if ($env->{PATH_INFO} eq '/manifest.js.gz') {
-               # grokmirror uses relative paths, so it's domain-dependent
-               my $list = $self->{manifest_cb}->($self, $env, 'manifest');
-               PublicInbox::ManifestJsGz::response($env, $list);
-       } else { # /
-               my $list = $self->{www_cb}->($self, $env, 'www');
-               html($env, $list);
-       }
-}
-
 1;