Bug（バグ） #2601
Minoru Takai が12年以上前に更新
h3. 概要
フレンド数ランキングページ（添付画像参照）には「第1位 OpenPNE君さん :4人」「第2位 fugaさん :1人」のように、ランキングとニックネーム、フレンド数が表示されるが、「フレンド数」に招待中のメンバーが含まれてしまっている。
h3. 対象バージョン
原因がソースコードにあれば、ソースコードは http://trac.openpne.jp/changeset/11826 以降から特に修正されていないため（ #619 では Action から Component へ変更されているが）、全てのバージョンで生じる問題かと思われる（ v0.9.0 で実装された部分なので、それ以降のバージョン全て http://plugins.openpne.jp/package/opRankingPlugin/releases ）。
少なくとも OpenPNE-3.6.0 にバンドルされる v1.0.1 で生じている。
h3. 原因
https://github.com/tejimaya/opRankingPlugin/blob/v1.0.1/lib/opRankingPlugin.php#L55 では、登録済みメンバーに対してフレンド数が多い順に結果を取得している。
<pre><code class="php">
$memberRelationships = Doctrine::getTable('MemberRelationship')->createQuery()
->select('COUNT(*), MemberRelationship.member_id_to')
->where('MemberRelationship.is_friend = ?', true)
->addWhere('m.is_active = ?', true)
->leftJoin('MemberRelationship.Member m')
->groupBy('m.id')
->orderBy('COUNT(*) DESC')
->offset($page * $max)
->limit($max)
->fetchArray();
</code></pre>
これで取得できる値は次のようなもの。
<pre>
array
0 =>
array
'id' => string '2' '1' (length=1)
'member_id_to' 'member_id_from' => string '1' (length=1)
'COUNT' => string '4' (length=1)
1 =>
array
'id' => string '1' '2' (length=1)
'member_id_to' 'member_id_from' => string '2' (length=1)
'COUNT' => string '1' (length=1)
2 =>
array
'id' => string '3' '4' (length=1)
'member_id_to' 'member_id_from' => string '3' (length=1)
'COUNT' => string '1' (length=1)
</pre>
ここで id は意味のない値であり、 member_id_to member_id_from が「登録済みメンバー」のIDであり、 COUNT が「フレンド数」である。
しかし、上記の結果を出力したときの DB は次のようになっている。
<pre>
mysql> select * from member;
+----+------------+------------------+-------------------+---------------------+---------------------+-----------+
| id | name | invite_member_id | is_login_rejected | created_at | updated_at | is_active |
+----+------------+------------------+-------------------+---------------------+---------------------+-----------+
| 1 | OpenPNE君 | NULL | 0 | 2011-10-18 21:39:29 | 2011-10-18 21:39:29 | 1 |
| 2 | fuga | 1 | 0 | 2011-10-24 20:14:11 | 2011-10-24 20:14:48 | 1 |
| 3 | hoge | 1 | 0 | 2011-11-08 19:45:36 | 2011-11-08 21:54:52 | 1 |
| 4 | | 1 | 0 | 2011-11-08 20:25:35 | 2011-11-08 20:25:35 | 0 |
| 5 | | 1 | 0 | 2011-11-16 19:09:39 | 2011-11-16 19:09:39 | 0 |
+----+------------+------------------+-------------------+---------------------+---------------------+-----------+
5 rows in set (0.00 sec)
mysql> select * from member_relationship;
+----+--------------+----------------+-----------+---------------+-----------------+---------------------+---------------------+
| id | member_id_to | member_id_from | is_friend | is_friend_pre | is_access_block | created_at | updated_at |
+----+--------------+----------------+-----------+---------------+-----------------+---------------------+---------------------+
| 1 | 2 | 1 | 1 | 0 | NULL | 2011-10-24 20:14:13 | 2011-10-24 20:14:13 |
| 2 | 1 | 2 | 1 | 0 | NULL | 2011-10-24 20:14:13 | 2011-10-24 20:14:13 |
| 3 | 3 | 1 | 1 | 0 | NULL | 2011-11-08 19:45:38 | 2011-11-08 19:45:38 |
| 4 | 1 | 3 | 1 | 0 | NULL | 2011-11-08 19:45:38 | 2011-11-08 19:45:38 |
| 5 | 4 | 1 | 1 | 0 | NULL | 2011-11-08 20:25:36 | 2011-11-08 20:25:37 |
| 6 | 1 | 4 | 1 | 0 | NULL | 2011-11-08 20:25:37 | 2011-11-08 20:25:37 |
| 7 | 5 | 1 | 1 | 0 | NULL | 2011-11-16 19:09:41 | 2011-11-16 19:09:41 |
| 8 | 1 | 5 | 1 | 0 | NULL | 2011-11-16 19:09:41 | 2011-11-16 19:09:41 |
+----+--------------+----------------+-----------+---------------+-----------------+---------------------+---------------------+
8 rows in set (0.00 sec)
</pre>
* 1, 2, 3 が登録済みメンバーで、 4, 5 は招待中（仮登録中）のメンバー
* 1 <-> 2 がフレンド
* 1 <-> 3 がフレンド
* 1 <-> 4 がフレンド（招待するとこのレコードができる）
* 1 <-> 5 がフレンド（招待するとこのレコードができる）
招待時に、後にフレンド関係とするための設計が悪いとも考えられるが、とにかく招待中のメンバーを考慮せずにフレンド数を計算してしまっている。これは MemberRelationship テーブルを参照するだけでは「相手が登録済みメンバーであるかどうか」が分からず、相手に対して Member.is_active 値が 1 であることを確認しなければならないが、それを行なっていないためである。
冒頭に示した DQL では次の 2 つの条件でしか絞り込んでいない。
* @where('MemberRelationship.is_friend = ?', true)@
** is_friend 値が 1 (true) であるレコードに限る
** これが 0 (false) であるとは、フレンド登録ではなくブラックリスト登録によってレコードが作られた場合や、フレンド登録後にフレンド解消した場合（レコードが削除されないため）を意味する
** しかし 1 (true) であっても、それは相手とフレンド状態である場合以外に、招待中の相手が登録完了した時点でフレンド関係になるためのレコードである場合もある
* @addWhere('m.is_active = ?', true)@
** 自分が登録済みメンバーである場合に限る
** これによって、ランクインする主体として仮登録中のメンバーが含まれないようにしている
しかし、次の条件も必要である。
* @addWhere('(SELECT Member.is_active FROM Member WHERE MemberRelationship.member_id_from = Member.id) = ?', true)@
** 自分とフレンド関係を示すレコードのある相手が登録済みメンバーである場合に限る
h3. 修正方針
原因に示した通り、「自分とフレンド関係にありそうな相手が登録済みメンバーである場合に限る」という条件を加える必要がある。次の差分を取り込むか、あるいは別の修正によってこの問題を解決することが考えられる。
* 原因に示した条件を単に追加した差分
<pre>
diff --git a/lib/opRankingPlugin.php b/lib/opRankingPlugin.php
index f6fc41f..b7b41fd 100644
--- a/lib/opRankingPlugin.php
+++ b/lib/opRankingPlugin.php
@@ -60,6 +60,7 @@ class opRankingPlugin
->select('COUNT(*), MemberRelationship.member_id_to')
->where('MemberRelationship.is_friend = ?', true)
->addWhere('m.is_active = ?', true)
+ ->addWhere('(SELECT Member.is_active FROM Member WHERE MemberRelationship.member_id_from = Member.id) = ?', true)
->leftJoin('MemberRelationship.Member m')
->groupBy('m.id')
->orderBy('COUNT(*) DESC')
</pre>
しかし、もともと記述されている MemberRelationship.member_id_to を主体とするのは不自然かもしれない（from, to を考えると主体は from であるべきである）。また MemberRelationship というテーブル名にエイリアスを付けずに記述しているのが冗長かもしれない。これも併せて修正することも検討したほうがよい。
フレンド数ランキングページ（添付画像参照）には「第1位 OpenPNE君さん :4人」「第2位 fugaさん :1人」のように、ランキングとニックネーム、フレンド数が表示されるが、「フレンド数」に招待中のメンバーが含まれてしまっている。
h3. 対象バージョン
原因がソースコードにあれば、ソースコードは http://trac.openpne.jp/changeset/11826 以降から特に修正されていないため（ #619 では Action から Component へ変更されているが）、全てのバージョンで生じる問題かと思われる（ v0.9.0 で実装された部分なので、それ以降のバージョン全て http://plugins.openpne.jp/package/opRankingPlugin/releases ）。
少なくとも OpenPNE-3.6.0 にバンドルされる v1.0.1 で生じている。
h3. 原因
https://github.com/tejimaya/opRankingPlugin/blob/v1.0.1/lib/opRankingPlugin.php#L55 では、登録済みメンバーに対してフレンド数が多い順に結果を取得している。
<pre><code class="php">
$memberRelationships = Doctrine::getTable('MemberRelationship')->createQuery()
->select('COUNT(*), MemberRelationship.member_id_to')
->where('MemberRelationship.is_friend = ?', true)
->addWhere('m.is_active = ?', true)
->leftJoin('MemberRelationship.Member m')
->groupBy('m.id')
->orderBy('COUNT(*) DESC')
->offset($page * $max)
->limit($max)
->fetchArray();
</code></pre>
これで取得できる値は次のようなもの。
<pre>
array
0 =>
array
'id' => string '2' '1' (length=1)
'member_id_to' 'member_id_from' => string '1' (length=1)
'COUNT' => string '4' (length=1)
1 =>
array
'id' => string '1' '2' (length=1)
'member_id_to' 'member_id_from' => string '2' (length=1)
'COUNT' => string '1' (length=1)
2 =>
array
'id' => string '3' '4' (length=1)
'member_id_to' 'member_id_from' => string '3' (length=1)
'COUNT' => string '1' (length=1)
</pre>
ここで id は意味のない値であり、 member_id_to member_id_from が「登録済みメンバー」のIDであり、 COUNT が「フレンド数」である。
しかし、上記の結果を出力したときの DB は次のようになっている。
<pre>
mysql> select * from member;
+----+------------+------------------+-------------------+---------------------+---------------------+-----------+
| id | name | invite_member_id | is_login_rejected | created_at | updated_at | is_active |
+----+------------+------------------+-------------------+---------------------+---------------------+-----------+
| 1 | OpenPNE君 | NULL | 0 | 2011-10-18 21:39:29 | 2011-10-18 21:39:29 | 1 |
| 2 | fuga | 1 | 0 | 2011-10-24 20:14:11 | 2011-10-24 20:14:48 | 1 |
| 3 | hoge | 1 | 0 | 2011-11-08 19:45:36 | 2011-11-08 21:54:52 | 1 |
| 4 | | 1 | 0 | 2011-11-08 20:25:35 | 2011-11-08 20:25:35 | 0 |
| 5 | | 1 | 0 | 2011-11-16 19:09:39 | 2011-11-16 19:09:39 | 0 |
+----+------------+------------------+-------------------+---------------------+---------------------+-----------+
5 rows in set (0.00 sec)
mysql> select * from member_relationship;
+----+--------------+----------------+-----------+---------------+-----------------+---------------------+---------------------+
| id | member_id_to | member_id_from | is_friend | is_friend_pre | is_access_block | created_at | updated_at |
+----+--------------+----------------+-----------+---------------+-----------------+---------------------+---------------------+
| 1 | 2 | 1 | 1 | 0 | NULL | 2011-10-24 20:14:13 | 2011-10-24 20:14:13 |
| 2 | 1 | 2 | 1 | 0 | NULL | 2011-10-24 20:14:13 | 2011-10-24 20:14:13 |
| 3 | 3 | 1 | 1 | 0 | NULL | 2011-11-08 19:45:38 | 2011-11-08 19:45:38 |
| 4 | 1 | 3 | 1 | 0 | NULL | 2011-11-08 19:45:38 | 2011-11-08 19:45:38 |
| 5 | 4 | 1 | 1 | 0 | NULL | 2011-11-08 20:25:36 | 2011-11-08 20:25:37 |
| 6 | 1 | 4 | 1 | 0 | NULL | 2011-11-08 20:25:37 | 2011-11-08 20:25:37 |
| 7 | 5 | 1 | 1 | 0 | NULL | 2011-11-16 19:09:41 | 2011-11-16 19:09:41 |
| 8 | 1 | 5 | 1 | 0 | NULL | 2011-11-16 19:09:41 | 2011-11-16 19:09:41 |
+----+--------------+----------------+-----------+---------------+-----------------+---------------------+---------------------+
8 rows in set (0.00 sec)
</pre>
* 1, 2, 3 が登録済みメンバーで、 4, 5 は招待中（仮登録中）のメンバー
* 1 <-> 2 がフレンド
* 1 <-> 3 がフレンド
* 1 <-> 4 がフレンド（招待するとこのレコードができる）
* 1 <-> 5 がフレンド（招待するとこのレコードができる）
招待時に、後にフレンド関係とするための設計が悪いとも考えられるが、とにかく招待中のメンバーを考慮せずにフレンド数を計算してしまっている。これは MemberRelationship テーブルを参照するだけでは「相手が登録済みメンバーであるかどうか」が分からず、相手に対して Member.is_active 値が 1 であることを確認しなければならないが、それを行なっていないためである。
冒頭に示した DQL では次の 2 つの条件でしか絞り込んでいない。
* @where('MemberRelationship.is_friend = ?', true)@
** is_friend 値が 1 (true) であるレコードに限る
** これが 0 (false) であるとは、フレンド登録ではなくブラックリスト登録によってレコードが作られた場合や、フレンド登録後にフレンド解消した場合（レコードが削除されないため）を意味する
** しかし 1 (true) であっても、それは相手とフレンド状態である場合以外に、招待中の相手が登録完了した時点でフレンド関係になるためのレコードである場合もある
* @addWhere('m.is_active = ?', true)@
** 自分が登録済みメンバーである場合に限る
** これによって、ランクインする主体として仮登録中のメンバーが含まれないようにしている
しかし、次の条件も必要である。
* @addWhere('(SELECT Member.is_active FROM Member WHERE MemberRelationship.member_id_from = Member.id) = ?', true)@
** 自分とフレンド関係を示すレコードのある相手が登録済みメンバーである場合に限る
h3. 修正方針
原因に示した通り、「自分とフレンド関係にありそうな相手が登録済みメンバーである場合に限る」という条件を加える必要がある。次の差分を取り込むか、あるいは別の修正によってこの問題を解決することが考えられる。
* 原因に示した条件を単に追加した差分
<pre>
diff --git a/lib/opRankingPlugin.php b/lib/opRankingPlugin.php
index f6fc41f..b7b41fd 100644
--- a/lib/opRankingPlugin.php
+++ b/lib/opRankingPlugin.php
@@ -60,6 +60,7 @@ class opRankingPlugin
->select('COUNT(*), MemberRelationship.member_id_to')
->where('MemberRelationship.is_friend = ?', true)
->addWhere('m.is_active = ?', true)
+ ->addWhere('(SELECT Member.is_active FROM Member WHERE MemberRelationship.member_id_from = Member.id) = ?', true)
->leftJoin('MemberRelationship.Member m')
->groupBy('m.id')
->orderBy('COUNT(*) DESC')
</pre>
しかし、もともと記述されている MemberRelationship.member_id_to を主体とするのは不自然かもしれない（from, to を考えると主体は from であるべきである）。また MemberRelationship というテーブル名にエイリアスを付けずに記述しているのが冗長かもしれない。これも併せて修正することも検討したほうがよい。