Bug(バグ) #2902
完了OpenPNE2系からコンバートした環境でデフォルトのプロフィール画像(no_image画像)が表示されない現象が発生する場合がある
100%
説明
Overview (現象)¶
OpenPNE2系からコンバートした環境でデフォルトのプロフィール画像(no_image画像)が表示されない現象が発生する場合がある。
2系のDBの c_member テーブルの image_filename の値が 0 の場合にコンバートを実施すると、
3系のDBの member_image テーブルの file_id の値も 0 になってしまい、プロフィール画像が表示されなくなる。
2系のDBの c_member テーブルの image_filename の値が 0 になる問題については未調査です。
再現バージョン¶
- OpenPNE3.6.2
再現手順¶
Causes (原因)¶
バグが発生した原因を記入
Way to fix (修正内容)¶
修正内容を記入
Yuya Watanabe さんが12年以上前に更新
メモ¶
data/upgrade/2/sql/member_image.sql
3 INSERT INTO member_image (id, member_id, file_id, is_primary, created_at, updated_at) (SELECT NULL, c_member_id, <?php echo $this->getSQLForFileId('image_filename_'.$i) ?>, 0, NOW(), NOW() FROM c_member WHERE image_filename_<?php echo
lib/upgrade/strategy/opUpgradeSQLImportStrategy.class.php
85 protected function getSQLForFileId($name) 86 { 87 return '(SELECT id FROM file WHERE name = '.$name.' LIMIT 1)'; 88 } 89 }
data/upgrade/2/sql/files.sql
1 INSERT IGNORE INTO file (id, name, type, original_filename, created_at, updated_at) (SELECT c_image_id, filename, type, filename, r_datetime, r_datetime FROM c_image);
getSQLForFileId() でファイルが存在する場合に何が帰ってくるかわからないが,予想としてファイルが存在しない場合に 0 が返ってくる可能性がある?
さらにメモ(再現方法予想)¶
- 2系でファイルをアップロードする
- ファイルのIDを取得する
select id from c_image
- 2で得られたID群のうちどれかを適当に選んで下記のSQLを実行する
delete from c_image where id = さっきのid
Hidenori Goto さんが12年以上前に更新
原因¶
OpenPNE2側で、c_imageに存在しない画像ファイル名がc_memberのimage_filenameに使われていると、コンバータ実行時にfile_id=0に変換される。
note-3 にあるとおり /data/upgrade/2/sql/member_image.sql が原因。
修正方針¶
- 根本的には member_image.sql を修正し、file_id = 0にならないようにinsert文を修正する等対応が必要であるが、SQLのみで対応が可能なのか、調査が必要。
- 表面的な対処として、プロフィール画像を表示するテンプレートブロックにて条件分岐を追加する。
- apps/pc_frontend/templates/_partsMemberImageBox.php にて画像ファイル名が空でないかどうかのチェックも追加する
Hidenori Goto さんが12年以上前に更新
- ステータス を Accepted(着手) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
Yuya Watanabe さんが12年以上前に更新
テーブル構造メモ¶
2系¶
mysql> show create table c_image\G *************************** 1. row *************************** Table: c_image Create Table: CREATE TABLE `c_image` ( `c_image_id` int(11) NOT NULL auto_increment, `filename` text NOT NULL, `bin` longblob NOT NULL, `r_datetime` datetime NOT NULL default '0000-00-00 00:00:00', `type` text, PRIMARY KEY (`c_image_id`), KEY `filename` (`filename`(100)) ) ENGINE=MyISAM AUTO_INCREMENT=8 DEFAULT CHARSET=utf8 MAX_ROWS=190000 1 row in set (0.00 sec)
3系¶
mysql> show create table file\G *************************** 1. row *************************** Table: file Create Table: CREATE TABLE `file` ( `id` int(11) NOT NULL auto_increment COMMENT 'Serial number', `name` varchar(64) collate utf8_unicode_ci NOT NULL default '' COMMENT 'File name', `type` varchar(64) collate utf8_unicode_ci NOT NULL default '' COMMENT 'Type of this file', `filesize` int(11) NOT NULL default '0' COMMENT 'File size', `original_filename` text collate utf8_unicode_ci COMMENT 'Original filename', `created_at` datetime NOT NULL, `updated_at` datetime NOT NULL, PRIMARY KEY (`id`), UNIQUE KEY `name_UNIQUE_idx` (`name`) ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci COMMENT='Saves informations of files uploaded' 1 row in set (0.08 sec)
mysql> show create table member_image\G *************************** 1. row *************************** Table: member_image Create Table: CREATE TABLE `member_image` ( `id` int(11) NOT NULL auto_increment COMMENT 'Serial number', `member_id` int(11) NOT NULL COMMENT 'Member id', `file_id` int(11) NOT NULL COMMENT 'Image file id in the ''file'' table', `is_primary` tinyint(1) default NULL COMMENT 'This is primary', `created_at` datetime NOT NULL, `updated_at` datetime NOT NULL, PRIMARY KEY (`id`), KEY `member_id_idx` (`member_id`), KEY `file_id_idx` (`file_id`), CONSTRAINT `member_image_file_id_file_id` FOREIGN KEY (`file_id`) REFERENCES `file` (`id`) ON DELETE CASCADE, CONSTRAINT `member_image_member_id_member_id` FOREIGN KEY (`member_id`) REFERENCES `member` (`id`) ON DELETE CASCADE ) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci COMMENT='Saves images in member profiles' 1 row in set (0.00 sec)
Yuya Watanabe さんが12年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Rejected(差し戻し) に変更
下記修正では ('' != $options->object->getImageFileName())
が用いられていますが,Member オブジェクトの場合は getImageFileName() の名前の期待に反して File オブジェクトあるいは false を返すため,文字列以外のものと文字列とを比較することになると思います.
getImageFileName() が文字列以外を返すということは期待に反する動作のようなのでバグとして扱うべきだとは思います( 報告チケット #3067 )ただ,下記修正の場合だとfalseの場合はif文の条件式が真となりますが(追記:この記述は誤り), op_image_tag_sf_image() のなかで 空文字列の場合なども含めて no_image.gif になるようになっていて,if 文の条件式にこれを追加するしないにかかわらず動作に影響がなさそうという認識ですがこの修正は正しいのでしょうか?コード追跡しかやっていませんが,確認お願いします.問題なければそのままレビュー待ちにしていただして構いません.
https://github.com/hidenorigoto/OpenPNE3/commit/72902e44f3e87aacbeeb8c14dc55ce6281641bd0
lib/helper/opUtilHelper.php
1081 function op_image_tag_sf_image($source, $options = array()) 1082 { 1083 if (!isset($options['no_image'])) 1084 { 1085 $options['no_image'] = op_image_path('no_image.gif'); 1086 } 1087 1088 return image_tag_sf_image($source, $options); 1089 }
plugins/sfImageHandlerPlugin/lib/helper/sfImageHelper.php
27 function image_tag_sf_image($filename, $options = array()) 28 { 29 if (empty($options['alt'])) 30 { 31 $options['alt'] = ''; 32 } 33 34 if (!$filename) 35 { 36 if (isset($options['no_image'])) 37 { 38 $filename = $options['no_image']; 39 unset($options['no_image']); 40 } 41 else 42 { 43 $filename = 'no_image.gif'; 44 } 45 return image_tag($filename, $options); 46 }
追記¶
「下記修正の場合だと false の場合は if 文の条件式が真となりますが」という点は PHP では誤りでした.
申し訳ありません.
$ php -r "var_dump(false != '');" bool(false)
Hidenori Goto さんが12年以上前に更新
https://github.com/hidenorigoto/OpenPNE3/commit/72902e44f3e87aacbeeb8c14dc55ce6281641bd0
このコミットの修正は、note-5 で記載した方針のうち表面的対応を行うものでしたが、この対応だけでは不正なmember_imageレコードが残ったままとなり、別の箇所で問題が発生することが分かりました。具体的にはメンバーのプロフィール写真の変更ページにて、file_id=0を特殊扱いするためのコードが必要になってしまいます。
表面的対応は取りやめ、コンバーターの時点で不正レコードが入らないようにする方針で再度別のPull Requestを作成します。
当初私のコンバーターに対する理解不足から、SQLImport Strategy向けのSQLのみでの対応は難しいと判断しておりましたが、member_imageのSQLImport実行後に、opUpgradeFrom2MemberImageCleanupStrategy.class.phpを実行するようにし、そこで不正データを除去するようにします。
Hidenori Goto さんが12年以上前に更新
- ステータス を Rejected(差し戻し) から Accepted(着手) に変更
- 進捗率 を 50 から 0 に変更
Hidenori Goto さんが12年以上前に更新
- ステータス を Accepted(着手) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
note-10 に記載した、コンバーターにて不正データ除去用Strategyを追加する方針にてPull Request送信しました。
Yuya Watanabe さんが12年以上前に更新
再現方法¶
- 2.14 をインストールして初期メンバを追加する
- 下記コマンドを実行してimage_filename_1 に 0 を入れる -> ホーム画面を表示して no_image が表示されることを確認する
$ mysql -u ユーザ DB名 -p -e "update c_member set image_filename_1 = 0 where c_member_id = 1;"
- 2.14 から 3.6.3 にアップグレードする
- 画像編集画面を開く -> no_image が表示されない部分があることを確認する
備考¶
- 2.14 のときで image_filename に 0 を入れても問題は確認できていない
- image_filename_1 に 0 以外の存在しないファイルをいれても同様の問題が発生するが,2.14 の状態でも no_image が表示されなくなる.
Yuya Watanabe さんが12年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更
下記コミットで stable-3.6.x にコミットをマージしました.
Fumie Toyooka さんが12年以上前に更新
- ステータス を Pending Testing(テスト待ち) から Fixed(完了) に変更
- 進捗率 を 70 から 100 に変更
テスト問題ありませんでした。