diff --git a/backend/app/controllers/counts_exports_controller.rb b/backend/app/controllers/counts_exports_controller.rb index c966bd7..a366aa4 100644 --- a/backend/app/controllers/counts_exports_controller.rb +++ b/backend/app/controllers/counts_exports_controller.rb @@ -12,6 +12,15 @@ class CountsExportsController < ApplicationController render json: { error: 'export_worker_unavailable' }, status: :service_unavailable rescue CountsExportDownload::Busy head :too_many_requests + rescue CountsAxes::TooManyAxes + render json: { error: 'too_many_axes', max_axes: CountsAxes::MAX_AXES }, status: :bad_request + rescue CountsAxes::PrivateCondition + render json: { error: 'private_condition' }, status: :forbidden + rescue CountsTableQuery::TooManyColumns + render json: { error: 'too_many_columns', max_columns: CountsTableQuery::MAX_COLUMNS }, + status: :unprocessable_entity + rescue ActiveRecord::StatementTimeout + render json: { error: 'counts_query_timeout' }, status: :service_unavailable rescue ArgumentError, TypeError, KeyError, JSON::ParserError head :bad_request end @@ -48,6 +57,15 @@ class CountsExportsController < ApplicationController head :gateway_timeout rescue CountsExportDownload::StorageFull, Errno::ENOSPC head :insufficient_storage + rescue CountsAxes::TooManyAxes + render json: { error: 'too_many_axes', max_axes: CountsAxes::MAX_AXES }, status: :bad_request + rescue CountsAxes::PrivateCondition + render json: { error: 'private_condition' }, status: :forbidden + rescue CountsTableQuery::TooManyColumns + render json: { error: 'too_many_columns', max_columns: CountsTableQuery::MAX_COLUMNS }, + status: :unprocessable_entity + rescue ActiveRecord::StatementTimeout + render json: { error: 'counts_query_timeout' }, status: :service_unavailable rescue ArgumentError, TypeError, KeyError, JSON::ParserError head :bad_request end @@ -79,36 +97,43 @@ class CountsExportsController < ApplicationController CountsExportJobs.download_attempt(params[:job], owner, **arguments, status: 'unavailable') return head :not_found end - # Open before committing the response, but never buffer the entire export. - stream = File.open(file.fetch('path'), 'rb') + 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' - job = params[:job] - authority = HistoryAccess.user - self.response_body = Enumerator.new do |output| - begin - first = true - while (chunk = stream.read(64 * 1024)) - output << chunk - if first - HistoryAccess.set(user: authority) do - CountsExportJobs.download_attempt(job, owner, **arguments, status: 'started') - end - first = false - end - end - rescue IOError, SystemCallError => error - Rails.logger.warn("counts_download_stream_failure: #{ error.class }") - HistoryAccess.set(user: authority) do - CountsExportJobs.download_attempt(job, owner, **arguments, status: 'failed') - end - ensure - stream.close - end - end + 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 @@ -138,6 +163,15 @@ class CountsExportsController < ApplicationController filename: "counts-#{ exporter.resource }.#{ exporter.format }", type: exporter.format == 'json' ? 'application/json' : 'text/csv', disposition: 'attachment' + rescue CountsAxes::TooManyAxes + render json: { error: 'too_many_axes', max_axes: CountsAxes::MAX_AXES }, status: :bad_request + rescue CountsAxes::PrivateCondition + render json: { error: 'private_condition' }, status: :forbidden + rescue CountsTableQuery::TooManyColumns + render json: { error: 'too_many_columns', max_columns: CountsTableQuery::MAX_COLUMNS }, + status: :unprocessable_entity + rescue ActiveRecord::StatementTimeout + render json: { error: 'counts_query_timeout' }, status: :service_unavailable rescue ArgumentError, TypeError, KeyError, JSON::ParserError head :bad_request end diff --git a/backend/app/models/history_access.rb b/backend/app/models/history_access.rb index 0bbee84..9e73210 100644 --- a/backend/app/models/history_access.rb +++ b/backend/app/models/history_access.rb @@ -4,6 +4,7 @@ require 'digest' class HistoryAccess < ActiveSupport::CurrentAttributes attribute :user class Forbidden < StandardError; end + ARTIFACT_POLICY = 'counts-masked-summary-1'.freeze KEYS = { 'post_versions' => ['post_id'], 'tag_versions' => ['tag_id'], 'nico_tag_versions' => ['tag_id'], @@ -18,10 +19,12 @@ class HistoryAccess < ActiveSupport::CurrentAttributes end def self.authority - { 'policy_revision' => revision, 'admin' => admin?, 'actor_id' => user&.id } + { 'artifact_policy' => ARTIFACT_POLICY, 'policy_revision' => revision, + 'admin' => admin?, 'actor_id' => user&.id } end def self.authorised_artifact?(metadata) + return false unless metadata['artifact_policy'] == ARTIFACT_POLICY return false unless metadata['policy_revision'].is_a?(String) if metadata['admin'] admin? && user.id == metadata['actor_id'] @@ -72,13 +75,47 @@ class HistoryAccess < ActiveSupport::CurrentAttributes "ON reference_entry.reference_id = CAST(blocked.#{ key } AS CHAR))" end + def self.snapshot_reference(model, row, external: false) + key = external ? 'external_tag_id' : 'tag_id' + table = model.table_name + source = "JSON_TABLE(COALESCE(#{ row }.tags_json, JSON_ARRAY()), '$[*]' COLUMNS (" + + "tag_id BIGINT PATH '$.#{ key }', version_no INT PATH '$.version_no')) tag_reference" + match = 'reference_version.tag_id = tag_reference.tag_id' + if external + match += " AND reference_version.created_at <= #{ row }.created_at" + latest = "AND NOT EXISTS (SELECT 1 FROM #{ table } later_reference " + + 'WHERE later_reference.tag_id = reference_version.tag_id ' + + "AND later_reference.created_at <= #{ row }.created_at " + + 'AND later_reference.version_no > reference_version.version_no)' + else + 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 })" + end + def self.reference_sql(table, row) case table when 'post_versions' [['tag_versions', 'tag_id', 'tag_id'], ['nico_tag_versions', 'tag_id', 'external_tag_id']].map do |target, key, field| json_reference(target, key, "JSON_EXTRACT(#{ row }.tags_json, '$[*].#{ field }')") - end + [list_reference('post_versions', 'post_id', "#{ row }.parent_post_ids")] + end + [list_reference('post_versions', 'post_id', "#{ row }.parent_post_ids"), + snapshot_reference(TagVersion, row), + snapshot_reference(NicoTagVersion, row, external: true)] + when 'nico_tag_versions' + # Legacy linked_tags snapshots contain names, not IDs. Match recorded names + # conservatively; never expose a private linked name through an external tag. + names = "JSON_MERGE_PRESERVE(JSON_ARRAY(private_tag.name), " + + "COALESCE(JSON_EXTRACT(private_tag.names, '$**.name'), JSON_ARRAY()))" + ["NOT EXISTS (SELECT 1 FROM tag_versions private_tag " + + "JOIN (#{ discarded_keys('tag_versions', 'tag_id') }) blocked " + + "ON blocked.tag_id = private_tag.tag_id " + + "JOIN JSON_TABLE(#{ names }, '$[*]' COLUMNS (name TEXT PATH '$')) private_name " + + "WHERE private_name.name <> '' AND " + + "LOCATE(CONCAT(' ', private_name.name, ' '), " + + "CONCAT(' ', COALESCE(#{ row }.linked_tags, ''), ' ')) > 0)"] when 'tag_versions' [list_reference('tag_versions', 'tag_id', "#{ row }.parent_tag_ids")] when 'tag_description_versions' diff --git a/backend/app/services/counts_axes.rb b/backend/app/services/counts_axes.rb index c7c6af8..cdd2907 100644 --- a/backend/app/services/counts_axes.rb +++ b/backend/app/services/counts_axes.rb @@ -61,7 +61,11 @@ class CountsAxes expression, axis.fetch('grain', 'month'), @population.timezone) : expression end if !HistoryAccess.admin? && !PUBLIC_FIELDS.include?(field) - expression = "CASE WHEN #{ HistoryAccess.sql(@definition.model, 'v') } " + + visible = HistoryAccess.sql(@definition.model, 'v') + if field == 'changed_field' + visible = "(#{ visible }) AND (#{ HistoryAccess.sql(@definition.model, 'previous') })" + end + expression = "CASE WHEN #{ visible } " + "THEN CAST((#{ expression }) AS CHAR) ELSE '#{ MASKED_VALUE }' END" end expression @@ -72,8 +76,10 @@ class CountsAxes return 'COUNT(DISTINCT v.id)' unless rows.any? { _1['field'] == 'tags' } # Keep tag occurrences even when several private tags share one masked label. - "COUNT(DISTINCT v.id, COALESCE(CONCAT('internal:', tag_entry.tag_id), " + - "CONCAT('external:', tag_entry.external_tag_id)))" + values = ['v.id', "COALESCE(CONCAT('internal:', tag_entry.tag_id), " + + "CONCAT('external:', tag_entry.external_tag_id))"] + values << 'changed.field_key' if rows.any? { _1['field'] == 'changed_field' } + "COUNT(DISTINCT #{ values.join(', ') })" end def joins(fields = all.map { _1['field'] }) diff --git a/backend/app/services/counts_export_download.rb b/backend/app/services/counts_export_download.rb index 3123441..3f3f08d 100644 --- a/backend/app/services/counts_export_download.rb +++ b/backend/app/services/counts_export_download.rb @@ -13,6 +13,46 @@ class CountsExportDownload class TimedOut < StandardError; end class StorageFull < StandardError; end + # 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 + @closed = false + @state = state + end + + def each + return if @closed + + @stream = File.open(@path, 'rb') + first = true + while (chunk = @stream.read(64 * 1024)) + 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') + first = false + end + end + @state.call('failed') if first + rescue StandardError => error + Rails.logger.warn("counts_download_stream_failure: #{ error.class }") + @state.call('failed') + raise + ensure + close + end + + def close + @closed = true + @stream.close if @stream && !@stream.closed? + end + end + def self.directory Rails.root.join(ENV.fetch('COUNTS_EXPORT_DIRECTORY', 'tmp/counts_exports')) end diff --git a/backend/app/services/counts_export_jobs.rb b/backend/app/services/counts_export_jobs.rb index 7c575ef..9e2cc1b 100644 --- a/backend/app/services/counts_export_jobs.rb +++ b/backend/app/services/counts_export_jobs.rb @@ -45,6 +45,20 @@ class CountsExportJobs write(state) end + def self.failure_code(error) + case error + when Cancelled then 'cancelled' + when CountsAxes::TooManyAxes then 'too_many_axes' + when CountsAxes::PrivateCondition then 'private_condition' + when CountsTableQuery::TooManyColumns then 'too_many_columns' + when ActiveRecord::StatementTimeout then 'counts_query_timeout' + when CountsExportDownload::TimedOut then 'generation_timeout' + when CountsExportDownload::TooLarge then 'export_too_large' + when CountsExportDownload::StorageFull, Errno::ENOSPC then 'export_storage_full' + else 'generation_failed' + end + end + def self.directory = CountsExportDownload.directory.join('jobs') def self.verifier = Rails.application.message_verifier('counts_export_job') @@ -176,6 +190,12 @@ class CountsExportJobs return unless state && state['owner'] == owner && state['expires_at'] > Time.current.to_f expire_stalled(state) + if (state['status'] == 'ready' && + ['pending', 'streaming'].include?(state['download_status']) && + state.fetch('download_expires_at', 0) <= Time.current.to_f) + state['download_status'] = 'timeout' + write(state) + end if cancel && ['queued', 'running', 'ready'].include?(state['status']) state['status'] = state['status'] == 'running' ? 'cancelling' : 'cancelled' @@ -183,8 +203,11 @@ class CountsExportJobs state.delete('token') write(state) end - state.slice('status', 'phase', 'processed', 'total', 'format', 'token', 'error', + result = state.slice('status', 'phase', 'processed', 'total', 'format', 'token', 'error', 'updated_at', 'heartbeat_at', 'download_attempt', 'download_status') + result['max_axes'] = CountsAxes::MAX_AXES if state['error'] == 'too_many_axes' + result['max_columns'] = CountsTableQuery::MAX_COLUMNS if state['error'] == 'too_many_columns' + result end end @@ -198,18 +221,53 @@ class CountsExportJobs state['expires_at'] > Time.current.to_f && HistoryAccess.authorised_artifact?(state)) if attempt return unless state['download_attempt'] == attempt && state['token'] == file_token - return unless ['pending', 'started'].include?(state['download_status']) + return unless state['download_status'] == 'started' || + state.fetch('download_expires_at', 0) > Time.current.to_f - state['download_status'] = status if status + transitions = { 'streaming' => ['pending'], 'started' => ['streaming'], + 'failed' => ['pending', 'streaming', 'started'], + 'unavailable' => ['pending', 'streaming'] } + if status == 'active' + return state.slice('download_attempt', 'download_status', 'token') if + ['streaming', 'started'].include?(state['download_status']) + return + end + if status + return unless transitions.fetch(status, []).include?(state['download_status']) + + state['download_status'] = status + else + return unless state['download_status'] == 'pending' + end else + # Repeated acceptance POSTs do not invalidate an in-flight GET or issue duplicates. + if (state['download_status'] == 'started' || + (['pending', 'streaming'].include?(state['download_status']) && + state.fetch('download_expires_at', 0) > Time.current.to_f)) + return state.slice('download_attempt', 'download_status', 'token') + end state['download_attempt'] = SecureRandom.hex(24) state['download_status'] = 'pending' + state['download_expires_at'] = Time.current.to_f + 45 end write(state) state.slice('download_attempt', 'download_status', 'token') end end + # GET already verified authority. During streaming, observe cancellation without + # repeating the six-table policy revision aggregate for every 64KiB chunk. + def self.stream_active?(token, owner, file_token:, attempt:) + id = authorised_id(token, owner) + state = id && read(id) + state && state['owner'] == owner && state['status'] == 'ready' && + state['expires_at'] > Time.current.to_f && state['token'] == file_token && + state['download_attempt'] == attempt && + (state['download_status'] == 'started' || + (state['download_status'] == 'streaming' && + state.fetch('download_expires_at', 0) > Time.current.to_f)) + end + def self.checkpoint(id, processed = nil, total = nil, phase: nil) synchronise do state = read(id) @@ -398,7 +456,7 @@ class CountsExportJobs if current unless current['status'] == 'failed' current['status'] = error.is_a?(Cancelled) ? 'cancelled' : 'failed' - current['error'] = error.class.name + current['error'] = failure_code(error) end current['updated_at'] = Time.current.to_f write(current) diff --git a/backend/app/services/counts_pivot_totals.rb b/backend/app/services/counts_pivot_totals.rb index 29c81b7..42125c7 100644 --- a/backend/app/services/counts_pivot_totals.rb +++ b/backend/app/services/counts_pivot_totals.rb @@ -1,4 +1,4 @@ -# Re-aggregate the population, never add up already grouped cells (multi-value axes overlap). +# Re-aggregate distinct membership; tag row tuples alone use additive contributions. class CountsPivotTotals def initialize(population, axes, row_headers, column_headers) @population = population @@ -8,8 +8,12 @@ class CountsPivotTotals end def call - # MySQL's unlimited LIMIT prevents CTE merging: authorise once for all branches. - basis = @population.scope.except(:select, :order).select('v.*') + # Materialise full-cell membership once, including omitted multi-value axes. + membership = @population.scope.except(:select, :order) + joins = @axes.joins + membership = membership.joins(joins) unless joins.empty? + # Retain exactly the records contributing a full cell, including changed_field. + basis = membership.select('v.*').distinct .limit(18_446_744_073_709_551_615).to_sql queries = @rows.flat_map do |row_depth, headers| @columns.filter_map do |column_depth, columns| @@ -18,7 +22,7 @@ class CountsPivotTotals indexes = (0...row_depth).to_a + (0...column_depth).map { @axes.rows.length + _1 } expressions = indexes.map { @axes.expressions[_1] } - scope = @population.scope.except(:select, :order, :where, :from).from('authorised v') + scope = @population.scope.except(:select, :order, :where, :from).from('population_members v') additive = @axes.rows.any? { _1['field'] == 'tags' } retained = additive ? (0...@axes.rows.length).to_a | indexes : indexes joins = @axes.joins(retained.map { @axes.all[_1].fetch('field') }) @@ -47,9 +51,12 @@ class CountsPivotTotals if additive # Sum full row tuples, not page cells; preserve DISTINCT inside each tuple. leaf = retained.map { @axes.expressions[_1] } - selected = leaf.each_with_index.map { |expression, index| "#{ expression } AS k#{ index }" } + selected = leaf.each_with_index.map do |expression, index| + "#{ expression } AS k#{ index }" + end + selected << "#{ @axes.count_expression } AS count" inner = scope.group(*leaf.map { Arel.sql(_1) }) - .select(Arel.sql((selected + ["#{ @axes.count_expression } AS count"]).join(', '))).to_sql + .select(Arel.sql(selected.join(', '))).to_sql coordinates = indexes.map { "k#{ retained.index(_1) }" } projected = coordinates.each_with_index.map { |key, index| "#{ key } AS v#{ index }" } output = ["#{ row_depth } AS row_depth", "#{ column_depth } AS column_depth"] + @@ -67,7 +74,7 @@ class CountsPivotTotals sql = queries.map { "(#{ _1 })" }.join(' UNION ALL ') @population.definition.model.connection.select_all( - "WITH authorised AS (#{ basis }) " + + "WITH population_members AS (#{ basis }) " + "SELECT /*+ MAX_EXECUTION_TIME(10000) */ * FROM (#{ sql }) totals").map do |record| row_depth = record['row_depth'].to_i column_depth = record['column_depth'].to_i diff --git a/backend/app/services/counts_population.rb b/backend/app/services/counts_population.rb index 9dd0c4e..feaa257 100644 --- a/backend/app/services/counts_population.rb +++ b/backend/app/services/counts_population.rb @@ -57,7 +57,8 @@ class CountsPopulation "(previous.id IS NOT NULL AND NOT (v.#{ column } <=> previous.#{ column }))" end - # Unrestricted predicates are used only for numerical totals, never record output. + # Aggregate sources are unrestricted; CountsAxes masks their output coordinates. + # Never serialise raw records from an unrestricted population. def access_sql(model, row) @authorisation_flg ? HistoryAccess.sql(model, row) : 'TRUE' end @@ -67,6 +68,16 @@ class CountsPopulation key = entry.fetch('field') raise ArgumentError unless definition.axes(history).include?(key) + value = entry['value'] + unless value.nil? + valid = case definition.fields[key] + when :integer + value.is_a?(Integer) || (value.is_a?(String) && value.match?(/\A-?[0-9]+\z/)) + when :boolean then [true, false].include?(value) + else value.is_a?(String) + end + raise ArgumentError unless valid + end if entry.key?('grain') unless definition.fields[key] == :datetime && CountsAxes::GRAINS.include?(entry['grain']) raise ArgumentError diff --git a/backend/app/services/counts_table_export.rb b/backend/app/services/counts_table_export.rb index 12c2625..bd03bd8 100644 --- a/backend/app/services/counts_table_export.rb +++ b/backend/app/services/counts_table_export.rb @@ -253,6 +253,8 @@ class CountsTableExport end def summary_value(value, index, page) + return '********' if value == CountsAxes::MASKED_VALUE + field = @query.axes.all.fetch(index).fetch('field') return @labels.fetch('bot', '') if field == 'user' && value.nil? if ['language_code', 'source_language_code'].include?(field) diff --git a/backend/app/services/counts_table_query.rb b/backend/app/services/counts_table_query.rb index 62f291d..049dc72 100644 --- a/backend/app/services/counts_table_query.rb +++ b/backend/app/services/counts_table_query.rb @@ -49,12 +49,18 @@ class CountsTableQuery result = metadata.merge(mode == 'summary' ? summary : details) visible = mode == 'summary' ? result[:visible_output_count] : result[:count] totals = output_totals(visible) - result.merge(visible_output_count: visible, + response = result.merge(visible_output_count: visible, **totals, output_count_scope: 'before_authorisation', output_count_unit: 'regular_result_rows', page_grand_total_repeated: mode == 'summary' && axes.rows.any? && result[:group_count] > limit) + if mode == 'summary' + response.except!(:excluded_count, :excluded_count_scope) + response[:column_values_editable] = HistoryAccess.admin? || + !result.fetch(:column_values, []).flatten.include?(CountsAxes::MASKED_VALUE) + end + response end end @@ -228,17 +234,17 @@ class CountsTableQuery fields = QueryState.records(@params, 'conditions') + QueryState.records(@params, 'drill') private_fields = fields.any? { !CountsAxes::PUBLIC_FIELDS.include?(_1['field']) } - private_parameters = ['tags', 'tag_refs', 'tag', 'kind', 'resource_id', 'user_id', - 'name_scope'].any? do |key| + keys = ['tags', 'tag_refs', 'tag', 'kind', 'resource_id', 'user_id'] + private_parameters = keys.any? do |key| @params.any? { |name, value| (name == key || name.start_with?("#{ key }.")) && value != '' } end raise CountsAxes::PrivateCondition if private_fields || private_parameters ['column_order', 'measures'].each do |key| - next if QueryState.records(@params, key).empty? - columns = QueryState.axes(@params.fetch('cols', '')) - raise CountsAxes::PrivateCondition if columns.any? do |axis| - !CountsAxes::PUBLIC_FIELDS.include?(axis['field']) + # Coordinates are compared only to already masked expressions, never raw + # values. Public coordinates remain sortable; a guessed private ID cannot match. + raise CountsAxes::PrivateCondition if QueryState.records(@params, key).any? do |record| + record['value'] == CountsAxes::MASKED_VALUE end end end @@ -350,6 +356,11 @@ class CountsTableQuery def measure_relation values row_expressions = axes.expressions.first(axes.rows.length) source = summary_population.scope.except(:select, :order) + # Omitted multi-value column axes still determine full-cell membership. + membership = summary_population.scope.except(:select, :order) + all_joins = axes.joins + membership = membership.joins(all_joins) unless all_joins.empty? + source = source.where("v.id IN (#{ membership.select('v.id').distinct.to_sql })") fields = axes.rows + axes.columns.first(values.length) joins = axes.joins(fields.map { _1.fetch('field') }) source = source.joins(joins) unless joins.empty? @@ -367,7 +378,15 @@ class CountsTableQuery def axis_order_sql key, direction = 'asc', qualifier: nil column = qualifier ? "#{ qualifier }.#{ key }" : key expression = column - if axes.all.fetch(key.delete_prefix('a').to_i).fetch('field') == 'category' + field = axes.all.fetch(key.delete_prefix('a').to_i).fetch('field') + if !HistoryAccess.admin? && [:integer, :boolean].include?(@definition.fields[field]) + masked = @connection.quote(CountsAxes::MASKED_VALUE) + expression = "CASE WHEN #{ column } <=> #{ masked } THEN NULL " + + "ELSE CAST(#{ column } AS SIGNED) END" + return "#{ column } IS NULL ASC, (#{ column } <=> #{ masked }) ASC, " + + "#{ expression } #{ direction.upcase }" + end + if 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 }" @@ -418,7 +437,10 @@ class CountsTableQuery labels = case field when 'tags' then tag_labels(values) when 'user' - User.where(id: values).pluck(:id, :name).to_h.transform_keys(&:to_s) + actors = HistoryAccess.visible(population.scope, 'v').except(:select, :order) + .select('v.created_by_user_id') + User.where(id: values).where(id: actors) + .pluck(:id, :name).to_h.transform_keys(&:to_s) else {} end [field, labels] diff --git a/backend/spec/models/history_access_spec.rb b/backend/spec/models/history_access_spec.rb index 313d196..c087c35 100644 --- a/backend/spec/models/history_access_spec.rb +++ b/backend/spec/models/history_access_spec.rb @@ -67,3 +67,64 @@ RSpec.describe HistoryAccess do end end end + +RSpec.describe 'Counts artifact policy and external references' do + it 'rejects files created before the masked-summary policy' do + HistoryAccess.set(user: nil) do + metadata = HistoryAccess.authority.except('artifact_policy') + expect(HistoryAccess.authorised_artifact?(metadata)).to be(false) + end + end + + it 'does not expose a private linked tag name through an external history snapshot' do + tag = create(:tag, primary_name: 'private-linked-name') + TagVersion.create!(tag:, version_no: 1, event_type: 'discard', name: 'private-linked-name', + category: 'general', aliases: '', parent_tag_ids: '', created_at: Time.utc(2024), + names: { ja: { name: 'private-linked-name', script_code: 'Jpan', + auto_generated: false, aliases: [] } }) + external = create(:external_tag, name: 'public-external') + row = NicoTagVersion.create!(external_tag: external, version_no: 1, event_type: 'create', + name: 'nico:public-external', linked_tags: 'private-linked-name', + created_at: Time.utc(2024)) + post = create(:post) + snapshot = PostVersion.create!(post:, version_no: 1, event_type: 'create', + title: 'external reference', url: post.url, tags: '', + tags_json: [{ external_tag_id: external.id }], parent_post_ids: '', + created_at: Time.utc(2024, 2)) + HistoryAccess.set(user: nil) do + expect(HistoryAccess.visible(NicoTagVersion.where(id: row.id))).to be_empty + expect(HistoryAccess.visible(PostVersion.where(id: snapshot.id))).to be_empty + end + HistoryAccess.set(user: create(:user, :admin)) do + expect(HistoryAccess.visible(NicoTagVersion.where(id: row.id))).to exist + end + public_row = NicoTagVersion.create!(external_tag: external, version_no: 2, event_type: 'update', + name: 'nico:public-external', linked_tags: 'private-linked-name-extra', + created_at: Time.utc(2024, 7)) + HistoryAccess.set(user: nil) do + expect(HistoryAccess.visible(NicoTagVersion.where(id: public_row.id))).to exist + expect(HistoryAccess.visible(PostVersion.where(id: snapshot.id))).to be_empty + end + end + + it 'propagates the recorded tag version policy to its post snapshot' do + parent = create(:tag, primary_name: 'private parent') + child = create(:tag, primary_name: 'referencing child') + [[parent, 'discard', '', 'private parent'], + [child, 'create', parent.id.to_s, 'referencing child']].each do |tag, event, parents, name| + TagVersion.create!(tag:, version_no: 1, event_type: event, name:, + category: 'general', aliases: '', parent_tag_ids: parents, created_at: Time.utc(2023), + names: { ja: { name:, script_code: 'Jpan', + auto_generated: false, aliases: [] } }) + end + post = create(:post) + row = PostVersion.create!(post:, version_no: 1, event_type: 'create', title: 'snapshot', + url: post.url, tags: '', parent_post_ids: '', + tags_json: [{ tag_id: child.id, version_no: 1, name: 'referencing child', sections: [] }], + created_at: Time.utc(2024)) + HistoryAccess.set(user: nil) do + expect(HistoryAccess.visible(PostVersion.where(id: row.id))).to be_empty + end + end + +end diff --git a/backend/spec/requests/counts_download_spec.rb b/backend/spec/requests/counts_download_spec.rb new file mode 100644 index 0000000..0b1d72a --- /dev/null +++ b/backend/spec/requests/counts_download_spec.rb @@ -0,0 +1,98 @@ +require 'rails_helper' +require 'tmpdir' + +RSpec.describe 'Counts native GET acknowledgement', type: :request do + include ActiveSupport::Testing::TimeHelpers + around do |example| + Dir.mktmpdir('counts-download-request') do |path| + allow(CountsExportDownload).to receive(:directory).and_return(Pathname.new(path)) + example.run + end + end + + def ready_job(format) + CountsExportJobs.pulse + post '/counts/export/jobs', params: { resource: 'posts', definition: 'history', + mode: 'details', columns: 'resource_id', locale: 'ja', format: } + expect(response).to have_http_status(:accepted) + 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']) + file = HistoryAccess.set(user: nil) { CountsExportDownload.prepare(exporter, owner) } + state = CountsExportJobs.read(signature.fetch('id')) + state.merge!('status' => 'ready', 'token' => file) + CountsExportJobs.write(state) + post "/counts/export/jobs/#{ job }/download" + expect(response).to have_http_status(:ok) + [job, file, response.parsed_body.fetch('download_attempt')] + end + + ['csv', 'json'].each do |format| + it "acknowledges only an authorised #{ format } GET body, not HEAD or acceptance" do + job, file, attempt = ready_job(format) + get "/counts/export/jobs/#{ job }" + expect(response.parsed_body['download_status']).to eq('pending') + head "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:method_not_allowed) + get "/counts/export/jobs/#{ job }" + expect(response.parsed_body['download_status']).to eq('pending') + get "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:ok) + expect(response.headers['Content-Disposition']).to include("counts-posts.#{ format }") + expect(response.body).to eq('payload') + get "/counts/export/jobs/#{ job }" + expect(response.parsed_body['download_status']).to eq('started') + get "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:not_found) + end + end + + it 'rejects wrong attempts, cancellation and a newly discarded resource' do + job, file, attempt = ready_job('json') + get "/counts/export/files/#{ file }", params: { job:, attempt: 'wrong' } + expect(response).to have_http_status(:not_found) + delete "/counts/export/jobs/#{ job }" + expect(response).to have_http_status(:ok) + get "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:not_found) + job, file, attempt = ready_job('json') + record = create(:post) + PostVersion.create!(post: record, version_no: 1, event_type: 'discard', title: 'private', + url: record.url, tags: '', tags_json: [], parent_post_ids: '', created_at: Time.utc(2024)) + get "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:not_found) + expect(response.body).not_to include('payload', 'private') + end + it 'does not let a second browser session inspect the job or download its file' do + job, file, attempt = ready_job('json') + other = ActionDispatch::Integration::Session.new(Rails.application) + other.get("/counts/export/jobs/#{ job }") + expect(other.response).to have_http_status(:not_found) + other.get("/counts/export/files/#{ file }", params: { job:, attempt: }) + expect(other.response).to have_http_status(:not_found) + expect(other.response.body).not_to include('payload') + end + + it 'does not acknowledge an expired attempt' do + job, file, attempt = ready_job('json') + travel_to(Time.current + 46.seconds) do + get "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:not_found) + get "/counts/export/jobs/#{ job }" + expect(response.parsed_body['download_status']).to eq('timeout') + end + end + + it 'records a missing file without accepting the GET as started' do + job, file, attempt = ready_job('json') + owner = CountsExportJobs.verifier.verified(job, purpose: 'counts_export_job').fetch('owner') + metadata = HistoryAccess.set(user: nil) { CountsExportDownload.find(file, owner) } + File.unlink(metadata.fetch('path')) + get "/counts/export/files/#{ file }", params: { job:, attempt: } + expect(response).to have_http_status(:not_found) + get "/counts/export/jobs/#{ job }" + expect(response.parsed_body['download_status']).to eq('unavailable') + end + +end diff --git a/backend/spec/requests/counts_table_spec.rb b/backend/spec/requests/counts_table_spec.rb index 4c96c40..b847cfd 100644 --- a/backend/spec/requests/counts_table_spec.rb +++ b/backend/spec/requests/counts_table_spec.rb @@ -266,4 +266,12 @@ RSpec.describe 'Counts table API', type: :request do expect(json).to eq('error' => 'private_condition') end + it 'reports the combined axis limit without replacing it with a generic 400' do + get '/counts/query', params: base.merge(mode: 'summary', sort: '', + rows: 'resource_id version_no event_type user created_at:month', + cols: 'title url thumbnail_base video_ms') + expect(response).to have_http_status(:bad_request) + expect(response.parsed_body).to eq('error' => 'too_many_axes', 'max_axes' => 8) + end + end diff --git a/backend/spec/services/counts_export_spec.rb b/backend/spec/services/counts_export_spec.rb index 15e3b73..b0671cb 100644 --- a/backend/spec/services/counts_export_spec.rb +++ b/backend/spec/services/counts_export_spec.rb @@ -193,11 +193,49 @@ RSpec.describe 'Counts download acknowledgement' do arguments = { file_token: 'file-token', attempt: receipt.fetch('download_attempt') } expect(CountsExportJobs.download_attempt(token, 'other', **arguments)).to be_nil expect(CountsExportJobs.download_attempt(token, 'owner', **arguments.merge(file_token: 'wrong'))).to be_nil + CountsExportJobs.download_attempt(token, 'owner', **arguments, status: 'streaming') CountsExportJobs.download_attempt(token, 'owner', **arguments, status: 'started') expect(CountsExportJobs.status(token, 'owner')['download_status']).to eq('started') - CountsExportJobs.download_attempt(token, 'owner') + expect(CountsExportJobs.download_attempt(token, 'owner')['download_attempt']) + .to eq(receipt['download_attempt']) + CountsExportJobs.download_attempt(token, 'owner', **arguments, status: 'failed') + fresh = CountsExportJobs.download_attempt(token, 'owner') + expect(fresh['download_attempt']).not_to eq(receipt['download_attempt']) expect(CountsExportJobs.download_attempt(token, 'owner', **arguments)).to be_nil CountsExportJobs.status(token, 'owner', cancel: true) expect(CountsExportJobs.download_attempt(token, 'owner')).to be_nil end end + +RSpec.describe CountsExportDownload::ResponseBody do + 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 } + 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(states.count('started')).to eq(1) + expect(body).not_to respond_to(:to_path) + expect { body.close }.not_to raise_error + end + end + + it 'records a failed stream without converting an exception into a second HTTP response' do + Dir.mktmpdir('counts-body-spec') do |directory| + path = File.join(directory, 'file') + File.binwrite(path, 'data') + states = [] + body = described_class.new(path) { |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') + expect { body.close }.not_to raise_error + end + end +end diff --git a/backend/spec/services/counts_pivot_security_spec.rb b/backend/spec/services/counts_pivot_security_spec.rb new file mode 100644 index 0000000..5bb234e --- /dev/null +++ b/backend/spec/services/counts_pivot_security_spec.rb @@ -0,0 +1,169 @@ +require 'rails_helper' +require 'csv' + +RSpec.describe 'Counts masked pivot contracts' do + let(:admin) { create(:user, :admin) } + let(:member) { create(:user, :member) } + let(:guest) { create(:user, :guest) } + let(:params) do + { 'resource' => 'posts', 'definition' => 'history', 'mode' => 'summary', + 'locale' => 'ja', 'execution_at' => '2025-01-01T00:00:00Z', + 'timezone' => 'UTC', 'rows' => 'tags', 'cols' => 'event_type', 'sort' => '' } + end + let!(:tags) do + ['A', 'B', 'C'].map do |name| + tag = create(:tag, primary_name: name) + TagVersion.create!(tag:, version_no: 1, event_type: 'create', name:, + category: 'general', aliases: '', parent_tag_ids: '', + names: { ja: { name:, script_code: 'Jpan', auto_generated: false, aliases: [] } }, + created_at: Time.utc(2023)) + tag + end + end + + before do + [[0, 1], [0], [1, 2]].each_with_index do |indexes, index| + post = create(:post) + snapshot = indexes.map { { tag_id: tags[_1].id, version_no: 1, sections: [] } } + PostVersion.create!(post:, version_no: 1, event_type: 'create', + title: "hidden-title-#{ index }", url: post.url, tags: '', tags_json: snapshot, + parent_post_ids: '', created_by_user: member, created_at: Time.utc(2024, 1)) + next if index == 2 + + PostVersion.create!(post:, version_no: 2, event_type: 'discard', + title: "private-discard-#{ index }", url: post.url, tags: '', tags_json: [], + parent_post_ids: '', created_by_user: member, created_at: Time.utc(2024, 2)) + end + end + + def result(user, extra = {}) + HistoryAccess.set(user:) { CountsTableQuery.new(params.merge(extra)).call } + end + + def grand(result) + result.fetch(:pivot_totals).find { _1[:row].empty? && _1[:column].empty? }.fetch(:count) + end + + it 'preserves five tag occurrences after merging private tags and across pages' do + [nil, guest, member, admin].each do |user| + page = result(user) + expect(grand(page)).to eq(5) + expect(page[:cells].sum { _1[:count] }).to eq(5) + expect(page).not_to have_key(:excluded_count) + paged = result(user, 'limit' => '1') + expect(grand(paged)).to eq(5) + expect(result(user, 'limit' => '1', 'page' => '2')[:group_count]) + .to eq(page[:group_count]) + end + public_page = result(nil) + hidden = public_page[:cells].select { _1[:values].first == CountsAxes::MASKED_VALUE } + expect(hidden.sum { _1[:count] }).to eq(3) + expect(public_page[:axis_labels]['a0']).not_to have_key("internal:#{ tags.first.id }") + expect(public_page.to_json).not_to include('hidden-title', 'private-discard') + end + + it 'retains distinct membership when tags are not a row axis' do + [nil, admin].each do |user| + expect(grand(result(user, 'rows' => 'event_type', 'cols' => 'tags'))).to eq(3) + expect(result(user, 'rows' => '', 'cols' => '')[:cells].sole[:count]).to eq(5) + details = result(user, 'mode' => 'details', 'rows' => '', 'cols' => '') + expect(details[:count]).to eq(user == admin ? 5 : 1) + end + end + + it 'uses changed-record membership for totals and hides changes to private predecessors' do + post = PostVersion.order(:id).last.post + PostVersion.create!(post:, version_no: 2, event_type: 'update', title: 'changed-public-title', + url: post.url, tags: '', + tags_json: [{ tag_id: tags.last.id, version_no: 1, sections: [] }], + parent_post_ids: '', created_at: Time.utc(2024, 3)) + restored = PostVersion.order(:id).first.post + PostVersion.create!(post: restored, version_no: 3, event_type: 'restore', + title: 'restored-public-title', url: restored.url, tags: '', + tags_json: [{ tag_id: tags.first.id, version_no: 1, sections: [] }], parent_post_ids: '', + created_at: Time.utc(2024, 4)) + [nil, admin].each do |user| + page = result(user, 'rows' => 'tags changed_field', 'cols' => 'event_type') + expect(grand(page)).to eq(page[:cells].sum { _1[:count] }) + expect(page[:cells]).not_to be_empty + if user.nil? + expect(page[:cells].any? { _1[:values][1] == CountsAxes::MASKED_VALUE }).to be(true) + end + expect(page.to_json).not_to include('hidden-title', 'private-discard') + end + end + + it 'counts distinct changed records when tags are not additive row tuples' do + [nil, admin].each do |user| + page = result(user, 'rows' => 'changed_field', 'cols' => 'event_type') + expect(grand(page)).to eq(2) + if user.nil? + expect(page[:cells].map { _1[:values].first }.uniq).to eq([CountsAxes::MASKED_VALUE]) + end + # Changed discard snapshots have no tags: no full cell contributes here. + empty = result(user, 'rows' => 'changed_field', 'cols' => 'tags') + expect(empty[:cells]).to be_empty + expect(grand(empty)).to eq(0) + end + end + + it 'keeps NULL as a public data value rather than treating it as a rollup' do + post = create(:post) + PostVersion.create!(post:, version_no: 1, event_type: 'create', title: 'public bot', + url: post.url, tags: '', tags_json: [], parent_post_ids: '', created_at: Time.utc(2024, 5)) + page = result(nil, 'rows' => 'user', 'cols' => 'event_type') + expect(page[:cells].select { _1[:values].first.nil? }.sum { _1[:count] }).to eq(1) + expect(grand(page)).to eq(6) + expect(page[:row_headers]).to include(hash_including(values: [nil], kind: 'data')) + end + + it 'sorts public numeric values numerically alongside a masked group' do + post = PostVersion.order(:id).last.post + [2, 10].each do |number| + PostVersion.create!(post:, version_no: number, event_type: 'update', + title: 'public numeric sort', url: post.url, tags: '', tags_json: [], + parent_post_ids: '', created_at: Time.utc(2024, 5)) + end + page = result(nil, 'rows' => 'version_no', 'cols' => '') + values = page[:row_headers].select { _1[:kind] == 'data' }.map { _1[:values].first } + expect(values.reject { _1 == CountsAxes::MASKED_VALUE }.map(&:to_i)).to eq([1, 2, 10]) + expect(values.last).to eq(CountsAxes::MASKED_VALUE) + end + + it 'rejects private predicates and supplied measure coordinates before SQL labels escape' do + probes = [{ 'resource_id' => PostVersion.first.post_id.to_s }, + { 'tags' => 'A' }, { 'tag_refs.0.name' => 'A', 'tag_refs.0.tag_id' => tags.first.id.to_s }, + { 'conditions.0.field' => 'title', 'conditions.0.operator' => 'eq', + 'conditions.0.value' => 'hidden-title-0' }, + { 'drill.0.field' => 'tags', 'drill.0.value' => "internal:#{ tags.first.id }" }, + { 'cols' => 'tags', 'column_order.0.value' => CountsAxes::MASKED_VALUE }] + HistoryAccess.set(user: nil) do + probes.each do |probe| + expect { CountsTableQuery.new(params.merge(probe)) } + .to raise_error(CountsAxes::PrivateCondition) + end + end + end + + it 'keeps public column sorting usable while guessed private coordinates match nothing' do + first = result(nil, 'rows' => 'event_type', 'cols' => 'tags') + probe = result(nil, 'rows' => 'event_type', 'cols' => 'tags', + 'column_order.0.value' => "internal:#{ tags.first.id }") + expect(probe[:column_values]).to eq(first[:column_values]) + expect(probe[:cells]).to eq(first[:cells]) + expect(probe[:column_values_editable]).to be(false) + expect(probe.to_json).not_to include("internal:#{ tags.first.id }") + end + + it 'exports masked coordinates and safe CSV labels from the same pivot' do + HistoryAccess.set(user: nil) do + payload = params.merge('format' => 'json') + json = JSON.parse(CountsTableExport.new(payload).body.to_a.join) + expect(json['data'].sum { _1['kind'] == 'data' ? _1['count'] : 0 }).to eq(5) + expect(json.to_json).not_to include('hidden-title', 'private-discard') + csv = CountsTableExport.new(payload.merge('format' => 'csv')).body.to_a.join + expect(csv).to include('********') + expect(csv).not_to include(CountsAxes::MASKED_VALUE, 'hidden-title', 'private-discard') + end + end +end diff --git a/docs/counts-exports.md b/docs/counts-exports.md index ec8d202..3d00934 100644 --- a/docs/counts-exports.md +++ b/docs/counts-exports.md @@ -35,7 +35,9 @@ Web とワーカーは同一リリース、Rails secret、アプリケーショ - POST `/counts/export/jobs`：要求検証・実行時点の固定・署名付きジョブトークンの発行。 - GET `/counts/export/jobs/:token`：状態、実際の処理済みレコード数、判明している総数。 - DELETE `/counts/export/jobs/:token`：取消を永続記録。 -- GET `/counts/export/files/:token`：生成完了後の署名付きファイルトークンによる取得。 +- POST `/counts/export/jobs/:token/download`：セッション所有者の開始確認要求を登録。 +- GET `/counts/export/files/:token?job=...&attempt=...`：実 GET の認可・送出開始の確認。 +- GET `/counts/export/files/:token`：既存クライアント向けの認可付きダウンロード。 ジョブ・ファイルのトークンは作成元のセッションに紐付く。 既存の BAN・CSRF 制御を使用する。フロントは約800ms間隔で状態を取得する。 @@ -80,16 +82,22 @@ processed は CSV では実際に生成したデータ行数、JSON のクロス 通常の Counts 応答では、使用していない全履歴の population_count は計算しない。 キーは null、population_count_computed は false とし、0に置き換えない。 -現在条件の output_count は権限適用前、visible_output_count は権限適用後の -通常の集計行数（明細では明細行数）。小計・合計の物理行は含めない。 +summary は非公開履歴を含む同じ母集団を集計する。非公開の軸はサーバーの CASE 式で +単一の予約値へまとめてから GROUP BY し、output_count / visible_output_count はその +安全化した通常行数を表す。excluded_count は返さない。details は従来どおり +output_count が権限適用前、visible_output_count が適用後の明細行数である。 +小計・合計の物理行は行数に含めない。 CSV の物理行と JSON の pivot_cell には集計レコードを含むため、両者を混同しない。 除票境界・参照禁止集合は GROUP BY で識別キー単位にまとめる。 投稿の参照判定は、全除票履歴を JSON_CONTAINS / FIND_IN_SET で反復照合せず、 保存済みスナップショットから参照IDを JSON_TABLE で展開して照合する。 権限条件付きの母集団件数は同一 export インスタンス内で再利用する。 -多段小計は、認可済み母集団の CTE を MySQL の無制限 LIMIT で実体化させ、 -各枝が同じ認可判定をやり直すことを避ける。計数は COUNT(DISTINCT v.id) のまま。 +多段小計は、全軸の JOIN へ実際に寄与するレコードを CTE に保持し、各枝で同じ +母集合を使う。changed_field には直前版がある実変更だけが寄与する。タグが行軸に +ない場合は DISTINCT 投稿版を数える。タグが行軸にある場合は投稿版×タグを数え、 +全行軸の葉単位から小計・合計を求める。変更項目も行軸ならその寄与も保持する。 +private のタグ・変更項目をまとめても、内部の寄与は捨てず、ラベルだけを保護する。 スキーマ上、以下の追加インデックスが候補となる。migration は作成・実行していない。 @@ -122,6 +130,35 @@ systemd の EnvironmentFile と Web の環境を照合し、directory の owner アクセス制限、容量、期限切れ削除、worker heartbeat と restart を公開前に確認する。 この作業では systemctl・worker 起動・本番環境への適用を行っていない。 -リリース保留：非公開グループと複数値・変更項目軸の交差集計、生成ファイルへの秘匿値の -非混入、実 GET の middleware/proxy 経由の開始通知は、回帰テストと実環境の確認が必要。 -テスト・型チェック・build は未実行。 +これらのコード対策と回帰テストを追加しているが、テスト・型チェック・build は未実行。 +middleware/proxy 経由の転送は別途実環境で確認する。 + + +## 開始確認の競合と権限 + +開始受付は pending、実 GET の単独取得は streaming、最初の Rack yield 後は started。 +同じ job の重複受付は既存 attempt を返す。streaming への遷移はプロセス間ロックで +単独取得し、同一 attempt の二重 GET を拒否する。開始前の attempt は45秒で失効する。 +Frontend は30秒の独立タイマーで確認を打ち切る。一時的なポーリング通信断は同じ +attempt で再取得するが、別の GET は始めない。取消失敗後の再試行は利用者の再操作で行う。 + +ファイル・ジョブは owner、署名、期限、現在の権限 revision 及び artifact_policy を +検証する。最初の送出前にも現ユーザー・BAN・ファイル権限を再確認する。旧ポリシーの +生成物は再利用しない。送出中は各チャンクで取消状態を確認し、例外時はログと failed +状態を記録して body を閉じる。コミット後に head で別レスポンスを書かない。 + +ResponseBody に to_path を提供せず、sendfile middleware による ACK の迂回を避ける。 +Last-Modified と X-Accel-Buffering: no を設定するが、プロキシの動作まで保証しない。 +サーバーの最初の Rack yield は、クライアントの受信開始の厳密な証明ではない。 + +非管理者の summary では、公開集計属性は操作日時・異動区分。その他の属性に対する +抽出条件・補助タグ検索は拒否する。measures / column_order はマスク後の式だけと +比較するため、推測した private ID は一致しない。予約値そのものの条件指定は拒否する。 +private ID の正誤に応じて許可可否を変えないため、条件の一致探索を避けられる。 +非公開列を含む列順変更は API の column_values_editable でも不可と通知する。 +公開列の値を使うソートは引き続き利用できる。 +明細には元の認可を適用し、抽出条件自体や取得済みの全言語値を表示設定で改変しない。 +外部タグ履歴の linked_tags は名前しか記録していないため、非公開タグの記録名を含む +スナップショットを保守的に非公開とする。名前全体を空白区切りの境界で照合する。 +区切りが曖昧な旧データでは過剰な秘匿の可能性はあるが、参照 ID がない旧形式から +名前の公開可否を楽観的に推定しない。 diff --git a/docs/counts-release-checklist.md b/docs/counts-release-checklist.md new file mode 100644 index 0000000..a6d9530 --- /dev/null +++ b/docs/counts-release-checklist.md @@ -0,0 +1,148 @@ +# Counts リリース前確認 + +この作業では Git、テスト、lint、typecheck、build、migration、サーバー・worker・ +systemd の起動、本番適用を行っていない。コードの静的確認とテストコードの編集のみ。 +以下の実行と結果確認は利用者が行う。未実行の状態は GO としない。 + +## 対応箇所 + +- 全件集計と安全な軸：counts_axes.rb / counts_table_query.rb / counts_pivot_totals.rb。 + 非公開値は GROUP BY 前に統合。通常セル・ソート用集計・小計は同じ寄与母集合を使う。 +- 出力：counts_table_export.rb / countsDisplay.ts / CountsSummaryResults.tsx。 + CSV とコピーは伏字。JSON の非公開座標は予約値のみ。非公開セルはリンクを作らない。 +- 配布：counts_exports_controller.rb / counts_export_jobs.rb / counts_export_download.rb。 + owner/job/attempt、単独 GET、期限・現在権限・旧 artifact policy の拒否を確認する。 +- 表示：summary 行番号を廃止し「行数」を表示。details の行番号・件数は維持。 + 非公開列座標のソート・並び替え可否はサーバーの能力フラグへ従う。 +- テスト基盤：requestAnimationFrame の同期・時刻0固定を修正。JSDOM の Canvas は + 計測非対応を返すテスト専用モックを置く。実ブラウザーの計測動作は変更していない。 + +## 利用者によるテスト + +backend ディレクトリで提案する対象（未実行）： + +```sh +bundle exec rspec spec/requests/counts_table_spec.rb spec/requests/counts_download_spec.rb spec/services/counts_pivot_security_spec.rb spec/services/counts_export_spec.rb spec/models/history_access_spec.rb spec/requests/post_locale_filter_identity_spec.rb spec/requests/tag_description_wiki_spec.rb +``` + +frontend ディレクトリで提案する対象（未実行）： + +```sh +npm run test:run -- src/lib/counts.test.ts src/lib/api.test.ts src/pages/CountsResourcePage.test.tsx src/pages/help/HelpChangesPage.test.tsx src/components/counts/CountsControls.test.tsx src/components/counts/CountsSummaryResults.test.tsx src/components/counts/CountsExportProgress.test.tsx +``` + +SSR は frontend/server.ssr.test.mjs が dist/server/entry-server.js を読み込む。 +ソースの PageHead はサイト名付きの共通タイトルを使用している。古い成果物のまま +SSR テストを実行しても現行ソースの検証にならない。利用者が最新成果物を用意してから +`npm run test:ssr` を実行する。Codex はビルド・SSR テストとも実行していない。 +主要テストの後に既存の全テスト群でも回帰の有無を確認する。 + +## ブラウザーと本番切替前 + +- anonymous / guest / member / admin で同じ期間・公開条件を指定し、通常セル・総計の + 数値を比較する。安全なグループ数は権限で異なり得る。details は秘匿版を出さない。 +- 投稿 A,B / A / B,C のタグ行軸は5。タグを行軸から外した合計は distinct 投稿数。 + changed_field、NULL、複数段小計、ページ境界、CSV / JSON でも値を比較する。 +- 伏字セル、API 辞書、DOM、コピー、CSV / JSON に非公開の値・ID がないこと。 + 非公開セルのドリルダウンはなく、private 条件の summary は安全な403となること。 +- 8軸・100列の専用エラー、SQL timeout、worker 不在、認証・権限、取消失敗を確認する。 +- CSV / JSON とも ready / 100% だけでは閉じず、実 GET の matching attempt が + started となってから閉じること。通信遮断・取消・再描画・タイムアウトも確認する。 + サーバー送出開始はブラウザーの受信完了・保存完了を保証しない。 +- PC・スマホで独立／共通スクロール、D&D、列幅、セル選択・文字列選択・キャレット、 + 内表 JSON の折返し、ページタイトル、Counts の noindex・先読み禁止を確認する。 + locale 切替後も対象 ID と集計値は同一で、全言語の登録値を保持すること。 +- Web と worker の同一リリース・secret・DB・COUNTS_EXPORT_DIRECTORY、所有者、 + flock / atomic rename、容量、期限切れ掃除、heartbeat、ログ、再起動を照合する。 +- systemd unit の User / WorkingDirectory / EnvironmentFile / PATH を実環境と照合する。 + 設定適用・サービス起動・本番切替はこの作業では行っていない。 +- ロール変更・logout・新しい discard 後に、生成済みファイルが再取得できないこと。 +- ロールバック時も公開ダウンロードを止め、旧生成物を再配布しない。古いコードへ戻すと + artifact_policy やマスクの保護を失う可能性があるため、無条件に巻き戻さない。 + +P0 の回帰テストと利用者の動作確認が済むまで判定は NO-GO。 + +## 静的な対応状況 + +| 対象 | 状態 | 根拠 | +| --- | --- | --- | +| 非公開軸・参照・全件母集団 | 対応済み（静的確認のみ） | HistoryAccess / CountsAxes / CountsTableQuery と回帰コード | +| タグ・変更項目・小計 | 対応済み（静的確認のみ） | CountsPivotTotals と全セル・ソートの母集合の統一 | +| API / CSV / JSON / コピー / URL | 対応済み（静的確認のみ） | マスク後の値のみを表示・比較し、予約座標の条件化を拒否 | +| GET ACK・競合・取消・期限 | 対応済み（静的確認のみ） | サーバー状態遷移、単独 GET、独立タイマー、回帰コード | +| P1・既知テストのソース上の問題 | 対応済み（静的確認のみ） | 行番号、行数、エラー、RAF、fixture、title 契約 | +| テスト成功・実ブラウザー・本番稼働 | 判定不能 | 実行禁止のため未検証 | + +## 今回の編集ファイル + +すべて Counts 本体に必要な変更。共有箇所の例外は HistoryAccess の参照秘匿と、 +api.ts の Abort/CSRF 待機・native download の機構であり、全域の安全性にも必要。 +SQL の速度改善・索引追加は別 Issue とし、今回 migration は追加していない。 + +### Backend App + +- backend/app/models/history_access.rb：外部タグの記録名参照と、投稿の記録済み参照版にも + 同じ可視性判定を適用。旧 artifact policy の生成物を拒否。 +- backend/app/services/counts_population.rb：軸条件の型・予約値の検証。 +- backend/app/services/counts_axes.rb：マスク前の内部寄与を計数し、直前版が非公開の場合も + changed_field の名前を秘匿。 +- backend/app/services/counts_table_query.rb：通常セル・ソートの寄与母集合、整数順、 + 条件表示名、公開列座標だけの操作、summary メタデータを整合。 +- backend/app/services/counts_pivot_totals.rb：全セルに寄与する母集合と、タグ行軸の加算。 +- backend/app/services/counts_table_export.rb：CSV の予約値を伏字化。 +- backend/app/services/counts_export_jobs.rb：開始確認の排他・状態遷移・失効と安全なエラーコード。 +- backend/app/services/counts_export_download.rb：64KiB の Rack body、開始確認、取消・close。 +- backend/app/controllers/counts_exports_controller.rb：GET 直前・送出直前の再認可と専用エラー。 + +### Frontend App + +- frontend/src/lib/api.ts：CSRF 待機中も中断可能にし、iframe の寿命・失敗通知を整理。 +- frontend/src/lib/counts.ts：マスク予約値と列操作の API 型。 +- frontend/src/lib/countsDisplay.ts：マスク表示と既知エラーの i18n 解決。 +- frontend/src/components/counts/CountsExportProgress.tsx：一つのジョブを維持し、matching ACK、 + 独立期限、取消・再描画・通信断を処理。保存完了の成功表示はしない。 +- frontend/src/components/counts/CountsSummaryResults.tsx：行番号コードの撤去、私的セルの + ドリルダウン抑止、公開列のソート維持。 +- frontend/src/components/counts/CountsSortHeader.tsx：禁止座標のソート抑止と伏字の理由表示。 +- frontend/src/pages/CountsResourcePage.tsx：専用エラーと、初回失敗後にも再実行できる状態。 +- frontend/src/locales/ja/counts.json：承認されたエラー文言。 +- frontend/src/locales/ain-Kana/counts.json：同じキーと未翻訳・日本語 fallback の管理。 + +### テスト・テスト専用基盤 + +- backend/spec/services/counts_pivot_security_spec.rb（新規）：4権限、タグ寄与5、distinct、 + changed_field、private group、NULL、整数順、ページ、座標探索、JSON/CSV。 +- backend/spec/requests/counts_download_spec.rb（新規）：CSV/JSON の実 GET、HEAD、二重取得、 + 別セッション、失効、取消、ファイル不在、新しい discard。 +- backend/spec/services/counts_export_spec.rb：owner/job/attempt の ACK と Rack body の失敗。 +- backend/spec/models/history_access_spec.rb：旧 policy、外部タグ参照、記録済みタグ参照版。 +- backend/spec/requests/counts_table_spec.rb：専用軸数エラー。 +- frontend/src/components/counts/CountsExportProgress.test.tsx（新規）：matching ACK、StrictMode、 + 再描画、開始受付失敗、通信断、独立期限、取消の競合・再試行、unmount、iframe 失敗。 +- frontend/src/components/counts/CountsSummaryResults.test.tsx（新規）：伏字・理由・コピー・旧行番号。 +- frontend/src/components/counts/CountsControls.test.tsx：summary 行番号廃止と表示件数選択。 +- frontend/src/pages/CountsResourcePage.test.tsx：初期エラーからの再実行、サイト名付き title。 +- frontend/src/lib/api.test.ts：共有 CSRF 待機中の中断、iframe の解放・重複失敗通知。 +- frontend/src/lib/counts.test.ts：理由別エラーと任意のサーバー文字列を表示しないこと。 +- frontend/src/test/setup.ts：時刻が進む非同期 RAF と Canvas のテスト専用 fallback。 +- frontend/server.ssr.test.mjs：共通 title の既存 assertion を維持し canonical と秘匿を補足。 + +### 文書 + +- docs/counts-exports.md：実装と一致する計数・ACK・権限・運用上の限界。 +- docs/counts-release-checklist.md：この一覧と利用者の検証・公開判定手順。 + +## 既知失敗の静的な確認結果 + +- counts_table_spec.rb：TagVersion.names の auto_generated、script_code、aliases があることを + 確認。新しい投稿タグ fixture にも schema 必須の sections を設定。 +- post_locale_filter_identity_spec.rb：検索文字列で基準集合を取得した後、tag_filters の + 応答から references を定義して構造化 URL を作る修正を維持。 +- tag_description_wiki_spec.rb：first / last とも対象 ID に限定した delete_all を維持。 +- CountsResourcePage.test.tsx / HelpChangesPage.test.tsx：共有 RAF の同期・時刻0固定を修正。 + Help の辞書応答は実 getHelpChanges が language を付ける契約も照合済み。 +- SSR title：現行ソースはサイト名を含む。dist を使うテストの assertion は削除していない。 + +上記はソース照合の結果であり、過去の失敗件数が0になったという実行結果ではない。 +既知の未修正 P0 を残して GO とする判断はしていない。現時点の阻害事項は、ユーザーに +よる MySQL・ブラウザー・worker/proxy を含む実検証がまだ行われていないことである。 diff --git a/frontend/server.ssr.test.mjs b/frontend/server.ssr.test.mjs index ab1b332..20d6ef2 100644 --- a/frontend/server.ssr.test.mjs +++ b/frontend/server.ssr.test.mjs @@ -345,6 +345,9 @@ test ('counts SSR uses query/definitions, noindex and the common site title', as assert.doesNotMatch (result.html, /counts:|columns\.changedAt|fields\.actor/) assert.match (result.head, /]*name="robots"[^>]*content="noindex"/) assert.match (result.head, /]*>[^<]*集計[^<]*\|[^<]*<\/title>/) + assert.match (result.head, /rel="canonical"/) + assert.doesNotMatch (result.html, /private-discard|hidden-title|__counts_private__/) + assert.ok (requests.some (request => { const url = new URL (request.url, origin) return url.pathname === '/counts/query' && url.searchParams.get ('resource') === resource diff --git a/frontend/src/components/counts/CountsControls.test.tsx b/frontend/src/components/counts/CountsControls.test.tsx index 92067b2..25932d8 100644 --- a/frontend/src/components/counts/CountsControls.test.tsx +++ b/frontend/src/components/counts/CountsControls.test.tsx @@ -117,3 +117,17 @@ it ('derives mixed parent selection and permits selecting all children again aft 'tags.name_ja', 'tags.name_ain', 'tags.aliases_ja', 'tags.aliases_ain']) expect (params.get ('columns')).toBe ('tags') }) + + +it ('offers row numbers only in details and keeps limit in display settings', () => { + const view = renderControls (appliedParams ({ mode: 'summary', row_numbers: '1' })) + expect (screen.queryByRole ('checkbox', { name: '行番号' })).not.toBeInTheDocument () + const limit = screen.getByRole ('combobox', { name: '表示件数' }) + expect (Array.from (limit.querySelectorAll ('option')).map (option => option.value)) + .toEqual (['20', '50', '100']) + expect (limit).toHaveValue ('100') + expect (limit.querySelector ('span')).toBeNull () + view.unmount () + renderControls (appliedParams ({ row_numbers: '1' })) + expect (screen.getByRole ('checkbox', { name: '行番号' })).toBeChecked () +}) diff --git a/frontend/src/components/counts/CountsExportProgress.test.tsx b/frontend/src/components/counts/CountsExportProgress.test.tsx new file mode 100644 index 0000000..923fdf2 --- /dev/null +++ b/frontend/src/components/counts/CountsExportProgress.test.tsx @@ -0,0 +1,198 @@ +import { act, cleanup, screen } from '@testing-library/react' +import { StrictMode } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import CountsExportProgress from '@/components/counts/CountsExportProgress' +import { renderWithProviders } from '@/test/render' + +const api = vi.hoisted (() => ({ + apiPost: vi.fn (), apiGet: vi.fn (), apiDelete: vi.fn (), startAPIDownload: vi.fn () })) +vi.mock ('@/lib/api', async original => ({ + ...await original (), ...api })) + +const ready = { status: 'ready', processed: 5, total: 5, token: 'file' } +const receipt = { downloadAttempt: 'attempt', downloadStatus: 'pending', token: 'file' } +const controls = () => ({ close: vi.fn (), setActions: vi.fn (), confirm: vi.fn () }) +const flush = async (milliseconds = 0) => act (async () => { + await vi.advanceTimersByTimeAsync (milliseconds) +}) + +beforeEach (() => { + vi.useFakeTimers () + Object.values (api).forEach (mock => mock.mockReset ()) + api.apiPost.mockImplementation (async (path: string) => + path.endsWith ('/download') ? receipt : { job: 'job' }) + api.apiGet.mockResolvedValue (ready) + api.apiDelete.mockResolvedValue ({ status: 'cancelled', processed: 5, total: 5 }) + api.startAPIDownload.mockReturnValue (vi.fn ()) +}) +afterEach (() => { cleanup (); vi.useRealTimers () }) + +const renderProgress = (format = 'csv') => { + const actions = controls () + const handle = { cancel: async () => false } + const element = (next = actions) => + + + return { ...renderWithProviders (element ()), actions, handle, element } +} + +describe ('Counts native download acknowledgement', () => { + it.each (['csv', 'json']) ('closes %s after the matching GET acknowledgement', async format => { + api.apiGet.mockResolvedValueOnce (ready).mockResolvedValue ({ ...ready, + downloadAttempt: 'attempt', downloadStatus: 'started' }) + const view = renderProgress (format) + await flush () + expect (api.apiPost).toHaveBeenCalledTimes (2) + expect (api.startAPIDownload).toHaveBeenCalledTimes (1) + expect (view.actions.close).not.toHaveBeenCalled () + const query = api.startAPIDownload.mock.calls[0][3] as URLSearchParams + expect (Object.fromEntries (query)).toEqual ({ job: 'job', attempt: 'attempt' }) + const next = controls () + view.rerender (view.element (next)) + await flush (800) + expect (next.close).toHaveBeenCalledTimes (1) + expect (api.startAPIDownload).toHaveBeenCalledTimes (1) + }) + + it ('times out independently of pending status requests', async () => { + api.apiGet.mockResolvedValueOnce (ready).mockImplementation (() => new Promise (() => {})) + const view = renderProgress () + await flush () + await flush (30_000) + expect (screen.getByRole ('alert')).toHaveTextContent ('タイムアウト') + expect (view.actions.close).not.toHaveBeenCalled () + expect (api.startAPIDownload.mock.results[0].value).toHaveBeenCalled () + }) + + it ('retains the modal after acceptance failure without repeated POSTs', async () => { + api.apiPost.mockResolvedValueOnce ({ job: 'job' }) + .mockRejectedValueOnce (new Error ('offline')) + const view = renderProgress () + await flush () + expect (screen.getByRole ('alert')).toHaveTextContent ('開始できません') + await flush (40_000) + expect (api.apiPost).toHaveBeenCalledTimes (2) + expect (api.startAPIDownload).not.toHaveBeenCalled () + expect (view.actions.close).not.toHaveBeenCalled () + }) + + it ('retries a temporary status transport failure without a second native GET', async () => { + api.apiGet.mockResolvedValueOnce (ready).mockRejectedValueOnce (new Error ('offline')) + .mockResolvedValue ({ ...ready, downloadAttempt: 'attempt', downloadStatus: 'started' }) + const view = renderProgress () + await flush () + await flush (2800) + expect (view.actions.close).toHaveBeenCalledTimes (1) + expect (api.startAPIDownload).toHaveBeenCalledTimes (1) + }) + + it ('cancels an acceptance in flight and never starts the resulting receipt', async () => { + let resolve!: (value: typeof receipt) => void + api.apiPost.mockResolvedValueOnce ({ job: 'job' }).mockImplementationOnce ( + () => new Promise (complete => { resolve = complete })) + const view = renderProgress () + await flush () + await act (async () => { expect (await view.handle.cancel ()).toBe (true) }) + await act (async () => { resolve (receipt) }) + await flush (1000) + expect (api.startAPIDownload).not.toHaveBeenCalled () + expect (view.actions.close).not.toHaveBeenCalled () + }) + + it ('releases an unmounted iframe and ignores late acknowledgements', async () => { + const view = renderProgress () + await flush () + const release = api.startAPIDownload.mock.results[0].value + view.unmount () + await flush () + expect (release).toHaveBeenCalledTimes (1) + await flush (40_000) + expect (view.actions.close).not.toHaveBeenCalled () + expect (api.startAPIDownload).toHaveBeenCalledTimes (1) + }) + + it ('keeps authentication expiry and iframe errors visible', async () => { + const view = renderProgress () + await flush () + act (() => { api.startAPIDownload.mock.calls[0][2] () }) + expect (screen.getByRole ('alert')).toHaveTextContent ('開始できません') + await flush (40_000) + expect (view.actions.close).not.toHaveBeenCalled () + }) + it ('shows the worker-specific reason instead of a download-success message', async () => { + api.apiPost.mockRejectedValueOnce ({ isAxiosError: true, + response: { status: 503, data: { error: 'export_worker_unavailable' } } }) + const view = renderProgress () + await flush () + expect (screen.getByRole ('alert')).toHaveTextContent ('ワーカーが稼働していません') + expect (view.actions.close).not.toHaveBeenCalled () + expect (await view.handle.cancel ()).toBe (true) + }) + + it ('shows revoked authority and does not close after a successful acceptance', async () => { + api.apiGet.mockResolvedValueOnce (ready).mockRejectedValue ({ isAxiosError: true, + response: { status: 404 } }) + const view = renderProgress () + await flush () + await flush (800) + expect (screen.getByRole ('alert')).toHaveTextContent ('認可が失われたか') + expect (view.actions.close).not.toHaveBeenCalled () + }) + + it ('stops after cancellation failure and permits an explicit cancellation retry', async () => { + const view = renderProgress () + await flush () + api.apiDelete.mockRejectedValueOnce (new Error ('offline')) + await act (async () => { expect (await view.handle.cancel ()).toBe (false) }) + const requests = api.apiGet.mock.calls.length + await flush (40_000) + expect (api.apiGet).toHaveBeenCalledTimes (requests) + expect (screen.getByRole ('alert')).toBeInTheDocument () + await act (async () => { expect (await view.handle.cancel ()).toBe (true) }) + expect (api.apiDelete).toHaveBeenCalledTimes (2) + expect (api.startAPIDownload).toHaveBeenCalledTimes (1) + }) + + it ('ignores a different attempt even when it reports started', async () => { + api.apiGet.mockResolvedValueOnce (ready).mockResolvedValueOnce ({ ...ready, + downloadAttempt: 'another', downloadStatus: 'started' }).mockResolvedValue ({ ...ready, + downloadAttempt: 'attempt', downloadStatus: 'started' }) + const view = renderProgress () + await flush () + await flush (800) + expect (view.actions.close).not.toHaveBeenCalled () + await flush (800) + expect (view.actions.close).toHaveBeenCalledTimes (1) + }) + + it ('keeps the created job available for cancellation retry during initial registration', async () => { + let resolve!: (value: { job: string }) => void + api.apiPost.mockImplementationOnce (() => new Promise (complete => { resolve = complete })) + api.apiDelete.mockRejectedValueOnce (new Error ('offline')) + const view = renderProgress () + await flush () + let pending!: Promise + act (() => { pending = view.handle.cancel () }) + await act (async () => { + resolve ({ job: 'created-during-cancel' }) + expect (await pending).toBe (false) + }) + await act (async () => { expect (await view.handle.cancel ()).toBe (true) }) + expect (api.apiDelete.mock.calls.map (call => call[0])).toEqual ([ + '/counts/export/jobs/created-during-cancel', '/counts/export/jobs/created-during-cancel']) + expect (api.startAPIDownload).not.toHaveBeenCalled () + }) + + it ('stops immediately if native download navigation cannot be created', async () => { + api.startAPIDownload.mockImplementationOnce (() => { throw new Error ('navigation blocked') }) + const view = renderProgress () + await flush () + expect (screen.getByRole ('alert')).toHaveTextContent ('開始できません') + await flush (40_000) + expect (api.apiGet).toHaveBeenCalledTimes (1) + expect (view.actions.close).not.toHaveBeenCalled () + }) + +}) diff --git a/frontend/src/components/counts/CountsExportProgress.tsx b/frontend/src/components/counts/CountsExportProgress.tsx index fdce32d..240b26b 100644 --- a/frontend/src/components/counts/CountsExportProgress.tsx +++ b/frontend/src/components/counts/CountsExportProgress.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from 'react' import LocalisedText from '@/components/LocalisedText' import { apiDelete, apiGet, apiPost, isApiError, startAPIDownload } from '@/lib/api' +import { countsErrorMessage } from '@/lib/countsDisplay' import { useClientMotion } from '@/lib/useClientMotion' import { useUITranslation } from '@/lib/useUITranslation' import { cn } from '@/lib/utils' @@ -26,178 +27,228 @@ const CountsExportProgress = ( const motion = useClientMotion () const [state, setState] = useState ({ status: 'starting', processed: 0, total: null }) - const [cancelError, setCancelError] = useState (false) + const [cancelError, setCancelError] = useState (null) + const [failure, setFailure] = useState (null) + const [downloadError, setDownloadError] = useState (null) + // One mounted dialogue owns one immutable job, even if its parent rerenders. + const initial = useRef ({ payload, locale }) + const actions = useRef (controls) + actions.current = controls const terminal = useRef (false) + const finished = useRef (false) + const cancelAction = useRef<() => Promise> (async () => false) const cancelled = useRef (false) + const generation = useRef (0) const job = useRef (null) const creation = useRef | null> (null) - const downloaded = useRef (false) - const registration = useRef (false) + const registration = useRef | null> (null) const attempt = useRef (null) const downloadDeadline = useRef (0) - const acknowledgementTimer = useRef | null> (null) const releaseDownload = useRef<(() => void) | null> (null) - const [downloadError, setDownloadError] = useState (null) - const pollRequest = useRef (null) + const cancellation = useRef | null> (null) + useEffect (() => { handle.cancel = () => cancelAction.current () }, [handle]) useEffect (() => { - handle.cancel = async () => { - if (terminal.current) return true + const epoch = ++generation.current + let active = true + let lastResponse = Date.now () + let timer: ReturnType | null = null + let acknowledgementTimer: ReturnType | null = null + let request: AbortController | null = null + const current = () => active && generation.current === epoch + const stop = (reason: string) => { + if (!current () || terminal.current) return + terminal.current = true + if (timer != null) clearTimeout (timer) + if (acknowledgementTimer != null) clearTimeout (acknowledgementTimer) + request?.abort () releaseDownload.current?.() + releaseDownload.current = null + setDownloadError (reason) + } + const cancel = (): Promise => { + if (finished.current || (terminal.current && job.current == null)) + return Promise.resolve (true) cancelled.current = true - if (job.current != null) - { + terminal.current = true + if (timer != null) clearTimeout (timer) + if (acknowledgementTimer != null) clearTimeout (acknowledgementTimer) + request?.abort () + releaseDownload.current?.() + releaseDownload.current = null + cancellation.current ??= (async () => { const controller = new AbortController () const deadline = setTimeout (() => controller.abort (), 15_000) try { - setCancelError (false) + const created = await creation.current + if (created == null) return false + job.current = created.job const result = await apiDelete ( - `/counts/export/jobs/${encodeURIComponent (job.current)}`, { - locale, signal: controller.signal }) - setState (result) + `/counts/export/jobs/${encodeURIComponent (created.job)}`, { + locale: initial.current.locale, signal: controller.signal }) + if (current ()) + { + setState (result) + setCancelError (null) + finished.current = ['cancelled', 'failed'].includes (result.status) + terminal.current = finished.current + if (!terminal.current) + { + if (timer != null) clearTimeout (timer) + timer = setTimeout (() => poll (created.job), 800) + } + } + return ['cancelled', 'failed'].includes (result.status) } - catch + catch (error) { - setCancelError (true) + if (isApiError (error) && error.response?.status === 404) return true + if (current ()) setCancelError (error) + return false } finally { clearTimeout (deadline) + cancellation.current = null } - } - return false + }) () + return cancellation.current + } + cancelAction.current = cancel + const armDeadline = () => { + if (downloadDeadline.current === 0) return + acknowledgementTimer = setTimeout (() => stop ('timeout'), + Math.max (0, downloadDeadline.current - Date.now ())) } - }, [handle, locale]) - useEffect (() => { - let active = true - let lastResponse = Date.now () - let timer: ReturnType | undefined const poll = async (token: string) => { + if (!current () || terminal.current) return const controller = new AbortController () - pollRequest.current = controller + request = controller const deadline = setTimeout (() => controller.abort (), 15_000) try { - const response = await apiGet ( + const result = await apiGet ( `/counts/export/jobs/${encodeURIComponent (token)}`, { - locale, signal: controller.signal }) - if (!active || terminal.current) return - const result = (cancelled.current && response.status === 'ready') - ? await apiDelete ( - `/counts/export/jobs/${encodeURIComponent (token)}`, { - locale, signal: controller.signal }) : response - if (!active || terminal.current) return + locale: initial.current.locale, signal: controller.signal }) + if (!current () || terminal.current) return lastResponse = Date.now () setState (result) - terminal.current = ['failed', 'cancelled'].includes (result.status) - if (result.status === 'ready' && result.token != null && !downloaded.current) + if (cancelled.current) { - registration.current = true - const receipt = await apiPost ( - `/counts/export/jobs/${encodeURIComponent (token)}/download`, {}, { - locale, signal: controller.signal }) - if (!active || cancelled.current) return - if (receipt.downloadAttempt == null) throw new Error ('Missing download attempt') - downloaded.current = true - attempt.current = receipt.downloadAttempt - downloadDeadline.current = Date.now () + 30_000 - acknowledgementTimer.current = setTimeout (() => { - if (!active || terminal.current) return - terminal.current = true - pollRequest.current?.abort () - releaseDownload.current?.() - setDownloadError ('timeout') - }, 30_000) - releaseDownload.current = startAPIDownload ('/counts/export', result.token, () => { - if (active) + if (await cancel ()) return + } + else if (result.status === 'ready') + { + if (result.token == null) { stop ('failed'); return } + if (attempt.current == null) + { + registration.current ??= apiPost ( + `/counts/export/jobs/${encodeURIComponent (token)}/download`, {}, { + locale: initial.current.locale, signal: controller.signal }) + const receipt = await registration.current + if (!current () || terminal.current || cancelled.current) return + if (receipt.downloadAttempt == null) { stop ('failed'); return } + attempt.current = receipt.downloadAttempt + downloadDeadline.current = Date.now () + 30_000 + armDeadline () + try + { + releaseDownload.current = startAPIDownload ('/counts/export', result.token, + () => stop ('failed'), new URLSearchParams ({ + job: token, attempt: receipt.downloadAttempt })) + } + catch + { + stop ('failed') + return + } + } + if (result.downloadAttempt === attempt.current) + { + if (result.downloadStatus === 'started') { terminal.current = true - setDownloadError ('failed') + finished.current = true + if (acknowledgementTimer != null) clearTimeout (acknowledgementTimer) + // Rack yielded a chunk; browser receipt and saving are not confirmed. + releaseDownload.current = null + actions.current.close () + return } - }, new URLSearchParams ({ job: token, attempt: receipt.downloadAttempt })) - } - if (attempt.current != null && result.downloadAttempt === attempt.current) - { - if (result.downloadStatus === 'started' && !cancelled.current) - { - terminal.current = true - // Native navigation continues after the dialogue unmounts. - releaseDownload.current = null - controls.close () - return - } - if (['failed', 'unavailable'].includes (result.downloadStatus ?? '')) - { - terminal.current = true - setDownloadError (result.downloadStatus!) + if (['failed', 'unavailable', 'timeout'].includes (result.downloadStatus ?? '')) + { stop (result.downloadStatus!); return } } } - if (attempt.current != null && Date.now () > downloadDeadline.current) + else if (['failed', 'cancelled'].includes (result.status)) { + finished.current = true terminal.current = true - setDownloadError ('timeout') - releaseDownload.current?.() + return } - if (!terminal.current) timer = setTimeout (() => poll (token), 800) + if (timer != null) clearTimeout (timer) + timer = setTimeout (() => poll (token), 800) } catch (error) { - if (active && !terminal.current) - { - if (downloaded.current || registration.current || (isApiError (error) && error.response?.status === 404) - || Date.now () - lastResponse > 30_000) - { - terminal.current = true - setDownloadError ('unavailable') - setState (previous => ({ ...previous, status: 'failed' })) - return - } - setCancelError (true) - timer = setTimeout (() => poll (token), 2000) - } + if (!current () || terminal.current) return + const status = isApiError (error) ? error.response?.status : null + if ([400, 401, 403, 422].includes (status ?? 0)) setFailure (error) + if (registration.current != null && attempt.current == null) + { stop ('failed'); return } + if (status != null && status >= 400 && status < 500) + { stop ('unavailable'); return } + if (Date.now () - lastResponse >= 30_000) + { stop ('timeout'); return } + timer = setTimeout (() => poll (token), 2000) } finally { clearTimeout (deadline) - if (pollRequest.current === controller) pollRequest.current = null + if (request === controller) request = null } } if (creation.current == null) { const controller = new AbortController () const deadline = setTimeout (() => controller.abort (), 15_000) - creation.current = apiPost<{ job: string }> ('/counts/export/jobs', payload, { - locale, signal: controller.signal }).finally (() => clearTimeout (deadline)) + creation.current = apiPost<{ job: string }> ( + '/counts/export/jobs', initial.current.payload, { + locale: initial.current.locale, signal: controller.signal }) + .finally (() => clearTimeout (deadline)) } - creation.current.then (async result => { - if (!active || terminal.current) return + armDeadline () + void creation.current.then (result => { + if (!current () || terminal.current) return job.current = result.job - if (cancelled.current) await handle.cancel () - await poll (result.job) - }).catch (() => { - if (active) - { - setDownloadError ('failed') - terminal.current = true - setState (previous => ({ ...previous, status: 'failed' })) - } + void poll (result.job) + }).catch (error => { + if (current ()) setFailure (error) + stop ('failed') }) return () => { active = false if (timer != null) clearTimeout (timer) - pollRequest.current?.abort () - releaseDownload.current?.() - if (acknowledgementTimer.current != null) clearTimeout (acknowledgementTimer.current) + if (acknowledgementTimer != null) clearTimeout (acknowledgementTimer) + request?.abort () + // StrictMode immediately installs another effect; real unmount releases the frame. + queueMicrotask (() => { + if (generation.current !== epoch) return + releaseDownload.current?.() + releaseDownload.current = null + }) } - }, [handle, locale]) - useEffect (() => { - controls.setActions ([]) - }, [controls]) + }, []) + useEffect (() => { controls.setActions ([]) }, [controls]) const percent = state.total == null ? null : (state.total === 0 ? 100 : Math.min (100, Math.floor (state.processed / state.total * 100))) - const busy = !['ready', 'failed', 'cancelled'].includes (state.status) - const phase = message (`exportProgress.${state.status === 'running' - ? state.phase ?? 'running' : state.status}`) + const busy = downloadError == null && cancelError == null + && !['failed', 'cancelled'].includes (state.status) + const phaseKey = (downloadError != null || cancelError != null) ? 'failed' + : (state.status === 'running' ? state.phase ?? 'running' + : (state.status === 'ready' ? 'writing' : state.status)) + const phase = message (`exportProgress.${phaseKey}`) + const failureMessage = countsErrorMessage (failure ?? state, locale) + const cancellationMessage = countsErrorMessage (cancelError, locale) return

{label ('exportProgress.format', { format: message (`exportFormats.${payload.format}`).text })}

@@ -214,9 +265,14 @@ const CountsExportProgress = ( ? label ('exportProgress.processed', { count: state.processed }) : label ('exportProgress.measured', { processed: state.processed, total: state.total ?? 0, percent })}

- {downloadError != null &&

- {label (`downloadErrors.${downloadError}`)}

} - {cancelError &&

{label ('common:fetchError')}

} + {(downloadError != null || state.status === 'failed') &&

+ {failure != null || state.status === 'failed' + ? failureMessage.text + : label (`downloadErrors.${downloadError}`)} + {downloadError != null && <>
{label ('errors.stopped')}}

} + {cancelError != null &&

+ {cancellationMessage.text}

}
} diff --git a/frontend/src/components/counts/CountsSortHeader.tsx b/frontend/src/components/counts/CountsSortHeader.tsx index 6f63bb5..c9de6dd 100644 --- a/frontend/src/components/counts/CountsSortHeader.tsx +++ b/frontend/src/components/counts/CountsSortHeader.tsx @@ -11,14 +11,15 @@ import type { CountSort } from '@/lib/counts' const CountsSortHeader = ( - { field, label, sorts, change, structured = false, rowSpan, colSpan, resizable = true, dragId, totalKind }: { + { field, label, sorts, change, structured = false, rowSpan, colSpan, + resizable = true, dragId, totalKind, disabled = false }: { field: string label: { text: string; language: string } sorts: CountSort[] change: (key: string, additive: boolean) => void structured?: boolean rowSpan?: number; colSpan?: number; resizable?: boolean - dragId?: string; totalKind?: string }, + dragId?: string; totalKind?: string; disabled?: boolean }, ) => { const { message } = useUITranslation ('counts') const family = useContext (CountsTableScope) @@ -84,7 +85,7 @@ const CountsSortHeader = ( aria-sort={(sorts.length > 1 && priority >= 0) ? 'other' : state}>
{shell?.handle} -
{resizable && } ) - return dragId == null ? render () : { }} renderShell={render}>{null} + return dragId == null ? render () : { }} renderShell={render}>{null} } diff --git a/frontend/src/components/counts/CountsSummaryResults.test.tsx b/frontend/src/components/counts/CountsSummaryResults.test.tsx new file mode 100644 index 0000000..22e88d7 --- /dev/null +++ b/frontend/src/components/counts/CountsSummaryResults.test.tsx @@ -0,0 +1,46 @@ +import { fireEvent, screen } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import CountsSummaryResults from '@/components/counts/CountsSummaryResults' +import CountsTableLayout from '@/components/counts/CountsTableLayout' +import { CountsDefinitionContext } from '@/lib/countsDefinitions' +import { postDefinition } from '@/test/fixtures/counts' +import { renderWithProviders } from '@/test/render' + +import type { CountsResult } from '@/lib/counts' + +const result: CountsResult = { + count: 4, page: 1, limit: 100, groupCount: 1, outputCount: 1, + rows: [{ field: 'resource_id' }], cols: [], columnValuesEditable: false, + rowHeaders: [{ values: ['__counts_private__'], kind: 'data' }, + { values: [], kind: 'grand_total' }], + columnHeaders: [{ values: [], kind: 'grand_total' }], + cells: [{ values: ['__counts_private__'], count: 4 }], + pivotTotals: [{ row: [], column: [], count: 4 }], axisLabels: {} } + +describe ('Counts summary privacy and row numbering', () => { + it ('copies masked labels and omits private links and legacy numbering', () => { + const { container } = renderWithProviders ( + + + , { route: '/ja/counts/posts' }) + const label = screen.getByText ('********') + expect (label).toHaveAttribute ('title', expect.stringContaining ('集計件数には含まれます')) + expect (label).toHaveClass ('text-red-600', 'dark:text-red-400') + const row = label.closest ('tr')! + expect (row.querySelector ('a')).toBeNull () + expect (container.innerHTML).not.toContain ('__counts_private__') + expect (screen.queryByText ('No.')).not.toBeInTheDocument () + expect (row.querySelectorAll ('td')).toHaveLength (2) + const table = screen.getByRole ('grid') + table.focus () + fireEvent.keyDown (table, { key: 'Home' }) + const setData = vi.fn () + fireEvent.copy (table, { clipboardData: { setData } }) + const plain = setData.mock.calls.find (entry => entry[0] === 'text/plain')?.[1] + expect (plain).toContain ('********') + expect (plain).not.toContain ('__counts_private__') + }) +}) diff --git a/frontend/src/components/counts/CountsSummaryResults.tsx b/frontend/src/components/counts/CountsSummaryResults.tsx index 97a41a0..764dc83 100644 --- a/frontend/src/components/counts/CountsSummaryResults.tsx +++ b/frontend/src/components/counts/CountsSummaryResults.tsx @@ -1,17 +1,17 @@ import SortableColumns from '@/components/common/SortableColumns' import { countsSummaryValue } from '@/lib/countsDisplay' import PrefetchLink from '@/components/PrefetchLink' -import CountsRowNumber from '@/components/counts/CountsRowNumber' import CountsSelectableTable from '@/components/counts/CountsSelectableTable' import CountsSortHeader from '@/components/counts/CountsSortHeader' -import { changeCountSort, countsRequestParams, readCountSort, writeCountSort } from '@/lib/counts' +import { changeCountSort, countsMaskedValue, countsRequestParams, readCountSort, writeCountSort } + from '@/lib/counts' import { countsAxisLabel, useCountsDefinition } from '@/lib/countsDefinitions' -import { cn } from '@/lib/utils' import { useLocale } from '@/lib/useLocale' import { useUITranslation } from '@/lib/useUITranslation' -import { queryRecordParams, setQueryRecords, readCountState, encodeCountState, setCountState } from '@/lib/queryState' +import { queryRecordParams, setQueryRecords, readCountState, encodeCountState, setCountState } + from '@/lib/queryState' import type { CountsPivotHeader, CountsResource, CountsResult } from '@/lib/counts' import type { CountsAxis } from '@/lib/countsDefinitions' @@ -21,12 +21,12 @@ const CountsSummaryTable = ( { resource, data, params, pending, update, dragIds = {} }: { dragIds?: Record resource: CountsResource; data: CountsResult; params: URLSearchParams - pending: boolean; update: (key: string, value: string, extra?: Record) => void }, + pending: boolean + update: (key: string, value: string, extra?: Record) => void }, ) => { const definition = useCountsDefinition () const locale = useLocale () const { message } = useUITranslation ('counts') - const numbered = false const rows = data.rows ?? [] const cols = data.cols ?? [] const cells = data.cells ?? [] @@ -49,7 +49,8 @@ const CountsSummaryTable = ( const headingLevels = cols.length === 0 ? 1 : levels * 2 const names = (header: CountsPivotHeader, axes: CountsAxis[], offset: number) => header.values.map ((value, index) => display (value, axes[index], offset + index)) - const columnKey = (header: CountsPivotHeader) => `${header.kind}:${JSON.stringify (header.values)}` + const columnKey = (header: CountsPivotHeader) => + `${header.kind}:${JSON.stringify (header.values)}` const measures = readCountState[]> ( params, 'measures', []) const measureKey = (header: CountsPivotHeader) => { @@ -66,13 +67,14 @@ const CountsSummaryTable = ( const next = new URLSearchParams (params) const nextSort = changeCountSort (sorts, key, additive) const active = new Set (nextSort.map (sort => sort.key)) - const records = measures.filter (record => record.key !== key && active.has (String (record.key))) + const records = measures.filter (record => + record.key !== key && active.has (String (record.key))) setQueryRecords (next, 'measures', [...records, ...(active.has (key) ? header.values.map ((value, axis) => ({ key, axis, value })) : [])]) update ('sort', writeCountSort (nextSort), queryRecordParams (next, 'measures')) } - const keys = [...(numbered ? ['row_number'] : []), + const keys = [ ...Array.from ({ length: leading }, (_, index) => `a${index}`), ...columnHeaders.map (columnKey)] const full = new Map (cells.map (cell => [JSON.stringify (cell.values), cell.count])) @@ -86,22 +88,21 @@ const CountsSummaryTable = ( : totals.get (JSON.stringify ([row.values, column.values])) ?? 0 const drill = (row: CountsPivotHeader, column: CountsPivotHeader) => { const next = new URLSearchParams (params) - const existing = readCountState[]> (params, 'conditions', []) + const existing = readCountState[]> ( + params, 'conditions', []) setCountState (next, 'conditions', encodeCountState ('conditions', [...existing, ...row.values.map ((value, index) => ({ ...rows[index], operator: 'axis_eq', value })), - ...column.values.map ((value, index) => ({ ...cols[index], operator: 'axis_eq', value }))])) + ...column.values.map ((value, index) => ({ + ...cols[index], operator: 'axis_eq', value }))])) next.set ('mode', 'details') next.delete ('sort') next.delete ('page') return `/counts/${resource}?${next}` } - let ordinal = (data.page - 1) * data.limit const rowLabels = rowHeaders.map (header => { const labels = names (header, rows, 0) - // With no row axes the sole result row is not an additional rollup row. - const number = rows.length === 0 ? '1' : (header.kind === 'data' ? String (++ordinal) : '') if (header.kind !== 'data') labels.push (aggregateLabel (header)) - return [...(numbered ? [number] : []), + return [ ...Array.from ({ length: leading }, (_, index) => labels[index] ?? '')] }) const spans = rowHeaders.map ((row, index) => rows.map ((_, axis) => { @@ -121,11 +122,10 @@ const CountsSummaryTable = ( return span })) rowLabels.forEach ((labels, row) => spans[row].forEach ((span, column) => { - if (span === 0) labels[column + (numbered ? 1 : 0)] = '' + if (span === 0) labels[column] = '' })) // This unmerged matrix is also the single source for TSV and HTML clipboard headings. const valueHeadingRows = Array.from ({ length: levels }, (_, level) => [ - ...(numbered ? [level === 0 ? message ('rowNumberHeading').text : ''] : []), ...Array.from ({ length: leading }, (_, index) => level === 0 && rows[index] != null ? countsAxisLabel (rows[index], definition, locale).text : ''), @@ -142,7 +142,7 @@ const CountsSummaryTable = ( return display (header.values[level], cols[level], rows.length + level) })]) const axisHeadingRows = cols.map ((axis, level) => [ - ...(numbered ? [''] : []), ...Array.from ({ length: leading }, () => ''), + ...Array.from ({ length: leading }, () => ''), ...columnHeaders.map ((header, index) => { if (header.values.length < level) return '' const previous = columnHeaders[index - 1] @@ -155,7 +155,7 @@ const CountsSummaryTable = ( : axisHeadingRows.flatMap ((headings, level) => [headings, valueHeadingRows[level]]) if (cols.length > 0) { - for (let index = 0; index < leading + (numbered ? 1 : 0); ++index) + for (let index = 0; index < leading; ++index) { headingRows[0][index] = valueHeadingRows[0][index] headingRows[1][index] = '' @@ -214,6 +214,7 @@ const CountsSummaryTable = ( const field = leaf ? columnKey (header) : sortKey output.push ( 0) ? (levels - level) * 2 - 1 : 1} totalKind={header.kind !== 'data' ? header.kind : undefined} dragId={level === 0 && header.values.length > 0 @@ -235,7 +236,6 @@ const CountsSummaryTable = ( {headingRows.map ((_, tier) => {tier === 0 && <> - {numbered && } {Array.from ({ length: leading }, (_, index) => rows[index] == null ? : {rowHeaders.map ((row, index) => {rowLabels[index].map ((value, offset) => { - const axis = offset - (numbered ? 1 : 0) + const axis = offset if (axis >= 0 && row.kind !== 'data' && axis > row.values.length) return null const aggregate = axis >= 0 && row.kind !== 'data' && axis === row.values.length const span = axis < 0 ? 1 : spans[index][axis] ?? 1 @@ -259,8 +259,8 @@ const CountsSummaryTable = ( data-counts-aggregate={row.kind !== 'data' ? '' : undefined} data-counts-total={row.kind !== 'data' ? row.kind : undefined} data-counts-row-total={row.kind !== 'data' ? row.kind : undefined} - className={cn ('overflow-hidden break-words border px-2 py-1 dark:border-slate-700', - numbered && offset === 0 && 'text-right tabular-nums')}>{value === '********' ? + {value === '********' ? {value} : value} })} @@ -273,8 +273,9 @@ const CountsSummaryTable = ( data-counts-aggregate={(row.kind !== 'data' || column.kind !== 'data') ? '' : undefined} className="border px-2 py-1 dark:border-slate-700"> - {[...row.values, ...column.values].includes ('__counts_private__') - ? {count (row, column)} : {count (row, column)} + : @@ -291,7 +292,8 @@ const CountsSummaryResults = (props: Parameters[0]) = const { message } = useUITranslation ('counts') const headers = props.data.columnHeaders ?? [] const columns = props.data.cols ?? [] - if (columns.length === 0) return + if (columns.length === 0 || props.data.columnValuesEditable === false) + return const groups = [...new Set (headers.filter (header => header.values.length > 0) .map (header => JSON.stringify (header.values.slice (0, 1))))] const ids = groups.map ((_, index) => `group${index}`) diff --git a/frontend/src/lib/api.test.ts b/frontend/src/lib/api.test.ts index 1e9f9d2..f718a3b 100644 --- a/frontend/src/lib/api.test.ts +++ b/frontend/src/lib/api.test.ts @@ -162,4 +162,40 @@ describe ('api helpers', () => { await apiPatch ('/posts/1') expect (mocks.client.get).toHaveBeenCalledTimes (2) }) + it ('aborts one write while a shared CSRF lookup remains pending for another write', async () => { + let complete!: (value: { data: { token: string } }) => void + mocks.client.get.mockReturnValueOnce (new Promise (resolve => { complete = resolve })) + const { apiPost } = await import ('@/lib/api') + const controller = new AbortController () + const first = apiPost ('/counts/export/jobs', {}, { signal: controller.signal }) + const second = apiPost ('/posts', {}) + const rejected = expect (first).rejects.toMatchObject ({ name: 'AbortError' }) + controller.abort () + await rejected + complete ({ data: { token: 'shared' } }) + await second + expect (mocks.client.get).toHaveBeenCalledWith ('/users/csrf', + expect.objectContaining ({ timeout: 15_000 })) + expect (mocks.client.post).toHaveBeenCalledTimes (1) + expect (mocks.client.post.mock.calls[0][0]).toBe ('/posts') + }) + + it ('releases native download frames after failure or explicit cancellation', async () => { + const { startAPIDownload } = await import ('@/lib/api') + const failure = vi.fn () + const release = startAPIDownload ('/counts/export', 'file', failure, + new URLSearchParams ({ job: 'job', attempt: 'attempt' })) + const frame = document.querySelector ('iframe')! + expect (new URL (frame.src).searchParams.get ('attempt')).toBe ('attempt') + frame.dispatchEvent (new Event ('error')) + frame.dispatchEvent (new Event ('error')) + expect (failure).toHaveBeenCalledTimes (1) + expect (frame).not.toBeInTheDocument () + release () + const cancel = startAPIDownload ('/counts/export', 'file', failure) + cancel () + expect (document.querySelector ('iframe')).toBeNull () + expect (failure).toHaveBeenCalledTimes (1) + }) + }) diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 210d47b..0727dec 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -63,7 +63,22 @@ const withSession = (opt?: Opt): AxiosRequestConfig => { const withCSRF = async (opt?: Opt): Promise => { - const token = await csrfToken () + const signal = opt?.signal + if (signal?.aborted) throw new DOMException ('Aborted', 'AbortError') + const pending = csrfToken () + // A shared CSRF lookup must not make an individual aborted write wait forever. + const token = signal == null ? await pending : await new Promise ((resolve, reject) => { + const abort = () => reject (new DOMException ('Aborted', 'AbortError')) + signal.addEventListener ('abort', abort, { once: true }) + if (signal.aborted) abort () + void pending.then (value => { + signal.removeEventListener ('abort', abort) + resolve (value) + }, error => { + signal.removeEventListener ('abort', abort) + reject (error) + }) + }) const config = withSession (opt) return { ...config, headers: { ...config.headers, 'X-CSRF-Token': token } } } @@ -73,7 +88,8 @@ const csrfToken = async (): Promise => { const state = csrfState () if (state.token != null) return state.token - state.pending ??= client.get<{ token: string }> ('/users/csrf', withSession ()) + state.pending ??= client.get<{ token: string }> ( + '/users/csrf', { ...withSession (), timeout: 15_000 }) .then (response => { if (csrfState () !== state) return csrfToken () @@ -223,19 +239,35 @@ export const startAPIDownload = ( ): (() => void) => { const frame = document.createElement ('iframe') frame.hidden = true + let timer: ReturnType | null = null + let closed = false + const release = () => { + if (closed) return + closed = true + if (timer != null) window.clearTimeout (timer) + frame.remove () + } + const failure = () => { + if (closed) return + release () + onError?.() + } frame.addEventListener ('load', () => { - const document = frame.contentDocument - if (document != null && document.URL !== 'about:blank') + try { - onError?.() - frame.remove () + const content = frame.contentDocument + // Attachment navigations normally have no document. An error document is failure. + if (content != null && content.URL !== 'about:blank') failure () + } + catch + { + // Cross-origin navigation cannot be inspected; the authenticated ACK decides. } }) - frame.addEventListener ('error', () => { onError?.(); frame.remove () }) + frame.addEventListener ('error', failure) const suffix = query == null ? '' : `?${query.toString ()}` frame.src = `${API_BASE_URL}${path}/files/${encodeURIComponent (token)}${suffix}` document.body.append (frame) - // Attachments do not reliably fire load; keep this isolated from page navigation. - const timer = window.setTimeout (() => frame.remove (), 10 * 60_000) - return () => { window.clearTimeout (timer); frame.remove () } + timer = window.setTimeout (release, 10 * 60_000) + return release } diff --git a/frontend/src/lib/counts.test.ts b/frontend/src/lib/counts.test.ts index df1454b..758d489 100644 --- a/frontend/src/lib/counts.test.ts +++ b/frontend/src/lib/counts.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { columnsForCounts, countColumns, countsCSV, countsDefinitionParams, countsRequestParams, normaliseCountsParams, selectedCountChildren } from '@/lib/counts' -import { countsConditionValue, countsNestedIsScalar, countsNestedScalarText } +import { countsErrorMessage, countsConditionValue, countsNestedIsScalar, countsNestedScalarText } from '@/lib/countsDisplay' import { countsClipboard } from '@/lib/countsClipboard' import { createUII18n, translateUIMessage } from '@/lib/i18n' @@ -157,3 +157,24 @@ describe ('Counts logical display and exports', () => { expect (isCountsPath (path.split ('?')[0])).toBe (true) }) }) + + +describe ('Counts safe failure messages', () => { + it.each ([ + [400, { error: 'too_many_axes', max_axes: 8 }, '最大8個'], + [422, { error: 'too_many_columns', max_columns: 73 }, '最大73列'], + [503, { error: 'counts_query_timeout' }, '時間制限'], + [503, { error: 'export_worker_unavailable' }, 'ワーカー'], + [400, {}, '入力条件'], [401, {}, '認証状態'], [403, {}, '権限']]) ( + 'distinguishes HTTP %s with a known safe response', (status, data, expected) => { + const failure = { isAxiosError: true, response: { status, data } } + expect (countsErrorMessage (failure, 'ja').text).toContain (expected) + }) + it ('never renders arbitrary exception messages or private identifiers', () => { + const failure = { isAxiosError: true, response: { status: 500, + data: { error: 'SELECT private_title FROM secret_table', message: 'private-user-17' } } } + const text = countsErrorMessage (failure, 'ja').text + expect (text).not.toMatch (/SELECT|private|secret_table/) + expect (countsErrorMessage ({ error: 'worker_stalled' }, 'ja').text).toContain ('停止') + }) +}) diff --git a/frontend/src/lib/counts.ts b/frontend/src/lib/counts.ts index b34feae..4e55c20 100644 --- a/frontend/src/lib/counts.ts +++ b/frontend/src/lib/counts.ts @@ -8,6 +8,8 @@ import type { CountsAxis, CountsDefinition } from '@/lib/countsDefinitions' import type { LocaleCode } from '@/lib/locales' import type { TagNameSnapshot } from '@/types' +export const countsMaskedValue = '__counts_private__' + export type CountsResource = 'posts' | 'tags' | 'nico_tags' | 'tag_descriptions' | 'tag_groups' | 'materials' export type CountsName = TagNameSnapshot & { @@ -77,6 +79,7 @@ export type CountsResult = { executionAt?: string columnValues?: (string | number | null)[][] cells?: { values: (string | number | null)[]; count: number }[] + columnValuesEditable?: boolean conditionLabels?: Record> axisLabels?: Record> axisNames?: Record>> diff --git a/frontend/src/lib/countsDisplay.ts b/frontend/src/lib/countsDisplay.ts index c021879..9c7a92d 100644 --- a/frontend/src/lib/countsDisplay.ts +++ b/frontend/src/lib/countsDisplay.ts @@ -1,4 +1,5 @@ -import { fieldsForCounts, orderedCountFields, selectedCountChildren, +import { isApiError } from '@/lib/api' +import { countsMaskedValue, fieldsForCounts, orderedCountFields, selectedCountChildren, sortedCountChildren } from '@/lib/counts' import { countsEventTypes, countsFieldLabel } from '@/lib/countsDefinitions' import { languageUIMessage, translateUIMessage } from '@/lib/i18n' @@ -12,6 +13,37 @@ import type { CountsAxis, CountsDefinition } from '@/lib/countsDefinitions' import type { LocaleCode } from '@/lib/locales' +// Render only known codes and numerical limits, never arbitrary server error text. +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 + max_columns?: unknown; maxColumns?: unknown } | null + const code = body != null && typeof body === 'object' ? body.error : null + const message = (key: string, values?: Record) => + translateUIMessage (key, 'counts', locale, values) + if (code === 'too_many_axes' || code === 'too_many_columns') + { + const maximum = code === 'too_many_axes' ? body?.maxAxes ?? body?.max_axes + : body?.maxColumns ?? body?.max_columns + if (typeof maximum === 'number') + return message (code === 'too_many_axes' ? 'axisLimit' : 'columnLimit.message', { + max: maximum }) + } + if (code === 'private_condition') return message ('privateCondition') + if (code === 'counts_query_timeout' || code === 'generation_timeout') + return message ('errors.queryTimeout') + if (code === 'export_worker_unavailable' || code === 'worker_unavailable') + return message ('errors.workerUnavailable') + if (['worker_stalled', 'interrupted', 'generation_failed', 'export_too_large', + 'export_storage_full'].includes (String (code))) return message ('errors.stopped') + 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') +} + + export const countsFieldValue = (row: CountRecord, field: string): unknown => row[field] ?? row[field.replace (/_([a-z])/g, (_, letter: string) => letter.toUpperCase ())] @@ -450,6 +482,7 @@ export const countsConditionValue = ( axis: CountsAxis, locale: LocaleCode, timezone: string, labels?: Record>, ): string => { + if (value === countsMaskedValue) return '********' if (axis.field === 'user' && value == null) return translateUIMessage ('bot', 'counts', locale).text if (value == null) return 'NULL' @@ -488,7 +521,7 @@ export const countsSummaryValue = ( ): string => { const message = (key: string, values?: Record) => translateUIMessage (key, 'counts', locale, values).text - if (value === '__counts_private__') return '********' + if (value === countsMaskedValue) return '********' if (value == null) return axis.field === 'user' ? message ('bot') : '' if (['language_code', 'source_language_code'].includes (axis.field)) return languageUIMessage (String (value), locale).text diff --git a/frontend/src/locales/ain-Kana/counts.json b/frontend/src/locales/ain-Kana/counts.json index fabea56..8ea919a 100644 --- a/frontend/src/locales/ain-Kana/counts.json +++ b/frontend/src/locales/ain-Kana/counts.json @@ -5,7 +5,8 @@ "maskedReason", "downloadErrors", "axisLimit", - "privateCondition" + "privateCondition", + "errors" ], "outputRows": "行数：{{value}} 件", "maskedReason": "閲覧権限のない履歴に由来するため、この項目の値は表示できません。集計件数には含まれます。", @@ -15,5 +16,13 @@ "timeout": "ダウンロード応答の開始確認がタイムアウトしました。" }, "axisLimit": "集計軸は最大{{max}}個です。行・列の項目を減らしてください。", - "privateCondition": "非公開情報を保護するため、この抽出条件では集計できません。" + "privateCondition": "非公開情報を保護するため、この抽出条件では集計できません。", + "errors": { + "queryTimeout": "集計処理が時間制限を超えました。抽出条件や集計軸を減らして再実行してください。", + "workerUnavailable": "エクスポートワーカーが稼働していません。", + "stopped": "エクスポート処理が停止しました。再実行してください。", + "input": "入力条件が正しくありません。", + "permission": "この操作を実行する権限がありません。", + "authentication": "認証状態を確認して再実行してください。" + } } diff --git a/frontend/src/locales/ja/counts.json b/frontend/src/locales/ja/counts.json index 81c7c17..4c9d687 100644 --- a/frontend/src/locales/ja/counts.json +++ b/frontend/src/locales/ja/counts.json @@ -299,5 +299,13 @@ "timeout": "ダウンロード応答の開始確認がタイムアウトしました。" }, "axisLimit": "集計軸は最大{{max}}個です。行・列の項目を減らしてください。", - "privateCondition": "非公開情報を保護するため、この抽出条件では集計できません。" + "privateCondition": "非公開情報を保護するため、この抽出条件では集計できません。", + "errors": { + "queryTimeout": "集計処理が時間制限を超えました。抽出条件や集計軸を減らして再実行してください。", + "workerUnavailable": "エクスポートワーカーが稼働していません。", + "stopped": "エクスポート処理が停止しました。再実行してください。", + "input": "入力条件が正しくありません。", + "permission": "この操作を実行する権限がありません。", + "authentication": "認証状態を確認して再実行してください。" + } } diff --git a/frontend/src/pages/CountsResourcePage.test.tsx b/frontend/src/pages/CountsResourcePage.test.tsx index 13a5a25..5b7f715 100644 --- a/frontend/src/pages/CountsResourcePage.test.tsx +++ b/frontend/src/pages/CountsResourcePage.test.tsx @@ -26,7 +26,6 @@ const renderPage = (extra = '') => renderWithProviders (<> , { route: `/ja/counts/posts?mode=details&timezone=UTC${extra}` }) beforeEach (() => { - vi.spyOn (HTMLCanvasElement.prototype, 'getContext').mockReturnValue (null) fetchCounts.mockReset ().mockResolvedValue (detailResult) fetchCountsDefinition.mockReset ().mockResolvedValue (postDefinition) }) @@ -104,4 +103,22 @@ describe ('Counts execution and URL state', () => { expect (await screen.findByRole ('alert')).toHaveTextContent ('73') expect (screen.getByRole ('alert')).toHaveTextContent ('列数が上限') }) + it ('can execute after an initial column-limit failure with no execution timestamp', async () => { + fetchCounts.mockRejectedValueOnce ({ isAxiosError: true, + response: { status: 422, data: { error: 'too_many_columns', max_columns: 73 } } }) + renderPage () + await screen.findByRole ('alert') + execute () + await screen.findByText ('Recorded title') + await waitFor (() => expect (currentParams ().get ('execution_at')).toBe (executionAt)) + expect (fetchCounts).toHaveBeenCalledTimes (2) + }) + + it ('keeps the page heading and common site title consistent', async () => { + renderPage () + await screen.findByText ('Recorded title') + await waitFor (() => expect (document.title).toContain ('広場集計 | ')) + expect (screen.getByRole ('heading', { name: '広場集計' })).toBeInTheDocument () + }) + }) diff --git a/frontend/src/pages/CountsResourcePage.tsx b/frontend/src/pages/CountsResourcePage.tsx index 1906c05..23e3809 100644 --- a/frontend/src/pages/CountsResourcePage.tsx +++ b/frontend/src/pages/CountsResourcePage.tsx @@ -17,7 +17,7 @@ import SidebarComponent from '@/components/layout/SidebarComponent' import { Button } from '@/components/ui/button' import { countsRequestParams, countsDefinitionParams, countsExportPayload, fieldsForCounts, fetchCounts, isCountsState, normaliseCountsParams, selectedCountColumns } from '@/lib/counts' -import { countsExportLabels } from '@/lib/countsDisplay' +import { countsErrorMessage, countsExportLabels } from '@/lib/countsDisplay' import { isApiError } from '@/lib/api' import { CountsDefinitionContext, countsAxisFields, countsAxisLabel, visibleCountsResources, countsFieldLabel, @@ -231,13 +231,7 @@ const CountsResourceContent = ( || pending || fetching || placeholder || (!params.has ('execution_at') && !errorFlg))) const failure = runError ? runFailure : queryError - const errorBody = isApiError (failure) ? failure.response?.data as { - error?: string; max_axes?: number; maxAxes?: number } : undefined - const errorCode = errorBody?.error - const failureMessage = errorCode === 'too_many_axes' - ? label ('axisLimit', { max: errorBody?.maxAxes ?? errorBody?.max_axes ?? '' }) - : (errorCode === 'private_condition' ? label ('privateCondition') - : label ('common:fetchError')) + const failureMessage = countsErrorMessage (failure, locale) const columnLimit = runColumnLimit ?? tooManyColumns (queryError) @@ -355,7 +349,7 @@ const CountsResourceContent = ( resource={resource} applied={params} conditionLabels={errorFlg ? undefined : data?.conditionLabels} conditionLabelsParams={resultParams.toString ()} - pending={running || fetching || !params.has ('execution_at')} + pending={running || fetching} open={open} setOpen={setOpen} reorder={value => update ('order', value)} displayJSON={enabled => update ('nested_json', enabled ? '1' : '0')} @@ -381,7 +375,7 @@ const CountsResourceContent = (

{label ('columnLimit.title')}

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

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

{failureMessage}

)} + &&

{failureMessage.text}

)}
diff --git a/frontend/src/test/setup.ts b/frontend/src/test/setup.ts index ef2725c..1f395af 100644 --- a/frontend/src/test/setup.ts +++ b/frontend/src/test/setup.ts @@ -1,6 +1,10 @@ import '@testing-library/jest-dom/vitest' import { cleanup } from '@testing-library/react' -import { afterEach, vi } from 'vitest' +import { afterEach, beforeEach, vi } from 'vitest' + +beforeEach (() => { + vi.spyOn (HTMLCanvasElement.prototype, 'getContext').mockReturnValue (null) +}) afterEach (() => { cleanup () @@ -16,10 +20,10 @@ window.scroll = function () { ; } -window.requestAnimationFrame = callback => { - callback (0) - return 0 -} +// Animation callbacks must advance time and must not recurse synchronously. +window.requestAnimationFrame = callback => window.setTimeout ( + () => callback (performance.now ()), 16) +window.cancelAnimationFrame = identifier => window.clearTimeout (identifier) Object.defineProperty (window, 'matchMedia', { configurable: true,