複雑化したプレスリリース検索の関数を安全に移行する

2025年9月25日

URLをコピーしました！

こんにちは。PR TIMESでインターンを行っている鐘ヶ江航です。レガシーコードの改善を中心に、バックエンド開発を行っています。

今回はそのレガシー改善の一環として、複雑化したプレスリリース検索の関数を改善した例についてご紹介します。

背景

PR TIMESには、一部のバッチ処理などで使われているプレスリリース検索関数が存在します。検索関数はRSSフィードの作成にも用いられています。PR TIMESのRSSは、転載先メディアとの連携にも使われている、重要な機能の1つです。具体的には1時間に2回、それぞれ約300件のRSSフィード作成バッチで実行されています。

また、この関数には多くのスロークエリ・N+1クエリが含まれており、バッチの実行時にはDBの負荷も非常に高くなっています。これを改善できれば、DBインスタンスのダウングレードによるコスト削減が可能となり、さらにRSSの更新頻度を上げることが可能となります。特に、将来的にプレスリリースの数が増えた際、現在の更新頻度では配信漏れが起こるリスクも考えられるため、その対策としてRSSの更新頻度を上げられることが望ましいです。

よって今回、私は古いプレスリリース検索関数の仕様を変えないまま、新しく作成したメソッドへ移行することを行いました。

既存の検索関数の問題点

既存のプレスリリース検索関数は、10年以上前から存在するレガシーコードであり、様々な問題点が存在しました。中でも主要な問題点は以下です。

可読性・保守性の低さ
スロークエリ・N+1クエリによるパフォーマンスの低下
影響範囲の広さ

最も大きな問題は、可読性の低さです。これまではさまざまな検索パターンに1つの関数で対応していました。そのため、検索関数の行数は1500行程度、条件分岐も複雑で解読困難なコードと化しています。またそもそも行数の多さから、ユニットテストを実装することも困難になります。

加えて、影響範囲の広さもこの関数の修正を難しくしています。特にRSSフィードの作成バッチは影響が大きい箇所であり、障害が発生した場合にはプレスリリースの誤配信や配信漏れなど、重大な問題につながります。その他、多くのバッチ・APIでも使用されているため、正確にQAを行うにも時間がかかります。

移行手順

今回の移行では、古い検索関数（以降、旧関数と呼称）には基本触れず、旧関数を模した新しい検索メソッド（以降、新メソッドと呼称）を作成します。そして旧関数の一部の呼び出し元では、新メソッドを呼び出すよう変更することを目指します。安全に、かつ現実的に移行を進めるため、私が工夫した点についてお伝えします。

メソッド移行の範囲を限定する

私が検索関数を移行するため初めに行ったことは、「移行範囲を絞ること」です。繰り返しになりますが、この検索関数は様々な検索パターンに対応した多機能な関数であり、様々なファイルから呼び出されています。

今回の移行の最終目的はDB負荷を減少させることであるため、修正範囲をRSSフィード作成バッチに限定しました。具体的には、旧関数を呼び出している箇所のうち1箇所に限定し、新メソッドに置き換えることを目指します。

旧検索関数は様々な検索パターンをオプションで指定ができますが、RSSフィードの作成時に使用されているオプションはごく一部です。実際にRSSフィードの生成時と同じオプションで旧関数を実行した場合、約1500行あるコードのうち500行ほどしか実行されていないことが分かりました。よって、メソッド移行の範囲を限定することで、移行後の新メソッドは最低限の機能を持った、小規模なメソッドにすることができます。

移行後、新メソッドはRSSフィードの作成にのみ責任を持ちます。これは単一責任原則（SRP）から考えても、良い改善です。例えば既存の検索関数では、RSS配信の仕様変更があった際、検索関数にオプションを追加する必要がありました。

function SearchRelease($terms)
{
    // ... 省略 ...

     // RSS作成時に位置情報データを取得する
    if ($terms['user'] == 'rss') {
        $press_release_location = PressReleaseLocationRepo::getRowsById($release_ids);
        foreach($press_release_location as $release_id => $location) {
            $data[$release_id]['location_data'] = $location;
        }
    }

    // ... 省略 ...
}

新メソッドは、RSSフィードの生成時に使われる検索機能としての責任しか持ちません。よって以下のように、簡潔に仕様変更をすることが可能となります。変更した際の影響範囲も少ないため、保守も容易です。

