Project

General

Profile

Enhancement(機能追加・改善) #3064

activity/post.json の画像投稿対応

Added by Shouta Kashiwagi about 7 years ago. Updated over 2 years ago.

Status:
Won't fix(対応せず)
Priority:
Normal(通常)
Target version:
Start date:
2012-06-01
Due date:
% Done:

0%


Description

概要

activity/post.json の画像投稿対応
現状OpenPNE3に従来からある「アクティビティ」は「ActivityData」テーブルと連携させる為に用意されたと考えられる「ActivityImage」テーブルがあるにも関わらず,
アクティビティの画像添付に対応していないので,API側で優先的に対応させる(タイムラインプラグインの画像投稿対応も視野に入れた想定)

仕様

"images" パラメーターに画像データを載せることにより投稿する

  • 実装予定

+    if ($request->getFiles('images'))
+    {
+      $imageFile = $request->getFiles('images');
+      $obj = new sfValidatedFile($imageFile['name'], $imageFile['type'], $imageFile['tmp_name'], $imageFile['size']);
+      $file = new File();
+      $file->setFromValidatedFile($obj);
+      $file->setName('ac_'.$this->getUser()->getMemberId().'_'.$file->getName());
+      $file->save();
+      $options['images'] = array('file_id' => $file->getId());
+    }
+


Related issues

Related to OpenPNE 3 - Backport(バックポート) #3075: activity/post.json の画像投稿対応 Fixed(完了) 2012-06-01
Blocks OpenPNE 3 - Enhancement(機能追加・改善) #3203: activity/post.json の images パラメータを複数枚の画像アップロードに対応させる Won't fix(対応せず) 2012-09-14
Blocks OpenPNE 3 - Enhancement(機能追加・改善) #3393: アクティビティに添付可能な画像の上限枚数を設けるべき New(新規) 2013-09-09
Blocks OpenPNE 3 - Task(タスク) #3394: JSON APIドキュメントのアクティビティ関連のページに画像投稿・取得のサンプルを追加する New(新規) 2013-09-09
Blocked by OpenPNE 3 - Enhancement(機能追加・改善) #3396: opBrowser::post() でファイルのアップロードをテストできるようにする Won't fix(対応せず) 2013-09-10
Blocked by OpenPNE 3 - Bug(バグ) #3202: ActivityDataTable::updateActivity() メソッドで画像付きのアクティビティが作成できない Won't fix(対応せず) 2012-09-14

Associated revisions

Revision 91745b9e (diff)
Added by Shouta Kashiwagi about 7 years ago

