From 22e57564a5f4ae61b19a657812dee63db7a0169c Mon Sep 17 00:00:00 2001 From: miteruzo Date: Sun, 1 Nov 2026 18:18:54 +0900 Subject: [PATCH] #434 --- .../controllers/counts_exports_controller.rb | 133 +++++++++--------- .../controllers/counts_tables_controller.rb | 5 + .../app/services/counts_export_download.rb | 35 +++-- .../app/services/counts_export_projection.rb | 2 +- backend/app/services/counts_pivot_totals.rb | 27 ++-- backend/app/services/counts_table_query.rb | 28 +++- backend/spec/requests/counts_download_spec.rb | 26 +++- backend/spec/requests/counts_table_spec.rb | 26 ++++ backend/spec/services/counts_export_spec.rb | 26 +++- .../services/counts_pivot_security_spec.rb | 14 ++ .../counts/CountsSummaryResults.tsx | 5 +- frontend/src/lib/counts.test.ts | 26 +++- frontend/src/lib/counts.ts | 4 + frontend/src/lib/countsDisplay.ts | 16 ++- frontend/src/lib/countsState.ts | 4 +- frontend/src/locales/ain-Kana/counts.json | 7 + frontend/src/locales/ja/counts.json | 9 +- .../src/pages/counts/CountsResourcePage.tsx | 41 +++--- 18 files changed, 313 insertions(+), 121 deletions(-) diff --git a/backend/app/controllers/counts_exports_controller.rb b/backend/app/controllers/counts_exports_controller.rb index a366aa4..dff8f05 100644 --- a/backend/app/controllers/counts_exports_controller.rb +++ b/backend/app/controllers/counts_exports_controller.rb @@ -18,7 +18,7 @@ class CountsExportsController < ApplicationController render json: { error: 'private_condition' }, status: :forbidden rescue CountsTableQuery::TooManyColumns render json: { error: 'too_many_columns', max_columns: CountsTableQuery::MAX_COLUMNS }, - status: :unprocessable_entity + status: :unprocessable_entity rescue ActiveRecord::StatementTimeout render json: { error: 'counts_query_timeout' }, status: :service_unavailable rescue ArgumentError, TypeError, KeyError, JSON::ParserError @@ -86,68 +86,6 @@ class CountsExportsController < ApplicationController disposition: 'attachment' end - def confirmed_download - owner = session[:counts_export_owner] - arguments = { file_token: params[:token], attempt: params[:attempt] } - return head :not_found unless arguments[:attempt].present? && - CountsExportJobs.download_attempt(params[:job], owner, **arguments) - - file = CountsExportDownload.find(params[:token], owner) - unless file - CountsExportJobs.download_attempt(params[:job], owner, **arguments, status: 'unavailable') - return head :not_found - end - job = params[:job] - authority = HistoryAccess.user - first_check = true - stream = CountsExportDownload::ResponseBody.new(file.fetch('path')) do |status| - if status == 'active' - if first_check - first_check = false - fresh = authority && User.find_by(id: authority.id) - next false if authority && (!fresh || fresh.banned?) - - authority = fresh - HistoryAccess.set(user: authority) do - CountsExportDownload.find(arguments[:file_token], owner) && - CountsExportJobs.stream_active?(job, owner, **arguments) - end - else - CountsExportJobs.stream_active?(job, owner, **arguments) - end - else - HistoryAccess.set(user: authority) do - CountsExportJobs.download_attempt(job, owner, **arguments, status:) - end - end - end - unless CountsExportJobs.download_attempt(job, owner, **arguments, status: 'streaming') - stream.close - return head :conflict - end - response.headers['Cache-Control'] = 'private, no-store' - send_file_headers! filename: "counts-#{ file.fetch('resource') }.#{ file.fetch('format') }", - type: file['format'] == 'json' ? 'application/json' : 'text/csv', - disposition: 'attachment' - response.headers['Last-Modified'] = File.mtime(file.fetch('path')).httpdate - response.headers['X-Accel-Buffering'] = 'no' - self.response_body = stream - rescue IOError, SystemCallError - stream&.close - CountsExportJobs.download_attempt(params[:job], owner, **arguments, status: 'failed') - head :service_unavailable - end - private :confirmed_download - - def job_state(cancel:) - result = CountsExportJobs.status(params[:token], session[:counts_export_owner], cancel:) - return head :not_found unless result - - response.headers['Cache-Control'] = 'private, no-store' - render json: result - end - private :job_state - def index locale = resolve_locale! payload = request.request_parameters.except('authenticity_token') @@ -175,4 +113,73 @@ class CountsExportsController < ApplicationController rescue ArgumentError, TypeError, KeyError, JSON::ParserError head :bad_request end + + private + + def confirmed_download + owner = session[:counts_export_owner] + arguments = { file_token: params[:token], attempt: params[:attempt] } + if (!(arguments[:attempt].present?) || + !(CountsExportJobs.download_attempt(params[:job], owner, **arguments))) + return head :not_found + end + + file = CountsExportDownload.find(params[:token], owner) + unless file + CountsExportJobs.download_attempt(params[:job], owner, **arguments, status: 'unavailable') + return head :not_found + end + job = params[:job] + authority = HistoryAccess.user + first_check = true + state = lambda do |status| + if status == 'active' + if first_check + first_check = false + fresh = authority && User.find_by(id: authority.id) + next false if authority && (!fresh || fresh.banned?) + + authority = fresh + HistoryAccess.set(user: authority) do + CountsExportDownload.find(arguments[:file_token], owner) && + CountsExportJobs.stream_active?(job, owner, **arguments) + end + else + CountsExportJobs.stream_active?(job, owner, **arguments) + end + else + HistoryAccess.set(user: authority) do + CountsExportJobs.download_attempt(job, owner, **arguments, status:) + end + end + end + unless CountsExportJobs.download_attempt(job, owner, **arguments, status: 'streaming') + return head :conflict + end + response.headers['Cache-Control'] = 'private, no-store' + response.headers['Last-Modified'] = File.mtime(file.fetch('path')).httpdate + response.headers['X-Accel-Buffering'] = 'no' + send_file file.fetch('path'), + filename: "counts-#{ file.fetch('resource') }.#{ file.fetch('format') }", + type: file['format'] == 'json' ? 'application/json' : 'text/csv', + disposition: 'attachment' + # Keep Rails' bounded FileBody reader, but hide to_path from Rack::Sendfile: + # offloading would bypass the GET acknowledgement and cancellation checks. + stream = CountsExportDownload::ResponseBody.new(response.stream, &state) + self.response_body = stream + rescue IOError, SystemCallError + stream&.close + CountsExportJobs.download_attempt(params[:job], owner, **arguments, status: 'failed') + raise if response.committed? + + head :service_unavailable + end + + def job_state cancel: + result = CountsExportJobs.status(params[:token], session[:counts_export_owner], cancel:) + return head :not_found unless result + + response.headers['Cache-Control'] = 'private, no-store' + render json: result + end end diff --git a/backend/app/controllers/counts_tables_controller.rb b/backend/app/controllers/counts_tables_controller.rb index 15ea3f1..6a6f5bb 100644 --- a/backend/app/controllers/counts_tables_controller.rb +++ b/backend/app/controllers/counts_tables_controller.rb @@ -25,6 +25,11 @@ class CountsTablesController < ApplicationController rescue CountsTableQuery::TooManyColumns render json: { error: 'too_many_columns', max_columns: CountsTableQuery::MAX_COLUMNS }, status: :unprocessable_entity + rescue CountsPivotTotals::TimedOut + Rails.logger.warn('counts_query_timeout: pivot_totals') + render json: { error: 'counts_query_timeout', stage: 'totals', + timeout_seconds: CountsTableQuery::SQL_TIMEOUT_SECONDS }, + status: :service_unavailable rescue ActiveRecord::StatementTimeout => error Rails.logger.warn("counts_query_timeout: #{ error.class }") render json: { error: 'counts_query_timeout' }, status: :service_unavailable diff --git a/backend/app/services/counts_export_download.rb b/backend/app/services/counts_export_download.rb index 3f3f08d..96c1d24 100644 --- a/backend/app/services/counts_export_download.rb +++ b/backend/app/services/counts_export_download.rb @@ -15,26 +15,38 @@ class CountsExportDownload # Rack body deliberately has no to_path: sendfile middleware must not bypass ACK. class ResponseBody - def initialize(path, &state) - File.open(path, 'rb') { |file| file.stat } - @path = path - @stream = nil + def initialize body, &state + @body = body @closed = false @state = state end - def each + def each &block + return enum_for(:each) unless block return if @closed - @stream = File.open(@path, 'rb') + transmit(&block) + end + + def close + return if @closed + + @closed = true + @body.close if @body.respond_to?(:close) + end + + private + + def transmit first = true - while (chunk = @stream.read(64 * 1024)) + @body.each do |chunk| raise IOError, 'Download no longer authorised' unless @state.call('active') yield chunk if first - # This confirms yielding to Rack, never browser receipt or saving. - @state.call('started') + # Rack accepted a chunk; browser receipt and saving are not guaranteed. + raise IOError, 'Download acknowledgement failed' unless @state.call('started') + first = false end end @@ -46,11 +58,6 @@ class CountsExportDownload ensure close end - - def close - @closed = true - @stream.close if @stream && !@stream.closed? - end end def self.directory diff --git a/backend/app/services/counts_export_projection.rb b/backend/app/services/counts_export_projection.rb index e04df4c..6e521de 100644 --- a/backend/app/services/counts_export_projection.rb +++ b/backend/app/services/counts_export_projection.rb @@ -157,7 +157,7 @@ class CountsExportProjection def csv_display(value, key) if key.start_with?('aliases_') && value.is_a?(Array) - return value.map { fetch(_1, 'name') }.compact.join("\n") + return value.map { fetch(_1, 'name') }.compact.join(' ') end return @labels.fetch('bot', '') if key == 'created_by_user' && value.nil? return @labels.fetch("event_type:#{ value }", value) if key == 'event_type' diff --git a/backend/app/services/counts_pivot_totals.rb b/backend/app/services/counts_pivot_totals.rb index d516171..65cc362 100644 --- a/backend/app/services/counts_pivot_totals.rb +++ b/backend/app/services/counts_pivot_totals.rb @@ -1,5 +1,6 @@ # Re-aggregate distinct membership; tag row tuples alone use additive contributions. class CountsPivotTotals + class TimedOut < ActiveRecord::StatementTimeout; end def initialize(population, axes, row_headers, column_headers) @population = population @axes = axes @@ -27,20 +28,26 @@ class CountsPivotTotals retained = additive ? (0...@axes.rows.length).to_a | indexes : indexes joins = @axes.joins(retained.map { @axes.all[_1].fetch('field') }) scope = scope.joins(joins) unless joins.empty? + predicates = [] if row_depth.positive? tuples = headers.map { _1[:values] }.uniq predicate = tuples.map { Array.new(row_depth) { |i| - "(#{ expressions[i] }) <=> ?" + "a#{ indexes[i] } <=> ?" }.join(' AND ') }.map { "(#{ _1 })" }.join(' OR ') - scope = scope.where(predicate, *tuples.flatten) + predicates << @population.definition.model.sanitize_sql_array([ + tuples.empty? ? 'FALSE' : "(#{ predicate })", *tuples.flatten]) end if column_depth.positive? tuples = columns.map { _1[:values] }.uniq predicate = tuples.map { Array.new(column_depth) { |i| - "(#{ expressions[row_depth + i] }) <=> ?" + "a#{ indexes[row_depth + i] } <=> ?" }.join(' AND ') }.map { "(#{ _1 })" }.join(' OR ') - scope = scope.where(predicate, *tuples.flatten) + predicates << @population.definition.model.sanitize_sql_array([ + tuples.empty? ? 'FALSE' : "(#{ predicate })", *tuples.flatten]) end + # Filter projected coordinates, not repeated masking/time expressions. + # Each member still participates in the complete DISTINCT population. + selection = predicates.empty? ? '' : " WHERE #{ predicates.join(' AND ') }" grouped = indexes.each_with_index.map { |key, index| "a#{ key } AS v#{ index }" } @@ -56,19 +63,19 @@ class CountsPivotTotals output = ["#{ row_depth } AS row_depth", "#{ column_depth } AS column_depth"] + projected + padding + ['SUM(count) AS count'] grouping = coordinates.empty? ? '' : " GROUP BY #{ coordinates.join(', ') }" - "SELECT #{ output.join(', ') } FROM (#{ inner }) leaf#{ grouping }" + "SELECT #{ output.join(', ') } FROM (#{ inner }) leaf#{ selection }#{ grouping }" else inner = @axes.grouped_sql(scope, indexes) - "SELECT #{ fields.join(', ') } FROM (#{ inner }) grouped" + "SELECT #{ fields.join(', ') } FROM (#{ inner }) grouped#{ selection }" end end end return [] if queries.empty? sql = queries.map { "(#{ _1 })" }.join(' UNION ALL ') - @population.definition.model.connection.select_all( - "WITH population_members AS (#{ basis }) " + - "SELECT /*+ MAX_EXECUTION_TIME(10000) */ * FROM (#{ sql }) totals").map do |record| + statement = "WITH population_members AS (#{ basis }) " + + "SELECT /*+ #{ CountsTableQuery::SQL_HINTS.join(' ') } */ * FROM (#{ sql }) totals" + @population.definition.model.connection.select_all(statement).map do |record| row_depth = record['row_depth'].to_i column_depth = record['column_depth'].to_i { row: (0...row_depth).map { record["v#{ _1 }"] }, @@ -78,5 +85,7 @@ class CountsPivotTotals row_depth, column_depth), count: record['count'].to_i } end + rescue ActiveRecord::StatementTimeout + raise TimedOut, 'Counts pivot totals exceeded the query time limit' end end diff --git a/backend/app/services/counts_table_query.rb b/backend/app/services/counts_table_query.rb index 8585c36..92acf8c 100644 --- a/backend/app/services/counts_table_query.rb +++ b/backend/app/services/counts_table_query.rb @@ -1,6 +1,7 @@ # Common entrypoint for six version resources; legacy endpoints keep their contracts. class CountsTableQuery - SQL_HINTS = ['MAX_EXECUTION_TIME(10000)', + SQL_TIMEOUT_SECONDS = 30 + SQL_HINTS = ["MAX_EXECUTION_TIME(#{ SQL_TIMEOUT_SECONDS * 1000 })", "SET_VAR(optimizer_switch='semijoin=off')"].freeze MAX_COLUMNS = 100 class TooManyColumns < ArgumentError; end @@ -176,7 +177,7 @@ class CountsTableQuery end def details - query = population.scope.select(Arel.sql( + query = population.scope.optimizer_hints(*SQL_HINTS).select(Arel.sql( "(#{ population.current_latest_sql }) AS counts_latest")) order = sort_sql { detail_expression(_1) } query = query.order(Arel.sql((order + ['v.id ASC']).join(', '))) @@ -373,7 +374,9 @@ class CountsTableQuery return "#{ column } IS NULL ASC, (#{ column } <=> #{ masked }) ASC, " + "#{ expression } #{ direction.upcase }" end - if field == 'category' + if field == 'event_type' + expression = event_order_sql(column) + elsif field == 'category' # Tag's enum preserves the same category order as TagsController. cases = Tag.categories.keys.each_with_index.map { |category, index| "WHEN #{ @connection.quote(category) } THEN #{ index }" @@ -398,6 +401,7 @@ class CountsTableQuery def detail_expression(key) return "(#{ population.current_latest_sql })" if key == 'latest' + return event_order_sql(@definition.expression(key)) if key == 'event_type' return @definition.name_expression(key) if @definition.resource == 'tags' && @definition.name_column(key) aliases = { 'tags' => 'tags_json', 'parents' => @definition.resource == 'posts' ? @@ -409,6 +413,11 @@ class CountsTableQuery "CAST(#{ expression } AS CHAR) COLLATE utf8mb4_bin" end + def event_order_sql(expression) + "CASE #{ expression } WHEN 'create' THEN 0 WHEN 'update' THEN 1 " + + "WHEN 'discard' THEN 2 WHEN 'restore' THEN 3 ELSE 4 END" + end + def sort_sql @sort.flat_map do |key, direction| expression = yield key @@ -423,6 +432,7 @@ class CountsTableQuery values = conditions.map { _1['value'] }.compact.uniq labels = case field when 'tags' then tag_labels(values) + when 'parent_tag_ids' then parent_tag_labels(values) when 'user' actors = HistoryAccess.visible(population.scope, 'v').except(:select, :order) .select('v.created_by_user_id') @@ -452,6 +462,7 @@ class CountsTableQuery labels = tag_labels(values) @axis_names["a#{ index }"] = @tag_names labels + when 'parent_tag_ids' then parent_tag_labels(values) else {} end result["a#{ index }"] = labels @@ -459,6 +470,17 @@ class CountsTableQuery result end + def parent_tag_labels(values) + ids = values.flat_map { _1.to_s.split }.uniq + tag_labels(ids.map { "internal:#{ _1 }" }) + values.to_h do |value| + names = value.to_s.split.map do |id| + @tag_names.fetch("internal:#{ id }", {}).dig(locale.language_code, 'name') + end + [value.to_s, names.compact.join(' ')] + end + end + def tag_labels(values) internal = values.grep(/\Ainternal:/).map { _1.split(':').last.to_i } external = values.grep(/\Aexternal:/).map { _1.split(':').last.to_i } diff --git a/backend/spec/requests/counts_download_spec.rb b/backend/spec/requests/counts_download_spec.rb index 9d03297..5734869 100644 --- a/backend/spec/requests/counts_download_spec.rb +++ b/backend/spec/requests/counts_download_spec.rb @@ -14,7 +14,7 @@ RSpec.describe 'Counts native GET acknowledgement', type: :request do allow(CountsExportDownload).to receive(:directory).and_return(@counts_download_dir) end - def ready_job(format) + def ready_job(format, payload: 'payload') CountsExportJobs.pulse post '/counts/export/jobs', params: { resource: 'posts', definition: 'history', mode: 'details', columns: 'resource_id', locale: 'ja', format: } @@ -22,7 +22,7 @@ RSpec.describe 'Counts native GET acknowledgement', type: :request do job = response.parsed_body.fetch('job') signature = CountsExportJobs.verifier.verified(job, purpose: 'counts_export_job') owner = signature.fetch('owner') - exporter = instance_double(CountsExporter, resource: 'posts', format:, body: ['payload']) + exporter = instance_double(CountsExporter, resource: 'posts', format:, body: [payload]) file = HistoryAccess.set(user: nil) { CountsExportDownload.prepare(exporter, owner) } state = CountsExportJobs.read(signature.fetch('id')) state.merge!('status' => 'ready', 'token' => file) @@ -33,6 +33,28 @@ RSpec.describe 'Counts native GET acknowledgement', type: :request do end ['csv', 'json'].each do |format| + it "delivers every byte of a multi-chunk #{ format } file through HTTP" do + sign_in_as(create(:user, :member)) + rows = Array.new(5000) { |index| { id: index, name: '日本語,名称' } } + payload = if format == 'json' + rows.to_json + else + "id,name\r\n" + rows.map { CSV.generate_line(_1.values) }.join + end + job, file, attempt = ready_job(format, payload:) + owner = CountsExportJobs.verifier.verified(job, purpose: 'counts_export_job').fetch('owner') + metadata = HistoryAccess.set(user: nil) { CountsExportDownload.find(file, owner) } + expected = File.binread(metadata.fetch('path')) + expect(expected.bytesize).to be > 64 * 1024 + get "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:ok) + expect(response.body.bytesize).to eq(expected.bytesize) + expect(response.body.b).to eq(expected) + expect(response.body.b).to eq(payload.b) + get "/counts/export/jobs/#{ job }" + expect(response.parsed_body['download_status']).to eq('started') + end + it "acknowledges only an authorised #{ format } GET body, not HEAD or acceptance" do job, file, attempt = ready_job(format) get "/counts/export/jobs/#{ job }" diff --git a/backend/spec/requests/counts_table_spec.rb b/backend/spec/requests/counts_table_spec.rb index 5c54067..5e13aac 100644 --- a/backend/spec/requests/counts_table_spec.rb +++ b/backend/spec/requests/counts_table_spec.rb @@ -50,6 +50,32 @@ RSpec.describe 'Counts table API', type: :request do def versions(result) = result.fetch('versions').map { _1.fetch('version_no') } + it 'identifies a totals timeout without exposing SQL in the error response' do + allow_any_instance_of(CountsPivotTotals).to receive(:call) + .and_raise(CountsPivotTotals::TimedOut, 'private SQL') + get '/counts/query', params: base.merge(mode: 'summary', rows: 'created_at:month') + expect(response).to have_http_status(:service_unavailable) + expect(response.parsed_body).to eq('error' => 'counts_query_timeout', + 'stage' => 'totals', 'timeout_seconds' => 30) + expect(response.body).not_to include('private SQL') + end + + it 'applies the same optimizer safeguards to the paginated detail SELECT' do + statements = [] + listener = lambda do |_name, _start, _finish, _id, payload| + statements << payload[:sql] + end + ActiveSupport::Notifications.subscribed(listener, 'sql.active_record') do + result = query(tags: 'alpha', limit: 1) + expect(result['count']).to eq(2) + expect(versions(result)).to eq([1]) + end + details = statements.find { _1.include?('AS counts_latest') } + expect(details).not_to be_nil + expect(details).to include('MAX_EXECUTION_TIME(30000)', + "SET_VAR(optimizer_switch='semijoin=off')") + end + it 'separates latest and history and defaults to stable detail ordering and limit 100' do history = query expect(versions(history)).to eq([1, 2, 3]) diff --git a/backend/spec/services/counts_export_spec.rb b/backend/spec/services/counts_export_spec.rb index 052bf2e..de205d1 100644 --- a/backend/spec/services/counts_export_spec.rb +++ b/backend/spec/services/counts_export_spec.rb @@ -211,18 +211,37 @@ RSpec.describe 'Counts download acknowledgement' do end RSpec.describe CountsExportDownload::ResponseBody do + it 'survives Rails buffer enumeration without closing before the first byte' do + Dir.mktmpdir('counts-body-spec') do |directory| + path = File.join(directory, 'file') + payload = 'data' * 20_000 + File.binwrite(path, payload) + states = [] + source = ActionDispatch::Response::FileBody.new(path) + body = described_class.new(source) { |state| states << state; true } + response = ActionDispatch::Response.new + response.body = body + deferred = response.stream.to_ary + expect(states).to eq([]) + expect(deferred.to_a.join).to eq(payload) + expect(states.count('started')).to eq(1) + end + end + it 'acknowledges only after the first Rack yield and closes its bounded file body' do Dir.mktmpdir('counts-body-spec') do |directory| path = File.join(directory, 'file') File.binwrite(path, 'x' * (64 * 1024 + 3)) states = [] - body = described_class.new(path) { |state| states << state; true } + source = ActionDispatch::Response::FileBody.new(path) + body = described_class.new(source) { |state| states << state; true } chunks = [] body.each do |chunk| expect(states).not_to include('started') if chunks.empty? chunks << chunk end - expect(chunks.map(&:bytesize)).to eq([64 * 1024, 3]) + expect(chunks.join).to eq('x' * (64 * 1024 + 3)) + expect(chunks.map(&:bytesize).max).to be <= 64 * 1024 expect(states.count('started')).to eq(1) expect(body).not_to respond_to(:to_path) expect { body.close }.not_to raise_error @@ -234,7 +253,8 @@ RSpec.describe CountsExportDownload::ResponseBody do path = File.join(directory, 'file') File.binwrite(path, 'data') states = [] - body = described_class.new(path) { |state| states << state; true } + source = ActionDispatch::Response::FileBody.new(path) + body = described_class.new(source) { |state| states << state; true } expect { body.each { raise IOError, 'connection closed' } }.to raise_error(IOError) expect(states).to include('failed') expect(states).not_to include('started') diff --git a/backend/spec/services/counts_pivot_security_spec.rb b/backend/spec/services/counts_pivot_security_spec.rb index 6c3bc74..19d0eac 100644 --- a/backend/spec/services/counts_pivot_security_spec.rb +++ b/backend/spec/services/counts_pivot_security_spec.rb @@ -44,6 +44,20 @@ RSpec.describe 'Counts masked pivot contracts' do result.fetch(:pivot_totals).find { _1[:row].empty? && _1[:column].empty? }.fetch(:count) end + it 'preserves monthly name subtotals and grand totals across pages' do + settings = { 'resource' => 'tags', 'rows' => 'created_at:month name_ja' } + complete = result(admin, settings) + paged = result(admin, settings.merge('limit' => '1')) + expect(complete[:group_count]).to eq(3) + expect(grand(complete)).to eq(3) + expect(grand(paged)).to eq(3) + subtotal = paged[:pivot_totals].find do |cell| + cell[:row] == ['2023-01'] && cell[:column].empty? + end + expect(subtotal[:count]).to eq(3) + expect(paged[:cells].sum { _1[:count] }).to eq(1) + end + it 'preserves five tag occurrences after merging private tags and across pages' do [nil, guest, member, admin].each do |user| page = result(user) diff --git a/frontend/src/components/counts/CountsSummaryResults.tsx b/frontend/src/components/counts/CountsSummaryResults.tsx index 764dc83..53660a3 100644 --- a/frontend/src/components/counts/CountsSummaryResults.tsx +++ b/frontend/src/components/counts/CountsSummaryResults.tsx @@ -3,7 +3,8 @@ import { countsSummaryValue } from '@/lib/countsDisplay' import PrefetchLink from '@/components/PrefetchLink' import CountsSelectableTable from '@/components/counts/CountsSelectableTable' import CountsSortHeader from '@/components/counts/CountsSortHeader' -import { changeCountSort, countsMaskedValue, countsRequestParams, readCountSort, writeCountSort } +import { changeCountSort, countsMaskedValue, countsRequestParams, countsResourcePath, + readCountSort, writeCountSort } from '@/lib/counts' import { countsAxisLabel, useCountsDefinition } from '@/lib/countsDefinitions' @@ -97,7 +98,7 @@ const CountsSummaryTable = ( next.set ('mode', 'details') next.delete ('sort') next.delete ('page') - return `/counts/${resource}?${next}` + return `${countsResourcePath (resource)}?${next}` } const rowLabels = rowHeaders.map (header => { const labels = names (header, rows, 0) diff --git a/frontend/src/lib/counts.test.ts b/frontend/src/lib/counts.test.ts index 758d489..ade516c 100644 --- a/frontend/src/lib/counts.test.ts +++ b/frontend/src/lib/counts.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { columnsForCounts, countColumns, countsCSV, countsDefinitionParams, +import { columnsForCounts, countColumns, countsCSV, countsDefinitionParams, countsResourcePath, countsRequestParams, normaliseCountsParams, selectedCountChildren } from '@/lib/counts' import { countsErrorMessage, countsConditionValue, countsNestedIsScalar, countsNestedScalarText } from '@/lib/countsDisplay' @@ -14,6 +14,30 @@ import type { CountsCondition } from '@/lib/countsDefinitions' describe ('Counts URL contract', () => { + it ('distinguishes HTTP, network and internal failures without server details', () => { + const http = { isAxiosError: true, response: { status: 503, data: '' } } + expect (countsErrorMessage (http, 'ja').text).toContain ('HTTP 503') + expect (countsErrorMessage (http, 'ja').text).not.toContain ('private SQL') + expect (countsErrorMessage ({ isAxiosError: true }, 'ja').text) + .toBe ('APIサーバーとの通信に失敗しました。') + expect (countsErrorMessage (new Error ('private stack'), 'ja').text) + .toBe ('内部処理で問題が発生しました。') + }) + it ('explains a totals timeout using the server time limit', () => { + const failure = { isAxiosError: true, response: { status: 503, + data: { error: 'counts_query_timeout', stage: 'totals', timeout_seconds: 30 } } } + const text = countsErrorMessage (failure, 'ja').text + expect (text).toContain ('集計処理が制限時間を超えました。') + expect (text).toContain ('小計・総計の算出が30秒以内に完了しませんでした。') + expect (text).toContain ('集計期間や行・列の項目を減らして') + expect (countsErrorMessage (failure, 'ain-Kana').language).toBe ('ja') + }) + it ('uses hyphenated page paths for internal resource identifiers', () => { + expect (countsResourcePath ('nico_tags')).toBe ('/counts/nico-tags') + expect (countsResourcePath ('tag_descriptions')).toBe ('/counts/tag-descriptions') + expect (countsResourcePath ('tag_groups')).toBe ('/counts/tag-groups') + expect (countsResourcePath ('posts')).toBe ('/counts/posts') + }) it ('defaults to latest summary, limit 100, and no axes', () => { const params = normaliseCountsParams ('posts', new URLSearchParams (), postDefinition) expect (params.get ('definition')).toBe ('latest') diff --git a/frontend/src/lib/counts.ts b/frontend/src/lib/counts.ts index 4e55c20..720069b 100644 --- a/frontend/src/lib/counts.ts +++ b/frontend/src/lib/counts.ts @@ -12,6 +12,10 @@ export const countsMaskedValue = '__counts_private__' export type CountsResource = 'posts' | 'tags' | 'nico_tags' | 'tag_descriptions' | 'tag_groups' | 'materials' + +export const countsResourcePath = (resource: CountsResource): string => + `/counts/${resource.replaceAll ('_', '-')}` + export type CountsName = TagNameSnapshot & { languageCode: string; languageName?: string | null; scriptName?: string | null } export type CountsTag = { diff --git a/frontend/src/lib/countsDisplay.ts b/frontend/src/lib/countsDisplay.ts index 9c7a92d..1fe88c8 100644 --- a/frontend/src/lib/countsDisplay.ts +++ b/frontend/src/lib/countsDisplay.ts @@ -17,7 +17,8 @@ import type { LocaleCode } from '@/lib/locales' export const countsErrorMessage = (failure: unknown, locale: LocaleCode) => { const response = isApiError (failure) ? failure.response : null const body = (response?.data ?? failure) as { - error?: unknown; max_axes?: unknown; maxAxes?: unknown + error?: unknown; stage?: unknown; timeout_seconds?: unknown; timeoutSeconds?: unknown + max_axes?: unknown; maxAxes?: unknown max_columns?: unknown; maxColumns?: unknown } | null const code = body != null && typeof body === 'object' ? body.error : null const message = (key: string, values?: Record) => @@ -31,6 +32,11 @@ export const countsErrorMessage = (failure: unknown, locale: LocaleCode) => { max: maximum }) } if (code === 'private_condition') return message ('privateCondition') + if (code === 'counts_query_timeout' && body?.stage === 'totals') + { + const seconds = body.timeoutSeconds ?? body.timeout_seconds + if (typeof seconds === 'number') return message ('errors.totalsTimeout', { seconds }) + } if (code === 'counts_query_timeout' || code === 'generation_timeout') return message ('errors.queryTimeout') if (code === 'export_worker_unavailable' || code === 'worker_unavailable') @@ -40,7 +46,9 @@ export const countsErrorMessage = (failure: unknown, locale: LocaleCode) => { if (response?.status === 401) return message ('errors.authentication') if (response?.status === 403) return message ('errors.permission') if (response?.status === 400 || response?.status === 422) return message ('errors.input') - return message ('common:fetchError') + if (response != null) return message ('errors.http', { status: response.status }) + if (isApiError (failure)) return message ('errors.network') + return message ('errors.internal') } @@ -158,7 +166,7 @@ export const countsLogicalName = ( if (value == null) return { text: '' } if (field.startsWith ('aliases_') && Array.isArray (value)) return { text: value.map (entry => typeof entry === 'string' ? entry - : String (entry?.name ?? '')).join ('\n') } + : String (entry?.name ?? '')).join (' ') } if (field === 'sections') return { text: (countsSectionRanges (value, locale) ?? []).join ( translateUIMessage ('sections.separator', 'counts', locale).text) } @@ -528,6 +536,8 @@ export const countsSummaryValue = ( const logical = data.axisLabels?.[`a${index}`]?.[String (value)] if (axis.field === 'user') return message ('actorWithId', { name: logical || message ('notRegistered'), id: value }) + if (axis.field === 'parent_tag_ids') + return String (value) === '' ? '' : logical || message ('notRegistered') if (logical != null) return logical if (axis.field === 'tags') return message ('notRegistered') diff --git a/frontend/src/lib/countsState.ts b/frontend/src/lib/countsState.ts index 92e5aad..87e59ba 100644 --- a/frontend/src/lib/countsState.ts +++ b/frontend/src/lib/countsState.ts @@ -1,4 +1,4 @@ -import { isCountsState, selectedCountFields } from '@/lib/counts' +import { countsResourcePath, isCountsState, selectedCountFields } from '@/lib/counts' import { translateUIMessage } from '@/lib/i18n' import type { CountField, CountsResource, CountsResult } from '@/lib/counts' @@ -90,5 +90,5 @@ export const countsSummaryDrill = ( next.set ('before', (before != null && Date.parse (before) < Date.parse (last)) ? before : last) } - return `/counts/${resource}?${next}` + return `${countsResourcePath (resource)}?${next}` } diff --git a/frontend/src/locales/ain-Kana/counts.json b/frontend/src/locales/ain-Kana/counts.json index 2bffce5..960f471 100644 --- a/frontend/src/locales/ain-Kana/counts.json +++ b/frontend/src/locales/ain-Kana/counts.json @@ -1,4 +1,6 @@ { + "_untranslated": ["errors.totalsTimeout", "errors.http", "errors.network", + "errors.internal", "errors.exportStart"], "titles": { "posts": "ミンタㇻ　ウコピㇱキ", "tags": "シロシ　ウコピㇱキ", @@ -301,6 +303,11 @@ "axisLimit": "ウコピㇱキ　アㇰシㇱ　{{max}}　パㇰノ　ネ。イキㇼ　ネワ　コラム　オルㇱペ　ポンノ　カㇻ　ヤン。", "privateCondition": "ソモ　アヌカレ　オルㇱペ　アシッケウ　クス、タン　イフナラ　イアヌ　アニ　ウコピㇱキ　エアイカㇷ゚。", "errors": { + "http": "HTTP {{status}} エラーが発生しました。管理者にサーバーログの確認を依頼してください。", + "network": "APIサーバーとの通信に失敗しました。", + "internal": "内部処理で問題が発生しました。", + "exportStart": "エクスポートを開始できませんでした。", + "totalsTimeout": "集計処理が制限時間を超えました。\n\n小計・総計の算出が{{seconds}}秒以内に完了しませんでした。集計期間や行・列の項目を減らして再実行してください。", "queryTimeout": "ウコピㇱキ　イカㇻ　トキ　オケレ。イフナラ　イアヌ　ネ　ヤ　ウコピㇱキ　アㇰシㇱ　ポンノ　カㇻ　ワ　スイ　キ　ヤン。", "workerUnavailable": "エクスポート　ワーカー　イカㇻ　ソモ　キ。", "stopped": "エクスポート　イカㇻ　オケレ　ソモ　キ。スイ　キ　ヤン。", diff --git a/frontend/src/locales/ja/counts.json b/frontend/src/locales/ja/counts.json index 5e6d66f..fa537b2 100644 --- a/frontend/src/locales/ja/counts.json +++ b/frontend/src/locales/ja/counts.json @@ -181,7 +181,7 @@ "blank": "空欄", "additionalConditions": "追加条件", "changedField": "変更項目", - "eventType": "異動区分", + "eventType": "異動等の事実", "count": "件数", "outputTotal": "件数：{{value}} 件", "excludedRecords": "（閲覧権限のないレコード {{count}} 件が除外されました）", @@ -246,7 +246,7 @@ "sourceLanguage": "翻訳元の言語", "source_language_code": "翻訳元の言語", "source_version_no": "翻訳元の版番号", - "event_type": "異動区分", + "event_type": "異動等の事実", "body": "本文", "linked_tags": "対応タグ", "description": "説明", @@ -301,6 +301,11 @@ "axisLimit": "集計軸は最大{{max}}個です。行・列の項目を減らしてください。", "privateCondition": "非公開情報を保護するため、この抽出条件では集計できません。", "errors": { + "http": "HTTP {{status}} エラーが発生しました。管理者にサーバーログの確認を依頼してください。", + "network": "APIサーバーとの通信に失敗しました。", + "internal": "内部処理で問題が発生しました。", + "exportStart": "エクスポートを開始できませんでした。", + "totalsTimeout": "集計処理が制限時間を超えました。\n\n小計・総計の算出が{{seconds}}秒以内に完了しませんでした。集計期間や行・列の項目を減らして再実行してください。", "queryTimeout": "集計処理が時間制限を超えました。抽出条件や集計軸を減らして再実行してください。", "workerUnavailable": "エクスポートワーカーが稼働していません。", "stopped": "エクスポート処理が停止しました。再実行してください。", diff --git a/frontend/src/pages/counts/CountsResourcePage.tsx b/frontend/src/pages/counts/CountsResourcePage.tsx index 294d464..b6be97f 100644 --- a/frontend/src/pages/counts/CountsResourcePage.tsx +++ b/frontend/src/pages/counts/CountsResourcePage.tsx @@ -1,4 +1,5 @@ import { useQuery, useQueryClient } from '@tanstack/react-query' +import { LoaderCircle } from 'lucide-react' import { useEffect, useRef, useState } from 'react' import { useSearchParams } from 'react-router-dom' @@ -15,7 +16,8 @@ import Pagination from '@/components/common/Pagination' import MainArea from '@/components/layout/MainArea' import SidebarComponent from '@/components/layout/SidebarComponent' import { Button } from '@/components/ui/button' -import { countsRequestParams, countsDefinitionParams, countsExportPayload, fieldsForCounts, fetchCounts, +import { countsResourcePath, countsRequestParams, countsDefinitionParams, + countsExportPayload, fieldsForCounts, fetchCounts, isCountsState, normaliseCountsParams, selectedCountColumns } from '@/lib/counts' import { countsErrorMessage, countsExportLabels } from '@/lib/countsDisplay' import { isApiError } from '@/lib/api' @@ -52,10 +54,10 @@ const tooManyColumns = (failure: unknown): number | null => { const CountsResourceContent = ( - { resource, definition, definitionError }: { + { resource, definition, definitionError, definitionFailure }: { resource: CountsResource definition?: CountsDefinition - definitionError: boolean }, + definitionError: boolean; definitionFailure?: unknown }, ) => { const queryClient = useQueryClient () const [mounted, setMounted] = useState (false) @@ -106,7 +108,7 @@ const CountsResourceContent = ( const requestKey = request.toString () const [exporting, setExporting] = useState (false) const exportInFlight = useRef (false) - const [exportError, setExportError] = useState (false) + const [exportError, setExportError] = useState<{ failure: unknown } | null> (null) const { data: cachedData, error: queryError, dataUpdatedAt, isFetching: fetching, isPending: pending, isError: errorFlg, isPlaceholderData: placeholder } = useQuery ({ @@ -236,7 +238,7 @@ const CountsResourceContent = ( const waiting = running || (!definitionError && (definition == null || pending || fetching || placeholder || (!params.has ('execution_at') && !errorFlg))) - const failure = runError ? runFailure : queryError + const failure = definitionError ? definitionFailure : (runError ? runFailure : queryError) const failureMessage = countsErrorMessage (failure, locale) const columnLimit = runColumnLimit ?? tooManyColumns (queryError) @@ -263,7 +265,7 @@ const CountsResourceContent = ( exportInFlight.current = true setExporting (true) - setExportError (false) + setExportError (null) try { const axes = [...readCountState (params, 'rows', []), @@ -291,9 +293,9 @@ const CountsResourceContent = ( handle={handle} controls={controls}/> }) } - catch + catch (failure) { - setExportError (true) + setExportError ({ failure }) } finally { @@ -344,7 +346,7 @@ const CountsResourceContent = ( return (
  • {label (definition)} @@ -385,16 +387,20 @@ const CountsResourceContent = ( {label (`exportFormats.${format}`)})} }/>} - {exportError &&

    {label ('common:fetchError')}

    } + {exportError != null &&

    + {label ('errors.exportStart')}
    + {countsErrorMessage (exportError.failure, locale).text}

    } {columnLimit != null ?

    {label ('columnLimit.title')}

    {label ('columnLimit.message', { max: columnLimit })}

    : ((definitionError || errorFlg || runError) - &&

    {failureMessage.text}

    )} + &&

    {failureMessage.text}

    )}
    -
    +
    {mounted && definition != null && data != null && <>

    {label (resultMode === 'summary' ? 'outputRows' : 'outputTotal', { value: data.outputCount ?? '—' })} @@ -413,10 +419,12 @@ const CountsResourceContent = ( }

    {waiting &&
    + className="pointer-events-none absolute inset-0 flex items-center justify-center"> + {label ('shared:messages.loadingEllipsis')}
    }
    @@ -428,11 +436,12 @@ const CountsResourceContent = ( const CountsResourcePage = ({ resource }: { resource: CountsResource }) => { const locale = useLocale () - const { data, isError: errorFlg } = useQuery ({ + const { data, error, isError: errorFlg } = useQuery ({ queryKey: countsKeys.definition (resource, locale), queryFn: () => fetchCountsDefinition (resource, locale), staleTime: Infinity }) return - + }