Project

General

Profile

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

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

Added by Youichi Kimura about 5 years ago. Updated over 2 years ago.

Status:
Rejected(差し戻し)
Priority:
Normal(通常)
Target version:
Start date:
2014-08-07
Due date:
% Done:

50%


Description

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: 次のページが存在するか否かを表す真偽値

Related issues

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

History

#1 Updated by Youichi Kimura about 5 years ago

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

#2 Updated by Youichi Kimura about 5 years ago

  • Description updated (diff)

#3 Updated by Youichi Kimura about 5 years ago

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

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

#4 Updated by Youichi Kimura about 5 years ago

  • Description updated (diff)

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

#5 Updated by 誠二 天重 about 5 years ago

  • Status changed from Pending Review(レビュー待ち) to 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 Updated by Youichi Kimura about 5 years ago

@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 Updated by kaoru n over 2 years ago

  • Target version changed from OpenPNE 3.9.0-old to OpenPNE 3.9.0

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

Also available in: Atom PDF