Skip to content

Commit 89709af

Browse files
authored
fix(model): override $supportsAdvisoryLocks=false on CockroachDB so lockingSpec skips (#2746)
* fix(model): override $supportsAdvisoryLocks=false on CockroachDB so lockingSpec skips CockroachDBModel extends PostgreSQLModel, which reports $supportsAdvisoryLocks()=true since pg_advisory_lock is native there. CockroachDB intentionally omits the primitive — its adapter throws "CockroachDB does not support advisory locks." from $acquireAdvisoryLock — but it never overrode the capability flag, so the four lockingSpec withAdvisoryLock tests bypassed the beforeEach skip-guard added in #2670 and reported as errors on every engine (lucee6, lucee7, boxlang). PR #2670's CHANGELOG entry already named CockroachDB in the "skips on" list, but the override itself was missed. This adds the one-method override, mirroring how MSSQL/SQLite/etc. each carry their own $supportsAdvisoryLocks override over Base.cfc's default-false. Fixes #2743 Signed-off-by: Peter Amiri <peter@alurium.com> * docs(model): trim redundant forUpdate hint from CockroachDB $supportsAdvisoryLocks docblock Reviewer A nit on #2746: the final sentence duplicates guidance already on the adjacent $acquireAdvisoryLock / $releaseAdvisoryLock docblocks. Keep the comment focused on the WHY (inheritance/capability-flag subtlety). Signed-off-by: Peter Amiri <peter@alurium.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com>
1 parent cb04f68 commit 89709af

2 files changed

Lines changed: 13 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo
3737

3838
### Fixed
3939

40+
- `CockroachDBModel` now overrides `$supportsAdvisoryLocks()` to return `false`, so the four `lockingSpec` `withAdvisoryLock` tests skip cleanly on CockroachDB instead of erroring with `CockroachDB does not support advisory locks.`. The PR that introduced the capability flag (#2670) claimed CockroachDB in its CHANGELOG entry but never added the override — CockroachDB inherits from `PostgreSQLModel`, which reports `true`, so the spec's `beforeEach` skip-guard never fired and the four specs proceeded to call `$acquireAdvisoryLock`, which the adapter throws from by design. Compat-matrix legs `lucee6/cockroachdb`, `lucee7/cockroachdb`, and `boxlang/cockroachdb` now report 4 skips where they previously reported 4 errors. No spec changes needed — the capability-flag layer added in #2670 already does the right thing once the flag is correct (#2743)
4041
- `wheels.middleware.Cors` now short-circuits unmatched `OPTIONS` preflight requests at the dispatch layer, preserving the legacy `set(allowCorsRequests=true)` contract under the new middleware pipeline. Previously, `$findMatchingRoute()` ran before middleware, so a preflight against a path that only declared `POST` (or any non-`OPTIONS` verb) 404'd with `Wheels.RouteNotFound` before the CORS middleware's preflight branch could fire — leaving the middleware strictly less capable than the 3.x global setting it was meant to replace and breaking cross-origin `POST`/`PUT`/`PATCH`/`DELETE` from configured browsers. `Dispatch.$request()` now checks for an `OPTIONS` verb plus a `wheels.middleware.Cors` instance in the global pipeline and, if both are present, runs the pipeline against a no-op core handler before route matching. Dispatch behavior for `OPTIONS` without CORS middleware (still 404s) and for non-`OPTIONS` verbs (still routed normally) is unchanged (#2703)
4142
- `paginationNav()` `showFirst` / `showLast` / `showPrevious` / `showNext` args now accept the tri-state strings `"auto"` / `"always"` / `"never"` (with backwards-compatible boolean coercion: `true` → `"always"`, `false` → `"never"`) and default to `"auto"`. Under `"auto"` the first/last anchors only render when the visible page-number window does not already reach the boundary — restoring the legacy 3.x `paginationLinks(alwaysShowAnchors=false)` semantics that a like-for-like swap to `paginationNav()` previously lost. Under `"auto"` the previous/next anchors always delegate to `previousPageLink()` / `nextPageLink()`, which render a disabled `<span class="disabled">` at the boundary by default — preserving the legacy `showPrevious=true` / `showNext=true` boundary indicator unless callers opt out with `"never"`. Adds a `windowSize` arg on `paginationNav()` so the auto-mode predicates stay coherent with `pageNumberLinks()`'s window (now passed explicitly to `pageNumberLinks()` instead of leaking through the anchor sub-helpers). Invalid strings throw `Wheels.InvalidArgument` at the call site
4243
- `QueryBuilder.whereIn()` / `whereNotIn()` with an empty array no longer emit malformed SQL (`property IN ()`). Previously, passing an empty list or array to either method produced syntactically invalid SQL that surfaced as a generic JDBC syntax error from the database, with no pointer back to the call site that built the empty collection. `whereIn(prop, [])` now sets an `$alwaysEmpty` flag on the builder so every terminal method (`count`, `findAll`, `findOne`, `first`, `exists`, `updateAll`, `deleteAll`, `findEach`, `findInBatches`) short-circuits to the appropriate zero-row sentinel before going through the finder. `whereNotIn(prop, [])` is a no-op (exclude-none = match-all), so the chain proceeds normally. Matches the user-facing behaviour every mature ORM converged on (Rails, Sequel, Django, Laravel Eloquent: empty `IN` matches no rows, empty `NOT IN` matches every row). The flag-based design avoids a runtime trap from Wheels' WHERE-clause parser (`vendor/wheels/model/sql.cfc` runs a property-extraction regex over every clause it sees — a raw `1 = 0` literal would be parsed as property `1` and trip `Wheels.ColumnNotFound`). Fourteen new specs in `vendor/wheels/tests/specs/model/queryBuilderSpec.cfc` cover empty-array, empty-list, composition with other clauses, the `whereNotIn` mirrors, every patched terminal (`findAll`, `first` / `findOne`, `exists`, `count`, `updateAll`, `deleteAll`, `findEach`, `findInBatches`), and the documented `select()` / `include()` silent-ignore caveat on the short-circuit path. Both copies of the query-builder guide were updated to document the short-circuit in the methods table (#2736)

vendor/wheels/databaseAdapters/CockroachDB/CockroachDBModel.cfc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@ component extends="wheels.databaseAdapters.PostgreSQL.PostgreSQLModel" output=fa
3838
return local.rv;
3939
}
4040

41+
/**
42+
* CockroachDB intentionally omits the PostgreSQL advisory-lock primitives
43+
* (`pg_advisory_lock` / `pg_advisory_unlock`) — it surfaces them as no-op
44+
* stubs that error rather than honoring the contract. Override the
45+
* PostgreSQL adapter's `true` and report unsupported so `withAdvisoryLock`
46+
* callers (and the capability-aware lockingSpec `beforeEach`) skip
47+
* standalone-lock paths instead of erroring.
48+
*/
49+
public boolean function $supportsAdvisoryLocks() {
50+
return false;
51+
}
52+
4153
/**
4254
* CockroachDB does not support advisory locks.
4355
* Use forUpdate() for row-level locking instead.

0 commit comments

Comments
 (0)