プロジェクト

全般

プロフィール

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

JSON APIの検索系エンドポイントで抽出範囲を指定するパラメータを共通して使えるようにする

Youichi Kimura9年以上前に追加. 約4年前に更新.

ステータス:
Rejected(差し戻し)
優先度:
Normal(通常)
担当者:
対象バージョン:
開始日:
2014-08-07
期日:
進捗率:

50%


説明

Overview (概要)

現行の activity/search.json に存在する count, max_id, since_id のパラメータを、member/search.json, member/community.json, community/search.json, community/member.json でも使えるようにする。

Spec (仕様)

  • count: 取得する検索結果の最大件数
    • OpenPNE.yml の json_api_fetch_limit で指定された値よりも大きい count を指定することは出来ない
    • count に上限以上の値が指定された場合は json_api_fetch_limit の件数だけ取得する
    • count が指定されていない場合もデフォルト値として json_api_fetch_limit の件数だけ取得する
  • max_id: IDが指定された値以下のメンバー(またはコミュニティ)を検索する (max_id 自体を 含む)
  • since_id: IDが指定された値より大きいメンバー(またはコミュニティ)を検索する (since_id 自体を 含まない)

以下のフィールドは API の レスポンス に新たに追加します。

  • hasNext: 次のページが存在するか否かを表す真偽値

関連するチケット

ブロック先 OpenPNE 3 - Enhancement(機能追加・改善) #3646: スマートフォン版のメンバー検索やフレンドリストガジェット等において、検索結果が20件以上表示できないのを全件表示できるようにする Rejected(差し戻し) 2014-06-25

履歴

#1 Youichi Kimura9年以上前に更新

  • ブロック先 Enhancement(機能追加・改善) #3646: スマートフォン版のメンバー検索やフレンドリストガジェット等において、検索結果が20件以上表示できないのを全件表示できるようにする を追加

#2 Youichi Kimura9年以上前に更新

  • 説明 を更新 (diff)

#3 Youichi Kimura9年以上前に更新

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

当チケットに対する修正は #3646 の Pull Request 内に含めました。
https://redmine.openpne.jp/issues/3646#note-5

#4 Youichi Kimura9年以上前に更新

  • 説明 を更新 (diff)

#3646 でのページングの実装に追加で必要となるフィールドを仕様に追記しました。

#5 誠二 天重9年以上前に更新

  • ステータスPending Review(レビュー待ち) から Rejected(差し戻し) に変更

気になった点について書いておきます。

  • fetch メソッドの実行は opCursorPager にとって必須な作業になるとおもいますが、現状の opCursorPager では外からクエリ操作ができるようになっているわけでもないので、 初期化用のメソッド(コンストラクタ内で余りやるべきではなさそうなので、initとかinitializeとかいう名前のメソッドなど)を作ってその中で fetch したほうがよいかとおもいました。アクション側に fetch を実行させる理由はあまり無いように思われます(fetch メソッドが since_id や max_id などの引数をとれるようなら、話は違いますが)。
  • もしくは、アクション側から opCursorPager の持っているクエリも操作できるようにしていれば、fetch をアクション側に実行させる理由はあるかとおもいます。
    $pager = new opCursorPager($q);
    $pager->baseQuery->addWhere('member_id = ?', $memberId);
    $pager->fetch();
    

    のような例であれば(現状では baseQuery が protected なので上記の例は実行できませんが)、fetch が外部から実行されるべき理由があるとおもいます(もっとも、こちらの例はもともと opCursorPager のインスタンスを生成する前にクエリ操作できるのだし、基本的にはそうすべきだとおもうので、あまり良くない例だとおもいます)
  • opJsonApiActions に addSearchCondition というメソッドを追加していますが、コントローラーのなかにこのメソッドがいる理由がないとおもいます。opJsonApiActions::addSearchCondition は内容的に見るなら、opCursorPager のなかで吸収すべきだとおもいます。
        $pager = new opCursorPager($query);
        self::addSearchCondition($pager, $request);
        $pager->fetch();
    

    opCursorPager のインスタンスを生成してから fetch を実行する間に opJsonApiActions::addSearchCondition の処理があり、そのなかでページャーオブジェクトが操作されている、という処理の流れには違和感があります。

とりあえず差し戻しにしておきます。

#6 Youichi Kimura9年以上前に更新

@amashige

  • opCursorPager::fetch() の目的は sfDoctrinePager::init() と同じなので、確かに init() などの名前に変えた方が sfPager の操作と統一性があって良いかもしれません。
    • 元々の意図としては、sfDoctrinePager では $pager->getResults() のタイミングでページに対するクエリが実行されるのに対して、opCursorPager では $pager->init() に相当するタイミングでクエリを実行するため、よりDBアクセスが行われることを意識しやすいメソッド名にしたというものでした。
    • init() 相当のタイミングでクエリを実行する必要がある理由は、1つのクエリで「対象ページの取得」と「次ページの有無の確認」を同時に済ませる都合によるものです。
  • opCursorPager::fetch() メソッドに since_id や max_id などの引数を設けなかった理由:
    • sfPager クラスの setPage(), setMaxPerPage() メソッドによる操作と統一性を保たせるため
    • fetch() メソッドに $sinceId, $maxId の2引数を設けた場合、引数の順序が伝わりにくく(あるいは順序の誤りに気付きにくくなり)確認のために毎回メソッドの定義を参照しなければならなくなる懸念があったため
  • opJsonApiActions::addSearchCondition() については、
    $pager = new opCursorPager($query);
    $pager->fromRequest($request);
    $pager->fetch();
    
    のように記述できないか考えていますが、op_json_api_fetch_limit をどう扱うべきかまだ悩んでいます。

#7 kaoru nほぼ7年前に更新

  • 対象バージョンOpenPNE 3.9.0-old から OpenPNE 3.9.0 に変更

対象バージョン変更のため、修正内容の確認を行います。

#8 kaoru n約4年前に更新

  • 対象バージョンOpenPNE 3.9.0 から OpenPNE 3.10.x に変更

他の形式にエクスポート: Atom PDF