#383

backend/app/controllers/materials_controller.rb

@@ -89,13 +89,15 @@ class MaterialsController < ApplicationController
 
     begin
       Material.transaction do
-        tag = resolve_material_tag!(tag_name_raw)
+        tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw)
+        tag = tag_name.tag
+        tag = Tag.create!(tag_name:, category: :material) unless tag
+
         material = Material.new(tag:, url:,
                                 created_by_user: current_user,
                                 updated_by_user: current_user)
         material.file.attach(uploaded_blob) if uploaded_blob
         material.save!
-        TagVersioning.record_tag_snapshot!(tag, created_by_user: current_user)
         upsert_export_paths!(material)
         MaterialVersionRecorder.record!(material:, event_type: :create,
                                         created_by_user: current_user)
@@ -137,7 +139,10 @@ class MaterialsController < ApplicationController
     begin
       Material.transaction do
         MaterialVersionRecorder.ensure_snapshot!(material, created_by_user: current_user)
-        tag = resolve_material_tag!(tag_name_raw)
+        tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw)
+        tag = tag_name.tag
+        tag = Tag.create!(tag_name:, category: :material) unless tag
+
         material.assign_attributes(tag:, url:, updated_by_user: current_user)
         if uploaded_blob
           material.file.attach(uploaded_blob)
@@ -145,7 +150,6 @@ class MaterialsController < ApplicationController
           material.file.detach
         end
         material.save!
-        TagVersioning.record_tag_snapshot!(tag, created_by_user: current_user)
         upsert_export_paths!(material)
         MaterialVersionRecorder.record!(material:, event_type: :update,
                                         created_by_user: current_user)
@@ -236,12 +240,6 @@ class MaterialsController < ApplicationController
     nil
   end
 
-  def resolve_material_tag! tag_name_raw
-    tag_name = TagName.find_undiscard_or_create_by!(name: tag_name_raw)
-    tag = tag_name.tag
-    tag || Tag.create!(tag_name:, category: :material)
-  end
-
   def material_index_needs_tag_name? filters
     filters[:q].present? || filters[:sort] == 'tag_name'
   end

backend/app/controllers/posts_controller.rb

@@ -140,11 +140,10 @@ class PostsController < ApplicationController
     original_created_from = params[:original_created_from]
     original_created_before = params[:original_created_before]
     parent_post_ids = parse_parent_post_ids
-    resized_thumbnail = thumbnail.present? ? Post.resized_thumbnail_attachment(thumbnail) : nil
 
     post = Post.new(title:, url:, thumbnail_base: nil, uploaded_user: current_user,
                     original_created_from:, original_created_before:)
-    post.thumbnail.attach(resized_thumbnail) if resized_thumbnail
+    post.thumbnail.attach(thumbnail) if thumbnail.present?
 
     ApplicationRecord.transaction do
       post.save!
@@ -157,6 +156,8 @@ class PostsController < ApplicationController
 
       sync_parent_posts!(post, parent_post_ids)
 
+      post.resized_thumbnail!
+
       PostVersionRecorder.record!(post:, event_type: :create, created_by_user: current_user)
     end
 
@@ -166,8 +167,6 @@ class PostsController < ApplicationController
     render_validation_error fields: { tags: 'ニコニコ・タグは直接指定できません．' }
   rescue Tag::DeprecatedTagNormalisationError
     render_unprocessable_entity '廃止済みタグは付与できません．', field: :tags
-  rescue MiniMagick::Error
-    render_validation_error fields: { thumbnail: ['サムネイル画像の変換に失敗しました．'] }
   rescue ArgumentError => e
     render_validation_error fields: { parent_post_ids: [e.message] }
   rescue ActiveRecord::RecordInvalid => e

backend/app/models/post.rb

@@ -1,19 +1,5 @@
 class Post < ApplicationRecord
   require 'mini_magick'
-  require 'stringio'
-
-  def self.resized_thumbnail_attachment(upload)
-    upload.rewind
-    image = MiniMagick::Image.read(upload.read)
-    image.resize '180x180'
-    image.format 'jpg'
-
-    { io: StringIO.new(image.to_blob),
-      filename: 'resized_thumbnail.jpg',
-      content_type: 'image/jpeg' }
-  ensure
-    upload.rewind
-  end
 
   belongs_to :uploaded_user, class_name: 'User', optional: true
 
@@ -101,7 +87,12 @@ class Post < ApplicationRecord
   def resized_thumbnail!
     return unless thumbnail.attached?
 
