#442

backend/app/controllers/post_versions_controller.rb

@@ -15,7 +15,6 @@ class PostVersionsController < ApplicationController
     offset = (page - 1) * limit
 
     q = HistoryAccess.visible(PostVersion.all)
-    q = q.with_previous_version if tag_id || external_tag_id
     q = q.where('post_versions.post_id = ?', post_id) if post_id
     q = q.with_previous_version if tag_id || external_tag_id
     if external_tag_id || (tag_id && !(Tag.exists?(id: tag_id)))
@@ -34,7 +33,7 @@ class PostVersionsController < ApplicationController
                 .limit(limit)
                 .offset(offset)
                 .pluck('post_versions.id')
-    versions = HistoryAccess.visible(version_scope.where(id: ids))
+    versions = version_scope.merge(q).where(id: ids)
         .order(Arel.sql('post_versions.created_at DESC, post_versions.id DESC'))
 
     render json: { versions: serialise_versions(versions, locale.language_code), count:, population_count: HistoryAccess.population_count(PostVersion) }

backend/app/controllers/posts_controller.rb

@@ -37,7 +37,7 @@ class PostsController < ApplicationController
 
     offset = (page - 1) * limit
 
-    updated_at_all_sql = 'posts.updated_at'
+    needs_history_join = order[0] == 'updated_at' || updated_between.any?(&:present?)
 
     q =
         filtered_posts(locale)
@@ -56,12 +56,25 @@ class PostsController < ApplicationController
     end
     q = q.where('posts.created_at >= ?', created_between[0]) if created_between[0]
     q = q.where('posts.created_at <= ?', created_between[1]) if created_between[1]
+    count_scope = q
+    if needs_history_join
+      history_times = HistoryAccess.visible(PostVersion.all)
+          .select('post_id, MAX(created_at) AS latest_at').group(:post_id)
+      q = q.joins("LEFT JOIN (#{ history_times.to_sql }) history_times " +
+                  'ON history_times.post_id = posts.id')
+      updated_at_all_sql = 'GREATEST(posts.updated_at, ' +
+          'COALESCE(history_times.latest_at, posts.updated_at))'
+    end
     if updated_between[0]
       q = q.where("#{ updated_at_all_sql } >= ?", updated_between[0])
     end
     if updated_between[1]
       q = q.where("#{ updated_at_all_sql } <= ?", updated_between[1])
     end
+    count_scope = q if updated_between.any?(&:present?)
+    if needs_history_join
+      q = q.select('posts.*', "#{ updated_at_all_sql } AS updated_at")
+    end
 
     sort_sql =
         case order[0]
@@ -78,14 +91,20 @@ class PostsController < ApplicationController
              .limit(limit)
              .offset(offset)
              .to_a
-    latest_version_times = HistoryAccess.visible(PostVersion.where(post_id: posts.map(&:id)))
-        .group(:post_id).maximum(:created_at)
+    latest_version_times =
+        if needs_history_join
+          { }
+        else
+          HistoryAccess.visible(PostVersion.where(post_id: posts.map(&:id)))
+              .group(:post_id).maximum(:created_at)
+        end
 
-    q = q.except(:select, :order)
+    count_scope = count_scope.except(:select, :order)
 
     render json: { posts: posts.map { |post|
+      updated_at = [post.updated_at, latest_version_times[post.id]].compact.max
       PostRepr.base(post, language_code: locale.language_code)
-              .merge(updated_at: post.updated_at).tap do |json|
+          .merge('updated_at' => updated_at).tap do |json|
         json['thumbnail'] =
             if post.thumbnail.attached?
               rails_storage_proxy_url(post.thumbnail, only_path: false)
@@ -93,7 +112,7 @@ class PostsController < ApplicationController
               nil
             end
       end
-    }, count: q.group_values.present? ? q.count.size : q.count }
+    }, count: count_scope.group_values.present? ? count_scope.count.size : count_scope.count }
   end
 
   def random

backend/app/models/history_access.rb