public function searchReleaseForRss($opts)
{
    // ... 省略 ...

    // 位置情報を取得
    $press_release_location = PressReleaseLocationRepo::getRowsById($release_ids);
    foreach($press_release_location as $release_id => $location) {
        $data[$release_id]['location_data'] = $location;
    }

    //... 省略 ...
}

本番環境で新・旧関数の差分を検証する

次のステップとして、新・旧関数の差分検証を行いました。

旧関数で叩かれているクエリは、複雑な条件分岐で構築されている上、クエリ単体で100行を超えるようなものも存在します。メソッド移行の範囲を限定することで必要な機能を絞ったとしても、新メソッドを1から書き直し、安全に実装することは現実的ではありません。よって、旧関数を丸ごとコピーした新メソッドを作成し、コードの削除・修正を進めていく方針で作業に着手しました。

旧関数にはユニットテストが存在せず、新メソッドに移行した際、同じ動作をする保証はどこにもありません。新メソッドに修正を加える前に、以下の手順で最初のプルリクエストを作成しました。

旧関数を丸ごとコピーし、新メソッドを作成
バッチ処理で旧関数実行後、新メソッドも実行する処理を追加
旧関数・新メソッドの実行結果が異なる場合、差分を要約してログに出力

これにより、その後の修正（不要なコードの削除・N+1クエリの改善など）において、QAの基準を設けることができました。

function checkSearchReleaseForRss($press_releases_old, $press_releases_new, $rss_id, $logger): void
{
    $diff = []; // プレスリリースの内容の差分
    $press_releases_old_only = []; // 旧関数でしか取得されなかったリリース
    $press_releases_new_only = []; // 新関数でしか取得されなかったリリース

    // ... 差分検証処理 ...

    // 差分がない場合
    if (count($diff) <= 0 && count($press_releases_new_only) <= 0) {
        return;
    }

    $diff_data = [
        'rss_id' => $rss_id,
        'diff' => $diff,
        'press_releases_new_only' => $press_releases_new_only,
        'press_releases_old_only' => $press_releases_old_only,
    ];
    $encoded_diff_data = json_encode($diff_data);
    $logger->warning('rss data is not equal', ["diff" => $encoded_diff_data]);
}

新メソッドから不要なコードを削除する

新メソッドは旧関数をコピーしたものであり、RSSフィードの作成時に到達しない条件分岐がいくつも含まれています。本来であれば新メソッドを小さいメソッドに細分化することから始めるべきかもしれませんが、今回はコード削除により削れる範囲が大きいと分かっていたため、こちらを先に行いました。

削除の際には、各条件分岐が本当に使われていないものかを確認するため、if文にログを仕込んで一定期間監視を行い、ログが出力されていなければ削除を行います。作業を2つに分割しなければならないため、当然効率は落ちますが、細かく作業を進めた方が結果的にスムーズに進むというのが、今回私が学んだことの一つです。

if (isset($opts) && $opts) {
    $logger->notice(' LINE : ' . __LINE__ . ' is used.', ['opts' => $opts]);
    
    // 省略...
    
}

テストコードを実装する

実際にコードの改善（N+1クエリの解消など）を進める前に、最後に行ったことは、メソッドの細分化とテストコードの実装です。今回の旧関数は行数も多いレガシーコードで、ユニットテストも存在しない状態でした。メソッドの修正は本来、テストコードを満たすことを確認しつつ行われるべきです。今回も、旧関数にテストを実装してから修正を行いました。

まず、旧関数全体のテストを作成します。旧関数の仕様を完全に理解することは難しいため、いわゆる「ハッピーパス」での最低限のテストを実装しました。

次に行ったことは、旧関数の細分化です。N+1クエリを含む、旧関数の修正したい箇所を別の関数にくり抜き、後のコード修正の軸となるユニットテストを実装しました。旧関数と新メソッドが同じテストを通ることを確認することで、同じ結果が返されることを確認できます。

苦労した点

私はPR TIMESでのインターンを始めるまで、実際に動いているレガシーコードをこれだけ読む機会はありませんでした。今回も実際に検索関数を移行する過程で、様々な場面で躓きつつ、時間をかけて対応してきました。その一部についてお伝えします。

レガシーコードの解読

