Skip to content

Fix element booting, holder writes, db path names and invalid route expressions - #672

Merged
daftspunk merged 5 commits into
octobercms:developfrom
samuelpatro:fix/halcyon-router-element-correctness
Sep 26, 2026
Merged

daftspunk merged 5 commits into
octobercms:developfrom
samuelpatro:fix/halcyon-router-element-correctness

Conversation

@samuelpatro

@samuelpatro samuelpatro commented Aug 30, 2026 •

Copy link
Copy Markdown
Member

Four unrelated correctness bugs. Each has a test that fails on develop.

  • ElementBase::__construct() never called parent::__construct(), so ::extend() callbacks and $implement behaviors were skipped for constructed elements but applied to unserialized ones. Extensions now boot after the config is applied, as they do in __wakeup(). Methods they add are now called instead of being stored as config by the fluent setter.
  • ElementHolder::get() kept returning the first value it read, so a later $holder['key'] = 'new' was never seen. It now reads the config each time and still records touched elements.
  • DbDatasource::pathToFileName() used str_replace(), which removed every occurrence of the directory name: pages/pages/about.htm became about.htm and pages/blog/pages.htm became blog/.htm. It now strips only the leading directory.
  • A route segment with an invalid expression, such as :id|[unclosed, matched any value. The ErrorException Laravel raises for the preg_match warning was swallowed and the segment counted as valid. Router and Rule now treat it as a non-match.

…d route expressions

ElementBase never called Extendable::__construct, so extend() callbacks and $implement were ignored on construction but applied on unserialize. ElementHolder::get served a value cached by an earlier read after offsetSet had replaced it. DbDatasource::pathToFileName removed every occurrence of the directory name instead of the leading one. Router and Rule accepted a URL when a parameter expression threw instead of rejecting it.
@samuelpatro
samuelpatro marked this pull request as ready for review August 30, 2026 11:29
Extensions now see the configured element, matching unserialized
elements. Unset holder entries stay touched as before. Tests go through
public APIs and cover each fix.
@samuelpatro

samuelpatro commented Sep 26, 2026 •

Copy link
Copy Markdown
Member Author

@daftspunk rechecked this against current develop and changed a few things:

  • ElementBase now boots extensions after initDefaultValues() and useConfig(), so ::extend() callbacks see the configured element, the same as after __wakeup(). Constructing a FieldDefinition goes from 0.92 to 1.13 µs.
  • Methods added by behaviors or extend() callbacks are now called, instead of being stored as config by the fluent __call() setter.
  • DbDatasource::pathToFileName() trims a trailing slash from the directory. buildDirectoryQuery() already accepts select('pages/'), and the previous version returned pages/about.htm for it instead of about.htm.
  • Dropped the ElementHolder::offsetUnset() override, so unset entries stay touched and Form still resets their widgets as before.
  • Tests go through select() instead of calling pathToFileName() directly, and the two router tests are now one. Each fix has a test that fails without it.

Ready for another pass.

Methods added by behaviors or extend() callbacks were swallowed by the
fluent config setter, so booting them had no visible effect.
@daftspunk
daftspunk merged commit 883b7d1 into octobercms:develop Sep 26, 2026
4 checks passed
@daftspunk

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants