Let callbacks resolve against something other than the controller#17
Open
NielBuys wants to merge 2 commits into
Open
Let callbacks resolve against something other than the controller#17NielBuys wants to merge 2 commits into
NielBuys wants to merge 2 commits into
Conversation
$CI was doing two unrelated jobs: it is the handle for lang, uri, router
and input, and it was also the only place a callback_ rule could be
looked up. Code wanting callbacks on anything other than the controller
- a model, a service - had to overwrite $CI outright, which handed away
the services too and made the callback target depend on whoever assigned
last. Because the library is shared for the whole request and the
assignment typically happens in a constructor, loading two such objects
left the first one's callbacks resolving to "unable to find callback
validation rule". The rule never ran, and the field failed with
Unable to access an error message corresponding to your field name X.
which reads like a missing language line rather than a missing callback.
Adds a separate _callback_object with set_callback_object() to set it and
callback_object() to read it, consulted only where callbacks are
resolved. It defaults to NULL, meaning the CodeIgniter instance, so every
existing caller behaves exactly as before and nothing needs to change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fifteen tests fail in a Windows clone with core.autocrlf=true and pass everywhere else: thirteen in Form_helper_test, one in Html_helper_test, one in Calendar_test. They are not PHP version failures. Each compares helper output against a literal multi-line string written in the test file. The helpers build their output from "\n" escapes, so they always emit LF; the expected value picks up whatever line ending the file on disk happens to have. A CRLF checkout therefore compares a CRLF expectation against LF output. .gitattributes only carried export-ignore rules, so nothing told git how to treat line endings and core.autocrlf won by default. Repository content is unchanged - git add --renormalize touched no file but this one, confirming the blobs were already LF and only the checkout differed. Nothing downstream is affected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent changes, both additive.
1. Separate the callback target from the
$CIreferenceCI_Form_validation::$CIwas doing two unrelated jobs:lang,uri,routerandinputcallback_rule could be looked upSo code that wanted callbacks on anything but the controller — a model, a
service — had to overwrite
$CIoutright. That also handed away the coreservices, and made the callback target depend on whoever assigned last.
The library is shared for the whole request and the assignment usually happens
in a constructor, so loading two such objects left the first one's callbacks
resolving to
Unable to find callback validation rule. The rule never ran andthe field failed with:
which reads like a missing language line, not a missing callback. That is a
long way from the cause, and it is why this took a while to find.
What changed
A separate
$_callback_object, consulted only where callbacks are resolved:$CIis left alone, so the services keep working.Backward compatibility
$_callback_objectdefaults toNULL, which means the CodeIgniter instance —the historic behaviour. No existing caller has to change and none behaves
differently. The only touched call site is the
method_exists()/ invokepair inside
_execute(), which now askscallback_object()instead of reading$this->CIdirectly.Because the library is request-scoped, the docblock tells callers to set it
immediately before
run()rather than once in a constructor — otherwisewhoever constructed last owns every lookup that follows, which is the original
bug wearing a different hat.
Tests
Seven new tests in
Form_validation_test, covering: the default is the CIinstance; a set object receives the callback;
$CIis untouched by setting it;NULLrestores the default; a non-object is ignored; the setter is chainable;and a genuinely missing callback still fails cleanly rather than erroring.
2.
.gitattributes: check out with LFFifteen tests fail in a Windows clone with
core.autocrlf=trueand passeverywhere else — thirteen in
Form_helper_test, one inHtml_helper_test,one in
Calendar_test.They are not PHP-version failures, which is what they look like at first.
Each compares helper output against a literal multi-line string written in the
test file. The helpers build output from
"\n"escapes so they always emit LF,while the expected value picks up whatever the file on disk uses. CRLF
checkout, CRLF expectation, LF actual, failure.
.gitattributesonly carriedexport-ignorerules, so nothing told git how totreat line endings and
core.autocrlfwon by default. Now* text=auto eol=lf,plus
*.bat text eol=crlfand binary rules.Repository content is unchanged.
git add --renormalizetouched no file but.gitattributesitself, confirming the blobs were already LF and only thecheckout differed. This affects the working tree only, and PHP does not care
either way.
Verification
Full suite, PHP 8.5:
developAll passing. The 54 PHPUnit deprecations and 33 skips are pre-existing on
developand unchanged here.