diff --git a/backend/app/models/post.rb b/backend/app/models/post.rb index cfbb50c..001fb6f 100644 --- a/backend/app/models/post.rb +++ b/backend/app/models/post.rb @@ -1,4 +1,5 @@ class Post < ApplicationRecord + require 'date' require 'mini_magick' require 'stringio' @@ -237,15 +238,43 @@ class Post < ApplicationRecord value = raw_value.to_s.strip return nil if value.blank? - if value.match?(/\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}\z/) - return Time.zone.local(value[0, 4].to_i, - value[5, 2].to_i, - value[8, 2].to_i, - value[11, 2].to_i, - value[14, 2].to_i) + if (match = value.match(/\A(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})\z/)) + year = match[1].to_i + month = match[2].to_i + day = match[3].to_i + hour = match[4].to_i + minute = match[5].to_i + return nil unless valid_original_created_components?(year, month, day, hour, minute, 0) + + return Time.zone.local(year, month, day, hour, minute) end - Time.iso8601(value).in_time_zone + match = + value.match( + /\A(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2})/ \ + '(?::(\d{2})(?:\.(\d+))?)?' \ + '(Z|[+-]\d{2}:?\d{2})\z/') + return nil if match.nil? + + year = match[1].to_i + month = match[2].to_i + day = match[3].to_i + hour = match[4].to_i + minute = match[5].to_i + second = match[6]&.to_i || 0 + fraction = match[7] + offset = match[8] + return nil unless valid_original_created_components?(year, month, day, hour, minute, second) + return nil unless valid_original_created_offset?(offset) + + Time.new( + year, + month, + day, + hour, + minute, + second + Rational(parse_original_created_nanoseconds(fraction), 1_000_000_000), + normalise_original_created_offset(offset)).in_time_zone rescue ArgumentError, TypeError nil end @@ -254,6 +283,38 @@ class Post < ApplicationRecord value.sec.zero? && value.nsec.zero? end + def valid_original_created_components? year, month, day, hour, minute, second + return false unless Date.valid_date?(year, month, day) + return false unless hour.between?(0, 23) + return false unless minute.between?(0, 59) + return false unless second.between?(0, 59) + + true + end + + def valid_original_created_offset? value + match = value.match(/\A([+-])(\d{2}):?(\d{2})\z/) + return true if value == 'Z' + return false if match.nil? + + hours = match[2].to_i + minutes = match[3].to_i + hours.between?(0, 23) && minutes.between?(0, 59) + end + + def parse_original_created_nanoseconds value + return 0 if value.blank? + + digits = value[0, 9].ljust(9, '0') + Integer(digits, 10) + end + + def normalise_original_created_offset value + return '+00:00' if value == 'Z' + + value.match?(/\A[+-]\d{2}:\d{2}\z/) ? value : "#{ value[0, 3] }:#{ value[3, 2] }" + end + def skip_original_created_validation? return false if new_record? return false if will_save_change_to_original_created_from? diff --git a/backend/app/services/post_creator.rb b/backend/app/services/post_creator.rb index 0ef0dde..6d96be8 100644 --- a/backend/app/services/post_creator.rb +++ b/backend/app/services/post_creator.rb @@ -75,8 +75,6 @@ class PostCreator duration = @attributes[:duration] return nil if duration.blank? - return duration.to_i if duration.is_a?(Numeric) && duration.to_i.positive? - value = Tag.time_to_ms!(duration.to_s, tag_name: '動画時間') raise VideoMsParseError unless value.positive? diff --git a/backend/app/services/post_import_previewer.rb b/backend/app/services/post_import_previewer.rb index 44a1f6f..1df1306 100644 --- a/backend/app/services/post_import_previewer.rb +++ b/backend/app/services/post_import_previewer.rb @@ -157,7 +157,15 @@ class PostImportPreviewer def initial_attributes row attributes = row[:attributes]&.stringify_keys || { } - FIELDS.to_h { |field| [field, attributes[field].to_s] } + FIELDS.to_h { |field| + value = attributes[field] + normalised = + if field == 'duration' + normalise_duration_attribute(value) + else + value.to_s + end + [field, normalised] } end def initial_provenance row @@ -256,9 +264,20 @@ class PostImportPreviewer end def sanitise_metadata_duration value - return nil unless value.is_a?(Numeric) + return nil unless value.is_a?(String) - value.positive? ? value.to_i : nil + value.presence + end + + def normalise_duration_attribute value + return '' if value.nil? + return value if value.is_a?(String) + + milliseconds = Integer(value, exception: false) + return value.to_s if milliseconds.nil? || milliseconds <= 0 + + seconds_string = (milliseconds / 1_000.0).to_s + seconds_string.end_with?('.0') ? seconds_string.delete_suffix('.0') : seconds_string end def preload_metadata! prepared_rows, fetch_metadata, metadata_cache, existing_posts, url_counts @@ -459,7 +478,7 @@ class PostImportPreviewer def parse_duration value, errors return nil if value.blank? - value.is_a?(Numeric) ? value.to_i : Tag.time_to_ms!(value.to_s, tag_name: '動画時間') + Tag.time_to_ms!(value.to_s, tag_name: '動画時間') rescue Tag::SectionLiteralParseError errors[:video_ms] = ['動画時間の記法が不正です．'] nil diff --git a/backend/app/services/post_import_row_normaliser.rb b/backend/app/services/post_import_row_normaliser.rb index 2dc7051..7362d3b 100644 --- a/backend/app/services/post_import_row_normaliser.rb +++ b/backend/app/services/post_import_row_normaliser.rb @@ -5,9 +5,10 @@ class PostImportRowNormaliser 'thumbnail_base', 'original_created_from', 'original_created_before', + 'duration', 'tags', 'parent_post_ids'].freeze - FLEXIBLE_FIELDS = ['duration', 'video_ms'].freeze + FLEXIBLE_FIELDS = ['video_ms'].freeze ATTRIBUTE_FIELDS = (STRING_FIELDS + FLEXIBLE_FIELDS).freeze def self.normalise! rows, allow_warning_fields: false diff --git a/backend/app/services/post_metadata_fetcher.rb b/backend/app/services/post_metadata_fetcher.rb index 8dc5f33..2db41b4 100644 --- a/backend/app/services/post_metadata_fetcher.rb +++ b/backend/app/services/post_metadata_fetcher.rb @@ -1,4 +1,5 @@ require 'time' +require 'date' class PostMetadataFetcher TIMESTAMP_PATTERN = @@ -30,7 +31,7 @@ class PostMetadataFetcher Preview::KnownSiteExtractor.thumbnail_url(uri) || metadata[:image_url], original_created_from: serialise_time(created_range&.first), original_created_before: serialise_time(created_range&.last), - duration: duration&.to_f&.then { _1.positive? ? (_1 * 1_000).round : nil }, + duration: serialise_duration(duration), tags: platform_tags.join(' ') } end @@ -54,12 +55,16 @@ class PostMetadataFetcher when /\A(\d{4})-(\d{2})\z/ year = Regexp.last_match(1).to_i month = Regexp.last_match(2).to_i + return nil unless month.between?(1, 12) + from = Time.zone.local(year, month, 1) [from, from + 1.month] when /\A(\d{4})-(\d{2})-(\d{2})\z/ year = Regexp.last_match(1).to_i month = Regexp.last_match(2).to_i day = Regexp.last_match(3).to_i + return nil unless Date.valid_date?(year, month, day) + from = Time.zone.local(year, month, day) [from, from + 1.day] else @@ -84,6 +89,9 @@ class PostMetadataFetcher second = match[6]&.to_i || 0 fraction = match[7] offset = match[8] + return nil unless valid_timestamp_components?(year, month, day, hour, minute, second) + return nil unless valid_offset?(offset) + nanoseconds = parse_nanoseconds(fraction) timestamp = if offset.present? @@ -117,6 +125,37 @@ class PostMetadataFetcher value.match?(/\A[+-]\d{2}:\d{2}\z/) ? value : "#{ value[0, 3] }:#{ value[3, 2] }" end + def self.serialise_duration value + seconds = Float(value) + return nil unless seconds.positive? + + milliseconds = (seconds * 1_000).round + seconds_string = (milliseconds / 1_000.0).to_s + seconds_string.end_with?('.0') ? seconds_string.delete_suffix('.0') : seconds_string + rescue ArgumentError, TypeError + nil + end + + def self.valid_timestamp_components? year, month, day, hour, minute, second + return false unless Date.valid_date?(year, month, day) + return false unless hour.between?(0, 23) + return false unless minute.between?(0, 59) + return false unless second.between?(0, 59) + + true + end + + def self.valid_offset? value + return true if value.nil? || value == 'Z' + + match = value.match(/\A([+-])(\d{2}):?(\d{2})\z/) + return false if match.nil? + + hours = match[2].to_i + minutes = match[3].to_i + hours.between?(0, 23) && minutes.between?(0, 59) + end + def self.serialise_time value return nil if value.nil? @@ -128,5 +167,8 @@ class PostMetadataFetcher :parse_timestamp_range, :parse_nanoseconds, :parse_offset, + :serialise_duration, + :valid_timestamp_components?, + :valid_offset?, :serialise_time end diff --git a/backend/spec/models/post_spec.rb b/backend/spec/models/post_spec.rb index 085d426..7c8977b 100644 --- a/backend/spec/models/post_spec.rb +++ b/backend/spec/models/post_spec.rb @@ -336,5 +336,26 @@ RSpec.describe Post, type: :model do expect(post).to be_valid end + + it 'rejects invalid calendar dates and invalid hours' do + invalid_dates = [ + '2024-02-31T12:00', + '2023-02-29T12:00', + '2024-02-29T24:00' + ] + + invalid_dates.each do |value| + post = described_class.new( + title: 'title', + url: 'https://example.com/post', + original_created_from: value + ) + + expect(post).to be_invalid + expect(post.errors[:original_created_from]).to eq( + [described_class::ORIGINAL_CREATED_INVALID_MESSAGE] + ) + end + end end end diff --git a/backend/spec/requests/post_imports_spec.rb b/backend/spec/requests/post_imports_spec.rb index 1fe54b1..78be75f 100644 --- a/backend/spec/requests/post_imports_spec.rb +++ b/backend/spec/requests/post_imports_spec.rb @@ -115,6 +115,50 @@ RSpec.describe 'Post imports API', type: :request do end describe 'POST /posts/import' do + it 'keeps the duration string contract through preview, validate, and import' do + sign_in_as(member) + allow(PostMetadataFetcher).to receive(:fetch).and_return( + title: 'fetched title', + thumbnail_base: nil, + duration: '2.5', + tags: '動画' + ) + + post '/posts/import/preview', params: { + source: 'https://example.com/video' + } + preview_row = json.fetch('rows').first + expect(preview_row.dig('attributes', 'duration')).to eq('2.5') + + post '/posts/import/validate', params: { + rows: [{ + sourceRow: preview_row.fetch('source_row'), + url: preview_row.fetch('url'), + attributes: preview_row.fetch('attributes'), + provenance: preview_row.fetch('provenance'), + tagSources: preview_row.fetch('tag_sources'), + metadataUrl: preview_row.fetch('metadata_url') + }], + changed_row: -1 + } + validated_row = json.fetch('rows').first + expect(validated_row.dig('attributes', 'duration')).to eq('2.5') + + post '/posts/import', params: { + rows: [{ + sourceRow: validated_row.fetch('source_row'), + url: validated_row.fetch('url'), + attributes: validated_row.fetch('attributes'), + provenance: validated_row.fetch('provenance'), + tagSources: validated_row.fetch('tag_sources'), + metadataUrl: validated_row.fetch('metadata_url') + }] + } + + expect(response).to have_http_status(:ok) + expect(Post.order(:id).last.video_ms).to eq(2_500) + end + it 'returns a formal skipped result for an existing post' do existing = create(:post, url: 'https://example.com/existing') sign_in_as(member) diff --git a/backend/spec/services/post_import_previewer_spec.rb b/backend/spec/services/post_import_previewer_spec.rb index fc6e16b..bd6e267 100644 --- a/backend/spec/services/post_import_previewer_spec.rb +++ b/backend/spec/services/post_import_previewer_spec.rb @@ -68,7 +68,7 @@ RSpec.describe PostImportPreviewer do allow(PostMetadataFetcher).to receive(:fetch).and_return( title: 'metadata title', thumbnail_base: 'https://example.com/thumb.jpg', - duration: 2_000, + duration: '2', tags: 'known-tag' ) @@ -79,7 +79,7 @@ RSpec.describe PostImportPreviewer do expect(result.fetch(:attributes)).to include( 'title' => 'metadata title', 'thumbnail_base' => 'https://example.com/thumb.jpg', - 'duration' => 2_000, + 'duration' => '2', 'tags' => 'known-tag' ) expect(result.fetch(:field_warnings)).not_to have_key('tags') diff --git a/backend/spec/services/post_import_row_normaliser_spec.rb b/backend/spec/services/post_import_row_normaliser_spec.rb index ec1a41c..5393228 100644 --- a/backend/spec/services/post_import_row_normaliser_spec.rb +++ b/backend/spec/services/post_import_row_normaliser_spec.rb @@ -6,7 +6,7 @@ RSpec.describe PostImportRowNormaliser do sourceRow: '1', url: 'https://example.com/post', metadataUrl: 'https://example.com/post', - attributes: { title: 'title', duration: 1_000 }, + attributes: { title: 'title', duration: '1' }, provenance: { url: 'manual', title: 'automatic' }, tagSources: { automatic: 'tag', manual: '' } }.deep_merge(overrides) @@ -21,7 +21,7 @@ RSpec.describe PostImportRowNormaliser do 'source_row' => 1, 'url' => 'https://example.com/post', 'metadata_url' => 'https://example.com/post', - 'attributes' => { 'title' => 'title', 'duration' => 1_000 }, + 'attributes' => { 'title' => 'title', 'duration' => '1' }, 'provenance' => { 'url' => 'manual', 'title' => 'automatic' }, 'tag_sources' => { 'automatic' => 'tag', 'manual' => '' } } diff --git a/backend/spec/services/post_metadata_fetcher_spec.rb b/backend/spec/services/post_metadata_fetcher_spec.rb index 2b2a468..bbc6575 100644 --- a/backend/spec/services/post_metadata_fetcher_spec.rb +++ b/backend/spec/services/post_metadata_fetcher_spec.rb @@ -68,4 +68,42 @@ RSpec.describe PostMetadataFetcher do expect(result.fetch(:original_created_from)).to be_nil expect(result.fetch(:original_created_before)).to be_nil end + + it 'returns nil dates for invalid calendar dates and invalid hour values' do + invalid_day = fetch_with_published_time('2024-02-31T12:00Z') + invalid_leap = fetch_with_published_time('2023-02-29T12:00Z') + invalid_hour = fetch_with_published_time('2024-02-29T24:00Z') + + [invalid_day, invalid_leap, invalid_hour].each do |result| + expect(result.fetch(:original_created_from)).to be_nil + expect(result.fetch(:original_created_before)).to be_nil + end + end + + it 'accepts a valid leap-day timestamp at minute precision' do + result = fetch_with_published_time('2024-02-29T12:34Z') + + expect(result.fetch(:original_created_from)).to eq('2024-02-29T12:34:00Z') + expect(result.fetch(:original_created_before)).to eq('2024-02-29T12:35:00Z') + end + + it 'serialises metadata duration as the same seconds string contract used by forms' do + html = <<~HTML + + + + + HTML + uri = URI.parse('https://example.com/video') + allow(Preview::UrlSafety).to receive(:validate) + .with('https://example.com/video') + .and_return([uri, ['8.8.8.8']]) + allow(Preview::HttpFetcher).to receive(:fetch).and_return(Response.new(html)) + allow(Preview::HtmlMetadataExtractor).to receive(:extract) + .and_return(title: 'title', image_url: nil) + + result = described_class.fetch('https://example.com/video') + + expect(result.fetch(:duration)).to eq('2.5') + end end diff --git a/frontend/src/components/PostEditForm.test.tsx b/frontend/src/components/PostEditForm.test.tsx index 2ff28b1..0523b70 100644 --- a/frontend/src/components/PostEditForm.test.tsx +++ b/frontend/src/components/PostEditForm.test.tsx @@ -19,8 +19,8 @@ const toastApi = vi.hoisted (() => ({ vi.mock ('@/lib/posts', () => postsApi) vi.mock ('@/lib/api', () => api) vi.mock ('@/components/ui/use-toast', () => toastApi) -vi.mock ('@/components/dialogues/DialogueProvider', () => ({ - useDialogue: () => ({ +vi.mock ('@/lib/dialogues/useDialogue', () => ({ + default: () => ({ choice: vi.fn (), }), })) @@ -78,15 +78,15 @@ describe ('PostEditForm', () => { render () - expect (screen.getByRole ('spinbutton')).toHaveValue (180.5) + expect (screen.getByPlaceholderText ('例: 2 / 2.5 / 1:23')).toHaveValue ('180.5') const tags = screen.getAllByRole ('textbox')[2] fireEvent.change (tags, { target: { value: 'general-tag' } }) - expect (screen.queryByRole ('spinbutton')).not.toBeInTheDocument () + expect (screen.queryByPlaceholderText ('例: 2 / 2.5 / 1:23')).not.toBeInTheDocument () fireEvent.change (tags, { target: { value: '動画 general-tag' }, }) - expect (screen.getByRole ('spinbutton')).toHaveValue (180.5) + expect (screen.getByPlaceholderText ('例: 2 / 2.5 / 1:23')).toHaveValue ('180.5') }) }) diff --git a/frontend/src/components/PostEditForm.tsx b/frontend/src/components/PostEditForm.tsx index 744c132..62e8d4b 100644 --- a/frontend/src/components/PostEditForm.tsx +++ b/frontend/src/components/PostEditForm.tsx @@ -1,15 +1,16 @@ import { useEffect, useMemo, useState } from 'react' -import PostFormTagsArea from '@/components/PostFormTagsArea' import PostOriginalCreatedTimeField from '@/components/PostOriginalCreatedTimeField' import FieldError from '@/components/common/FieldError' -import FormField from '@/components/common/FormField' -import { useDialogue } from '@/components/dialogues/DialogueProvider' +import PostDurationField from '@/components/posts/PostDurationField' +import PostTagsField from '@/components/posts/PostTagsField' +import PostTextField from '@/components/posts/PostTextField' import { Button } from '@/components/ui/button' import { toast } from '@/components/ui/use-toast' import { isApiError } from '@/lib/api' +import useDialogue from '@/lib/dialogues/useDialogue' import { updatePost } from '@/lib/posts' -import { inputClass, msToTime } from '@/lib/utils' +import { msToTime } from '@/lib/utils' import { useValidationErrors } from '@/lib/useValidationErrors' import type { FC, FormEvent } from 'react' @@ -157,32 +158,20 @@ const PostEditForm: FC = ({ post, onSave }) => {
- {/* タイトル */} - - {({ invalid }) => ( - setTitle (e.target.value)}/>)} - + - {/* 親投稿 */} - - {({ describedBy, invalid }) => ( - setParentPostIds (e.target.value)} - aria-describedby={describedBy} - aria-invalid={invalid} - className={inputClass (invalid)}/>)} - + - {/* タグ */} - = ({ post, onSave }) => { setOriginalCreatedBefore={setOriginalCreatedBefore} errors={fieldErrors.originalCreatedAt}/> - {/* 動画時間 */} {videoFlg && ( - - {({ invalid }) => ( - setDuration (e.target.value)}/>)} - )} + )} {/* 送信 */}