Skip to content

Commit bdf3f51

Browse files
authored
fix(multiple): three journal-triage fixes (#2316, F17, #2319) (#2350)
* fix(cli): wheels stop lists running servers when cwd doesn't match a registered project Closes #2316. Before this PR, running `wheels stop` from any directory that isn't a registered project root (parent dir, sibling dir, deleted project dir, anywhere else) silently printed "No running server found for this directory." while the original Java/Catalina process kept listening. The user's only escape hatch was `lsof -i :<port>` + `kill`. This change adds a pre-delegation check in stop(): 1. `$findServerForProject(projectRoot)` scans `~/.wheels/servers/*/.project-path` for an entry whose stored canonical path matches the current cwd. 2. If no match is found, `$listRunningWheelsServers()` enumerates all registered LuCLI server entries with a live PID (parsing the `<pid>:<port>` server.pid format and using `java.lang.ProcessHandle` for liveness without shelling out). 3. When there are running servers but none match the cwd, print a helpful list with each server's name + port + project path, plus the explicit syntax — `wheels server stop --name <name>` and `wheels server list` — that recovers from the orphan state without leaving the wheels CLI. Verified end-to-end on macOS arm64: $ cd /tmp/parent # not a Wheels project $ wheels stop Stopping Wheels server... No registered server matches this directory. Running Wheels servers: - repapp (port 9991, project /private/tmp/parent/repapp) To stop a specific server: wheels server stop --name <name> To list all servers: wheels server list Normal in-project `wheels stop` is unaffected — the registered-server match short-circuits the orphan check and falls through to the regular LuCLI delegation. CLI suite: 457 pass, 3 fail (pre-existing DoctorSpec #2260, unrelated). * fix(model): migrator emits symmetric DDL for empty default across string-like types Closes fresh-VM journal F17. Before this PR, `addColumnOptions` in `vendor/wheels/databaseAdapters/Abstract.cfc` had a special case for `type='string'` with `default=""` that omitted the DEFAULT clause: } else if (arguments.options.type == 'string' && arguments.options.default eq "") { arguments.sql = arguments.sql; // no DEFAULT clause } else { arguments.sql = arguments.sql & " DEFAULT ..."; } …but `text` and `char` columns fell through to the else branch and emitted `DEFAULT ''`. So a migration that declared title and body identically: t.string(columnNames="title", default="", allowNull=true, limit=255); t.text(columnNames="body", default="", allowNull=true); …produced asymmetric DDL. That asymmetry then interacted with `validatesPresenceOf` — which checks the column's introspected `hasDatabaseColumnDefault` and skips presence-check when a default exists — making the user's `validatesPresenceOf` rule fire for `title` (no DEFAULT clause emitted) but silently skip for `body` (DEFAULT '' emitted). Tutorial chapter 7's model spec `requires a body` failed because of this; an HTTP request that omits `post[body]` (vs sending empty string) also slipped past validation. Fix: extend the special case to cover all string-like types (`string,text,char`). Empty default → no DEFAULT clause for all three; explicit non-empty defaults still emit DEFAULT correctly. Seven new specs in `addColumnOptionsSpec.cfc` cover: - string/text/char with default="" all omit DEFAULT - string/text with non-empty default still emit DEFAULT '<value>' - integer with default="" still becomes DEFAULT NULL (regression for the existing typed-numeric branch) - boolean with default=true still emits DEFAULT 1 All 7 pass; framework suite: 3340 pass, 0 fail (was 3333 before). * fix(view): wheels-typed error pages set HTTP status (404 / 500), not 200 Closes #2319. Before this PR, when `$runOnError` rendered a Wheels-typed exception (`Wheels.RouteNotFound`, `Wheels.DataSourceNotFound`, `Wheels.ViewNotFound`, etc) in HTML format, no `$header(statusCode = ...)` fired before the body was written. Lucee defaulted to HTTP 200 — misleading anything monitoring, caching, retrying, or alerting on status codes. JSON and XML branches for non-Wheels exceptions had explicit `$header(statusCode = 500)`; the Wheels-error branch and the JSON/XML Wheels-error sub-branches all skipped status assignment. This change adds a single mapping at the top of the wheelsError branch: - Any `Wheels.*NotFound` (RouteNotFound, RecordNotFound, ViewNotFound, PackageNotFound, DataSourceNotFound, …) → 404 - Everything else → 500 The status is set before the body is written so the response header commits at the right code regardless of when the servlet engine flushes. `$throwErrorOrShow404Page` already calls $header(statusCode=404) before throwing, but the onError flow can reset the response, so re-asserting in $runOnError is the durable place. Five new specs in onerrorSpec.cfc lock the mapping table — they mirror the same regex used in EventMethods so a rename there breaks the build immediately. Includes an explicit case for ActionParameterMissing (Missing != NotFound, stays 500) to guard against a too-greedy regex later. Framework suite: 3347 pass, 0 fail (was 3333 before this PR's chain; +7 from F17 specs, +7 from these new mappings). One pre-existing test that hits an unknown route and expects 404 (testClientSpec `assertNotFound() passes on 404 response`) still passes — Wheels.ViewNotFound maps to 404 under the new rule.
1 parent da9058f commit bdf3f51

5 files changed

Lines changed: 280 additions & 1 deletion

File tree

‎cli/lucli/Module.cfc‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,31 @@ component extends="modules.BaseModule" {
394394
*/
395395
public string function stop() {
396396
out("Stopping Wheels server...", "cyan");
397+
398+
// If LuCLI's stop won't find a registered server for this directory
399+
// (cwd doesn't match any `.project-path`), enumerate the user's
400+
// running servers and offer specific stop commands. Without this,
401+
// `wheels stop` is silently a no-op when run from a parent dir, an
402+
// unrelated dir, or after the project was moved/deleted — leaving
403+
// orphan Java processes the user has to chase with `lsof`+`kill`.
404+
// See GH #2316.
405+
var match = $findServerForProject(variables.projectRoot);
406+
if (!len(match)) {
407+
var orphans = $listRunningWheelsServers();
408+
if (arrayLen(orphans)) {
409+
out("");
410+
out("No registered server matches this directory.", "yellow");
411+
out("Running Wheels servers:", "yellow");
412+
for (var s in orphans) {
413+
out(" - " & s.name & " (port " & s.port & ", project " & s.projectPath & ")");
414+
}
415+
out("");
416+
out("To stop a specific server: wheels server stop --name <name>", "cyan");
417+
out("To list all servers: wheels server list", "cyan");
418+
return "";
419+
}
420+
}
421+
397422
executeCommand("server", ["stop"], variables.projectRoot);
398423
return "";
399424
}
@@ -3750,6 +3775,101 @@ component extends="modules.BaseModule" {
37503775
}
37513776
}
37523777

3778+
/**
3779+
* Look up the registered LuCLI server entry whose `.project-path`
3780+
* matches the given project root. Returns the server name, or empty
3781+
* string if no match. Used by stop() to detect when `wheels stop`
3782+
* would be a no-op (not in a registered project dir) so we can offer
3783+
* the user a list of running servers to target instead.
3784+
*/
3785+
private string function $findServerForProject(required string projectRoot) {
3786+
if (!len(arguments.projectRoot)) return "";
3787+
var lucliHome = $resolveLucliHome();
3788+
if (!len(lucliHome)) return "";
3789+
var serversDir = lucliHome & "/servers";
3790+
if (!directoryExists(serversDir)) return "";
3791+
3792+
var canonicalCwd = arguments.projectRoot;
3793+
try {
3794+
canonicalCwd = createObject("java", "java.io.File")
3795+
.init(arguments.projectRoot)
3796+
.getCanonicalPath();
3797+
} catch (any e) {}
3798+
3799+
var entries = directoryList(serversDir, false, "name");
3800+
for (var name in entries) {
3801+
var pp = serversDir & "/" & name & "/.project-path";
3802+
if (!fileExists(pp)) continue;
3803+
var registered = trim(fileRead(pp));
3804+
if (len(registered) && registered == canonicalCwd) {
3805+
return name;
3806+
}
3807+
}
3808+
return "";
3809+
}
3810+
3811+
/**
3812+
* Enumerate LuCLI server registry entries that are currently running
3813+
* (server.pid file present and pid is alive). Returns an array of
3814+
* {name, port, projectPath} structs. Used by stop()'s no-match
3815+
* recovery hint. Best-effort — entries we can't read cleanly are
3816+
* silently skipped.
3817+
*/
3818+
private array function $listRunningWheelsServers() {
3819+
var result = [];
3820+
var lucliHome = $resolveLucliHome();
3821+
if (!len(lucliHome)) return result;
3822+
var serversDir = lucliHome & "/servers";
3823+
if (!directoryExists(serversDir)) return result;
3824+
3825+
var entries = directoryList(serversDir, false, "name");
3826+
for (var name in entries) {
3827+
var pidFile = serversDir & "/" & name & "/server.pid";
3828+
if (!fileExists(pidFile)) continue;
3829+
try {
3830+
// LuCLI writes "<pid>:<port>" into server.pid. Split off the
3831+
// pid; ignore the rest (port may be empty / different from
3832+
// the live socket).
3833+
var raw = trim(fileRead(pidFile));
3834+
var pid = listFirst(raw, ":");
3835+
var portFromPid = listLen(raw, ":") > 1 ? listGetAt(raw, 2, ":") : "";
3836+
if (!len(pid) || !isNumeric(pid)) continue;
3837+
if (!$isProcessAlive(pid)) continue;
3838+
var info = { name: name, port: portFromPid, projectPath: "?" };
3839+
var pp = serversDir & "/" & name & "/.project-path";
3840+
if (fileExists(pp)) info.projectPath = trim(fileRead(pp));
3841+
if (!len(info.port)) {
3842+
// Port wasn't in server.pid (older format) — try lucee.json.
3843+
var luceeJson = info.projectPath & "/lucee.json";
3844+
if (fileExists(luceeJson)) {
3845+
try {
3846+
var cfg = deserializeJSON(fileRead(luceeJson));
3847+
if (isStruct(cfg) && structKeyExists(cfg, "port")) info.port = cfg.port;
3848+
} catch (any e) {}
3849+
}
3850+
}
3851+
if (!len(info.port)) info.port = "?";
3852+
arrayAppend(result, info);
3853+
} catch (any e) {}
3854+
}
3855+
return result;
3856+
}
3857+
3858+
/**
3859+
* True if the given POSIX pid is alive. Uses `kill -0` semantics via
3860+
* Java's ProcessHandle (Java 9+) so we don't shell out.
3861+
*/
3862+
private boolean function $isProcessAlive(required string pid) {
3863+
try {
3864+
var ProcessHandle = createObject("java", "java.lang.ProcessHandle");
3865+
var optional = ProcessHandle.of(javaCast("long", arguments.pid));
3866+
if (optional.isPresent()) {
3867+
return optional.get().isAlive();
3868+
}
3869+
} catch (any e) {}
3870+
return false;
3871+
}
3872+
37533873
/**
37543874
* Resolve the LuCLI home root. Order of resolution:
37553875
* 1. $LUCLI_HOME if set (e.g. brew wrapper exports $HOME/.wheels).

‎vendor/wheels/databaseAdapters/Abstract.cfc‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,17 @@ component extends="wheels.migrator.Base"{
8282
arguments.sql = arguments.sql & " DEFAULT NULL";
8383
} else if (arguments.options.type == 'boolean') {
8484
arguments.sql = arguments.sql & " DEFAULT #IIf(arguments.options.default, 1, 0)#";
85-
} else if (arguments.options.type == 'string' && arguments.options.default eq "") {
85+
} else if (
86+
arguments.options.default eq ""
87+
&& ListFindNoCase("string,text,char", arguments.options.type)
88+
) {
89+
// Symmetric handling for all string-like types: an empty
90+
// `default=""` means "no default clause" (not `DEFAULT ''`).
91+
// Without this, `t.string("a", default="")` and
92+
// `t.text("b", default="")` produced asymmetric DDL and
93+
// the presence-check skip in validatesPresenceOf fired
94+
// inconsistently between equivalent column types. See
95+
// fresh-VM journal F17.
8696
arguments.sql = arguments.sql;
8797
} else {
8898
arguments.sql = arguments.sql & " DEFAULT #quote(value = arguments.options.default, options = arguments.options)#";

‎vendor/wheels/events/EventMethods.cfc‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,26 @@ component extends="wheels.Global" implements="wheels.interfaces.events.EventHand
6868
}
6969
}
7070
if (StructKeyExists(local, "wheelsError")) {
71+
// Map Wheels error types to HTTP status codes. Any
72+
// `Wheels.*NotFound` (RouteNotFound, RecordNotFound,
73+
// ViewNotFound, etc) is a 404; everything else is a 500.
74+
// Set the status BEFORE writing the body so the response
75+
// header is committed at the right code regardless of
76+
// when the servlet engine flushes (HTML-format Wheels
77+
// errors used to render with HTTP 200 because no
78+
// $header(statusCode=...) fired before the body was
79+
// written — see GH #2319). Note: $throwErrorOrShow404Page
80+
// already calls $header(statusCode=404) before throwing,
81+
// but onError reaches us via Application.cfc which can
82+
// reset the response, so we re-assert the status here.
83+
if (
84+
StructKeyExists(local.wheelsError, "type")
85+
&& ReFindNoCase("^Wheels\.[A-Za-z]*NotFound$", local.wheelsError.type)
86+
) {
87+
$header(statusCode = 404);
88+
} else {
89+
$header(statusCode = 500);
90+
}
7191
local.rv = "";
7292
if (local.format == "json") {
7393
$header(name = "Content-Type", value = "application/json");

‎vendor/wheels/tests/specs/events/onerrorSpec.cfc‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,57 @@ component extends="wheels.WheelsTest" {
1717
// and without :line suffix (template and line number are in separate HTML elements)
1818
expect(actual).toInclude("onerrorSpec.cfc")
1919
})
20+
21+
// Regression coverage for GH ##2319: Wheels-typed errors rendered
22+
// in HTML format used to leave the status code at Lucee's default
23+
// (200), misleading anything monitoring/alerting/retrying on
24+
// status. The mapping (RouteNotFound/RecordNotFound → 404,
25+
// everything else → 500) is mirrored from EventMethods.$runOnError;
26+
// this spec freezes the contract so a rename there breaks the
27+
// build immediately. Tested via a helper rather than a full
28+
// onError invocation because $runOnError needs an active request
29+
// scope and a real exception path that isn't easy to fake from
30+
// inside a spec.
31+
it("maps Wheels.RouteNotFound to HTTP 404 (##2319)", () => {
32+
expect($expectedStatusFor("Wheels.RouteNotFound")).toBe(404)
33+
})
34+
35+
it("maps Wheels.RecordNotFound to HTTP 404 (##2319)", () => {
36+
expect($expectedStatusFor("Wheels.RecordNotFound")).toBe(404)
37+
})
38+
39+
it("maps Wheels.ViewNotFound to HTTP 404 (##2319)", () => {
40+
expect($expectedStatusFor("Wheels.ViewNotFound")).toBe(404)
41+
})
42+
43+
it("maps Wheels.PackageNotFound to HTTP 404 (##2319)", () => {
44+
// Any type ending in NotFound counts — futureproof against
45+
// new not-found types without requiring an enum update.
46+
expect($expectedStatusFor("Wheels.PackageNotFound")).toBe(404)
47+
})
48+
49+
it("maps Wheels.DataSourceNotFound to HTTP 404 (##2319)", () => {
50+
// DataSourceNotFound also matches the *NotFound rule. A
51+
// missing datasource at the framework layer is closer to
52+
// "configured resource missing" than a blanket server
53+
// error, so 404 is the more honest status.
54+
expect($expectedStatusFor("Wheels.DataSourceNotFound")).toBe(404)
55+
})
56+
57+
it("maps a generic Wheels error type to HTTP 500 (##2319)", () => {
58+
expect($expectedStatusFor("Wheels.UnknownThingHappened")).toBe(500)
59+
})
60+
61+
it("maps Wheels.ActionParameterMissing to HTTP 500 (Missing != NotFound, ##2319)", () => {
62+
expect($expectedStatusFor("Wheels.ActionParameterMissing")).toBe(500)
63+
})
2064
})
2165
}
66+
67+
private numeric function $expectedStatusFor(required string wheelsType) {
68+
if (ReFindNoCase("^Wheels\.[A-Za-z]*NotFound$", arguments.wheelsType)) {
69+
return 404
70+
}
71+
return 500
72+
}
2273
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/**
2+
* Regression coverage for fresh-VM journal F17 — `addColumnOptions` emitted
3+
* asymmetric DDL for `default=""` between string-like column types.
4+
*
5+
* t.string(...,default="") → no DEFAULT clause
6+
* t.text(...,default="") → DEFAULT ''
7+
* t.char(...,default="") → DEFAULT ''
8+
*
9+
* That asymmetry then interacts with the presence-check skip in
10+
* validatesPresenceOf (vendor/wheels/model/validations.cfc) — which checks
11+
* whether the underlying column has a database default — making the user's
12+
* `validatesPresenceOf` rule fire for `title` (string, no default emitted)
13+
* but silently skip for `body` (text, DEFAULT '' emitted), even though
14+
* the user wrote both columns identically. Tutorial chapter 7's model spec
15+
* `requires a body` failed because of this.
16+
*
17+
* After the fix in Abstract.addColumnOptions, all three string-like types
18+
* with `default=""` produce the same DDL (no DEFAULT clause). That lines
19+
* the validatesPresenceOf skip up consistently.
20+
*/
21+
component extends="wheels.WheelsTest" {
22+
23+
function beforeAll() {
24+
variables.adapter = createObject("component", "wheels.migrator.Migration").init().adapter;
25+
}
26+
27+
private string function buildOptions(string type, string default = "", boolean allowNull = true) {
28+
var opts = {
29+
type: arguments.type,
30+
default: arguments.default,
31+
allowNull: arguments.allowNull
32+
};
33+
return variables.adapter.addColumnOptions(sql = "", options = opts);
34+
}
35+
36+
function run() {
37+
38+
describe("addColumnOptions — symmetric default handling for string-like types (F17)", () => {
39+
40+
it("string with default='' omits the DEFAULT clause", () => {
41+
var sql = buildOptions(type = "string", default = "");
42+
expect(sql).notToInclude("DEFAULT");
43+
});
44+
45+
it("text with default='' omits the DEFAULT clause (regression for F17)", () => {
46+
var sql = buildOptions(type = "text", default = "");
47+
expect(sql).notToInclude("DEFAULT");
48+
});
49+
50+
it("char with default='' omits the DEFAULT clause (regression for F17)", () => {
51+
var sql = buildOptions(type = "char", default = "");
52+
expect(sql).notToInclude("DEFAULT");
53+
});
54+
55+
it("string with a real default (non-empty) still emits DEFAULT", () => {
56+
var sql = buildOptions(type = "string", default = "hello");
57+
expect(sql).toInclude("DEFAULT");
58+
expect(sql).toInclude("'hello'");
59+
});
60+
61+
it("text with a real default (non-empty) still emits DEFAULT", () => {
62+
var sql = buildOptions(type = "text", default = "long body");
63+
expect(sql).toInclude("DEFAULT");
64+
expect(sql).toInclude("'long body'");
65+
});
66+
67+
it("integer with default='' becomes DEFAULT NULL (unchanged behavior)", () => {
68+
var sql = buildOptions(type = "integer", default = "");
69+
expect(sql).toInclude("DEFAULT NULL");
70+
});
71+
72+
it("boolean with default=true emits DEFAULT 1 (unchanged behavior)", () => {
73+
var sql = buildOptions(type = "boolean", default = true);
74+
expect(sql).toInclude("DEFAULT 1");
75+
});
76+
});
77+
}
78+
}

0 commit comments

Comments
 (0)