Skip to content

fix(model): evaluate positional args in this.method() validation conditions (#3238) - #3239

Merged
bpamiri merged 1 commit into
developfrom
peter/fix-3238-positional-condition-args
Jun 22, 2026
Merged

fix(model): evaluate positional args in this.method() validation conditions (#3238)#3239
bpamiri merged 1 commit into
developfrom
peter/fix-3238-positional-condition-args

Conversation

@bpamiri

@bpamiri bpamiri commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #3238. A custom validation condition/unless expression that calls a model method with a positional argument threw instead of evaluating:

validate(method="_validateSomething", condition="this.propertyIsPresent('productid')");
// → The parameter [property] to function [propertyIsPresent] is required but was not passed in

Root cause

vendor/wheels/model/validations.cfc::$resolveThisReference() parses this.propertyIsPresent('productid') into methodName="propertyIsPresent" / argsRaw="'productid'", then calls invoke(this, methodName, $parseConditionArgs(argsRaw)). $parseConditionArgs() only handled named arguments — it guarded on Find("=", param), so a bare positional argument like 'productid' was silently dropped and invoke(this, "propertyIsPresent", {}) threw because property is required.

Fix

  • $parseConditionArgs() now also collects positional arguments and maps them onto the target function's declared parameter names (via a new $conditionFunctionParameterNames() helper using GetMetaData()), producing a single named-argument struct.
  • Named-argument invoke() resolves uniformly across Lucee/Adobe/BoxLang; a numeric-keyed positional argumentCollection does not — hence the name-mapping rather than passing positionals straight through.
  • Existing named-arg conditions (this.stupid_mixin(b='1', a='2')) are preserved; quote-stripping now goes through $unquoteConditionValue() (strips only surrounding quotes) so inner apostrophes survive.
  • Updated the Wheels.InvalidValidationCondition extendedInfo "Supported forms…" message to note method calls now accept named or positional arguments.

A pre-existing limitation is left documented in a code comment: a quoted value containing a comma ('a,b') is still split on the comma — this matched the prior named-arg behaviour and is out of scope here.

Tests

Added 4 specs to validationsSpec.cfc under Tests conditional validations:

  • $evaluateConditionString with a single positional arg (true/false)
  • $evaluateConditionString with multiple positional args
  • if validation runs when a positional-arg method condition is true
  • if validation is skipped when a positional-arg method condition is false

All 4 fail on develop with the exact reported error, pass with this change.

Verification

  • bash tools/test-local.sh model921 pass (917 + 4 new), 0 failures.
  • tools/test-matrix.sh lucee7 mysql — my 4 specs pass (the one unrelated updateAll with multiple includes failure is pre-existing on develop).
  • tools/test-matrix.sh adobe2023 mysql — my 4 specs pass (confirms the GetMetaData().parameters + named-arg invoke() approach works on Adobe CF — the cross-engine risk flagged in triage). The 3 unrelated failures (float() defaults, updateAll with includes, verb-mismatch HTML encoding) were confirmed pre-existing by re-running the same engine against a stashed (clean-develop) tree: 4554 pass clean vs 4558 with this change = exactly the 4 new tests, same 3 pre-existing failures.

🤖 Generated with Claude Code

…itions

A custom validation `condition`/`unless` expression of the form
`this.propertyIsPresent('productid')` threw `The parameter [property] to
function [propertyIsPresent] is required but was not passed in`.

`$parseConditionArgs()` only understood named arguments (`key='val'`),
guarding on `Find("=", param)`, so a bare positional argument was silently
dropped and the target method was invoked with an empty struct.

Teach the parser to also collect positional arguments and map them onto the
target function's declared parameter names (via GetMetaData), producing a
single named-argument struct — named-arg invoke() resolves uniformly across
Lucee/Adobe/BoxLang where a numeric-keyed positional argumentCollection does
not. Verified on Lucee 7 + SQLite/MySQL and Adobe CF 2023 + MySQL.

Fixes #3238

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR — This PR fixes #3238: a validation condition/unless expression calling a model method with a positional argument (e.g. this.propertyIsPresent('productid')) previously threw because $parseConditionArgs() only understood named args and silently dropped positionals. The fix collects positional arguments and maps them onto the target function's declared parameter names via GetMetaData(), producing a uniform named-argument struct for invoke(). The change is small, correctly scoped, well-tested, and documented. Verdict: approve.

Cross-engine

No blocking issue. The new $conditionFunctionParameterNames() reads GetMetaData(targetFunction).parameters, which the PR verified on Lucee 7 and Adobe CF 2023. The access is defensively guarded:

if (StructKeyExists(local.meta, "parameters") && IsArray(local.meta.parameters)) {

so on any engine where function metadata lacks parameters (e.g. a BoxLang shape difference), positional mapping is simply skipped — the behaviour degrades to the prior empty-struct path rather than regressing. Both new helpers are correctly declared public with the $ prefix (Cross-Engine Invariant 7 — private mixin helpers in vendor/wheels/model/*.cfc are not integrated), and the new targetFunction parameter name avoids the reserved-scope shadowing trap (Anti-Pattern 11). Named-arg invoke() is the right call over a numeric-keyed positional argumentCollection, as the code comment explains.

Tests

Coverage is solid. vendor/wheels/tests/specs/model/validationsSpec.cfc adds four BDD specs extending wheels.WheelsTest:

  • single positional arg, true and false branches (propertyIsPresent('username') / 'nonexistent')
  • multiple positional args (stupid_mixin('1', '2') eq 3)
  • if validation runs when the positional-arg condition is true
  • if validation is skipped when it is false

The fixture wiring checks out: the conditional-validations beforeEach sets user.username = "TheLongestNameInTheWorld" with validatesLengthOf(maximum=5), so a true condition yields an invalid record and a false condition is skipped — the assertions match. The pre-existing named-arg specs (this.stupid_mixin(b='1', a='2')) remain and still pass, confirming no regression to the named path.

Docs / Commits

Changelog fragment changelog.d/3238-condition-positional-args.fixed.md follows the <slug>.<type>.md convention (no direct CHANGELOG.md edit). The Wheels.InvalidValidationCondition extendedInfo message was updated to advertise positional args. Commit header fix(model): evaluate positional args in this.method() validation conditions is a conforming conventional-commit header under 100 chars with a DCO Signed-off-by trailer.

Minor (non-blocking): the PR description says quote-stripping "strips only surrounding quotes so inner apostrophes survive," but $unquoteConditionValue() (pre-existing, validations.cfc:1013) does Replace(rv, "'", "", "all") once it confirms the value is quote-wrapped — so inner apostrophes in a '...'-wrapped value are also removed. Not a code defect and out of this PR's scope; just flagging the description nuance.

@bpamiri
bpamiri merged commit a748630 into develop Jun 22, 2026
10 checks passed
@bpamiri
bpamiri deleted the peter/fix-3238-positional-condition-args branch June 22, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

model validate method cannot evaluate this.propertyIsPresent in a condition argument

1 participant