Enhancement（機能追加・改善） #2698: OpenPNE3 コアに JSON API を追加する - OpenPNE 3 - OpenPNE Issue Tracking System

 32     $enableEscaping = true;
 33     if (isset($request['escaping']))
 34     {
 35       $enableEscaping = in_array($request['escaping'], array('1', 'on', 'true'));
 36     }
 37     sfConfig::set('sf_escaping_strategy', $enableEscaping);

疑問点¶

404 専用のエラーを継承して他のステータスコードでも用いているのが気になる．これを継承する利点はなんだろうか．

lib/exception/opErrorHttpException.class.php

 18 class opErrorHttpException extends sfError404Exception

lib/request/opWebRequest.class.php を用いない理由は？（用いて問題があるかどうかは未調査）
apps/api/modules/activity/actions/actions.class.php
apps/api/modules/community/actions/actions.class.php
apps/api/modules/member/actions/actions.class.php
apps/api/modules/push/actions/actions.class.php

DB操作を主にMVCのコントローラ側で行われているのが気になる．
apps/api/modules/activity/actions/actions.class.php
apps/api/modules/community/actions/actions.class.php
apps/api/modules/member/actions/actions.class.php

API経由で日本語を表示する場合だとSNS名称設定の値が反映されない可能性がある．（pc_backend.php/sns/term）
現状では日本語表示の部分はなさそうなので問題は発生しないと思われる．
参考: #1843 「公開設定デフォルト値が my_friend と表示されている」

apps/api/config/factories.yml

100   i18n:
101     class: sfI18N

差し戻し理由¶

ログイン停止状態のメンバの apiKey でも api 経由で SNS 内の情報にアクセスで来てしまう．
携帯から api にアクセスできるが，ブラックリストに登録されているメンバの apiKey でも SNS 内の情報にアクセスできてしまう．

コーディング規約違反

apps/api/lib/myUser.class.php is_null()を用いていない

 46     if (null !== $this->member)
...
 61     if (null === $member)

lib/exception/opErrorHttpException.class.php is_null()を用いていない, returnの前に改行がない

 30     $this->httpStatusCode = $statusCode;
 31     return $this;
...
 39     $exception = null === $this->wrappedException ? $this : $this->wrappedException;
...
 44       if (null === $response)

lib/model/doctrine/Member.class.php is_null()を用いていない

394     if (null === $apiKey)

apps/api/lib/helper/opJsonApiHelper.php 変数名はアンダースコアを用いてはいけない

 54 function op_api_member_profile_url($member_id)
 55 {
 56   return app_url_for('pc_frontend', array('sf_route' => 'obj_member_profile', 'id' => $member_id), true);
 57 }

適切なファイルフォーマットでファイルパスを取得できない可能性がある．sf_image_path() の第一引数はファイルオブジェクトで渡したほうがよい．
参考: #2864 「OpenPNE 2 からコンバートされたか、管理画面からアップロードされた画像が常に JPEG として表示されてしまう箇所がある」

 29     $memberImage = sf_image_path($memberImageFileName, array('size' => '48x48'), true);

$request['reject']が未定義の場合エラーが出るのでは？

apps/api/modules/member/actions/actions.class.php 112 行目

112     if (!$request['reject'])
113     {
114       $preRequest->setFriend();
115     }
116     else
117     {
118       $preRequest->removeFriendPre();
119     }

その他¶

今回の編集部分以外でのコーディング規約違反．

lib/util/opToolkit.class.php

532           while (feof($fp) === false)

lib/model/doctrine/ActivityDataTable.class.php returnの前に改行がない， switch文でdefaultが記述されていない，is_null()を用いていない

