LINEヤフー Tech Blog

LINEヤフー株式会社のサービスを支える、技術・開発文化を発信しています。

コード品質向上のテクニック:第41回 「アーキテクチャ」ただいま工事中

こんにちは。コミュニケーションアプリ「LINE」のモバイルクライアントを開発している石川です。

この記事は、毎週木曜の定期連載 "Weekly Report" 共有の第 41 回です。 LINEヤフー社内には、高い開発生産性を維持するための Review Committee という活動があります。ここで集まった知見を、Weekly Report と称して毎週社内に共有しており、その一部を本ブログ上でも公開しています。(Weekly Report の詳細については、過去の記事一覧を参照してください)

「アーキテクチャ」ただいま工事中

メッセージを送受信するアプリケーションを作成していると仮定しましょう。送受信するメッセージのデータモデルは以下のように定義されています。

class MessageModel internal constructor(
    val messageId: String,
    val sender: UserId,
    val contentModel: MessageContentModel
)

sealed interface MessageContentModel {
    class Text(
        val messageText: String
    ): MessageContentModel

    class Image(
        val imageUri: Uri,
        ...
    ): MessageContentModel

    ...

    class ExternalResource(
        val resourceId: ...,
        ...
    ): MessageContentModel
}

MessageContentModel の示す通り、メッセージにはいくつかのタイプがあります。このとき、タイプは messageId の先頭文字で決められるとします。(更に、この messageId の仕様は、歴史的経緯により変えることができないことを前提にしてください。)

  • "t": テキスト
  • "i": 画像
  • ...
  • "e": 外部リソース

例えば、"t02af6d450d056" という ID の先頭文字は "t" なので、これはテキストメッセージの ID です。

ここで、ExternalResource インスタンスのは他の MessageContentModel インスタンスの作成ロジックは全く異なるとしましょう。その場合、MessageModel を提供するクラス (...Dao) も ExternalResource とそれ以外で分けて実装することもあります。

class MainMessageDao {
    fun queryMessageModel(messageId: String): MessageModel? { ... }
}

class ExternalResourceMessageDao {
    fun queryMessageModel(messageId: String): MessageModel? { ... }
}

messageId に対応するメッセージが存在しない場合、関数 queryMessageModel は null を返します。したがって、テキストメッセージの ID を ExternalResourceMessageModelDao.queryMessageModel に渡すと、null が返されます。

メッセージのタイプに合わせて MainMessageDaoExternalResourceMessageDao を使い分けるのは煩雑で、バグの原因にもなります。そこで、正しい ...Dao を選択する役割を持つ MessageModelRepository というクラスを以下のように作りました。ここで、各 ...Daosupports 関数は、与えられた messageId に対応しているかどうかを Boolean で返します。

class MessageModelRepository(
    private val mainMessageDao: MainMessageDao,
    private val externalResourceMessageDao: ExternalResourceMessageDao,
    ...
) {
    fun queryMessageModel(messageId: String): MessageModel? {
        ... // other code
        val dao = when {
            MainMessageDao.supports(messageId) -> mainMessageDao
            ExternalResourceMessageDao.supports(messageId) -> externalResourceMessageDao
            else -> null
        }
        ...
        return dao?.queryMessageModel(messageId)
    }

    companion object {
        fun isMainMessage(messageId: String): Boolean = ...
        fun isExternalResourceMessage(messageId: String): Boolean = ...
    }
}

このようにすることで、MessageModelRepository を使う側としては、...Dao の存在を意識することなく MessageModel を取得できます。

以下は MessageModelRepository のテストコードです。

class MessageModelRepositoryTest {
    @get:Rule
    val mockitoRule: MockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS)

    private val mockMainMessageDao: MainMessageDao = mock()

    private val mockExternalResourceMessageDao: ExternalResourceMessageDao = mock()

    private lateinit var subject: MessageModelRepository

    @Before
    fun setUp() {
        ... // Setup code
        subject = MessageModelRepository(
            mockMainMessageDao,
            mockExternalResourceMessageDao,
            ...,
        )
    }

    @Test
    fun queryMessageModel_returnsNull_forMalformedId() {
        val nullMessage = subject.queryMessageModel(MALFORMED_MESSAGE_ID)

        assertNull(nullMessage)
        verify(mockMainMessageDao, never()).queryMessageModel(any())
        verify(mockExternalResourceMessageDao, never()).queryMessageModel(any())
    }

    @Test
    fun queryMessageModel_returnsModelOfMainMessage() {
        mockMainMessageDao.stub {
            on { queryMessageModel(TEXT_MESSAGE_ID) } doReturn
                TestMessageModels.TEXT
            on { queryMessageModel(IMAGE_MESSAGE_ID) } doReturn
                TestMessageModels.IMAGE
            ...
        }

        assertIs<MessageContentModel.Text>(subject.queryMessageModel(TEXT_MESSAGE_ID))
        assertIs<MessageContentModel.Image>(subject.queryMessageModel(IMAGE_MESSAGE_ID))
        ...
    }

    @Test
    fun queryMessageModel_returnsModelOfExternalResource() {
        mockExternalResourceMessageDao.stub {
            on { queryMessageModel(EXTERNAL_RESOURCE_MESSAGE_ID) } doReturn
                TestMessageModels.EXTERNAL_RESOURCE
        }

        val model = subject.queryMessageModel(EXTERNAL_RESOURCE_MESSAGE_ID)
        assertIs<MessageContentModel.ExternalResource>(model)
    }

    companion object {
        private const val MALFORMED_MESSAGE_ID = "x123456"
        private const val TEXT_MESSAGE_ID = "t123456"
        ...
    }
}