@@ -59,20 +59,21 @@ class HistoryAccess < ActiveSupport::CurrentAttributes
   end
 
   def self.json_reference(table, key, document)
-    "NOT EXISTS (SELECT 1 FROM JSON_TABLE(COALESCE(#{ document }, JSON_ARRAY()), " +
+    # A scalar lookup avoids MySQL's semijoin rewrite losing the JSON row correlation.
+    "(SELECT 1 FROM JSON_TABLE(COALESCE(#{ document }, JSON_ARRAY()), " +
         "'$[*]' COLUMNS (reference_id BIGINT PATH '$' NULL ON EMPTY)) reference_entry " +
         "JOIN (#{ discarded_keys(table, key) }) blocked " +
-        "ON reference_entry.reference_id = blocked.#{ key })"
+        "ON reference_entry.reference_id = blocked.#{ key } LIMIT 1) IS NULL"
   end
 
   def self.list_reference(table, key, document)
     # Preserve FIND_IN_SET's exact token semantics, including empty tokens.
     array = "CONCAT('[', REPLACE(JSON_QUOTE(REPLACE(COALESCE(#{ document }, ''), " +
         "' ', ',')), ',', '\",\"'), ']')"
-    "NOT EXISTS (SELECT 1 FROM JSON_TABLE(#{ array }, '$[*]' " +
+    "(SELECT 1 FROM JSON_TABLE(#{ array }, '$[*]' " +
         "COLUMNS (reference_id VARCHAR(255) PATH '$')) reference_entry " +
         "JOIN (#{ discarded_keys(table, key) }) blocked " +
-        "ON reference_entry.reference_id = CAST(blocked.#{ key } AS CHAR))"
+        "ON reference_entry.reference_id = CAST(blocked.#{ key } AS CHAR) LIMIT 1) IS NULL"
   end
 
   def self.snapshot_reference(model, row, external: false)
@@ -91,8 +92,8 @@ class HistoryAccess < ActiveSupport::CurrentAttributes
       match += ' AND reference_version.version_no = tag_reference.version_no'
       latest = ''
     end
-    "NOT EXISTS (SELECT 1 FROM #{ source } JOIN #{ table } reference_version ON #{ match } " +
-        "WHERE NOT (#{ sql(model, 'reference_version') }) #{ latest })"
+    "(SELECT 1 FROM #{ source } JOIN #{ table } reference_version ON #{ match } " +
+        "WHERE NOT (#{ sql(model, 'reference_version') }) #{ latest } LIMIT 1) IS NULL"
   end
 
   def self.reference_sql(table, row)

backend/spec/requests/post_history_privacy_spec.rb

@@ -37,6 +37,32 @@ RSpec.describe 'Post history privacy', type: :request do
     end
   end
 
+  it 'evaluates each reference condition against the restored snapshot alone' do
+    HistoryAccess.set(user: nil) do
+      restored = PostVersion.where(post_id: post_record.id, version_no: 3)
+      HistoryAccess.reference_sql('post_versions', 'post_versions').each do |condition|
+        expect(restored.where(Arel.sql(condition)).pluck(:version_no)).to eq([3])
+      end
+      expect(HistoryAccess.visible(PostVersion.where(post_id: post_record.id))
+          .pluck(:version_no)).to eq([3])
+    end
+  end
+
+  it 'keeps every version through a later discard private after another restore' do
+    ['discard', 'restore'].each_with_index do |event, index|
+      PostVersion.create!(post: post_record, version_no: index + 5, event_type: event,
+          title: 'later snapshot', url: post_record.url, tags: '', tags_json: [],
+          parent_post_ids: '', created_at: origin + (index + 5).days)
+    end
+
+    get '/posts/versions', params: { post: post_record.id }
+
+    expect(response).to have_http_status(:ok)
+    expect(response.parsed_body.fetch('versions').pluck('version_no')).to eq([6])
+    expect(response.parsed_body.fetch('count')).to eq(1)
+    expect(response.parsed_body.fetch('versions').first.fetch('title').fetch('prev')).to be_nil
+  end
+
   ['anonymous', 'member', 'admin'].each do |role|
     context "as #{ role }" do
       before do