解体しつつ可読性を上げてきましたが、当然一度は1500行あるコードに目を通さないと修正ができません。これはこの検索関数が放置されてきた理由の一つであるはずです。RSSフィード生成バッチに関連するコードはほとんどがレガシーコードです。そのため何度もレガシーコードを読む機会がありました。

初め私は「RSS」という仕組み自体を知らない状態からこのタスクを進めたため、レガシーコードで無くても分からないことだらけでした。コメントやドキュメントが残っていないようなコードは、知っている人に聞くことでしか仕様が分からないことも多いため、もっと頻繁に質問したり、ペアプロをお願いすべきだったかもしれないと今では考えています。

同時に、ドキュメントやコメントが残っていないコードの怖さが身に沁みて分かりました。コードレビューの際にも「ここが分かりにくいので、コメントを書いてください」と指摘されたこともあり、現在は後から見て仕様や意図が分かるコードを書くよう意識するようになりました。

差分結果の許容範囲

新・旧関数で差分を取りログを出力する際、旧関数をコピーして新メソッドを作成したにも関わらず、初めは大量のログが出力されてしまう状態でした。主な原因は、旧関数を実行してから新メソッドを実行する間に、DBの中の値が更新されてしまうことです。

そのため、差分が出る原因を1つ1つ特定し場合分けをすることで、出力されるログを減らしていきました。例として、RSSフィード作成バッチの実行時間内に更新されたプレスリリースについては、差分としてカウントしない、などです。

$releases_new_only = []
foreach ($releases__new as $press_release_id => $release_data) {
    // 新メソッドでは取得されているが、旧関数では取得されていないプレスリリースを差分に追加
    if (!key_exists($press_release_id, $releases_old)) {
    
        // バッチ実行時間内に更新されたプレスリリースは無視する
        $update_date = (new DateTime($release_data['update_date']))->getTimestamp();
        if ($start_time <= $update_date && $update_date <= $end_time) {
            continue;
        }

        $releases_new_only[] = [
            'rid' => $rid,
            'update_date' => $release_data['update_date'],
        ];
    }
}

検索関数内で参照しているテーブルは複数あるため、場合分けをしてもなお、差分ログは一日数件程度は出力されていました。そのため最終的には、それらの差分ログが作業影響ではないか、それぞれ目視で確認を行いました。

結果

次に実際に私がレガシーコード改善を行い、移行した結果についてお伝えします。私が行った主な改善内容は、

不要なコードを削除し、コードを500行程度まで削減
新・旧関数へテストコードを実装
新・旧関数の一部をさらに細分化した関数へ分割
N+1クエリは一部を除き全て解消

です（一部のN+1クエリは、影響範囲が大きくすぐには解消できなかったため残しています）。

可読性・保守性

移行後、旧関数に存在した複雑な条件分岐はまだ一部残っていますが、100行程度に削減でき、メンテナンスもしやすくなりました。一部を小さいメソッドに切り分けたことで、今後の修正も行いやすくなったといえます。

また、ハッピーパスのテストもありますが、ユニットテストで担保できる範囲が増えたことも大きいです。いままでファイル内に直接書かれていたクエリも、一部をレポジトリ層に切り分けることでユニットテストを通すことができています。

パフォーマンス

N+1クエリをほとんど解消することができたため、当初はパフォーマンスが大きく改善されることを期待していました。しかし実際に新メソッドを適用後、DBの負荷・バッチの実行時間ともに、ほとんど変化は感じられませんでした。

今回の変更では目に見えた改善はありませんでしたが、

DB負荷の原因として、可能性を一つ無くすことができた
今後スロークエリを改善するための基盤を整えることができた

の2点は、成果として十分だと考えています。今後の改善に期待したいところです。

最後に

RSSはPR TIMESのサービスの中でも重要な機能の一つなので、今回の一連の作業はやりがいがあると同時に、もし障害を起こしてしまったらと思うと怖い作業でもありました。レガシーコードを触った経験がない状態からインターンに参加し、このような経験ができたことを嬉しく思っています。

最終的に目に見えたパフォーマンス改善ができなかったことは心残りですが、初めは何から進めれば良いか分からなかったレガシー改善を一段落させることができて安心しました。

今後もやりがいのある課題に、積極的にチャレンジしていきたいと思います！

URLをコピーしました！

この記事を書いた人

鐘ヶ江航

PR TIMESでインターン生としてバックエンド開発を行っています！

この著者の記事一覧へ