全件取得のつもりが、最初のページだけ落ちていた

会員が自分の商品の注文を見る画面のコードを読んでいて、手が止まった箇所がある。

let allCmsItems = [];
let cmsResults = await wixData.query('商品コレクション')
  .eq('member', memberId)
  .limit(1000)
  .find();

while (cmsResults.hasNext()) {
  cmsResults = await cmsResults.next();
  allCmsItems = allCmsItems.concat(cmsResults.items);
}

最初の find() が返したページの中身が、allCmsItems に入っていない。ループが連結しているのは、2ページ目以降だけである。

1ページで収まるときに全部消える

この書き方は、件数によって壊れ方が変わる。

• 登録商品が1000件以内なら、hasNext() は最初から false になる。ループは一度も回らず、allCmsItems は空のままになる

• 1000件を超えていれば、2ページ目以降だけが集まる。先頭の1000件が欠ける

現実的な運用ではほぼ前者だから、症状は「登録商品があるのに、注文が1件も出ない」になる。しかも例外は出ない。

直後の分岐が、この状態をさらに見えにくくしている。

if (cmsResults.items.length === 0) {
  return { success: true, orders: [], message: '登録商品がありません。' };
}

判定に使っているのは cmsResults.items であって、集めたはずの配列ではない。ループが回っていなければ、これは最初のページの中身を指している。つまり件数は正しく入っていて、この分岐は素通りする。空なのは、その後で使う allCmsItems のほうである。

結果として、処理は先へ進み、商品 ID が1件も集まらず、最後に「商品データが見つかりません」という別のメッセージを返して終わる。利用者から見れば、商品を登録しているのに登録されていないと言われている状態になる。

同じファイルの20行下に、正しい形がある

おもしろいのは、同じ関数の少し下で、別のコレクションを同じ目的で読んでいることである。

let allOrders = [];
let ordersResult = await wixData.query('注文コレクション')
  .descending('_createdDate')
  .limit(100)
  .find();

allOrders = allOrders.concat(ordersResult.items);   // 初回の結果を入れている

while (ordersResult.hasNext()) {
  ordersResult = await ordersResult.next();
  allOrders = allOrders.concat(ordersResult.items);
}

こちらは初回の結果を先に入れている。書いた人が理解していなかったわけではない、ということでもある。1行を書き忘れただけで、しかもその1行が無いことは、テストでも例外でも表面化しない。

形で防ぐ

この種の書き忘れは、注意では減らない。ページを取得したら必ず連結する、という順番を構造として持たせるほうが早い。

async function findAll(query, maxPages = 30) {
  const items = [];
  let page = await query.find();
  let pages = 1;

  for (;;) {
    items.push(...page.items);          // 取得したページは必ず入れる
    if (!page.hasNext() || pages >= maxPages) break;
    page = await page.next();
    pages += 1;
  }

  return { items, pages, truncated: page.hasNext() };
}

ページを1つ取ったら必ず連結して、その後で終わりかどうかを判定する。上限に達したかどうかも返す。ページングの終了条件が1つしか無いと暴走することは別の題材になるが、上限を持つ形にしておけば、そちらの問題も同時に閉じられる。