-    thumbnail.attach(self.class.resized_thumbnail_attachment(StringIO.new(thumbnail.download)))
+    image = MiniMagick::Image.read(thumbnail.download)
+    image.resize '180x180'
+    thumbnail.purge
+    thumbnail.attach(io:            File.open(image.path),
+                     filename:      'resized_thumbnail.jpg',
+                     content_type:  'image/jpeg')
   end
 
   private

backend/app/models/tag_implication.rb

@@ -6,7 +6,6 @@ class TagImplication < ApplicationRecord
   validates :parent_tag_id, presence: true
 
   validate :parent_tag_mustnt_be_itself
-  validate :parent_tag_mustnt_create_cycle
 
   private
 
@@ -15,27 +14,4 @@ class TagImplication < ApplicationRecord
       errors.add :parent_tag_id, '親タグは子タグと同一であってはなりません．'
     end
   end
-
-  def parent_tag_mustnt_create_cycle
-    return if tag_id.blank? || parent_tag_id.blank?
-    return if errors[:parent_tag_id].present?
-
-    seen = { }
-    stack = [parent_tag_id]
-
-    until stack.empty?
-      current_id = stack.pop
-      next if seen[current_id]
-
-      seen[current_id] = true
-
-      if current_id == tag_id
-        errors.add :parent_tag_id, '親タグに子孫タグを指定すると循環します．'
-        errors.add :base, 'タグの親子関係が循環します．'
-        return
-      end
-
-      stack.concat(TagImplication.where(tag_id: current_id).pluck(:parent_tag_id))
-    end
-  end
 end

backend/spec/models/tag_implication_spec.rb

@@ -1,36 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe TagImplication, type: :model do
-  it 'rejects a parent tag that would create a cycle' do
-    child = create(:tag, name: 'tag_implication_cycle_child')
-    parent = create(:tag, name: 'tag_implication_cycle_parent')
-
-    described_class.create!(tag: child, parent_tag: parent)
-
-    implication = described_class.new(tag: parent, parent_tag: child)
-
-    expect(implication).not_to be_valid
-    expect(implication.errors[:parent_tag_id]).to include(
-      '親タグに子孫タグを指定すると循環します．'
-    )
-    expect(implication.errors[:base]).to be_present
-  end
-
-  it 'terminates even when existing data already contains a cycle' do
-    child = create(:tag, name: 'tag_implication_existing_cycle_child')
-    parent = create(:tag, name: 'tag_implication_existing_cycle_parent')
-    ancestor = create(:tag, name: 'tag_implication_existing_cycle_ancestor')
-
-    described_class.create!(tag: parent, parent_tag: ancestor)
-    described_class.insert_all!(
-      [
-        { tag_id: ancestor.id, parent_tag_id: parent.id,
-          created_at: Time.current, updated_at: Time.current }
-      ]
-    )
-
-    implication = described_class.new(tag: child, parent_tag: parent)
-
-    expect(implication).to be_valid
-  end
-end

backend/spec/models/tag_spec.rb

@@ -54,17 +54,7 @@ RSpec.describe Tag, type: :model do
       first = create(:tag, name: 'expand_cycle_first')
       second = create(:tag, name: 'expand_cycle_second')
       TagImplication.create!(tag: first, parent_tag: second)
-      now = Time.current
+      TagImplication.create!(tag: second, parent_tag: first)
-      TagImplication.insert_all!(
-        [
-          {
-            tag_id: second.id,
-            parent_tag_id: first.id,
-            created_at: now,
-            updated_at: now
-          }
-        ]
-      )
 
       expect(described_class.expand_parent_tags([first])).to contain_exactly(first, second)
     end

backend/spec/requests/materials_spec.rb

@@ -287,25 +287,6 @@ RSpec.describe 'Materials API', type: :request do
         expect(json.dig('export_paths', 'legacy_drive')).to eq('伊地知ニジカ/created.png')
       end
 
