Project

General

Profile

Backport(バックポート) #2684

プロフィール項目の不具合について

Added by Kousuke Ebihara almost 8 years ago. Updated almost 7 years ago.

Status:
Fixed(完了)
Priority:
Normal(通常)
Assignee:
Target version:
Start date:
Due date:
% Done:

100%


Description

http://sns.openpne.jp/communityTopic/6709 より転記

http://sns.openpne.jp/cache/img/png/w_h/a5e00bca971049df8db393b769b1381a1f20d217_png.png
 http://sns.openpne.jp/cache/img/png/w_h/9499800169021d9a54fef4b90d8f792535b16bd0_png.png

(1) OpenPNE-3.4.2とOpenPNE-3.4.9.2で確認しております。

(2) さくらレンタルサーバー プレミアムプラン
OS: FreeBSD 7.1-RELEASE-p15 i386
PHP: 5.2.14
MySQL: 5.1

(3) OpenPNEの管理画面の「プロフィール項目設定」→「プロフィール項目登録」を表示しまして
「公開設定の選択」を「メンバー選択」、「フォームタイプ」を「複数選択(チェックボックス)」を選択、
項目名・説明・識別名は適切な文字列を入力後、
その他の設定はデフォルトで追加しました。
同様の手順で項目を追加し、全部で三つのプロフィール項目を追加しました。

そして、プロフィール選択肢一覧で項目名に適切な文字列を入力後
追加して、それぞれ、三つの選択肢を追加しました。

ここまでで三つのプロフィール項目と、それぞれ三つの選択肢が作成されています。

その後、SNSにログインしまして、プロフィール編集で
先ほど追加した三つの項目のチェックボックスを全て選択後、送信。
三つの項目のチェックボックスを全て外して、また送信。
最後に、再び全てのチェックボックスを選択して送信しますと
一番目の項目は、三番目の選択肢のみが表示されて
二番目の項目は、二番目の選択肢のみが表示されて
三番目の項目は、全ての選択肢が表示されるという
現象が発生しました。

また、OpenPNE-3.4.9.2で上記の手順を最後まで試したところ
三番目の項目のみ、全ての選択肢が表示され、
その他の項目は表示されないという異なった結果となりました。
その後に、プロフィール編集を表示しますとOpenPNE-3.4.2と同じ
選択肢がチェックされていました。

これらの動作検証は管理アカウント(sns@example.com)で行っておりました。
しかし、その後に新しくアカウントを作成して動作検証をしたところ、正常に動作するものと同様の症状が発生するものとで別れることが分かっております。

それと色々と試行錯誤したところ、表示される選択肢と表示されない選択肢が
ランダムで選ばれるように見受けられ、パターンが把握できないのも特徴の一つとして挙げられます。
(先の例で言いますと、何故一番目の項目は三番目の選択肢だけが表示され
二番目の項目は二番目の選択肢だけが表示されるのかが分からないという事です)

文面だけでは伝わりづらいかと思いまして、スクリーンショットを添付しました。
写真1は管理画面のプロフィール項目設定で、写真2はプロフィール編集→プロフィール確認の画面と、phpmyadminの画面及び補足です。

以上、宜しくお願いいたします。


Related issues

Related to OpenPNE 3 - Bug(バグ) #1865: プロフィール項目の不具合について Fixed(完了) 2011-01-12
Related to OpenPNE 3 - Bug(バグ) #3294: 選択肢を持つプロフィール項目を作成し、ユーザーが選択肢を入力後に選択肢IDを削除すると、ユーザーのプロフィールを更新することができなくなる。 Won't fix(対応せず) 2013-01-11

Associated revisions

Revision 08e534fd (diff)
Added by Yuya Watanabe almost 8 years ago

