OpenPNE 3

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

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

参考から転記: 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" 

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

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 として扱われる際に正常に動作しない可能性があり，修正として不十分であると思います．
本問題はバックポート元の修正によって表面上問題を解決できるため，本チケットではこの修正を適用して上記の問題については別チケットで扱うことを検討します．