-      it 'creates a create tag_version for a newly created material tag' do
-        expect do
-          post '/materials', params: {
-            tag: 'material_create_versioned_tag',
-            file: dummy_upload(filename: 'created.png')
-          }
-        end.to change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:created)
-
-        tag = Tag.joins(:tag_name).find_by!(tag_names: { name: 'material_create_versioned_tag' })
-        version = tag.tag_versions.order(:version_no).last
-
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_create_versioned_tag')
-        expect(version.category).to eq('material')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
       it 'snapshots attached file metadata and sha256' do
         post '/materials', params: {
           tag: 'material_create_file_version',
@@ -485,73 +466,6 @@ RSpec.describe 'Materials API', type: :request do
         expect(json.dig('tag', 'name')).to eq('material_update_new')
       end
 
-      it 'creates a create tag_version when update creates a new tag' do
-        expect do
-          put "/materials/#{ material.id }", params: {
-            tag: 'material_update_versioned_tag',
-            file: dummy_upload(filename: 'updated.png')
-          }
-        end.to change(Tag, :count).by(1)
-          .and change(TagName, :count).by(1)
-          .and change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:ok)
-
-        tag = Tag.joins(:tag_name).find_by!(tag_names: { name: 'material_update_versioned_tag' })
-        version = tag.tag_versions.order(:version_no).last
-
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_update_versioned_tag')
-        expect(version.category).to eq('material')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
-      it 'backfills a create tag_version for an existing material tag without history' do
-        existing_tag =
-            Tag.create!(tag_name: TagName.create!(name: 'material_update_existing_no_history'),
-                        category: :material)
-
-        expect(existing_tag.tag_versions).to be_empty
-
-        expect do
-          put "/materials/#{ material.id }", params: {
-            tag: 'material_update_existing_no_history',
-            file: dummy_upload(filename: 'updated.png')
-          }
-        end.to change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:ok)
-
-        version = existing_tag.reload.tag_versions.order(:version_no).last
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_update_existing_no_history')
-        expect(version.category).to eq('material')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
-      it 'backfills a create tag_version for an existing character tag without history' do
-        existing_tag =
-            Tag.create!(tag_name: TagName.create!(name: 'material_update_character_no_history'),
-                        category: :character)
-
-        expect(existing_tag.tag_versions).to be_empty
-
-        expect do
-          put "/materials/#{ material.id }", params: {
-            tag: 'material_update_character_no_history',
-            file: dummy_upload(filename: 'updated.png')
-          }
-        end.to change(TagVersion, :count).by(1)
-
-        expect(response).to have_http_status(:ok)
-
-        version = existing_tag.reload.tag_versions.order(:version_no).last
-        expect(version.event_type).to eq('create')
-        expect(version.name).to eq('material_update_character_no_history')
-        expect(version.category).to eq('character')
-        expect(version.created_by_user).to eq(member_user)
-      end
-
       it 'detaches the existing file without purging blob when url replaces file' do
         old_blob_id = material.file.blob.id
 
@@ -580,7 +494,6 @@ RSpec.describe 'Materials API', type: :request do
       it 'does not increase version for the same snapshot update' do
         MaterialVersionRecorder.record!(material:, event_type: :create,
                                         created_by_user: member_user)
-        TagVersioning.ensure_snapshot!(tag, created_by_user: member_user)
 
         expect do
           put "/materials/#{ material.id }", params: {
@@ -588,8 +501,6 @@ RSpec.describe 'Materials API', type: :request do
           }
         end.not_to change(MaterialVersion, :count)
 
-        expect(tag.reload.tag_versions.count).to eq(1)
-
         expect(response).to have_http_status(:ok)
         expect(material.reload.version_no).to eq(1)
       end

backend/spec/requests/posts_spec.rb

@@ -1,5 +1,4 @@
 require 'rails_helper'
-require 'base64'
 require 'set'
 
 include ActiveSupport::Testing::TimeHelpers
@@ -9,11 +8,6 @@ RSpec.describe 'Posts API', type: :request do
   # resized_thumbnail! が MiniMagick 依存でコケやすいので request spec ではスタブしとくのが無難。
   before do
     allow_any_instance_of(Post).to receive(:resized_thumbnail!).and_return(true)
-    allow(Post).to receive(:resized_thumbnail_attachment).and_return(
-      io: StringIO.new('dummy'),
-      filename: 'resized_thumbnail.jpg',
-      content_type: 'image/jpeg'
-    )
   end
 
   def create_nico_tag!(name)
@@ -25,18 +19,6 @@ RSpec.describe 'Posts API', type: :request do
     Rack::Test::UploadedFile.new(StringIO.new('dummy'), 'image/jpeg', original_filename: 'dummy.jpg')
   end
 
