/polls/voters.json でページングされたリクエスト間で重複したユーザーが返される

バグ
, ,
1

/polls/voters.json エンドポイントを呼び出す際にページネーションを使用すると、最初の呼び出しでは予想どおり 25 人のユーザーが返されますが、2 ページ目では 26 人のユーザーが返され、そのうち 1 人は最初の呼び出しで返されたユーザーと同じです。これは、25 人を超えるユーザーがいる場合に常に発生します。

問題は、discourse の poll プラグインのここにあります。

オフセットは、Postgres SQL が BETWEEN を使用して行を排他的であると仮定して計算されていますが、実際には包括的です。

計算された行は 0 ではなく 1 から始まるため、最初のクエリは実際には 25 を返します。

私の提案する修正は、次のように簡単です。

   params = {
      offset: offset + 1,
      offset_plus_limit: offset + limit,
      option_digest: opts[:option_id].presence,
    }

または、よりエレガントな解決策として、Postgres の LIMIT と OFFSET を使用することもできます。

    params = {
      limit: limit,
      offset: offset,
      option_digest: opts[:option_id].presence,
    }

          WHERE pv.poll_id IN (:poll_ids)
                /* where */
        ) v
        ORDER BY digest, CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END, username
        LIMIT :limit OFFSET :offset
      SQL
「いいね！」 2
2

素晴らしい発見です。誰かが修正を試みる場合に備えて、ここに #pr-welcome を付けます。

ここでは、フロントエンドがここで誤った前提に誤って依存している場合に、既存の投票のものが壊れないことを確認する必要があります。

「いいね！」 1
3

迅速な返信ありがとうございます！
テストなしでPRを作成したくなかったので、環境をセットアップしてみます。誰でも自由に先にやってください。私はまったくRuby開発者ではありません。specやテストの変更についてもわかりません。

おかしくて笑えることに、UIにはこのバグを回避するために結果をセットにダンプするコードがあります。

「いいね！」 2
4

こちらでも同様のUI修正を抽選ソフトウェアに適用しました。

問題箇所を特定してくれたVHSの@lukecycaさんに感謝します。

5

1月に行われた、より多くの投票者を読み込むことに関する関連レポートもあります。

「いいね！」 1
7

さらに面白いのは、以前、フロントエンドの近代化とランク付け選択の追加のために、非常に大規模なPRをいくつか提出したことです。

それらのPRの巨大な規模が、私がこれを見逃した原因となったことは間違いありません。バックエンドの修正は実際には範囲外でした。

2018年に設定されたフロントエンドの先例をコピーしたことは認めます😅

「いいね！」 3
8

初めてのコントリビューションとしてこの件に取り組んでみようと思ったのですが、予想以上に複雑なようです。:wink:

Draft PRを公開しました: FIX: do not return duplicates from /polls/voters.json by clechasseur · Pull Request #1 · clechasseur/discourse · GitHub

このPRでは、まず問題が再現するテストを追加し、次にRobの簡単な修正を適用したところ、テストがパスしました。

よりエレガントな解決策（私が望んでいた方）も試しましたが、重複する投票者を防ぐことはできましたが、最初の投票者を含む、どの投票者がどのページで返されるかも変更されてしまいました。これは、フロントエンドがどのように処理するかによっては、破壊的な変更と見なされる可能性があります（まだ確認していません）。

しかし、一歩引いて考えると、このエンドポイントを呼び出す際の limit パラメータの真の意味は何なのか疑問に思っています。これは、返される合計投票者の数を制限するのではなく、各投票オプションごとに返される投票者の数を制限するだけです。これは、私がこちらに追加したマルチチョイス投票のテストで確認できます。最初のページは確かにオプションごとに2人の投票者に制限されていますが、合計で3人の異なる投票者が返されます（オプションに分散しています）。エレガントな解決策（つまり、LIMIT :limit OFFSET :offset を使用する）に切り替えると、limit は投票者ではなく、投票の合計数に適用されます。どちらがより良いか、またはより直感的かは100％確信が持てません。

とにかく、私はこの分野の初心者なので、考えすぎているのかもしれません。簡単な解決策は重複する投票者を削除し、あまり大きな問題を引き起こさないので、それが良い方法かもしれません。親リポジトリにPRを送信する前に、フィードバックを待ちます。

–

ちなみに、このコードの部分にもう一つバグがあると思います。投票者をロードするためのクエリは、digest、rank、usernameで並べ替えられますが、rankで並べ替える際に、この条件を使用しています。

CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END

しかし、'Abstain' は実際にはランク0に対応しており、1ではありません。ランク1も'1'として返される可能性があります。これにより、クエリ間で順序が非決定的になる可能性があり、投票者の数と使用されるlimit値によっては、ページネーションクエリを実行する際に投票者を見逃すことが可能になるかもしれません。新しいテストでは、順序の非決定的性を回避するために、返された投票者をソートする必要がありました。（非決定的であるため、テストで再現するのは簡単ではないと思いますが、試してみることはできます…）

「いいね！」 1
9

上記リンクをたどり、以前のレポートを確認したところ、さらにその前のレポートへのリンクが含まれており、そこにも同様の可能性を示唆する記述がありました。

しかし、それはかなり前のことなので、コードは当時とは全く異なっている可能性があります。（Gitで時間を戻そうとしましたが、履歴は2021年で止まっていたので、コードはどこかで移動したと推測されます。）

10

いくつかの実験の後、非決定的な順序付けが返された投票者で問題を引き起こすテストを作成できませんでした。これは、クエリの作成方法と投票の作成方法によるものだと思いますが、100％確信はありません。

テストと提案された修正を含むPRを作成しました：FIX: clechasseur による /polls/voters.json エンドポイントから重複する投票者を返すのを回避 · Pull Request #34433 · discourse/discourse

「いいね！」 3
12

修正とレビューへのご協力、誠にありがとうございました。承認してマージしました。 :partying_face:

「いいね！」 3
13

このトピックは3日後に自動的に閉じられました。新しい返信はもう許可されていません。