Fix: #442 カスタム絵文字のaliasesにnullが入る場合がある (#443)

* Fix: #442 カスタム絵文字のaliasesにnullが入る場合がある

* Fix test

* WebのほうにもNULLチェックを追加

* ローカルのエイリアス名を編集するところもついでにスリムに

app/helpers/jsonld_helper.rb

@@ -51,6 +51,14 @@ module JsonLdHelper
     end
   end
 
+  def as_array_ex(value)
+    if value.is_a?(Hash)
+      []
+    else
+      as_array(value)
+    end
+  end
+
   def value_or_id(value)
     value.is_a?(String) || value.nil? ? value : value['id']
   end

app/javascript/mastodon/features/emoji/emoji_utils.js

@@ -12,7 +12,7 @@ const buildSearch = (data) => {
     }
 
     (Array.isArray(strings) ? strings : [strings]).forEach((string) => {
-      (split ? string.split(/[-|_|\s]+/) : [string]).forEach((s) => {
+      (split ? string.split(/[-|_|\s]+/) : [string]).filter((s) => s).forEach((s) => {
         s = s.toLowerCase();
 
         if (search.indexOf(s) === -1) {

app/lib/activitypub/parser/custom_emoji_parser.rb

@@ -16,7 +16,7 @@ class ActivityPub::Parser::CustomEmojiParser
   end
 
   def aliases
-    as_array(@json['keywords'])
+    as_array_ex(@json['keywords'])
   end
 
   def image_remote_url

app/models/custom_emoji.rb

@@ -72,11 +72,11 @@ class CustomEmoji < ApplicationRecord
   def copy!
     copy = self.class.find_or_initialize_by(
       domain: nil,
-      shortcode: shortcode,
-      license: license,
-      aliases: aliases,
-      is_sensitive: is_sensitive
+      shortcode: shortcode
     )
+    copy.aliases = (aliases || []).compact_blank
+    copy.license = license
+    copy.is_sensitive = is_sensitive
     copy.image = image
     copy.tap(&:save!)
   end
@@ -96,7 +96,7 @@ class CustomEmoji < ApplicationRecord
   end
 
   def aliases_raw=(raw)
-    aliases = raw.split(',').filter(&:present?).uniq
+    aliases = raw.split(',').compact_blank.uniq
     self[:aliases] = aliases
   end
 

app/serializers/rest/custom_emoji_serializer.rb

@@ -5,9 +5,13 @@ class REST::CustomEmojiSerializer < REST::CustomEmojiSlimSerializer
 
   # Please update `app/javascript/mastodon/api_types/custom_emoji.ts` when making changes to the attributes
 
-  attribute :aliases, if: :aliases?
+  attribute :aliases
 
-  def aliases?
-    object.respond_to?(:aliases) && object.aliases.present?
+  def aliases
+    if object.respond_to?(:aliases) && object.aliases.present?
+      object.aliases.compact_blank
+    else
+      []
+    end
   end
 end

spec/models/custom_emoji_spec.rb

@@ -49,6 +49,52 @@ RSpec.describe CustomEmoji do
     end
   end
 
+  describe '#copy!' do
+    subject do
+      custom_emoji.copy!
+      described_class.where.not(id: custom_emoji.id).find_by(domain: nil, shortcode: custom_emoji.shortcode)
+    end
+
+    context 'when a simple case' do
+      let(:custom_emoji) { Fabricate(:custom_emoji, license: 'Ohagi', aliases: %w(aaa bbb), domain: 'example.com', uri: 'https://example.com/emoji') }
+
+      it 'makes a copy ot the emoji' do
+        emoji = subject
+        expect(emoji).to_not be_nil
+        expect(emoji.license).to eq 'Ohagi'
+        expect(emoji.aliases).to eq %w(aaa bbb)
+      end
+    end
+
+    context 'when local has already same emoji' do
+      let(:custom_emoji) { Fabricate(:custom_emoji, domain: nil) }
+
+      it 'does not make a copy of the emoji' do
+        expect(subject).to be_nil
+      end
+    end
+
+    context 'when aliases is null' do
+      let(:custom_emoji) { Fabricate(:custom_emoji, aliases: nil, domain: 'example.com', uri: 'https://example.com/emoji') }
+
+      it 'makes a copy of the emoji but aliases property is normalized' do
+        emoji = subject
+        expect(emoji).to_not be_nil
+        expect(emoji.aliases).to eq []
+      end
+    end
+
+    context 'when aliases contains null' do
+      let(:custom_emoji) { Fabricate(:custom_emoji, aliases: [nil], domain: 'example.com', uri: 'https://example.com/emoji') }
+
+      it 'makes a copy of the emoji but aliases property is normalized' do
+        emoji = subject
+        expect(emoji).to_not be_nil
+        expect(emoji.aliases).to eq []
+      end
+    end
+  end
+
   describe '#object_type' do
     it 'returns :emoji' do
       custom_emoji = Fabricate(:custom_emoji)

spec/models/form/custom_emoji_batch_spec.rb

@@ -95,12 +95,12 @@ describe Form::CustomEmojiBatch do
     end
 
     describe 'the copy action' do
-      let(:custom_emoji) { Fabricate(:custom_emoji) }
+      let(:custom_emoji) { Fabricate(:custom_emoji, domain: 'example.com', uri: 'https://example.com/emoji') }
       let(:options) { { action: 'copy', custom_emoji_ids: [custom_emoji.id] } }
 
       it 'makes a copy of the emoji' do
         expect { subject.save }
-          .to change(CustomEmoji, :count).by(1)
+          .to change(CustomEmoji, :count).by(2)
       end
     end
 

spec/serializers/rest/custom_emoji_serializer_spec.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe REST::CustomEmojiSerializer do
+  let(:serialization) { serialized_record_json(record, described_class) }
+  let(:record) do
+    Fabricate(:custom_emoji, shortcode: 'ohagi', aliases: aliases)
+  end
+  let(:aliases) { [] }
+
+  context 'when empty aliases' do
+    it 'returns normalized aliases' do
+      expect(serialization['aliases']).to eq []
+    end
+  end
+
+  context 'when null aliases' do
+    let(:aliases) { nil }
+
+    it 'returns normalized aliases' do
+      expect(serialization['aliases']).to eq []
+    end
+  end
+
+  context 'when aliases contains null' do
+    let(:aliases) { [nil] }
+
+    it 'returns normalized aliases' do
+      expect(serialization['aliases']).to eq []
+    end
+  end
+
+  context 'when aliases contains normal text' do
+    let(:aliases) { ['neko'] }
+
+    it 'returns normalized aliases' do
+      expect(serialization['aliases']).to eq ['neko']
+    end
+  end
+end