add an implement of image post onto activity/post.json (fixes #3064)

Revision 96df88d1 (diff)
Added by Youichi Kimura almost 6 years ago

some cleanup in activity/post.json (fixes #3064)

Revision bbf98395 (diff)
Added by Youichi Kimura almost 6 years ago

add testcases for activity/post.json (refs #3064)

History

#1 Updated by Shouta Kashiwagi about 7 years ago

  • 実装予定
diff --git a/apps/api/modules/activity/actions/actions.class.php b/apps/api/modules/activity/actions/actions.class.php
index 9cf307f..891f9d8 100644
--- a/apps/api/modules/activity/actions/actions.class.php
+++ b/apps/api/modules/activity/actions/actions.class.php
@@ -188,6 +188,30 @@ class activityActions extends opJsonApiActions

     $options['source'] = 'API';

+    $imageFile = $request->getFiles('images');
+    if (!empty($imageFile))
+    {
+      $validator = new opValidatorImageFile(array('required' => false));
+      try
+      {
+        $obj = $validator->clean($imageFile);
+      }
+      catch (sfValidatorError $e)
+      {
+        $this->forward400('This image file is invalid.');
+      }
+      $file = new File();
+      $file->setFromValidatedFile($obj);
+      $file->setName('ac_'.$this->getUser()->getMemberId().'_'.$file->getName());
+      $file->save();
+      $options['images'][] = array('file_id' => $file->getId());
+    }
+
     $this->activity = Doctrine::getTable('ActivityData')->updateActivity($memberId, $body, $options);

     $this->setTemplate('object');

#2 Updated by Shouta Kashiwagi about 7 years ago

簡単なお試し例

<form action="http://{$sns_url}/api.php/activity/post.json" method="post">
<input type="hidden" name="apiKey" value="XXXXXXXXXXXXXXXXXX" />
<input type="text" name="body" value="hoge" />
<input type="file" name="images" />
<input type="submit" name="submit" value="投稿" />
</form>

#3 Updated by Shouta Kashiwagi about 7 years ago

  • APIレスポンス部分
diff --git a/apps/api/lib/helper/opJsonApiHelper.php b/apps/api/lib/helper/opJsonApiHelper.php
index 502ff90..9adc42c 100644
--- a/apps/api/lib/helper/opJsonApiHelper.php
+++ b/apps/api/lib/helper/opJsonApiHelper.php
@@ -58,8 +58,14 @@ function op_api_member_profile_url($memberId)

 function op_api_activity($activity)
 {
   $viewMemberId = sfContext::getInstance()->getUser()->getMemberId();
   $member = $activity->getMember();
+  $images = null;
+  if ($activity->getImages()->count())
+  {
+    $images = $activity->getImages();
+  }

   return array(
     'id' => $activity->getId(),
@@ -69,7 +75,9 @@ function op_api_activity($activity)
     'uri' => $activity->getUri(),
     'source' => $activity->getSource(),
     'source_uri' => $activity->getSourceUri(),
+    'images' => !is_null($images) ? sf_image_path($images[0]->getFile(), array('size' => '48x48'), true) : null, 
+    'images_large' => !is_null($images) ? sf_image_path($images[0]->getFile(), array(), true) : null,
     'created_at' => date('r', strtotime($activity->getCreatedAt())),
   );
 }

  • 追記: 余計な修正が入っていたので除去 (2012/06/11 19:54)

#4 Updated by Shouta Kashiwagi about 7 years ago

  • Target version changed from OpenPNE 3.8.x to OpenPNE 3.9.0-old

#5 Updated by Shouta Kashiwagi about 7 years ago

  • Status changed from New(新規) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

更新履歴 91745b9e8bb9c56460b04d561e5be926920ad083 で適用されました。

#6 Updated by Shinichi Urabe almost 6 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

気づいた点から順にフィードバックします。

  • まず、投稿する側の画面は本チケットでは作成されていないということから、APIで投稿する受け側だけ存在すると言うことになります。そのことから、この機能を使うためのドキュメントがないと使い方が分からないという状況になります。 http://houou.github.io/api.php/activity_post.html には画像投稿についての説明がないので、使い方は分からないでしょう。別チケットでもよいですが、何かしらのドキュメントがあったほうがよいのではないでしょうか。また、取り急ぎ参考にする情報としても、以下の理由から #note-2 の説明は参考にはできないです。
    • #note-2 についてですが、検証されずに記載されているものと考えますが、 form 要素に enctype="multipart/form-data" の指定がないため、画像投稿はできません。
    • 続いて #note-2 のサンプルの input[type=file] の要素の name 属性に images という複数形の名称を設定されていながら、1つしか画像投稿はできない例となっていますが、 apps/api/modules/activity/actions/actions.class.php の 198行目では複数の画像を処理するような実装となっていますので、ただしく処理されず、サンプルでは画像投稿ができません。

#7 Updated by Shinichi Urabe almost 6 years ago

  • 画像の投稿枚数に上限がないですが、制限は不要でしょうか。OpenPNE に存在する画像投稿機能の場合、一度に画像を投稿できる枚数は 3枚 か 5枚 であることが多いと思います。
  • APIでの アクティビティのデータ取得の画像表示についての仕様が見当たりません。 http://houou.github.io/api.php/activity_mentions.html http://houou.github.io/api.php/activity_mentions.html 投稿時の説明と同じく、ドキュメントの修正をしたほうがいいと思います。
  • API の応答は 固定で1枚だけ画像のURLを出力するような実装ですが、複数枚投稿できるのであれば、複数枚出力した方が良くないでしょうか。
    • 1枚だけ出力するような仕様に意図的にするのであれば、例えば一番新しい画像のURLを出力する といった対応にしたほうがよいのではないかと思います。
  • ActivityData::getImages()->count() で分岐した後、もう一度 ActivityData::getImages() をしていますが、処理に無駄があります。修正案ですが、以下のように処理をすれば、不要な分岐は減ります。
diff --git a/apps/api/lib/helper/opJsonApiHelper.php b/apps/api/lib/helper/opJsonApiHelper.php
index c460778..6228a91 100644
--- a/apps/api/lib/helper/opJsonApiHelper.php
+++ b/apps/api/lib/helper/opJsonApiHelper.php
@@ -60,11 +60,7 @@ function op_api_activity($activity)
 {
   $viewMemberId = sfContext::getInstance()->getUser()->getMemberId();
   $member = $activity->getMember();
-  $images = null;
-  if ($activity->getImages()->count())
-  {
-    $images = $activity->getImages();
-  }
+  $images = $activity->getImages();

   return array(
     'id' => $activity->getId(),
@@ -74,8 +70,8 @@ function op_api_activity($activity)
     'uri' => $activity->getUri(),
     'source' => $activity->getSource(),
     'source_uri' => $activity->getSourceUri(),
-    'image_url' => !is_null($images) ? sf_image_path($images[0]->getFile(), array('size' => '48x48'), true) : null, 
-    'image_large_url' => !is_null($images) ? sf_image_path($images[0]->getFile(), array(), true) : null,
+    'image_url' => count($images) ? sf_image_path($images[0]->getFile(), array('size' => '48x48'), true) : null, 
+    'image_large_url' => count($images) ? sf_image_path($images[0]->getFile(), array(), true) : null,
     'created_at' => date('r', strtotime($activity->getCreatedAt())),
   );
 }

  • アクション の209行目以下の処理はファイルのフィールドが空で送信された場合の continue だと思いますが、何の処理であるか、直感で分かりにくいように思います。コメントで説明があったほうがよいかと思います。また、データがないのであれば、バリデーションする必要もないので、先に continue した方がいいのではないでしょうか。
        if (is_null($obj))
        {   
          continue;
        }  

  • git show 91745b9e --check で確認してみてください。不要な空白文字が多いです。
  • $options['images'][] = array('file_id' => $file->getId()); が可読性がよくないので、 $options['images'][]['file_id'] = $file->getId(); としたほうがよいかもしれません。
  • テストコードを書いた方がよいと思います。

#8 Updated by Mutsumi Imamura almost 6 years ago

  • Assignee changed from Shouta Kashiwagi to Youichi Kimura

#9 Updated by Youichi Kimura almost 6 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • % Done changed from 50 to 0

修正着手します

#10 Updated by Youichi Kimura almost 6 years ago

取り扱う内容が多いため、いくつかチケットを分割しました。

ドキュメント修正 (#3394)

まず、投稿する側の画面は本チケットでは作成されていないということから、APIで投稿する受け側だけ存在すると言うことになります。そのことから、この機能を使うためのドキュメントがないと使い方が分からないという状況になります。 http://houou.github.io/api.php/activity_post.html には画像投稿についての説明がないので、使い方は分からないでしょう。

新たにチケットを作成しました。ドキュメントの修正については #3394 にて扱います。

APIでの アクティビティのデータ取得の画像表示についての仕様が見当たりません。 http://houou.github.io/api.php/activity_mentions.html http://houou.github.io/api.php/activity_mentions.html 投稿時の説明と同じく、ドキュメントの修正をしたほうがいいと思います。

画像投稿のドキュメントと同じく #3394 にて扱います。

アクティビティ画像の複数枚対応 (#3203)

続いて #note-2 のサンプルの input[type=file] の要素の name 属性に images という複数形の名称を設定されていながら、1つしか画像投稿はできない例となっていますが、 apps/api/modules/activity/actions/actions.class.php の 198行目では複数の画像を処理するような実装となっていますので、ただしく処理されず、サンプルでは画像投稿ができません。

当初の実装である 91745b9e8bb9c56460b04d561e5be926920ad083 は画像一枚のみを受け付ける実装となっていました (しかしパラメータ名は images です)。パラメーター名に合わせて複数枚対応を行ったのは、当チケットの後に作られた #3203 です。そのため、アクティビティ画像の複数枚対応については (後述のレスポンス形式の修正と合わせて) #3203 で扱います。

API の応答は 固定で1枚だけ画像のURLを出力するような実装ですが、複数枚投稿できるのであれば、複数枚出力した方が良くないでしょうか。
1枚だけ出力するような仕様に意図的にするのであれば、例えば一番新しい画像のURLを出力する といった対応にしたほうがよいのではないかと思います。

これについても #3203 にて扱います。

アクティビティ画像の枚数制限 (#3393)

画像の投稿枚数に上限がないですが、制限は不要でしょうか。OpenPNE に存在する画像投稿機能の場合、一度に画像を投稿できる枚数は 3枚 か 5枚 であることが多いと思います。

画像枚数の制限については JSON API に限らない問題であるため、上記の複数枚対応とは別に #3393 にて扱います。

#11 Updated by Youichi Kimura almost 6 years ago

そして、当チケットでは 91745b9e8bb9c56460b04d561e5be926920ad083 に対する可読性や冗長な記述の指摘とテストコードの追加について (note-7 の最後の5つ) を取り扱うものとします。

#12 Updated by Youichi Kimura almost 6 years ago

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

更新履歴 96df88d12b0e15f418b3a6a6da4aaae6240b40cc で適用されました。

#13 Updated by Youichi Kimura almost 6 years ago

  • Status changed from Pending Review(レビュー待ち) to Accepted(着手)
  • % Done changed from 50 to 0
  • アクション の209行目以下の処理はファイルのフィールドが空で送信された場合の continue だと思いますが、何の処理であるか、直感で分かりにくいように思います。コメントで説明があったほうがよいかと思います。また、データがないのであれば、バリデーションする必要もないので、先に continue した方がいいのではないでしょうか。
        if (is_null($obj))
        {   
          continue;
        }  

上記の「先に continue した方が...」の指摘については、バリデーターが sfValidatorBase::isEmpty() によって型厳密に空値か否かの判定を行っていたため下手に empty() で判定するよりもこのままの方が良いと判断し、コメントの追記のみ行いました。

#14 Updated by Youichi Kimura almost 6 years ago

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

ステータス変更ミスです

#15 Updated by Youichi Kimura almost 6 years ago

補足: bbf9839510c25ad9da71c5a1a464f46745d2a370 のテストケースは #3396 の修正に依存しています

#16 Updated by Youichi Kimura almost 6 years ago

動作確認は下記コマンドで行いました。

curl -v -F 'apiKey=<APIキー>' -F 'body=hogehoge' -F 'images[0]=@<ファイルパス>' http://sns.example.com/api.php/activity/post.json

#17 Updated by Shinichi Urabe almost 6 years ago

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

レビューOKです。
ファイルが送信されていない場合の判定については、 https://redmine.openpne.jp/issues/3064#note-13 の対応は適切だと思います。
幅の広い指摘をしていましたが、別チケットでの対応とのことで、本チケットでの追加の指摘はないです。

#18 Updated by 誠二 天重 almost 6 years ago

  • Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
  • % Done changed from 70 to 50

以下のコマンドで確認したところ,500エラーが返ってきます。また,この500エラーのときは,アクティビティそのものは投稿されていない状態ですが,画像だけ投稿されています。

curl -v -F 'apiKey=(apiキー)' -F 'body=hogehoge' -F 'images=@(ローカルのファイルパス)' http://op.stable-3.8.x2.local/api_dev.php/activity/post.json

↓デバッグモードで実施
{
  "error": {
    "debug": {
      "traces": [
        "at () in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Connection.php line 1082",
        "at Doctrine_Connection->rethrowException(object('PDOException'), object('Doctrine_Connection_Statement')) in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Connection/Statement.php line 269",
        "at Doctrine_Connection_Statement->execute(array('1', '1', '2013-09-12 11:59:43', '2013-09-12 11:59:43')) in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Connection.php line 1042",
        "at Doctrine_Connection->exec('INSERT INTO activity_image (file_id, activity_data_id, created_at, updated_at) VALUES (?, ?, ?, ?)', array('1', '1', '2013-09-12 11:59:43', '2013-09-12 11:59:43')) in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Connection.php line 687",
        "at Doctrine_Connection->insert(object('ActivityImageTable'), array('file_id' => '1', 'activity_data_id' => '1', 'created_at' => '2013-09-12 11:59:43', 'updated_at' => '2013-09-12 11:59:43')) in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Connection/UnitOfWork.php line 647",
        "at Doctrine_Connection_UnitOfWork->processSingleInsert(object('ActivityImage')) in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Connection/UnitOfWork.php line 571",
        "at Doctrine_Connection_UnitOfWork->insert(object('ActivityImage')) in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Connection/UnitOfWork.php line 81",
        "at Doctrine_Connection_UnitOfWork->saveGraph(object('ActivityImage')) in SF_ROOT_DIR/lib/vendor/symfony/lib/plugins/sfDoctrinePlugin/lib/vendor/doctrine/Doctrine/Record.php line 1718",
        "at Doctrine_Record->save(object('opDoctrineConnectionMysql')) in SF_ROOT_DIR/lib/util/opDoctrineRecord.class.php line 66",
        "at opDoctrineRecord->save() in SF_ROOT_DIR/lib/model/doctrine/ActivityDataTable.class.php line 135",
        "at ActivityDataTable->updateActivity('1', 'hogehoge', array('source' => 'API', 'images' => <em>array</em>(<em>array</em>('file_id' => '1')))) in SF_ROOT_DIR/apps/api/modules/activity/actions/actions.class.php line 210",
        "at activityActions->executePost(object('sfWebRequest')) in SF_ROOT_DIR/lib/vendor/symfony/lib/action/sfActions.class.php line 60",
        "at sfActions->execute(object('sfWebRequest')) in SF_ROOT_DIR/lib/action/opJsonApiActions.class.php line 39",
        "at opJsonApiActions->execute(object('sfWebRequest')) in SF_ROOT_DIR/lib/vendor/symfony/lib/filter/sfExecutionFilter.class.php line 92",
        "at sfExecutionFilter->executeAction(object('activityActions')) in SF_ROOT_DIR/lib/vendor/symfony/lib/filter/sfExecutionFilter.class.php line 78",
        "at sfExecutionFilter->handleAction(object('sfFilterChain'), object('activityActions')) in SF_ROOT_DIR/lib/vendor/symfony/lib/filter/sfExecutionFilter.class.php line 42",
        "at sfExecutionFilter->execute(object('sfFilterChain')) in SF_ROOT_DIR/lib/vendor/symfony/lib/filter/sfFilterChain.class.php line 53",
        "at sfFilterChain->execute() in SF_ROOT_DIR/lib/vendor/symfony/lib/filter/sfRenderingFilter.class.php line 33",
        "at sfRenderingFilter->execute(object('sfFilterChain')) in SF_ROOT_DIR/lib/vendor/symfony/lib/filter/sfFilterChain.class.php line 53",
        "at sfFilterChain->execute() in SF_ROOT_DIR/lib/vendor/symfony/lib/controller/sfController.class.php line 238",
        "at sfController->forward('activity', 'post') in SF_ROOT_DIR/lib/vendor/symfony/lib/controller/sfFrontWebController.class.php line 48",
        "at sfFrontWebController->dispatch() in SF_ROOT_DIR/lib/vendor/symfony/lib/util/sfContext.class.php line 170",
        "at sfContext->dispatch() in SF_ROOT_DIR/web/api_dev.php line 13" 
      ],
      "message": "SQLSTATE[HY000]: General error: 1364 Field 'mime_type' doesn't have a default value",
      "name": "Doctrine_Connection_Mysql_Exception" 
    },
    "message": "SQLSTATE[HY000]: General error: 1364 Field 'mime_type' doesn't have a default value",
    "code": 500
  }
}

#19 Updated by Youichi Kimura almost 6 years ago

note-18 の問題について、activity/post.json のアクションが #3202 のバグを踏んでいたことが分かりました。現時点で #3202 の修正は master ブランチのみに入っています。

#20 Updated by Youichi Kimura almost 6 years ago

  • Status changed from Rejected(差し戻し) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

note-18 のエラーは stable-3.8.x ブランチのみで発生するエラーであることと、原因となる #3202 が stable-3.8.x にバックポート中であることから、このチケットで扱う問題ではないとしステータスを戻します。

#21 Updated by isao sano over 2 years ago

  • Status changed from Pending Testing(テスト待ち) to Won't fix(対応せず)
  • % Done changed from 70 to 0

OpenPNE 3.8.8 にて対応済みであったため、対応せずとします。

Also available in: Atom PDF