@@ -71,6 +97,15 @@ RSpec.describe 'Post history privacy', type: :request do
         end
       end
 
+      it 'applies authorised timestamp ordering before pagination' do
+        get '/posts', params: { order: 'updated_at:desc', limit: 1 }
+
+        expect(response).to have_http_status(:ok)
+        expected = role == 'admin' ? post_record.id : other_post.id
+        expect(response.parsed_body.fetch('posts').pluck('id')).to eq([expected])
+        expect(response.parsed_body.fetch('count')).to eq(2)
+      end
+
       it 'counts only visible versions and masks a private previous version' do
         get '/posts/versions', params: { post: post_record.id }
         expect(response).to have_http_status(:ok)

backend/spec/requests/posts_spec.rb

@@ -629,6 +629,37 @@ RSpec.describe 'Posts API', type: :request do
         create(:post_tag, post: plain_post, tag:, created_at: t3)
       end
 
+      it 'displays history time after pagination without joining history to posts or count' do
+        queries = []
+        subscriber = ->(event) { queries << event.payload.fetch(:sql) }
+        ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do
+          get '/posts', params: { url: '/version-time/', order: 'url:asc', limit: 1 }
+        end
+
+        expect(response).to have_http_status(:ok)
+        expect(json.fetch('posts').pluck('id')).to eq([history_post.id])
+        expect(Time.zone.parse(json.fetch('posts').first.fetch('updated_at'))).to eq(t2)
+        expect(json.fetch('count')).to eq(3)
+        post_queries = queries.select { _1.include?('FROM `posts`') }
+        expect(post_queries).not_to be_empty
+        expect(post_queries.grep(/post_versions/)).to be_empty
+      end
+
+      it 'omits the history aggregate from count when only sorting by update time' do
+        queries = []
+        subscriber = ->(event) { queries << event.payload.fetch(:sql) }
+        ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do
+          get '/posts', params: { url: '/version-time/', order: 'updated_at:desc', limit: 1 }
+        end
+
+        expect(response).to have_http_status(:ok)
+        expect(json.fetch('posts').pluck('id')).to eq([newer_post.id])
+        expect(json.fetch('count')).to eq(3)
+        count_queries = queries.select { _1.include?('FROM `posts`') && _1.match?(/COUNT\(/i) }
+        expect(count_queries).not_to be_empty
+        expect(count_queries.grep(/post_versions/)).to be_empty
+      end
+
       ['asc', 'desc'].each do |direction|
         it "sorts by the later of post update and latest version time (#{ direction })" do
           get '/posts', params: { url: '/version-time/', order: "updated_at:#{ direction }" }
@@ -658,6 +689,16 @@ RSpec.describe 'Posts API', type: :request do
         expect(json.fetch('posts').map { |item| item.fetch('id') }).to eq([history_post.id])
         expect(json.fetch('count')).to eq(1)
       end
+
+      it 'counts every timestamp match before pagination' do
+        get '/posts', params: { url: '/version-time/', order: 'updated_at:asc',
+                               updated_from: t1.iso8601, updated_to: t3.iso8601,
+                               limit: 1, page: 2 }
+
+        expect(response).to have_http_status(:ok)
+        expect(json.fetch('posts').pluck('id')).to eq([history_post.id])
+        expect(json.fetch('count')).to eq(3)
+      end
     end
 
     context 'when original_created_from/original_created_to are provided' do

backend/spec/services/counts_export_spec.rb

@@ -221,9 +221,15 @@ RSpec.describe CountsExportDownload::ResponseBody do
       body = described_class.new(source) { |state| states << state; true }
       response = ActionDispatch::Response.new
       response.body = body
-      deferred = response.stream.to_ary
+
+      _, _, rack_body = response.to_a
+
       expect(states).to eq([])
-      expect(deferred.to_a.join).to eq(payload)
+
+      chunks = []
+      rack_body.each { |chunk| chunks << chunk }
+
+      expect(chunks.join).to eq(payload)
       expect(states.count('started')).to eq(1)
     end
   end