143     $i18n = sfContext::getInstance()->getI18N();
144     return $i18n->__(self::$publicFlags[$flag]);
...
179     switch ($flag)
180     {
181       case self::PUBLIC_FLAG_PRIVATE:
182         $flags[] = self::PUBLIC_FLAG_PRIVATE;
183       case self::PUBLIC_FLAG_FRIEND:
184         $flags[] = self::PUBLIC_FLAG_FRIEND;
185       case self::PUBLIC_FLAG_SNS:
186         $flags[] = self::PUBLIC_FLAG_SNS;
187       case self::PUBLIC_FLAG_OPEN:
188         $flags[] = self::PUBLIC_FLAG_OPEN;
189         break;
190     }
...
205     }
206     return null;
...
221     if (null === $memberId)
...
224       if (null === $memberId)
...
248       if (sfConfig::get('sf_app') == 'mobile_frontend')
...
263     if (null !== $limit)
...
266     }
267     return $q->execute();
...
273     $this->addFriendActivityQuery($q, $memberId, $isCheckApp);
274     return $this->getPager($q, $page, $size);
...
279     if (null === $memberId)
...
282       if (null === $memberId)
...
288     if (null === $viewerMemberId)
...
293     if (null === $viewerMemberId)
...
326       if (sfConfig::get('sf_app') == 'mobile_frontend')
...
341     if (null !== $limit)
...
344     }
345     return $q->execute();
...
351     $this->addActivityQuery($q, $memberId, $viewerMemberId, $isCheckApp);
352     return $this->getPager($q, $page, $size);
...
361       if (sfConfig::get('sf_app') == 'mobile_frontend')
...
378     if (null !== $limit)
...
389     $this->addAllMemberActivityQuery($q, $isCheckApp);
390     return $this->getPager($q, $page, $size);
...
395     if (null === $this->templateConfig)
...

lib/i18n/opI18N.class.php is_null()を用いていない

 25     if($application == 'pc_backend')
...
 77       if (null === $dirs)
...
 87     if (null !== $this->cache)
...
 92     if (null !== $culture)
...

lib/model/doctrine/MemberRelationship.class.php 波括弧は関数名の次の行に書かなければならない，return文の前に改行がない

