Skip the query for a caller who can see nothing, and widen acls in one place

From review of #354.

_acl_clause() had an if with an empty body for raw, and spiked the query
with '0 = 1' for a caller holding no acls.  It now returns early for raw,
and returns false when nothing can match.  _where() and _build_query()
return nothing then, and read() answers [] without querying at all.

"Add public, unlisted for a direct link, private for an admin" was
written out five times in Trog::Routes::HTML: posts(), series(), the
relation resolver in series(), avatars() and users().  It is now
_viewer_acls().  Looking for other callers that passed the raw user_acls
found one with the same bug: /api/posts_of_form, which fills the
relation picker, so an editor who was not an admin could not pick a
public post.  It uses _viewer_acls() too.  The guest routes also pass the
raw list, but they are admin only, and admin skips the acl check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

lib/Trog/Data/SQLite.pm

@@ -86,7 +86,7 @@ that Trog::DataModule::_dedup_versions() would have computed by hand.
 =cut
 
 sub read ( $self, $query = {} ) {
-    my ( $sql, @bind ) = _build_query($query);
+    my ( $sql, @bind ) = _build_query($query) or return [];
 
     my $rows = _dbh()->selectall_arrayref( $sql, { Slice => {} }, @bind );
     return [] unless ref $rows eq 'ARRAY';
@@ -142,7 +142,7 @@ sub _pinned ($query) {
 sub _build_query ($query) {
     my $select = q{SELECT p.post_data, p.uuid, p.version, v.version_max, v.first_created, f.author AS first_author};
 
-    my ( $where, $bind ) = _where($query);
+    my ( $where, $bind ) = _where($query) or return;
 
     # A raw read with no filters has no clauses at all, and "WHERE" on its own
     # is a syntax error rather than "everything".
@@ -172,6 +172,7 @@ sub _build_query ($query) {
 }
 
 # filter(), clause for clause.  Read the two side by side when changing either.
+# Returns nothing when no post can match, and then there is no query to run.
 sub _where ($query) {
     my ( @where, @bind );
 
@@ -195,7 +196,7 @@ sub _where ($query) {
         my $column = $key eq 'id' ? 'p.uuid' : "p.$key";
         push( @where, "$column = ?" );
         push( @bind,  $query->{$key} );
-        _acl_clause( $query, \@where, \@bind );
+        return unless _acl_clause( $query, \@where, \@bind );
         return ( \@where, \@bind );
     }
 
@@ -229,7 +230,7 @@ sub _where ($query) {
         push( @bind,  @{ $query->{exclude_tags} } );
     }
 
-    _acl_clause( $query, \@where, \@bind );
+    return unless _acl_clause( $query, \@where, \@bind );
 
     if ( $query->{form} ) {
         push( @where, 'COALESCE(p.form, ?) = ?' );
@@ -250,27 +251,23 @@ sub _where ($query) {
     return ( \@where, \@bind );
 }
 
-# The load bearing one: a post is only visible to a caller holding one of its
-# tags, since visibility and acls are both stored as tags.  admin skips it, as
-# it does in filter().
+# A post is only visible to a caller holding one of its tags, since visibility
+# and acls are both stored as tags.  admin skips it, as it does in filter().
+# False when the caller can see nothing at all, so there is nothing to query.
 sub _acl_clause ( $query, $where, $bind ) {
-    my $acls = ref $query->{acls} eq 'ARRAY' ? $query->{acls} : [];
-    if ( $query->{raw} ) {
 
-        # raw skips filter() altogether in the flat file model, acls included.
-        # Only bin/migrate.pl and an index rebuild ask for it.
-    }
-    elsif ( @$acls && !any { $_ eq 'admin' } @$acls ) {
-        my $binds = join( ',', map { '?' } @$acls );
-        push( @$where, "p.id IN (SELECT post_id FROM post_tags WHERE tag IN ($binds))" );
-        push( @$bind,  @$acls );
-    }
-    elsif ( !@$acls ) {
+    # raw skips filter() altogether in the flat file model, acls included.
+    # Only bin/migrate.pl and an index rebuild ask for it.
+    return 1 if $query->{raw};
 
-        # filter() greps against an empty list here, which matches nothing.
-        push( @$where, '0 = 1' );
-    }
-    return;
+    my $acls = ref $query->{acls} eq 'ARRAY' ? $query->{acls} : [];
+    return 0 unless @$acls;
+    return 1 if any { $_ eq 'admin' } @$acls;
+
+    my $binds = join( ',', map { '?' } @$acls );
+    push( @$where, "p.id IN (SELECT post_id FROM post_tags WHERE tag IN ($binds))" );
+    push( @$bind,  @$acls );
+    return 1;
 }
 
 =head2 _like_clause($term)

lib/Trog/Routes/HTML.pm

@@ -1485,13 +1485,7 @@ sub series ($query) {
     # Asking for page two of a single post gets nothing, so a series carrying a
     # ?page= in its URL could not find itself and 404'd on its own page.  Latent
     # until something started emitting page numbers -- see _paginate_offset.
-    #
-    # Seen through what a viewer can actually see, as posts() does further on:
-    # the raw user_acls of a logged out visitor are empty, and match nothing.
-    # Unlisted too, since the route names this series directly.
-    my @series_acls = ( @{ $query->{user_acls} }, qw{public unlisted} );
-    push( @series_acls, 'private' ) if $is_admin;
-    my @posts = _post_helper( { %$query, page => 1, limit => 1 }, ['series'], \@series_acls );
+    my @posts = _post_helper( { %$query, page => 1, limit => 1 }, ['series'], [ _viewer_acls( $query, 1 ) ] );
 
     # The series post carries its *children's* relations, so that an editor
     # picker rendered on this page has something to populate from -- that is
@@ -1507,14 +1501,7 @@ sub series ($query) {
             # post gets overwritten.
             my $stand_in = { %{ $posts[0] }, form => $posts[0]{child_form} };
 
-            # posts() widens user_acls to what a viewer can actually see, but
-            # that happens after this -- so resolving here against the raw list
-            # means a logged out visitor resolves every relation against no
-            # acls at all, and finds nothing however public the target is.
-            my @acls = ( @{ $query->{user_acls} }, 'public' );
-            push( @acls, 'private' ) if $is_admin;
-
-            _enrich_post( $stand_in, { %$query, user_acls => \@acls } );
+            _enrich_post( $stand_in, { %$query, user_acls => [ _viewer_acls($query) ] } );
             @{ $posts[0] }{ keys(%$relations) } = @{$stand_in}{ keys(%$relations) };
         }
     }
@@ -1538,7 +1525,7 @@ Returns the avatars.css.
 =cut
 
 sub avatars ($query) {
-    push( @{ $query->{user_acls} }, 'public' );
+    $query->{user_acls} = [ _viewer_acls($query) ];
     my $tags = Trog::Utils::coerce_array( $query->{tag} );
 
     my @posts = _post_helper( $query, $tags, $query->{user_acls} );
@@ -1613,7 +1600,7 @@ sub users ($query) {
     return $query->{tpsgi}->notfound($query) unless $username;
 
     $query->{username} //= $username;
-    push( @{ $query->{user_acls} }, 'public' );
+    $query->{user_acls}    = [ _viewer_acls($query) ];
     $query->{exclude_tags} = ['about'];
 
     # Don't show topbar series on the series page.  That said, don't exclude it from direct series view.
@@ -1657,9 +1644,7 @@ sub posts ( $query, $direct = 0 ) {
     $query->{id} = $id if $id && !$query->{in_series};
 
     my $is_admin = grep { $_ eq 'admin' } @{ $query->{user_acls} };
-    push( @{ $query->{user_acls} }, 'public' );
-    push( @{ $query->{user_acls} }, 'unlisted' ) if $query->{id};
-    push( @{ $query->{user_acls} }, 'private' )  if $is_admin;
+    $query->{user_acls} = [ _viewer_acls( $query, $query->{id} ) ];
     my @posts;
 
     # Discover this user's visibility, so we can make them post in this category by default
@@ -2028,6 +2013,22 @@ sub _search_language ($query) {
     return ( scalar $lang->(), scalar $help->() );
 }
 
+=head2 _viewer_acls($query, $direct) = LIST
+
+The acls to fetch posts for this viewer with: their own, and public, plus
+unlisted when the route names one post directly, plus private for an admin.
+A logged out visitor holds no acls of their own, and would see nothing without
+this.
+
+=cut
+
+sub _viewer_acls ( $query, $direct = 0 ) {
+    my @acls = ( @{ $query->{user_acls} // [] }, 'public' );
+    push( @acls, 'unlisted' ) if $direct;
+    push( @acls, 'private' )  if grep { $_ eq 'admin' } @acls;
+    return List::Util::uniq(@acls);
+}
+
 sub _post_helper ( $query, $tags, $acls ) {
     state $data;
     $data //= Trog::Data->new( Trog::Config::get() );

lib/Trog/Routes/JSON.pm

@@ -136,7 +136,7 @@ nobody's business here.
 =cut
 
 sub posts_of_form ($query) {
-    my @posts = Trog::Routes::HTML::_post_helper( { form => $query->{form}, limit => 0 }, [], $query->{user_acls} );
+    my @posts = Trog::Routes::HTML::_post_helper( { form => $query->{form}, limit => 0 }, [], [ Trog::Routes::HTML::_viewer_acls($query) ] );
 
     return _render(
         200, undef,

t/Trog-Data-SQLite.t

@@ -223,6 +223,15 @@ subtest 'a direct lookup still answers only to the acls' => sub {
     }
 };
 
+subtest 'a caller who can see nothing is not queried for' => sub {
+    my $mock = Test::MockModule->new('Trog::Data::SQLite');
+    $mock->redefine( _dbh => sub { die "queried the database\n" } );
+
+    is( exception { is_deeply( $sqlite->read( { limit => 0, acls => [] } ), [], 'no acls get nothing' ) }, undef, 'without asking the database' );
+    is( exception { is_deeply( $sqlite->read( { limit => 0, acls => [], id => 'p-public' } ), [], 'nor by id' ) }, undef, 'nor for a direct lookup' );
+    like( exception { $sqlite->read( { limit => 0, acls => ['public'] } ) }, qr/queried the database/a, 'while a caller with acls is queried' );
+};
+
 subtest 'versions' => sub {
     my ($latest) = $sqlite->get( limit => 0, acls => [qw{admin}], id => 'p-public' );
     is( $latest->{version},     1,                               'the newest version is the one you get' );

t/Trog-Routes-HTML-series-lifecycle.t

@@ -1412,6 +1412,12 @@ subtest '/api/posts_of_form feeds the relation picker' => sub {
         'listing exactly the posts of that type, id and title only'
     );
 
+    # An editor who is not an admin holds only acls of their own, which no
+    # public post carries.
+    $res     = Trog::Routes::JSON::posts_of_form( _query( user => 'spec-editor', user_acls => ['spec-editors'], route => '/api/posts_of_form', form => 'spec_target.tx' ) );
+    $payload = JSON::MaybeXS::decode_json( join( '', @{ $res->[2] } ) );
+    is_deeply( [ map { $_->{id} } @{ $payload->{posts} } ], [ $target->{id} ], 'an editor who is not an admin can pick a public post too' );
+
     # An unknown type is an empty list, not an error -- a type with no posts
     # yet is the normal state of affairs right after you create it.
     $res     = Trog::Routes::JSON::posts_of_form( _admin( route => '/api/posts_of_form', form => 'nosuchtype.tx' ) );