-  def real_thumbnail_upload
-    gif =
-        Base64.decode64(
-            'R0lGODdhAQABAIABAP///wAAACwAAAAAAQABAAACAkQBADs=')
-
-    Rack::Test::UploadedFile.new(
-      StringIO.new(gif),
-      'image/gif',
-      original_filename: 'thumbnail.gif'
-    )
-  end
-
   def post_write_params params = { }
     { parent_post_ids: '' }.merge(params)
   end
@@ -714,66 +696,6 @@ RSpec.describe 'Posts API', type: :request do
       expect(json['tags'][0]).to have_key('name')
     end
 
-    it '201 when posting manually with a thumbnail' do
-      sign_in_as(member)
-      allow(Post).to receive(:resized_thumbnail_attachment).and_call_original
-
-      post '/posts', params: post_write_params(
-        title: 'thumbnail post',
-        url: 'https://example.com/thumbnail-post',
-        tags: 'spec_tag',
-        thumbnail: real_thumbnail_upload
-      )
-
-      expect(response).to have_http_status(:created)
-      post_record = Post.find(json.fetch('id'))
-      expect(post_record.thumbnail).to be_attached
-      expect(post_record.thumbnail.blob.content_type).to eq('image/jpeg')
-      expect { post_record.thumbnail.download }.not_to raise_error
-    end
-
-    it 'resizes the thumbnail before the create transaction begins' do
-      sign_in_as(member)
-
-      open_transactions = []
-      baseline_open_transactions = Post.connection.open_transactions
-      allow(Post).to receive(:resized_thumbnail_attachment) do |_upload|
-        open_transactions << Post.connection.open_transactions
-        { io: StringIO.new('dummy'),
-          filename: 'resized_thumbnail.jpg',
-          content_type: 'image/jpeg' }
-      end
-
-      post '/posts', params: post_write_params(
-        title: 'transaction post',
-        url: 'https://example.com/transaction-post',
-        tags: 'spec_tag',
-        thumbnail: dummy_upload
-      )
-
-      expect(response).to have_http_status(:created)
-      expect(open_transactions).to eq([baseline_open_transactions])
-    end
-
-    it 'returns 422 and does not create a post when thumbnail resize fails' do
-      sign_in_as(member)
-      allow(Post).to receive(:resized_thumbnail_attachment).and_raise(MiniMagick::Error)
-
-      expect {
-        post '/posts', params: post_write_params(
-          title: 'broken thumbnail post',
-          url: 'https://example.com/broken-thumbnail-post',
-          tags: 'spec_tag',
-          thumbnail: dummy_upload
-        )
-      }.not_to change(Post, :count)
-
-      expect(response).to have_http_status(:unprocessable_entity)
-      expect(json.fetch('errors')).to include(
-        'thumbnail' => ['サムネイル画像の変換に失敗しました．']
-      )
-    end
-
     it '201 and creates post + tags when member and tags have aliases' do
       sign_in_as(member)
 

backend/spec/requests/tag_children_spec.rb

@@ -56,17 +56,6 @@ RSpec.describe "TagChildren", type: :request do
 
         expect(response).to have_http_status(:no_content)
       end
-
-      it 'returns 422 and does not create relation when the new link makes a cycle' do
-        TagImplication.create!(tag: parent, parent_tag: child)
-
-        expect {
-          do_request
-        }.not_to change(TagImplication, :count)
-
-        expect(response).to have_http_status(:unprocessable_entity)
-        expect(TagImplication.where(tag: child, parent_tag: parent)).not_to exist
-      end
     end
 
     context "when Tag.find raises (invalid ids)" do

backend/spec/requests/tags_spec.rb

@@ -797,17 +797,7 @@ RSpec.describe 'Tags API', type: :request do
       )
       TagImplication.create!(tag: first, parent_tag: root_material)
       TagImplication.create!(tag: second, parent_tag: first)
-      now = Time.current
+      TagImplication.create!(tag: first, parent_tag: second)
-      TagImplication.insert_all!(
-        [
-          {
-            tag_id: first.id,
-            parent_tag_id: second.id,
-            created_at: now,
-            updated_at: now
-          }
-        ]
-      )
 
       get '/tags/with-depth', params: { parent: root_material.id }
 
@@ -1374,6 +1364,8 @@ RSpec.describe 'Tags API', type: :request do
       end
 
       it 'parent_tags に指定すると循環する tag は 422 にする' do
+        pending '#332 で対応予定'
+
         child = Tag.create!(
           tag_name: TagName.create!(name: 'put_cycle_child'),
           category: :general