(fixes #2684, BP from #1865) added 'and query' and 'order query'.

BP from #1865
b51157e2bdcf378d31bcf25553b64b3de698f5df

History

#1 Updated by Kousuke Ebihara almost 8 years ago

  • Due date set to 2012-01-12

#2 Updated by Yuya Watanabe almost 8 years ago

  • Status changed from New(新規) to Accepted(着手)
  • Assignee set to Yuya Watanabe

#3 Updated by Yuya Watanabe almost 8 years ago

  • Description updated (diff)

#4 Updated by Yuya Watanabe almost 8 years ago

バックポート元の修正が正しいかどうかについての中間報告.

複数選択で用いられるデータ構造について

参考から転記: https://redmine.openpne.jp/issues/2478#note-5

データベース内で木構造を表現する手法として「入れ子集合モデル」というものがあり,本チケットで扱われている複数項目のレコード記録方法としてこのモデルが使われているようである.

config/doctrine/schema.yml 145行目でNestedSetと定義されている

143 MemberProfile:
144   actAs:
145     NestedSet:
146       hasManyRoots: true
147       rootColumnName: tree_key
148     Timestampable:
149   columns:
150     id: { type: integer(4), primary: true, autoincrement: true, comment: "Serial number" }
151     member_id: { type: integer(4), notnull: true, comment: "Member id" }
152     profile_id: { type: integer(4), notnull: true, comment: "Profile id" }
153     profile_option_id: { type: integer(4), comment: "Profile option id" }
154     value: { type: string, default: "", notnull: true, comment: "Text content for this profile item" }
155     value_datetime: { type: timestamp, comment: "Profile datetime value" }
156     public_flag: { type: integer(1), comment: "Public flag" }
157   relations:
158     Member: { local: member_id, foreign: id, onDelete: cascade }
159     Profile: { local: profile_id, foreign: id, onDelete: cascade }
160     ProfileOption: { local: profile_option_id, foreign: id, onDelete: cascade }
161   indexes:
162     lft_INDEX:
163       fields: [lft]
164   options:
165     type: INNODB
166     collate: utf8_unicode_ci
167     charset: utf8
168     comment: "Saves informations of every member''s profile" 

NestedSetについての詳しい実装については lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Node/NestedSet.php を参照してください.

チケットに記載されたURLの画像からわかること

対象画像: http://sns.openpne.jp/cache/img/png/w_h/9499800169021d9a54fef4b90d8f792535b16bd0_png.png

この例としておそらく複数選択のレコードである member_id=1 and profile_id=6 を考えてみることにします.
該当するレコードを見てみると以下のようなことがわかります.
  1. tree_key から見た構造とlevelの値

tree_keyに格納されている数値がそのレコードの親となるレコードのidです.自分自身を指している場合はルートとなります.

  • id=18 level=0 tree_key=18
    • id=19 level=1 tree_key=18
    • id=20 level=1 tree_key=18
    • id=21 level=1 tree_key=18
    • id=22 level=2 tree_key=18
    • id=23 level=2 tree_key=18
    • id=24 level=2 tree_key=18

id=22〜24がid-18を親としているのにlevelは2であるという矛盾が発生しています.

  1. lftとrgtから見た構造

lftとrgtの間の値を持つレコードを子として持つので以下のようになります.通常はlftが1であるレコードがルートとなります.

  • id=18 level=0 tree_key=18 lft=1 rgt=14
    • id=19 level=1 tree_key=18 lft=2 rgt=3
    • id=20 level=1 tree_key=18 lft=4 rgt=5
    • id=21 level=1 tree_key=18 lft=6 rgt=13
      • id=22 level=2 tree_key=18 lft=11 rgt=12
      • id=23 level=2 tree_key=18 lft=9 rgt=10
      • id=24 level=2 tree_key=18 lft=7 rgt=8

id=22〜24がlevelとは矛盾していませんが,tree_keyに入っているid-18を親としているという点が矛盾しています.

問題再現結果

MyISAMで再現することがあることを確認しました.ただこのストレージエンジンを用いる際にInnoDBの外部キーを削除した状態であるため本当にMyISAMが原因であるかどうかまでは調査していません.

同様に詳しくは調査していませんが,操作時に公開範囲については扱っていないはずがレコード上ではNULLではなく1となっていたり,挙動が安定しない状態がありました.

#5 Updated by Yuya Watanabe almost 8 years ago

バックポート元の修正は以下のようになっています.

commit b51157e2bdcf378d31bcf25553b64b3de698f5df
Author: touri <tourimgr@gmail.com>
Date:   Wed Aug 3 16:48:40 2011 +0900

    added 'and query' and 'order query'.(fixes #1865)

diff --git a/lib/model/doctrine/MemberProfileTable.class.php b/lib/model/doctrine/MemberProfileTable.class.php
index 953cec9..6dbd2a7 100644
--- a/lib/model/doctrine/MemberProfileTable.class.php
+++ b/lib/model/doctrine/MemberProfileTable.class.php
@@ -21,7 +21,9 @@ class MemberProfileTable extends opAccessControlDoctrineTable

     $q = $this->createQuery()
       ->where('member_id = ?')
-      ->andWhere('profile_id = ?');
+      ->andWhere('profile_id = ?')
+      ->andWhere('level = 0')
+      ->orderBy('id');

     $memberProfiles = array();
     foreach ($profiles as $profile)

ここでは上記の修正が妥当かどうかを考察しました.

まず,再現手順に「すべてのチェックを外して送信をする」旨の記述があります.
この手順によって以下のメソッドが呼び出されます.

lib/model/doctrine/MemberProfile.class.php

200   public function clearChildren()
201   {
202     if ($this->getTreeKey() && $this->getNode()->hasChildren())
203     {
204       $children = $this->getNode()->getChildren();
205       $children->delete();
206     }
207   }

ここで注目すべきなのは 205行目の $children 変数を直接 delete() を呼び出していることです.
この時点では $children は Doctrine_Collection クラスです.

lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Collection.php

 922     /**
 923      * Deletes all records from this collection
 924      *
 925      * @return Doctrine_Collection
 926      */
 927     public function delete(Doctrine_Connection $conn = null, $clearColl = true)
 928     {
 929         if ($conn == null) {
 930             $conn = $this->_table->getConnection();
 931         }
 932 
 933         try {
 934             $conn->beginInternalTransaction();
 935             $conn->transaction->addCollection($this);
 936 
 937             foreach ($this as $key => $record) {
 938                 $record->delete($conn);
 939             }
 940 
 941             $conn->commit();
 942         } catch (Exception $e) {
 943             $conn->rollback();
 944             throw $e;
 945         }
 946 
 947         if ($clearColl) {
 948             $this->clear();
 949         }
 950 
 951         return $this;
 952     }

ここで削除を行う MemberProfile の削除について考察するために継承関係を見てみます.
MemberProfile -> BaseMemberProfile -> opDoctrineRecord -> sfDoctrineRecord -> Doctrine_Record -> Record

Record クラスには以下の記述があります.

lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Record.php

2629     /**
2630      * used to delete node from tree - MUST BE USE TO DELETE RECORD IF TABLE ACTS AS TREE
2631      *
2632      */
2633     public function deleteNode()
2634     {
2635         $this->getNode()->delete();
2636     }

また, Recode クラスや MemberProfile クラスで呼び出される getNode() は Doctrine_Node_NestedSet を返します.

lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Node/NestedSet.php

  56     /**
  57      * test if node has children
  58      *
  59      * @return bool            
  60      */
  61     public function hasChildren()
  62     {
  63         return (($this->getRightValue() - $this->getLeftValue()) > 1);
  64     }

 327     /**
 328      * gets number of descendants (children and their children)
 329      *
 330      * @return int            
 331      */
 332     public function getNumberDescendants()
 333     {
 334         return ($this->getRightValue() - $this->getLeftValue() - 1) / 2;
 335     }

 939     /**
 940      * deletes node and it's descendants
 941      * @todo Delete more efficiently. Wrap in transaction if needed.      
 942      */
 943     public function delete()
 944     {
 945         $conn = $this->record->getTable()->getConnection();
 946         try {
 947             $conn->beginInternalTransaction();
 948 
 949             // TODO: add the setting whether or not to delete descendants or relocate children
 950             $oldRoot = $this->getRootValue();
 951             $q = $this->_tree->getBaseQuery();
 952             
 953             $baseAlias = $this->_tree->getBaseAlias();
 954             $componentName = $this->_tree->getBaseComponent();
 955 
 956             $q = $q->addWhere("$baseAlias.lft >= ? AND $baseAlias.rgt <= ?", array($this->getLeftValue(), $this->getRightValue()));
 957 
 958             $q = $this->_tree->returnQueryWithRootId($q, $oldRoot);
 959 
 960             $coll = $q->execute();
 961 
 962             $coll->delete();
 963 
 964             $first = $this->getRightValue() + 1;
 965             $delta = $this->getLeftValue() - $this->getRightValue() - 1;
 966             $this->shiftRLValues($first, $delta, $oldRoot);
 967 
 968             $conn->commit();
 969         } catch (Exception $e) {
 970             $conn->rollback();
 971             throw $e;
 972         }
 973 
 974         return true;
 975     }

ここに記述されているとおり, Node を削除する際には 966 行目のように lft と rgt を更新する必要がありますが, Doctrine_Collection から delete() を呼び出した場合はおそらく NestedSet で定義されている delete() は呼び出されておらず lft および rgt は更新されないと思われます.そうすると hasChildren() や getNumberDescendants() などの lft や rgt を利用しているメソッドが正しく機能しなくなる可能性があります.これは別の不具合が発生する元となり得るため,Doctrine_Collection の delete() を呼び出している部分は修正されることが望ましいと考えられます.

バックポート元の修正ではおそらく正しいと思われるレコードの取得には成功しますが,ここまでに記述したとおり NestedSet として扱われる際に正常に動作しない可能性があり,修正として不十分であると思います.
本問題はバックポート元の修正によって表面上問題を解決できるため,本チケットではこの修正を適用して上記の問題については別チケットで扱うことを検討します.

#6 Updated by Yuya Watanabe almost 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

更新履歴 08e534fd312f43242cd36208429217de58cf180c で適用されました。

#7 Updated by Kousuke Ebihara almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#8 Updated by Yuma Sakata almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テストOKです。

Also available in: Atom PDF