100     if ($this->toInstance) {
...
105     if (!$relation) {
...
111     $this->toInstance = $relation;
112     return $this->toInstance;

getImage() を二回呼び出すことで，それぞれでクエリを実行されることになり，これは無駄．

lib/model/doctrine/Member.class.php

196   public function getImageFileName()
197   {
198     if ($this->getImage())
199     {
200       return $this->getImage()->getFile();
201     }
202 
203     return false;
204   }

操作

リンクをコピー

#14

Yuya Watanabe さんが12年以上前に更新

ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更

操作

リンクをコピー

#15

Shouta Kashiwagi さんが12年以上前に更新

ステータス を Rejected（差し戻し） から Accepted（着手） に変更
進捗率 を 50 から 0 に変更

操作

リンクをコピー

#16

Shouta Kashiwagi さんが12年以上前に更新

ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
進捗率 を 0 から 50 に変更

(master) 15f247ca6e9f25c8082e377e20f21881ff61b4a9
(release-3.8beta1) 94515d98f87747603b42b4397c0561039bff011a
で適用しました．

操作

リンクをコピー

#17

Shouta Kashiwagi さんが12年以上前に更新

更新履歴 15f247ca6e9f25c8082e377e20f21881ff61b4a9 で適用されました。

操作

リンクをコピー

#18

Shouta Kashiwagi さんが12年以上前に更新

更新履歴 94515d98f87747603b42b4397c0561039bff011a で適用されました。

操作

リンクをコピー

#19

Yuya Watanabe さんが12年以上前に更新

ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更

下記項目は仕様でしょうか？念のため確認として差し戻します．

ログイン停止状態のメンバの apiKey でも api 経由で SNS 内の情報にアクセスで来てしまう．
ブラックリストに登録されているメンバの apiKey でも SNS 内の情報にアクセスできてしまう．

操作

リンクをコピー

#20

Shouta Kashiwagi さんが12年以上前に更新

すみませんコーディング規約の方しかまだ修正できていませんでした．
note-19 につきましては実装者と相談の上修正をコミットします．

操作

リンクをコピー

#21

Shouta Kashiwagi さんが12年以上前に更新

Yuya Watanabe は書きました:

下記項目は仕様でしょうか？念のため確認として差し戻します．

ログイン停止状態のメンバの apiKey でも api 経由で SNS 内の情報にアクセスで来てしまう．

ブラックリストに登録されているメンバの apiKey でも SNS 内の情報にアクセスできてしまう．

上記にあります「ブラックリスト」というのはメンバーが別のメンバーを「アクセスブロック」している場合において，という認識で間違いありませんでしょうか？

操作

リンクをコピー

#22

Yuya Watanabe さんが12年以上前に更新

OpenPNE には個体識別番号を指定してそのメンバーをログインできないようにするブラックリストの機能があります．管理画面ブラックリスト（pc_backend.php/member/blacklist）で管理画面メンバーリスト（pc_backend.php/member/list）にある「携帯電話個体識別番号（暗号化済）」の部分を Uid に入力するとそのメンバの携帯電話からのアクセスをできないようにする仕組みです．

操作

リンクをコピー

#23

Shouta Kashiwagi さんが12年以上前に更新

ステータス を Rejected（差し戻し） から Pending Review（レビュー待ち） に変更

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

操作

リンクをコピー

#24

Shouta Kashiwagi さんが12年以上前に更新

更新履歴 86db92b2d49011b02d456874622ae6bd8950a2aa で適用されました。

操作

リンクをコピー

#25

Shouta Kashiwagi さんが12年以上前に更新

更新履歴 636b6cea1e9cf691ab5566299bf22530b65ec604 で適用されました。

操作

リンクをコピー

#26

Shouta Kashiwagi さんが12年以上前に更新

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

操作

リンクをコピー

#27

Shouta Kashiwagi さんが12年以上前に更新

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

操作

リンクをコピー

#28

Shouta Kashiwagi さんが12年以上前に更新

更新履歴 693e3fcf73698d997b5c251d6139fe4e3e830251 で適用されました。

操作

リンクをコピー

#29

Youichi Kimura さんが12年以上前に更新

note-13 での指摘について（一部）

$request['reject']が未定義の場合エラーが出るのでは？

lib/vendor/symfony/lib/request/sfRequest.class.php の165行目

  public function offsetGet($name)
  {
    return $this->getParameter($name, false);
  }

となっているため、rejectパラメータが指定されていない場合は false が返ります。

getImage() を二回呼び出すことで，それぞれでクエリを実行されることになり，これは無駄．

Member::getImageFileName() については #2535 にて修正されています。

操作

リンクをコピー

#30

Yuya Watanabe さんが12年以上前に更新

ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更

ログイン停止状態のメンバの apiKey でも api 経由で SNS 内の情報にアクセスで来てしまう．
ブラックリストに登録されているメンバの apiKey でも SNS 内の情報にアクセスできてしまう．

について下記のように修正されていますが，PC版及び携帯版ではメンバ側ではログイン停止あるいはブラックリスト登録されているということをメンバに表示していません．ログイン停止あるいはブラックリスト登録の状態でPC版あるいは携帯版にアクセスしたときは「ログインに失敗しました」と通常のログイン失敗と同様の表示を行なっています．APIについてもPCおよび携帯版と同等の表示を行なってください．これは「ログインに失敗しました」と表示すべきという指摘ではなく，APIにおける同等のメッセージを表示してくださいということです．

apps/api/lib/myUser.class.php

 67     if ($member->isOnBlackList() || $member->getIsLoginRejected())
 68     {
 69       $exception = new opErrorHttpException('You are not allowd to log in.');
 70       throw $exception->setHttpStatusCode(403);
 71     }
 72

操作

リンクをコピー

#31