このテストでは、以下の動作が正しいことを確認しています。

  • メッセージ ID の形式が正しくない場合は null を返し、どの DAO も使われない。
  • テキスト、画像などのタイプでは MainMessageDao だけが使われる (Strictness.STRICT_STUBS により保証される)
  • 外部リソースのタイプでは ExternalResourceMessageDao だけが使われる (Strictness.STRICT_STUBS により保証される)

このテストにより、val dao = when { のすべてのケースが網羅されています。これに加えてテストするべき点は何かありますか?

工事中専用のテスト

上記のテストは、「現在のコードが正しい」ことを証明するためなら十分です。しかし、将来の変更によるバグは検出できません。新しい MessageContentModel とそれに対応する DAO を追加したとき、MessageModelRepository の更新を忘れることが考えられます。その場合、新しいタイプの ID は単に不正な ID として扱われてしまうため、実装漏れのバグが見過ごされてしまうかもしれません。例えば、MessageContentModel.FooFooMessageDao を追加したときに queryMessageModel の更新を忘れるとバグになるのですが、それは実行時にしか気付きにくいです。

これを防ぐには、以下のように「テストされた MessageContentModel の一覧」と「定義された MessageContentModel の一覧」を比較すると良いでしょう。

    ...

    @Test
    fun queryMessageModel_returnsModelOfMainMessage() {
        mockMainMessageDao.stub {
            on { queryMessageModel(TEXT_TEST_DATA.id) } doReturn
                TestMessageModels.TEXT
            on { queryMessageModel(IMAGE_TEST_DATA.id) } doReturn
                TestMessageModels.IMAGE
            ...
        }

        assertTypeOf(TEXT_TEST_DATA.type, subject.queryMessageModel(TEXT_TEST_DATA.id))
        assertTypeOf(IMAGE_TEST_DATA.type, subject.queryMessageModel(IMAGE_TEST_DATA.id))
        ...
    }

    @Test
    fun queryMessageModel_returnsModelOfExternalResource() {
        mockExternalResourceMessageDao.stub {
            on { queryMessageModel(EXTERNAL_RESOURCE_TEST_DATA.id) } doReturn
                TestMessageModels.EXTERNAL_RESOURCE
        }

        val model = subject.queryMessageModel(EXTERNAL_RESOURCE_TEST_DATA.id)
        assertTypeOf(EXTERNAL_RESOURCE_TEST_DATA.type, model)
    }

    @Test
    fun ensureCompletenessOfMessageContentModel() {
        val knownTypes: Set<KClass<out MessageContentModel>> = setOf(
            TEXT_TEST_DATA.type,
            IMAGE_TEST_DATA.type,
            ...,
            EXTERNAL_RESOURCE_TEST_DATA.type
        )
        ContentModel::class.sealedSubclasses.forEach { type ->
            assertTrue(
                type in knownTypes,
                "Unknown type: $type. Update [MessageModelRepository] to cover it."
            )
        }
    }

    private class TestData<T : MessageContentModel>(val id: String, val type: KClass<T>)

    companion object {
        @Suppress("UNUSED_PARAMETER") // `type` is to specify `T` without a type parameter.
        private inline fun <reified T : Any> assertTypeOf(type: KClass<T>, actual: Any?) =
            assertTrue(actual is T)

        private val TEXT_TEST_DATA = TestData("t123456", MessageContentModel.Text::class)
        private val IMAGE_TEST_DATA = TestData("i123456", MessageContentModel.Image::class)
        ...
        private val EXTERNAL_RESOURCE_TEST_DATA = TestData("e123456", MessageContentModel.ExternalResource::class)
    }

このようにすることで、新たなタイプが追加されたときに「MessageModelRepository も更新しなければならない」というメッセージとともにテストが失敗するため、開発者は何をすればよいかが分かり、実装漏れを防ぐことができます。

一言まとめ

テストは現在のコードだけでなく、将来の変更にも対応できるとよい。

タグ: test, completeness, specification update

コード品質向上のテクニックの他の記事を読む

コード品質向上のテクニックの記事一覧