diff --git a/packages/cubejs-backend-native/test/bridge/args-names.test.ts b/packages/cubejs-backend-native/test/bridge/args-names.test.ts index c61b29c4c7eeb..cb541a4f89aea 100644 --- a/packages/cubejs-backend-native/test/bridge/args-names.test.ts +++ b/packages/cubejs-backend-native/test/bridge/args-names.test.ts @@ -29,51 +29,34 @@ describeBridge('bridge: args_names parser', () => { }); }); - // The skipped tests below assert how a correct parser should behave. - // They are buggy today because both this bridge and the JS-side - // schema-compiler use the same regex; a fix must touch both. - describe('known bugs (skip = expected behavior after fix)', () => { - // The named-function branch of the regex captures [A-Za-z0-9_,]* — - // no whitespace allowed. V8 renders `function f(x, y)` with a space - // after the comma, so capture stops after the first arg. - it.skip('parses named function declaration with multiple args', () => { + describe('edge cases', () => { + it('parses named function declaration with multiple args', () => { function named(x: any, y: any) { return [x, y]; } expect(parseArgsNames(named)).toEqual(['x', 'y']); }); - it.skip('parses async named function declaration with multiple args', () => { + it('parses async named function declaration with multiple args', () => { async function named(x: any, y: any) { return [x, y]; } expect(parseArgsNames(named)).toEqual(['x', 'y']); }); - // Default args land inside the (...) capture as a single token "x = 1" - // and survive the comma split unchanged. Special names like - // SECURITY_CONTEXT in this position fail to dispatch. - it.skip('parses default args, returning just the identifier', () => { + it('parses default args, returning just the identifier', () => { expect(parseArgsNames((x: any = 1) => x)).toEqual(['x']); }); - // Rest args keep their leading dots after capture+split, yielding - // "...args" instead of "args". - it.skip('parses rest args, dropping the spread dots', () => { + it('parses rest args, dropping the spread dots', () => { expect(parseArgsNames((...args: any[]) => args)).toEqual(['args']); }); - // Destructuring patterns get split by the comma inside the braces, - // breaking the pattern into half-tokens. - it.skip('parses destructuring args, returning the destructured identifiers', () => { + it('parses destructuring args, returning the destructured identifiers', () => { expect(parseArgsNames(({ a, b }: any) => [a, b])).toEqual(['a', 'b']); }); - // Anonymous function expressions (`function (x){}`) match no branch - // of the regex at all. Today Rust silently returns []; the JS side - // throws `Can't match args for: ...`. Neither is the desired - // behavior — the parser should just return the args. - it.skip('parses anonymous function expressions', () => { + it('parses anonymous function expressions', () => { // eslint-disable-next-line func-names, prefer-arrow-callback const fn = function (x: any) { return x; }; expect(parseArgsNames(fn)).toEqual(['x']); diff --git a/packages/cubejs-backend-native/test/bridge/multi-arg.test.ts b/packages/cubejs-backend-native/test/bridge/multi-arg.test.ts index 3e985061c346d..22e63e4fbfc39 100644 --- a/packages/cubejs-backend-native/test/bridge/multi-arg.test.ts +++ b/packages/cubejs-backend-native/test/bridge/multi-arg.test.ts @@ -12,13 +12,13 @@ describeBridge('bridge: multi-arg dispatch', () => { ); expect(result.template).toBe( - 'SUM({arg:0}) WHERE {fp:0} AND t = {sv:1}' + 'SUM({arg:0}) WHERE {fp:0} AND t = {sv:0}' ); expect(result.args.symbol_paths).toEqual([['CUBE', 'amount']]); expect(result.args.filter_params).toEqual([ { cube_name: 'orders', name: 'status', column: 'col' }, ]); - expect(result.args.security_context.values).toEqual(['acme', 'acme']); + expect(result.args.security_context.values).toEqual(['acme']); }); it('throws an internal error from the StubBaseTools when SQL_UTILS is referenced', () => { diff --git a/packages/cubejs-backend-native/test/bridge/result-shape.test.ts b/packages/cubejs-backend-native/test/bridge/result-shape.test.ts index 3b58b0357030d..38bdcab7b1418 100644 --- a/packages/cubejs-backend-native/test/bridge/result-shape.test.ts +++ b/packages/cubejs-backend-native/test/bridge/result-shape.test.ts @@ -26,21 +26,20 @@ describeBridge('bridge: result shape', () => { expect(result.args.symbol_paths).toEqual([['orders', 'status']]); }); - it('errors when the user function returns a primitive number', () => { - // Bug: bridge eagerly coerces the return value via convert_to_string, - // which only handles JsString and null — it then tries to call - // toString as a struct method, which errors on primitives. JS - // reference returns the value unchanged and lets downstream template - // literals coerce naturally. See skipped 'JS-ref: numeric return…'. - expect(() => compileMemberSql(() => 42 as any)).toThrow( - /Object is not the Struct object/ - ); + it('coerces a numeric return to its string form', () => { + const integerResult = compileMemberSql(() => 42 as any); + const decimalResult = compileMemberSql(() => 1.5 as any); + + expect(integerResult.template).toBe('42'); + expect(decimalResult.template).toBe('1.5'); }); - it('errors when the user function returns a primitive boolean', () => { - expect(() => compileMemberSql(() => true as any)).toThrow( - /Object is not the Struct object/ - ); + it('coerces a boolean return to its string form', () => { + const trueResult = compileMemberSql(() => true as any); + const falseResult = compileMemberSql(() => false as any); + + expect(trueResult.template).toBe('true'); + expect(falseResult.template).toBe('false'); }); it('returns an empty string template when the user function returns null', () => { @@ -49,17 +48,4 @@ describeBridge('bridge: result shape', () => { expect(result.template).toBe(''); expect(result.args.symbol_paths).toEqual([]); }); - - // JS reference does no coercion at the bridge boundary — the user - // function's return value flows through resolveSymbolsCall verbatim and - // is later embedded via template literal, which yields String(value). - it.skip('JS-ref: numeric return is coerced to its string form', () => { - const result = compileMemberSql(() => 42 as any); - expect(result.template).toBe('42'); - }); - - it.skip('JS-ref: boolean return is coerced to its string form', () => { - const result = compileMemberSql(() => true as any); - expect(result.template).toBe('true'); - }); }); diff --git a/packages/cubejs-backend-native/test/bridge/security-context.test.ts b/packages/cubejs-backend-native/test/bridge/security-context.test.ts index 885999d0f6442..480852c4ba68d 100644 --- a/packages/cubejs-backend-native/test/bridge/security-context.test.ts +++ b/packages/cubejs-backend-native/test/bridge/security-context.test.ts @@ -3,35 +3,34 @@ import { bridgeHarnessAvailable, compileMemberSql } from './helpers'; const describeBridge = bridgeHarnessAvailable ? describe : describe.skip; describeBridge('bridge: SECURITY_CONTEXT — filter input shapes', () => { - it('handles a string filter value as col = {sv:N}', () => { - // Pin current Rust behavior. Eager double-registration is a known - // divergence vs JS — see skipped 'JS-ref: .filter pushes value once'. + it('handles a string filter value as col = {sv:0}', () => { const result = compileMemberSql( (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.filter('col')}`, { tenant: 'acme' } ); - expect(result.template).toBe('col = {sv:1}'); - expect(result.args.security_context.values).toEqual(['acme', 'acme']); + expect(result.template).toBe('col = {sv:0}'); + expect(result.args.security_context.values).toEqual(['acme']); }); - it('handles a string array as col IN (sv0, sv1, ...)', () => { + it('handles a string array as col IN (sv0, sv1, sv2)', () => { const result = compileMemberSql( (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups.filter('col')}`, { groups: ['a', 'b', 'c'] } ); - // Eager double-registration: 3 from leaf-proxy construction + 3 from - // .filter call. See skipped 'JS-ref: .filter pushes value once'. - expect(result.template).toBe('col IN ({sv:3}, {sv:4}, {sv:5})'); - expect(result.args.security_context.values).toEqual([ - 'a', - 'b', - 'c', - 'a', - 'b', - 'c', - ]); + expect(result.template).toBe('col IN ({sv:0}, {sv:1}, {sv:2})'); + expect(result.args.security_context.values).toEqual(['a', 'b', 'c']); + }); + + it('handles a numeric array by stringifying each element', () => { + const result = compileMemberSql( + (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.ids.filter('id')}`, + { ids: [1, 2, 3] } + ); + + expect(result.template).toBe('id IN ({sv:0}, {sv:1}, {sv:2})'); + expect(result.args.security_context.values).toEqual(['1', '2', '3']); }); it('renders an empty string array as 1 = 0 with no values registered for the filter', () => { @@ -41,8 +40,6 @@ describeBridge('bridge: SECURITY_CONTEXT — filter input shapes', () => { ); expect(result.template).toBe('1 = 0'); - // Leaf proxy still constructed; an empty array contributes no eager - // registrations and the filter callback also produces none. expect(result.args.security_context.values).toEqual([]); }); @@ -63,8 +60,8 @@ describeBridge('bridge: SECURITY_CONTEXT — filter input shapes', () => { { user_id: 42 } ); - expect(result.template).toBe('uid = {sv:1}'); - expect(result.args.security_context.values).toEqual(['42', '42']); + expect(result.template).toBe('uid = {sv:0}'); + expect(result.args.security_context.values).toEqual(['42']); }); it('formats a non-integer number as a decimal string', () => { @@ -73,34 +70,49 @@ describeBridge('bridge: SECURITY_CONTEXT — filter input shapes', () => { { factor: 1.5 } ); - expect(result.template).toBe('f = {sv:1}'); - expect(result.args.security_context.values).toEqual(['1.5', '1.5']); + expect(result.template).toBe('f = {sv:0}'); + expect(result.args.security_context.values).toEqual(['1.5']); }); - it('formats a boolean as the string true/false', () => { + it('formats a truthy boolean as the string "true"', () => { const result = compileMemberSql( (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.flag.filter('f')}`, { flag: true } ); - expect(result.template).toBe('f = {sv:1}'); - expect(result.args.security_context.values).toEqual(['true', 'true']); + expect(result.template).toBe('f = {sv:0}'); + expect(result.args.security_context.values).toEqual(['true']); }); - it('returns 1 = 1 when an optional filter field is missing', () => { + it.each([ + ['missing', undefined], + ['null', null], + ['empty string', ''], + ['zero', 0], + ['false', false], + ])('returns 1 = 1 when filter value is %s', (_, value) => { + const ctx = value === undefined ? {} : { tenant: value }; const result = compileMemberSql( (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.filter('col')}`, - {} + ctx ); expect(result.template).toBe('1 = 1'); expect(result.args.security_context.values).toEqual([]); }); - it('throws a user error when requiredFilter field is missing', () => { + it.each([ + ['missing', undefined], + ['null', null], + ['empty string', ''], + ['zero', 0], + ['false', false], + ])('throws when requiredFilter value is %s', (_, value) => { + const ctx = value === undefined ? {} : { tenant: value }; + expect(() => compileMemberSql( (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.requiredFilter('col')}`, - {} + ctx )).toThrow(/Filter for col is required/); }); @@ -119,8 +131,8 @@ describeBridge('bridge: SECURITY_CONTEXT — proxy structure', () => { { tenant: { id: '123' } } ); - expect(result.template).toBe('col = {sv:1}'); - expect(result.args.security_context.values).toEqual(['123', '123']); + expect(result.template).toBe('col = {sv:0}'); + expect(result.args.security_context.values).toEqual(['123']); }); it('does not crash on a deep leaf-proxy path that does not exist in the context', () => { @@ -133,6 +145,7 @@ describeBridge('bridge: SECURITY_CONTEXT — proxy structure', () => { // the filter falls back to "1 = 1" (the path is treated as an absent // optional filter, not an error). expect(result.template).toBe('1 = 1'); + expect(result.args.security_context.values).toEqual([]); }); it('exposes unsafeValue() that returns the raw value without registering a placeholder', () => { @@ -142,15 +155,14 @@ describeBridge('bridge: SECURITY_CONTEXT — proxy structure', () => { ); expect(result.template).toBe('prefix-acme-suffix'); - // Eager toString registration still pushes once; unsafeValue itself - // does not register anything. - expect(result.args.security_context.values).toEqual(['acme']); + expect(result.args.security_context.values).toEqual([]); }); it('lets the user branch the template at compile time via unsafeValue()', () => { // Real prod pattern: unsafeValue() returns the actual JS value, so a // ternary in the template literal picks one branch at compile time - // and the resulting template is just the picked literal. + // and the resulting template is just the picked literal — no + // placeholders registered. const adminResult = compileMemberSql( (SECURITY_CONTEXT: any) => `SELECT * FROM ${ SECURITY_CONTEXT.cubeCloud.groups.unsafeValue() === 'admin' @@ -170,18 +182,13 @@ describeBridge('bridge: SECURITY_CONTEXT — proxy structure', () => { expect(adminResult.template).toBe('SELECT * FROM admin_orders'); expect(viewerResult.template).toBe('SELECT * FROM public_orders'); - // unsafeValue() itself does not push to values, BUT just accessing - // .groups constructs a leaf proxy whose toString function eagerly - // pushes the leaf value. So the bridge state still records the leaf - // even though the rendered template never references {sv:N}. - expect(adminResult.args.security_context.values).toEqual(['admin']); - expect(viewerResult.args.security_context.values).toEqual(['viewer']); + expect(adminResult.args.security_context.values).toEqual([]); + expect(viewerResult.args.security_context.values).toEqual([]); }); - it('renders a leaf used directly in a template (no filter call) without duplicating values', () => { - // tenant_id = ${SECURITY_CONTEXT.cubeCloud.tenantId} — common in prod. - // Here the eager to_string_fn baked in during leaf-proxy construction - // returns the placeholder when JS coerces; nothing else pushes. + it('renders a scalar leaf used directly in a template as a single placeholder', () => { + // `tenant_id = ${SECURITY_CONTEXT.cubeCloud.tenantId}` — common in prod. + // Coerce-time toString fires once and registers a single placeholder. const result = compileMemberSql( (SECURITY_CONTEXT: any) => `tenant_id = ${SECURITY_CONTEXT.cubeCloud.tenantId}`, { cubeCloud: { tenantId: '123' } } @@ -191,142 +198,65 @@ describeBridge('bridge: SECURITY_CONTEXT — proxy structure', () => { expect(result.args.security_context.values).toEqual(['123']); }); - it('supports the canonical array-filter callback pattern with groups.join(...)', () => { - // Canonical prod pattern. The callback receives the prepared - // placeholder strings; join glues them into the SQL fragment. + it('renders an array leaf directly in a template as comma-joined placeholders', () => { const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.cubeCloud.groups.filter( - (groups: string[]) => `source IN (${groups.join(',')})` - )}`, - { cubeCloud: { groups: ['a', 'b'] } } - ); - - // 2 from eager toString registration + 2 from .filter() — the last two - // ({sv:2}, {sv:3}) are passed to the callback. - expect(result.template).toBe('source IN ({sv:2},{sv:3})'); - expect(result.args.security_context.values).toEqual(['a', 'b', 'a', 'b']); - }); - - it('accepts both camelCase securityContext and snake_case security_context arg names', () => { - const camel = compileMemberSql( - (securityContext: any) => `${securityContext.tenant.filter('col')}`, - { tenant: 'acme' } - ); - const snake = compileMemberSql( - // eslint-disable-next-line camelcase - (security_context: any) => `${security_context.tenant.filter('col')}`, - { tenant: 'acme' } + (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups}`, + { groups: ['a', 'b'] } ); - expect(camel.template).toBe('col = {sv:1}'); - expect(snake.template).toBe('col = {sv:1}'); + expect(result.template).toBe('{sv:0},{sv:1}'); + expect(result.args.security_context.values).toEqual(['a', 'b']); }); -}); -// Each skipped test below asserts what the JS reference proxy -// (`contextSymbolsProxyFrom` in schema-compiler) does today. The Rust -// bridge diverges; unskip together with a fix. -describeBridge('bridge: SECURITY_CONTEXT — known divergences from JS reference', () => { - // Bug: bridge treats falsy non-null values as real values and emits a - // bind. JS short-circuits truthy on the param: false / 0 / '' return - // "1 = 1". Rust cascades through String/f64/bool deserialization and - // pushes the value. - it.skip('JS-ref: .filter on false returns 1 = 1', () => { + it('renders an empty array leaf directly in a template as an empty string', () => { const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.flag.filter('f')}`, - { flag: false } + (SECURITY_CONTEXT: any) => `[${SECURITY_CONTEXT.groups}]`, + { groups: [] } ); - expect(result.template).toBe('1 = 1'); - expect(result.args.security_context.values).toEqual([]); - }); - it.skip('JS-ref: .filter on 0 returns 1 = 1', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.user_id.filter('uid')}`, - { user_id: 0 } - ); - expect(result.template).toBe('1 = 1'); + expect(result.template).toBe('[]'); expect(result.args.security_context.values).toEqual([]); }); - it.skip('JS-ref: .filter on \'\' returns 1 = 1', () => { + it('allocates a fresh placeholder on every coercion of the same leaf proxy', () => { const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.filter('col')}`, - { tenant: '' } + (SECURITY_CONTEXT: any) => { + const t = SECURITY_CONTEXT.tenant; + return `${t} | ${t}`; + }, + { tenant: 'acme' } ); - expect(result.template).toBe('1 = 1'); - expect(result.args.security_context.values).toEqual([]); - }); - // Bug: requiredFilter accepts falsy non-null values. JS throws on any - // falsy value (including 0 / false / ''). Rust only throws when the - // value is undefined or null. - it.skip('JS-ref: .requiredFilter on 0 throws', () => { - expect(() => compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.requiredFilter('col')}`, - { tenant: 0 } - )).toThrow(/Filter for col is required/); - }); - - it.skip('JS-ref: .requiredFilter on false throws', () => { - expect(() => compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.flag.requiredFilter('f')}`, - { flag: false } - )).toThrow(/Filter for f is required/); - }); - - // Bug: numeric arrays in security context error out. JS maps each - // element through allocateParam without type coercion and produces an - // IN clause. Rust requires every element to deserialize as a string and - // falls through to "Invalid param for security context". - it.skip('JS-ref: .filter on number[] emits IN clause', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.ids.filter('id')}`, - { ids: [1, 2, 3] } - ); - expect(result.template).toMatch( - /^id IN \(\{sv:\d+\}, \{sv:\d+\}, \{sv:\d+\}\)$/ - ); + expect(result.template).toBe('{sv:0} | {sv:1}'); + expect(result.args.security_context.values).toEqual(['acme', 'acme']); }); - // Bug: leaf proxy eagerly allocates the value at construction time, so - // .filter pushes a duplicate and {sv:N} starts at 1. JS allocates - // lazily — each .filter call pushes exactly once. Rust pre-bakes the - // toString output when the leaf proxy is built. - it.skip('JS-ref: .filter pushes value once at {sv:0}', () => { + it('supports the canonical array-filter callback pattern with groups.join(...)', () => { + // Canonical prod pattern. The callback receives the prepared + // placeholder strings; join glues them into the SQL fragment. const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.tenant.filter('col')}`, - { tenant: 'acme' } + (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.cubeCloud.groups.filter( + (groups: string[]) => `source IN (${groups.join(',')})` + )}`, + { cubeCloud: { groups: ['a', 'b'] } } ); - expect(result.template).toBe('col = {sv:0}'); - expect(result.args.security_context.values).toEqual(['acme']); - }); - it.skip('JS-ref: .filter on string[] uses indices 0..N once', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups.filter('col')}`, - { groups: ['a', 'b'] } - ); - expect(result.template).toBe('col IN ({sv:0}, {sv:1})'); + expect(result.template).toBe('source IN ({sv:0},{sv:1})'); expect(result.args.security_context.values).toEqual(['a', 'b']); }); - it.skip('JS-ref: unsafeValue() does not register a placeholder', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `prefix-${SECURITY_CONTEXT.tenant.unsafeValue()}-suffix`, + it('accepts both camelCase securityContext and snake_case security_context arg names', () => { + const camel = compileMemberSql( + (securityContext: any) => `${securityContext.tenant.filter('col')}`, { tenant: 'acme' } ); - expect(result.template).toBe('prefix-acme-suffix'); - expect(result.args.security_context.values).toEqual([]); - }); - - // Bug: array toString joins with ", " (with space). JS joins with - // "," (no space). - it.skip('JS-ref: array toString joins without a space', () => { - const result = compileMemberSql( - (SECURITY_CONTEXT: any) => `${SECURITY_CONTEXT.groups}`, - { groups: ['a', 'b'] } + const snake = compileMemberSql( + // eslint-disable-next-line camelcase + (security_context: any) => `${security_context.tenant.filter('col')}`, + { tenant: 'acme' } ); - expect(result.template).toBe('{sv:0},{sv:1}'); + + expect(camel.template).toBe('col = {sv:0}'); + expect(snake.template).toBe('col = {sv:0}'); }); }); diff --git a/rust/cube/cubenativeutils/src/wrappers/neon/object/neon_function.rs b/rust/cube/cubenativeutils/src/wrappers/neon/object/neon_function.rs index 96bf382250f8f..8095acb844171 100644 --- a/rust/cube/cubenativeutils/src/wrappers/neon/object/neon_function.rs +++ b/rust/cube/cubenativeutils/src/wrappers/neon/object/neon_function.rs @@ -75,33 +75,159 @@ impl + 'static> NativeFunction> for NeonFu } fn args_names(&self) -> Result, CubeError> { - lazy_static! { - static ref FUNCTION_RE: Regex = Regex::new( - r"function\s+\w+\(([A-Za-z0-9_,]*)|\(([\s\S]*?)\)\s*=>|\(?(\w+)\)?\s*=>" - ) - .unwrap(); + Ok(parse_args_names(&self.definition()?)) + } +} + +fn parse_args_names(definition: &str) -> Vec { + lazy_static! { + // Strips an optional `async` and `function [*] [name]` prefix — + // anything left is either `(args) ...` or `name => ...`. + static ref PREFIX_RE: Regex = + Regex::new(r"^\s*(?:async\s+)?(?:function\s*\*?\s*\w*\s*)?").unwrap(); + static ref IDENT_RE: Regex = Regex::new(r"[A-Za-z_$][A-Za-z0-9_$]*").unwrap(); + } + + let prefix_end = PREFIX_RE.find(definition).map(|m| m.end()).unwrap_or(0); + let rest = definition[prefix_end..].trim_start(); + + if !rest.starts_with('(') { + return IDENT_RE + .find(rest) + .filter(|m| rest[m.end()..].trim_start().starts_with("=>")) + .map(|m| vec![m.as_str().to_string()]) + .unwrap_or_default(); + } + + let Some(end) = matching_paren(rest) else { + return vec![]; + }; + let inner = &rest[1..end]; + + let mut out = Vec::new(); + for tok in split_top_level(inner, ',') { + let tok = strip_default(tok).trim().trim_start_matches('.').trim(); + if tok.is_empty() { + continue; } - let definition = self.definition()?; - if let Some(captures) = FUNCTION_RE.captures(&definition) { - let args_string = captures.get(1).or(captures.get(2)).or(captures.get(3)); - if let Some(args_string) = args_string { - Ok(args_string - .as_str() - .split(',') - .filter_map(|s| { - let arg = s.trim().to_string(); - if arg.is_empty() { - None - } else { - Some(arg) - } - }) - .collect()) - } else { - Ok(vec![]) - } + if tok.starts_with('{') || tok.starts_with('[') { + out.extend(IDENT_RE.find_iter(tok).map(|m| m.as_str().to_string())); } else { - Ok(vec![]) + out.push(tok.to_string()); + } + } + out +} + +fn matching_paren(s: &str) -> Option { + let mut depth = 0i32; + for (i, b) in s.bytes().enumerate() { + match b { + b'(' | b'[' | b'{' => depth += 1, + b')' | b']' | b'}' => { + depth -= 1; + if depth == 0 && b == b')' { + return Some(i); + } + } + _ => {} + } + } + None +} + +fn split_top_level(s: &str, sep: char) -> Vec<&str> { + let mut depth = 0i32; + let mut start = 0usize; + let mut out = Vec::new(); + for (i, c) in s.char_indices() { + match c { + '(' | '[' | '{' => depth += 1, + ')' | ']' | '}' => depth -= 1, + c if c == sep && depth == 0 => { + out.push(&s[start..i]); + start = i + c.len_utf8(); + } + _ => {} } } + out.push(&s[start..]); + out +} + +fn strip_default(tok: &str) -> &str { + let mut depth = 0i32; + for (i, c) in tok.char_indices() { + match c { + '(' | '[' | '{' => depth += 1, + ')' | ']' | '}' => depth -= 1, + '=' if depth == 0 => return &tok[..i], + _ => {} + } + } + tok +} + +#[cfg(test)] +mod tests { + use super::*; + + fn names(def: &str) -> Vec { + parse_args_names(def) + } + + #[test] + fn matching_paren_balanced_with_nested_default() { + // `(x = f())` — naive lazy regex would stop at the inner `)`. + assert_eq!(matching_paren("(x = f())"), Some(8)); + assert_eq!(matching_paren("(x = (1, 2))"), Some(11)); + assert_eq!(matching_paren("(a, {b: [c]})"), Some(12)); + assert_eq!(matching_paren("(unbalanced"), None); + } + + #[test] + fn split_top_level_respects_nested_brackets() { + assert_eq!(split_top_level("a, b, c", ','), vec!["a", " b", " c"]); + assert_eq!(split_top_level("{a, b}, c", ','), vec!["{a, b}", " c"]); + assert_eq!(split_top_level("(a, b), c", ','), vec!["(a, b)", " c"]); + assert_eq!(split_top_level("", ','), vec![""]); + } + + #[test] + fn strip_default_cuts_at_top_level_equals_only() { + assert_eq!(strip_default("x"), "x"); + assert_eq!(strip_default("x = 1"), "x "); + assert_eq!(strip_default("x = (1, 2)"), "x "); + // Arrow body in the default value: only the first top-level `=` counts. + assert_eq!(strip_default("x = (y) => y"), "x "); + // Nested `=` inside a destructuring default stays untouched. + assert_eq!(strip_default("{a = 1}"), "{a = 1}"); + } + + #[test] + fn parses_arrow_forms() { + assert_eq!(names("(x) => x"), vec!["x"]); + assert_eq!(names("(x, y) => x"), vec!["x", "y"]); + assert_eq!(names("() => 42"), Vec::::new()); + assert_eq!(names("async (x) => x"), vec!["x"]); + assert_eq!(names("x => x"), vec!["x"]); + } + + #[test] + fn parses_function_forms() { + assert_eq!(names("function named(x, y) { return x; }"), vec!["x", "y"]); + assert_eq!(names("async function n(a) { return a; }"), vec!["a"]); + assert_eq!(names("function (x) { return x; }"), vec!["x"]); + assert_eq!(names("function* gen(a, b) {}"), vec!["a", "b"]); + } + + #[test] + fn handles_defaults_rest_and_destructuring() { + assert_eq!(names("(x = 1) => x"), vec!["x"]); + assert_eq!(names("(...args) => args"), vec!["args"]); + assert_eq!(names("({ a, b }) => a"), vec!["a", "b"]); + assert_eq!(names("([a, b]) => a"), vec!["a", "b"]); + // Default value with nested parens — no foot-gun on lazy regex. + assert_eq!(names("(x = f(1, 2)) => x"), vec!["x"]); + } } diff --git a/rust/cube/cubenativeutils/src/wrappers/object_handle.rs b/rust/cube/cubenativeutils/src/wrappers/object_handle.rs index 4068750655102..e367c789260ba 100644 --- a/rust/cube/cubenativeutils/src/wrappers/object_handle.rs +++ b/rust/cube/cubenativeutils/src/wrappers/object_handle.rs @@ -1,5 +1,8 @@ use super::{inner_types::InnerTypes, object::NativeObject}; -use super::{NativeContextHolder, NativeContextHolderRef, NativeString, NativeStruct}; +use super::{ + NativeBoolean, NativeContextHolder, NativeContextHolderRef, NativeNumber, NativeString, + NativeStruct, +}; use crate::CubeError; #[derive(Clone)] @@ -68,15 +71,23 @@ impl NativeObjectHandle { pub fn convert_to_string(&self) -> Result { if let Ok(str) = self.to_string() { - str.value() - } else if self.is_null()? { - Ok("".to_string()) - } else { - self.to_struct()? - .call_method("toString", vec![])? - .into_string()? - .value() + return str.value(); + } + if self.is_null()? { + return Ok("".to_string()); + } + // Primitives don't expose a struct-side `toString`, so coerce them + // inline before the struct fallback below — otherwise it errors. + if let Ok(n) = self.to_number() { + return Ok(n.value()?.to_string()); + } + if let Ok(b) = self.to_boolean() { + return Ok(b.value()?.to_string()); } + self.to_struct()? + .call_method("toString", vec![])? + .into_string()? + .value() } pub fn try_clone_to_context_ref( diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs index 1e9b95505f759..f913103fff43a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs @@ -383,6 +383,23 @@ impl NativeMemberSql { } } + fn coerce_scalar_to_string( + handle: NativeObjectHandle, + ) -> Result { + if let Ok(s) = String::from_native(handle.clone()) { + return Ok(s); + } + if let Ok(n) = f64::from_native(handle.clone()) { + return Ok(n.to_string()); + } + if let Ok(b) = bool::from_native(handle.clone()) { + return Ok(b.to_string()); + } + Err(CubeError::user( + "Invalid param for security context".to_string(), + )) + } + fn security_context_filter_fn( context_holder: NativeContextHolder, property_value: NativeObjectHandle, @@ -394,20 +411,37 @@ impl NativeMemberSql { StringVec(Vec), None, } - let param_value = if let Ok(prop_vec) = Vec::::from_native(property_value.clone()) { - ParamValue::StringVec(prop_vec) + // Falsy scalars (undefined / null / "" / 0 / NaN / false) collapse to + // `ParamValue::None` and emit `1 = 1`. Empty arrays stay as an empty + // `StringVec` and emit `1 = 0` separately below — `IN ()` is invalid + // SQL in most dialects. + let param_value = if property_value.is_undefined()? || property_value.is_null()? { + ParamValue::None + } else if let Ok(arr) = property_value.to_array() { + let values = arr + .to_vec()? + .into_iter() + .map(Self::coerce_scalar_to_string) + .collect::, _>>()?; + ParamValue::StringVec(values) } else if let Ok(prop) = String::from_native(property_value.clone()) { - ParamValue::String(prop) + if prop.is_empty() { + ParamValue::None + } else { + ParamValue::String(prop) + } } else if let Ok(prop) = f64::from_native(property_value.clone()) { - if prop.fract() == 0.0 && prop.is_finite() { - ParamValue::String(format!("{}", prop as i64)) + if prop == 0.0 || prop.is_nan() { + ParamValue::None } else { ParamValue::String(prop.to_string()) } } else if let Ok(prop) = bool::from_native(property_value.clone()) { - ParamValue::String(prop.to_string()) - } else if property_value.is_undefined()? || property_value.is_null()? { - ParamValue::None + if prop { + ParamValue::String("true".to_string()) + } else { + ParamValue::None + } } else { return Err(CubeError::user( "Invalid param for security context".to_string(), @@ -500,30 +534,40 @@ impl NativeMemberSql { property_value: NativeObjectHandle, proxy_state: ProxyStateWeak, ) -> Result, CubeError> { - let str_value = if let Ok(prop_vec) = Vec::::from_native(property_value.clone()) { - Some(prop_vec) - } else if let Ok(prop) = String::from_native(property_value.clone()) { - Some(vec![prop]) - } else if let Ok(prop) = f64::from_native(property_value.clone()) { - if prop.fract() == 0.0 && prop.is_finite() { - Some(vec![format!("{}", prop as i64)]) - } else { + // Type extraction is read-only and runs eagerly. Placeholder + // allocation happens lazily inside the returned function so it only + // fires when the proxy is actually coerced via `${...}` — otherwise + // every property access would register a placeholder. + let str_value: Option> = + if property_value.is_undefined()? || property_value.is_null()? { + None + } else if let Ok(arr) = property_value.to_array() { + let elements = arr.to_vec()?; + let mut values = Vec::with_capacity(elements.len()); + for el in elements { + values.push(Self::coerce_scalar_to_string(el)?); + } + Some(values) + } else if let Ok(prop) = String::from_native(property_value.clone()) { + Some(vec![prop]) + } else if let Ok(prop) = f64::from_native(property_value.clone()) { Some(vec![prop.to_string()]) - } - } else if let Ok(prop) = bool::from_native(property_value.clone()) { - Some(vec![prop.to_string()]) - } else { - None - }; - let allocated = match str_value { - Some(values) => values - .iter() - .map(|v| Self::process_secutity_context_value(&proxy_state, v)) - .collect::, _>>()? - .join(", "), - None => String::new(), - }; - let result = context_holder.to_string_fn(allocated)?; + } else if let Ok(prop) = bool::from_native(property_value.clone()) { + Some(vec![prop.to_string()]) + } else { + None + }; + let result = + context_holder.make_vararg_function(move |_, _| -> Result { + match &str_value { + Some(values) => Ok(values + .iter() + .map(|v| Self::process_secutity_context_value(&proxy_state, v)) + .collect::, _>>()? + .join(",")), + None => Ok(String::new()), + } + })?; Ok(NativeObjectHandle::new(result.into_object())) }