-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix(billing): 修复时间规则区间恒真表达式导致倍率全天生效 #6934
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
Changes from all commits
af1819b
0492b0a
df0dc36
49582f8
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 |
|---|---|---|
| @@ -0,0 +1,218 @@ | ||
| /* | ||
| Copyright (C) 2023-2026 QuantumNous | ||
|
|
||
| This program is free software: you can redistribute it and/or modify | ||
| it under the terms of the GNU Affero General Public License as | ||
| published by the Free Software Foundation, either version 3 of the | ||
| License, or (at your option) any later version. | ||
|
|
||
| This program is distributed in the hope that it will be useful, | ||
| but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| GNU Affero General Public License for more details. | ||
|
|
||
| You should have received a copy of the GNU Affero General Public License | ||
| along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
|
|
||
| For commercial licensing, please contact support@quantumnous.com | ||
| */ | ||
| import { describe, expect, test } from 'vitest' | ||
|
|
||
| import { | ||
| buildRequestRuleExpr, | ||
| MATCH_EQ, | ||
| MATCH_GTE, | ||
| MATCH_RANGE, | ||
| type RequestCondition, | ||
| type RequestRuleGroup, | ||
| type TimeCondition, | ||
| type TimeFunc, | ||
| tryParseRequestRuleExpr, | ||
| } from '../billing-expr' | ||
|
|
||
| function timeCondition(overrides: Partial<TimeCondition> = {}): TimeCondition { | ||
| return { | ||
| source: 'time', | ||
| timeFunc: 'hour', | ||
| timezone: 'Asia/Shanghai', | ||
| mode: MATCH_RANGE, | ||
| value: '', | ||
| rangeStart: '', | ||
| rangeEnd: '', | ||
| ...overrides, | ||
| } | ||
| } | ||
|
|
||
| function timeRangeGroup(start: string, end: string): RequestRuleGroup { | ||
| return { | ||
| conditions: [timeCondition({ rangeStart: start, rangeEnd: end })], | ||
| multiplier: '2', | ||
| } | ||
| } | ||
|
|
||
| function scalarTimeGroup( | ||
| value: string, | ||
| timeFunc: TimeFunc = 'hour' | ||
| ): RequestRuleGroup { | ||
| return { | ||
| conditions: [timeCondition({ mode: MATCH_GTE, value, timeFunc })], | ||
| multiplier: '2', | ||
| } | ||
| } | ||
|
|
||
| describe('time range expression generation', () => { | ||
| test('within-day range (start < end) builds an && condition', () => { | ||
| // Regression test for #6923: the || form is a tautology that applies the | ||
| // multiplier 24/7. | ||
| expect(buildRequestRuleExpr([timeRangeGroup('9', '12')])).toBe( | ||
| '(hour("Asia/Shanghai") >= 9 && hour("Asia/Shanghai") < 12 ? 2 : 1)' | ||
| ) | ||
| }) | ||
|
|
||
| test('overnight range (start > end) keeps the || condition', () => { | ||
| expect(buildRequestRuleExpr([timeRangeGroup('21', '6')])).toBe( | ||
| '(hour("Asia/Shanghai") >= 21 || hour("Asia/Shanghai") < 6 ? 2 : 1)' | ||
| ) | ||
| }) | ||
|
|
||
| test('equal bounds build an always-false && range instead of a tautology', () => { | ||
| expect(buildRequestRuleExpr([timeRangeGroup('9', '9')])).toBe( | ||
| '(hour("Asia/Shanghai") >= 9 && hour("Asia/Shanghai") < 9 ? 2 : 1)' | ||
| ) | ||
| }) | ||
|
|
||
| test.each([ | ||
| ['out-of-domain negative bounds', '-1', '-5'], | ||
| ['out-of-domain upper bound', '9', '24'], | ||
| ['non-integer bound', '9.5', '12'], | ||
| ])('drops the rule for %s', (_name, start, end) => { | ||
| expect(buildRequestRuleExpr([timeRangeGroup(start, end)])).toBe('') | ||
| }) | ||
|
|
||
| test('drops a scalar rule whose value is out of domain', () => { | ||
| expect(buildRequestRuleExpr([scalarTimeGroup('25')])).toBe('') | ||
| }) | ||
|
|
||
| test.each([ | ||
| ['hour', '0', true], | ||
| ['hour', '23', true], | ||
| ['hour', '24', false], | ||
| ['minute', '59', true], | ||
| ['minute', '60', false], | ||
| ['weekday', '0', true], | ||
| ['weekday', '6', true], | ||
| ['weekday', '7', false], | ||
| ['month', '1', true], | ||
| ['month', '12', true], | ||
| ['month', '0', false], | ||
| ['month', '13', false], | ||
| ['day', '1', true], | ||
| ['day', '31', true], | ||
| ['day', '32', false], | ||
| ])('keeps %s value %s in domain: %s', (timeFunc, value, inDomain) => { | ||
| const expr = buildRequestRuleExpr([ | ||
| scalarTimeGroup(value, timeFunc as TimeFunc), | ||
| ]) | ||
| expect(expr !== '').toBe(inDomain) | ||
| }) | ||
| }) | ||
|
|
||
| describe('time range expression parsing', () => { | ||
| test('parses an && range back into a single MATCH_RANGE condition', () => { | ||
| const groups = tryParseRequestRuleExpr( | ||
| '(hour("Asia/Shanghai") >= 9 && hour("Asia/Shanghai") < 12 ? 2 : 1)' | ||
| ) | ||
| expect(groups).toHaveLength(1) | ||
| expect(groups?.[0].conditions).toHaveLength(1) | ||
| const condition = groups?.[0].conditions[0] as TimeCondition | ||
| expect(condition.mode).toBe(MATCH_RANGE) | ||
| expect(condition.rangeStart).toBe('9') | ||
| expect(condition.rangeEnd).toBe('12') | ||
| }) | ||
|
|
||
| test('still parses a legacy || range into a single MATCH_RANGE condition', () => { | ||
| const groups = tryParseRequestRuleExpr( | ||
| '(hour("Asia/Shanghai") >= 21 || hour("Asia/Shanghai") < 6 ? 2 : 1)' | ||
| ) | ||
| expect(groups?.[0].conditions).toHaveLength(1) | ||
| const condition = groups?.[0].conditions[0] as TimeCondition | ||
| expect(condition.mode).toBe(MATCH_RANGE) | ||
| expect(condition.rangeStart).toBe('21') | ||
| expect(condition.rangeEnd).toBe('6') | ||
| }) | ||
|
|
||
| test('merges adjacent time bounds into MATCH_RANGE when other conditions follow', () => { | ||
| const groups = tryParseRequestRuleExpr( | ||
| '(param("service_tier") == "fast" && hour("Asia/Shanghai") >= 9 && hour("Asia/Shanghai") < 12 ? 2 : 1)' | ||
| ) | ||
| expect(groups?.[0].conditions.map((c) => c.mode)).toEqual([ | ||
| MATCH_EQ, | ||
| MATCH_RANGE, | ||
| ]) | ||
| const range = groups?.[0].conditions[1] as TimeCondition | ||
| expect(range.rangeStart).toBe('9') | ||
| expect(range.rangeEnd).toBe('12') | ||
| }) | ||
|
|
||
| test('keeps a parenthesized overnight range as MATCH_RANGE in a mixed group', () => { | ||
| const groups = tryParseRequestRuleExpr( | ||
| '((hour("Asia/Shanghai") >= 21 || hour("Asia/Shanghai") < 6) && param("service_tier") == "fast" ? 3 : 1)' | ||
| ) | ||
| expect(groups?.[0].conditions.map((c) => c.mode)).toEqual([ | ||
| MATCH_RANGE, | ||
| MATCH_EQ, | ||
| ]) | ||
| expect(groups?.[0].multiplier).toBe('3') | ||
| }) | ||
|
|
||
| test('parses the issue #6923 two-scalar workaround groups as single ranges', () => { | ||
| const groups = tryParseRequestRuleExpr( | ||
| '(hour("Asia/Shanghai") >= 9 && hour("Asia/Shanghai") < 12 ? 2 : 1) * (hour("Asia/Shanghai") >= 14 && hour("Asia/Shanghai") < 18 ? 2 : 1)' | ||
| ) | ||
| expect(groups).toHaveLength(2) | ||
| for (const group of groups ?? []) { | ||
| expect(group.conditions).toHaveLength(1) | ||
| expect(group.conditions[0].mode).toBe(MATCH_RANGE) | ||
| } | ||
| }) | ||
|
|
||
| test.each([ | ||
| [ | ||
| 'out-of-domain range bounds', | ||
| '(hour("Asia/Shanghai") >= 25 && hour("Asia/Shanghai") < 30 ? 2 : 1)', | ||
| ], | ||
| [ | ||
| 'fractional range bounds', | ||
| '(hour("Asia/Shanghai") >= 1.5 && hour("Asia/Shanghai") < 2.5 ? 2 : 1)', | ||
| ], | ||
| ['out-of-domain scalar value', '(hour("Asia/Shanghai") >= 25 ? 2 : 1)'], | ||
| ['out-of-domain weekday value', '(weekday("UTC") >= 7 ? 2 : 1)'], | ||
| ])('rejects %s instead of parsing them', (_name, expr) => { | ||
| // Rejected rules keep the editor in raw mode; a lenient parse would let | ||
| // the visual editor silently drop the rule on rebuild. | ||
| expect(tryParseRequestRuleExpr(expr)).toBeNull() | ||
| }) | ||
| }) | ||
|
|
||
| describe('time range round-trip stability', () => { | ||
| test('build → parse → build yields the identical mixed-group expression', () => { | ||
| const groups: RequestRuleGroup[] = [ | ||
| { | ||
| conditions: [ | ||
| { | ||
| source: 'param', | ||
| path: 'service_tier', | ||
| mode: MATCH_EQ, | ||
| value: 'fast', | ||
| } satisfies RequestCondition, | ||
| timeCondition({ rangeStart: '9', rangeEnd: '12' }), | ||
| ], | ||
| multiplier: '2', | ||
| }, | ||
| ] | ||
| const expr = buildRequestRuleExpr(groups) | ||
| const parsed = tryParseRequestRuleExpr(expr) | ||
| expect(parsed).not.toBeNull() | ||
| expect(buildRequestRuleExpr(parsed ?? [])).toBe(expr) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -372,25 +372,44 @@ function parseExprLiteral(raw: string): string | null { | |
| } | ||
| } | ||
|
|
||
| // Time function value domains. Values outside these ranges are invalid for | ||
| // the corresponding time function (e.g. hour() is 0-23) and would otherwise | ||
| // produce always-true conditions like hour >= -1 || hour < -5. | ||
| const TIME_FUNC_RANGES: Record<TimeFunc, [number, number]> = { | ||
| hour: [0, 23], | ||
| minute: [0, 59], | ||
| weekday: [0, 6], | ||
| month: [1, 12], | ||
| day: [1, 31], | ||
| } | ||
|
|
||
| function isTimeValueInRange(timeFunc: TimeFunc, text: string): boolean { | ||
| if (!NUMERIC_LITERAL_REGEX.test(text)) return false | ||
| const value = Number(text) | ||
| if (!Number.isInteger(value)) return false | ||
| const [min, max] = TIME_FUNC_RANGES[timeFunc] | ||
| return value >= min && value <= max | ||
| } | ||
|
|
||
| function tryParseTimeCondition(expr: string): RequestCondition | null { | ||
| let m = expr.match( | ||
| /^(hour|minute|weekday|month|day)\("([^"]+)"\) >= ([\d.eE+-]+) \|\| \1\("\2"\) < ([\d.eE+-]+)$/ | ||
| /^(hour|minute|weekday|month|day)\("([^"]+)"\) >= ([\d.eE+-]+) (?:&&|\|\|) \1\("\2"\) < ([\d.eE+-]+)$/ | ||
| ) | ||
| if (m) { | ||
| return { | ||
| source: 'time', | ||
| timeFunc: m[1] as TimeFunc, | ||
| timezone: m[2], | ||
| mode: MATCH_RANGE, | ||
| value: '', | ||
| rangeStart: m[3], | ||
| rangeEnd: m[4], | ||
| } | ||
| if (!m) { | ||
| m = expr.match( | ||
| /^\((hour|minute|weekday|month|day)\("([^"]+)"\) >= ([\d.eE+-]+) (?:&&|\|\|) \1\("\2"\) < ([\d.eE+-]+)\)$/ | ||
| ) | ||
| } | ||
| m = expr.match( | ||
| /^\((hour|minute|weekday|month|day)\("([^"]+)"\) >= ([\d.eE+-]+) \|\| \1\("\2"\) < ([\d.eE+-]+)\)$/ | ||
| ) | ||
| if (m) { | ||
| // Reject invalid bounds at parse time too: an unparseable rule keeps the | ||
| // editor in raw mode, while a leniently parsed one would be silently | ||
| // dropped when the visual editor rebuilds the expression. | ||
| if ( | ||
| !isTimeValueInRange(m[1] as TimeFunc, m[3]) || | ||
| !isTimeValueInRange(m[1] as TimeFunc, m[4]) | ||
| ) { | ||
| return null | ||
| } | ||
| return { | ||
| source: 'time', | ||
| timeFunc: m[1] as TimeFunc, | ||
|
|
@@ -405,6 +424,7 @@ function tryParseTimeCondition(expr: string): RequestCondition | null { | |
| /^(hour|minute|weekday|month|day)\("([^"]+)"\) (==|>=|<) ([\d.eE+-]+)$/ | ||
| ) | ||
| if (m) { | ||
| if (!isTimeValueInRange(m[1] as TimeFunc, m[4])) return null | ||
| const opMap: Record<string, string> = { | ||
| '==': MATCH_EQ, | ||
| '>=': MATCH_GTE, | ||
|
|
@@ -483,13 +503,51 @@ function tryParseRequestCondition(expr: string): RequestCondition | null { | |
| return null | ||
| } | ||
|
|
||
| function tryParseTimeRangePair( | ||
| lower: string, | ||
| upper: string | ||
| ): RequestCondition | null { | ||
| const a = tryParseTimeCondition(lower) | ||
| const b = tryParseTimeCondition(upper) | ||
| if (!a || !b || a.source !== 'time' || b.source !== 'time') return null | ||
| const ta = a as TimeCondition | ||
| const tb = b as TimeCondition | ||
| if (ta.timeFunc !== tb.timeFunc || ta.timezone !== tb.timezone) return null | ||
| if (ta.mode !== MATCH_GTE || tb.mode !== MATCH_LT) return null | ||
| return { | ||
| source: 'time', | ||
| timeFunc: ta.timeFunc, | ||
| timezone: ta.timezone, | ||
| mode: MATCH_RANGE, | ||
| value: '', | ||
| rangeStart: ta.value, | ||
| rangeEnd: tb.value, | ||
| } | ||
| } | ||
|
|
||
| function tryParseRequestConditions( | ||
| conditionStr: string | ||
| ): RequestCondition[] | null { | ||
| // A single time range like hour(tz) >= 9 && hour(tz) < 12 must stay one | ||
| // MATCH_RANGE condition instead of being split into two scalar conditions. | ||
| const wholeTimeCond = tryParseTimeCondition(conditionStr.trim()) | ||
| if (wholeTimeCond) return [wholeTimeCond] | ||
|
|
||
| const andParts = splitTopLevelAnd(conditionStr) | ||
| const conditions: RequestCondition[] = [] | ||
| for (const part of andParts) { | ||
| const condition = tryParseRequestCondition(part.trim()) | ||
| for (let i = 0; i < andParts.length; i += 1) { | ||
| const part = andParts[i].trim() | ||
| // Adjacent matching time bounds (fn >= X && fn < Y) form one range; merge | ||
| // them so the visual editor keeps a single MATCH_RANGE row even when | ||
| // other conditions follow in the same group. | ||
| const next = i + 1 < andParts.length ? andParts[i + 1].trim() : '' | ||
| const merged = next ? tryParseTimeRangePair(part, next) : null | ||
| if (merged) { | ||
| conditions.push(merged) | ||
| i += 1 | ||
| continue | ||
| } | ||
| const condition = tryParseRequestCondition(part) | ||
| if (!condition) return null | ||
| conditions.push(condition) | ||
| } | ||
|
|
@@ -732,13 +790,21 @@ function buildTimeConditionExpr(cond: TimeCondition): string { | |
| if (mode === MATCH_RANGE) { | ||
| const s = normalized.rangeStart.trim() | ||
| const e = normalized.rangeEnd.trim() | ||
| if (!NUMERIC_LITERAL_REGEX.test(s) || !NUMERIC_LITERAL_REGEX.test(e)) { | ||
| if (!isTimeValueInRange(timeFunc, s) || !isTimeValueInRange(timeFunc, e)) { | ||
| return '' | ||
| } | ||
| return `${fn} >= ${s} || ${fn} < ${e}` | ||
| // Overnight range (start > end) crosses the day boundary, e.g. 21-6. | ||
| // A within-day range (start <= end), e.g. 9-12, must use && so the | ||
| // condition is not a tautology that always applies the multiplier. | ||
| const sNum = Number(s) | ||
| const eNum = Number(e) | ||
| if (sNum > eNum) { | ||
| return `${fn} >= ${s} || ${fn} < ${e}` | ||
| } | ||
| return `${fn} >= ${s} && ${fn} < ${e}` | ||
|
Comment on lines
+796
to
+804
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Use a neutral label for These lines make Replace the key with a neutral i18n key such as 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
| const v = normalized.value.trim() | ||
| if (!NUMERIC_LITERAL_REGEX.test(v)) return '' | ||
| if (!isTimeValueInRange(timeFunc, v)) return '' | ||
| const opMap: Record<string, string> = { | ||
| [MATCH_EQ]: '==', | ||
| [MATCH_GTE]: '>=', | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.