-
Notifications
You must be signed in to change notification settings - Fork 0
feat: allow object format for range parameters #222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
af33ddb
bf7d4d1
cd3a0a6
0c85cd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,7 +21,7 @@ import type { | |||||||||||||||||||||
| DateParam, | ||||||||||||||||||||||
| } from "./params.js"; | ||||||||||||||||||||||
| import { BooleanNumber, StopParam } from "./params.js"; | ||||||||||||||||||||||
| import type { Join } from "./util/type.js"; | ||||||||||||||||||||||
| import type { Join, RangeParam } from "./util/type.js"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export type DefaultSearchResultFields = keyof Omit< | ||||||||||||||||||||||
| NarouSearchResult, | ||||||||||||||||||||||
|
|
@@ -54,6 +54,34 @@ export abstract class SearchBuilderBase< | |||||||||||||||||||||
| return Array.from(new Set(array)); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * 範囲指定や配列をハイフン区切りの文字列に変換する | ||||||||||||||||||||||
| * @protected | ||||||||||||||||||||||
| * @static | ||||||||||||||||||||||
| * @param n 範囲指定オブジェクト、数値の配列、あるいは単一の数値 | ||||||||||||||||||||||
| * @returns ハイフン区切りの文字列 | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| protected static range2string<T extends number>( | ||||||||||||||||||||||
| n: T | readonly [T, T] | RangeParam<T> | ||||||||||||||||||||||
| ): Join<T | ""> | undefined { | ||||||||||||||||||||||
| if (typeof n === "object" && n !== null && !Array.isArray(n)) { | ||||||||||||||||||||||
| if ("equal" in n && typeof n.equal === "number") { | ||||||||||||||||||||||
| return n.equal.toString() as Join<T>; | ||||||||||||||||||||||
| } else if ("min" in n || "max" in n) { | ||||||||||||||||||||||
| const obj = n as Extract<RangeParam<T>, { min?: T, max?: T }>; | ||||||||||||||||||||||
| if (obj.min === undefined && obj.max === undefined) { | ||||||||||||||||||||||
| return undefined; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return `${obj.min ?? ""}-${obj.max ?? ""}` as Join<T | "">; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return undefined; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if (Array.isArray(n) && n.length > 2) { | ||||||||||||||||||||||
| throw new Error("範囲指定の配列は要素数を2つ以内にする必要があります"); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return SearchBuilderBase.array2string(n) as Join<T>; | ||||||||||||||||||||||
|
||||||||||||||||||||||
| return SearchBuilderBase.array2string(n) as Join<T>; | |
| if (Array.isArray(n)) { | |
| if (n.length !== 2) { | |
| throw new TypeError( | |
| "range2string only accepts arrays with exactly two elements: [min, max]" | |
| ); | |
| } | |
| return `${n[0]}-${n[1]}` as Join<T>; | |
| } | |
| return n.toString() as Join<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array2stringが意図しているタプルを返すかを(型かランタイムで)検証をしっかりすること
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
range2string は配列入力をそのまま array2string に委譲しているため、数値配列の要素数が 2 以外(例: [1,2,3])でも 1-2-3 のような文字列になり、range 系パラメータとしては不正なクエリを生成します。range 用ユーティリティとしては、配列はタプル [min, max] のみ受け付ける(型を readonly [T, T] にする)か、ランタイムで長さチェックしてエラーにするなどの防止策を入れた方が安全です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ご指摘ありがとうございます!range2stringが不適切な要素数(3以上)の配列を受け付けないようにランタイムチェックを追加し、さらに安全性を高めるため、TypeScriptの型定義でも readonly [number, number] のタプル型を要求するように変更しました。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with length, sasie, and time, the kaiwaritu method should also support readonly number[] in its signature. The implementation already correctly handles arrays via range2string because typeof array === "object" is true.
kaiwaritu(
minOrRange: number | readonly number[] | RangeParam<number>,
max?: number
): this {There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
会話率も同じシグネチャのほうがいい
Copilot
AI
Apr 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kaiwaritu のオブジェクト分岐が typeof minOrRange === "object" だけなので、配列もここに入ってしまいます(例: JS利用者が誤って kaiwaritu([10, 20]) を渡す)。結果として range2string が配列を受け付けて文字列化し、意図しない入力をサイレントに受理する/空配列で "" を生成する可能性があります。RangeParam として扱う条件を !Array.isArray(minOrRange) まで含めるか、"min" in ... || "max" in ... || "equal" in ... のようにキーで判定して配列を除外してください。
Copilot
AI
Apr 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sasie / time も RangeParam を受け付けるようになりましたが、テストが一部の形(例: sasie は { min } のみ、time は { min, max } のみ)しかカバーしていません。{ max } や { equal } など、サポートするとして公開したオブジェクト形状を最低限網羅するテストを追加してください。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
テストが足りてない
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ご指摘ありがとうございます。sasie および time に対しても { min: x, max: y } や { equal: z } といったオブジェクト形式での網羅的なテストケースを追加し、不足を補いました。
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,3 +24,12 @@ type Stringable = string | number | bigint | boolean | null | undefined; | |||||||||||||||||||||||
| * type JoinedNumbers = Join<Numbers>; // '1' | '2' | '3' | '1-1' | '1-2' | '1-3' | '2-1' | '2-2' | '2-3' | '3-1' | '3-2' | '3-3' | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export type Join<T extends Stringable> = `${T}-${T}` | `${T}`; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * 範囲指定のためのオブジェクトの型。 | ||||||||||||||||||||||||
| * @template T - 範囲指定する値の型 | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| export type RangeParam<T extends number> = | ||||||||||||||||||||||||
| | { min: T; max?: T } | ||||||||||||||||||||||||
| | { min?: T; max: T } | ||||||||||||||||||||||||
|
Comment on lines
+30
to
+34
|
||||||||||||||||||||||||
| * @template T - 範囲指定する値の型 | |
| */ | |
| export type RangeParam<T extends number> = | |
| | { min: T; max?: T } | |
| | { min?: T; max: T } | |
| * `min` / `max` による範囲指定、または `equal` による完全一致を表します。 | |
| * `min` と `max` はどちらも省略可能です。 | |
| * @template T - 範囲指定する値の型 | |
| */ | |
| export type RangeParam<T extends number> = | |
| | { min?: T; max?: T } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -787,6 +787,46 @@ describe("SearchBuilder", () => { | |||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`0-${length}`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| test("if length = { min: 1000 }", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const mockFn = vi.fn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setupMockHandler(mockFn, ["length", "gzip", "out"]); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await NarouAPI.search().length({ min: 1000 }).execute(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.allcount).toBe(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`1000-`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| test("if length = { max: 1000 }", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const mockFn = vi.fn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setupMockHandler(mockFn, ["length", "gzip", "out"]); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await NarouAPI.search().length({ max: 1000 }).execute(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.allcount).toBe(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`-1000`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| test("if length = { min: 100, max: 1000 }", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const mockFn = vi.fn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setupMockHandler(mockFn, ["length", "gzip", "out"]); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await NarouAPI.search().length({ min: 100, max: 1000 }).execute(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.allcount).toBe(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`100-1000`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| test("if length = { equal: 1000 }", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const mockFn = vi.fn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setupMockHandler(mockFn, ["length", "gzip", "out"]); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await NarouAPI.search().length({ equal: 1000 }).execute(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.allcount).toBe(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`1000`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| describe("kaiwaritu", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -823,6 +863,26 @@ describe("SearchBuilder", () => { | |||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`${min}-${max}`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| test("if kaiwaritu = { min: 10 }", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const mockFn = vi.fn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setupMockHandler(mockFn, ["kaiwaritu", "gzip", "out"]); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await NarouAPI.search().kaiwaritu({ min: 10 }).execute(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.allcount).toBe(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`10-`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| test("if kaiwaritu = { max: 50 }", async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const mockFn = vi.fn(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setupMockHandler(mockFn, ["kaiwaritu", "gzip", "out"]); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await NarouAPI.search().kaiwaritu({ max: 50 }).execute(); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(result.allcount).toBe(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(mockFn).toHaveBeenCalledWith(`-50`, "5", "json", 3); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | |
| }); | |
| test("if kaiwaritu = { min: 10, max: 50 }", async () => { | |
| const mockFn = vi.fn(); | |
| setupMockHandler(mockFn, ["kaiwaritu", "gzip", "out"]); | |
| const result = await NarouAPI.search() | |
| .kaiwaritu({ min: 10, max: 50 }) | |
| .execute(); | |
| expect(result.allcount).toBe(1); | |
| expect(mockFn).toHaveBeenCalledTimes(1); | |
| expect(mockFn).toHaveBeenCalledWith(`10-50`, "5", "json", 3); | |
| }); | |
| test("if kaiwaritu = { equal: 10 }", async () => { | |
| const mockFn = vi.fn(); | |
| setupMockHandler(mockFn, ["kaiwaritu", "gzip", "out"]); | |
| const result = await NarouAPI.search().kaiwaritu({ equal: 10 }).execute(); | |
| expect(result.allcount).toBe(1); | |
| expect(mockFn).toHaveBeenCalledTimes(1); | |
| expect(mockFn).toHaveBeenCalledWith(`10`, "5", "json", 3); | |
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
range2stringの配列チェックがn.length > 2のみになっているため、ランタイムで[]や[1]を渡すと例外にならず、array2string経由で""(空文字)や単一値が生成されます。範囲指定用ユーティリティとしては配列は[min, max]の2要素のみを許可し、length !== 2の場合は例外にする(またはundefinedを返してsetしない)ようにして、無効なクエリの生成を防いでください。