There should be no need to call hook handlers directly,
this should only be done by HookContainer.
Change-Id: I8fa46c2eb6a40ad98e564c31dcfb103825608426
This reduces the acceptable forms for hook handlers to three things:
* a callable (in the form of a string, an array, or a closure)
* an object, which is expected to have a public "on" method that
matches the hook name.
* an array containing an object spec in the "handler" key, for use
with ExtensionRegistry.
All other forms will trigger a deprecation warning.
Bug: T339167
Depends-On: I980f2d45e6bb8c6a04058e68c758f71bbcf709de
Depends-On: Ieae405f70caa01d84602583cc214b0ee3fadc796
Depends-On: If15df4b598c02ed9bda5eea0ae89a16ebbf4f2e2
Depends-On: Id70276fa1e1821bd400dc0ae5cea722a21d524d5
Change-Id: I83bc81d1b3033c38b9313884a9c70a187fdde227
This reverts change I50c3d1c5df (commit b0317287bc), thus reinstating
change I7d690a1172 (commit d139eb07fe). The only change from the
original is in getHookMethodName(), additionally replacing '-' with '_'
(not just ':' and '\'). The original commit message follows:
This converts all hook handlers to the same internal representation.
This is done lazily, when the hook is run for the first time.
The logic for temporarily disabling handlers by calling scopedRegister()
with the $replace parameter set has been greatly simplified.
There are some minor changes to the class's interface and behavior,
none of which should be breaking changes:
* run() will emit deprecation warnings if and only if it was called
with the deprecationVersion option set, for all kinds of handlers.
The idea is that deprecated hooks should emit a warning either from
run(), or from emitDeprecationWarnings(). The latter happens if the
hook is listed in DeprecatedHooks.
* register() now also accepts hook handlers declared in the way that
extensions register hooks.
* Attempts to call register() with an invalid hook definition now
result in an invalidArgumentException.
* Attempts to call register() for a deprecated hook will consistently
result in a deprecation warning.
* The internal getRegisteredHooks() method has been removed in favor
of the identical getHookNames() method.
* The internal getLegacyHandlers method has been removed in favor
of getHandlerDescriptions() and getHandlerCallbacks().
* The call order changed so that dynamically registered handlers
are called last, instead of getting called before handler objects
from extensions.
Bug: T338213
Change-Id: I6efb09e314ad2b124a33a757fdda2a07ae0d8f7c
This apparently caused some change in how hook handlers are called (it
now calls e.g. AbuseFilterHookHandler::onAbuseFilter-generateUserVars()
instead of AbuseFilterHookHandler::onAbuseFilter_generateUserVars()),
causing both test failures and errors on Beta.
This reverts commit d139eb07fe.
Bug: T338213
Change-Id: I50c3d1c5dfd2d7eeac59992156a8a644cf0197e5
This converts all hook handlers to the same internal representation.
This is done lazily, when the hook is run for the first time.
The logic for temporarily disabling handlers by calling scopedRegister()
with the $replace parameter set has been greatly simplified.
There are some minor changes to the class's interface and behavior,
none of which should be breaking changes:
* run() will emit deprecation warnings if and only if it was called
with the deprecationVersion option set, for all kinds of handlers.
The idea is that deprecated hooks should emit a warning either from
run(), or from emitDeprecationWarnings(). The latter happens if the
hook is listed in DeprecatedHooks.
* register() now also accepts hook handlers declared in the way that
extensions register hooks.
* Attempts to call register() with an invalid hook definition now
result in an invalidArgumentException.
* Attempts to call register() for a deprecated hook will consistently
result in a deprecation warning.
* The internal getRegisteredHooks() method has been removed in favor
of the identical getHookNames() method.
* The internal getLegacyHandlers method has been removed in favor
of getHandlerDescriptions() and getHandlerCallbacks().
* The call order changed so that dynamically registered handlers
are called last, instead of getting called before handler objects
from extensions.
Change-Id: I7d690a1172af44a90b957b2274d68e51b7f09938
This will allow us to treat $wgHooks as a normal config
variable in 1.41.
Note: should be backported to 1.40
Bug: T331602
Change-Id: Iddcb760cf8961316d6527e81b9aa968657d8354c
$wgHooks should be treated like a regular setting, which cannot be
manipulated after bootstrapping is complete. This will allow us to
greatly simplify the logic in HookContainer.
Replacing $wgHooks with a fake array after bootstrapping allows us to
detect any remaining live access to $wgHooks without breaking
functionality.
The plan is to have the fake array emit deprecation warnings in the 1.40
release, and make it throw exceptions in later releases.
See Iddcb760cf8961316d6527e81b9aa968657d8354c for the deprecation
warnings.
Bug: T331602
Change-Id: I0ebba9a29f81b0d86ad8fd84d478fb244f9e9c15
Also:
* In .phpcs.xml, remove "showUsage" from the list; transstat.php was
removed in e19f4e16a6.
* In styleTest.css.php, check the type of the $_GET value as part of the
"basic sanitization".
Change-Id: Icb82d8a42dd6cd062f41a04696f092b43b04f54f
Previously, clear() and scopedRegister() would not disable hook handlers
registered using the new mechanism. Only old style hook handlers would
be suppressed.
The new implementation is based on tombstones: Instead of actually
removing registered handlers to override them, tombstone markers are
put into place to disable existing handlers until the tombstone is
removed again. This allows hook handlers to be disabled temporarily
in a consistent way.
This removes HookContainer::getOriginalHooksForTest(), which is
incompatible with the new logic.
Bug: T255056
Change-Id: I08c1824797ac60a5098a52b5781af8ac4dd38928
Use a unique key to assign handlers registered via
scopedRegister(). Using unique keys instead of
array indices ensures that handlers registered in other
ways previously (e.g. via global hook registry or via
HookContainer::register() won't be removed).
Remove the temporary hook for AlternateUserMailer
as the ticket it references is for a class that
no longer exists
Bug: T255056
Change-Id: I491f281e60511a5bdd695ac123611e408324ccff
The name change happened some time ago, and I think its
about time to start using the name name!
(Done with a find and replace)
My personal motivation for doing this is that I have started
trying out vscode as an IDE for mediawiki development, and
right now it doesn't appear to handle php aliases very well
or at all.
Change-Id: I412235d91ae26e4c1c6a62e0dbb7e7cf3c5ed4a6
Done with `composer fix` and suppressing the rest (i.e. sniffs for
global variables, which for core should be suppressed anyway).
Additionally, add `-p` to `phpcbf`, as otherwise it just seems stuck.
Change-Id: Ide8d6cdd083655891b6d654e78440fbda81ab2bc
New classes and modificatons to existing classes to support the new Hooks system. All changes are documented in RFC https://phabricator.wikimedia.org/T240307.
- HookContainer.php: Class for doing much of what Hooks.php has historically done, but enabling new-style hooks to be processed and registered. Changes include new ways of defining hook handler functions as an object with defined dependencies in extension.json, removing runWithoutAbort() and addit it to an $options parameter to be passed to HookContainer::run(), being able to decipher whether a hook handler is legacy or non-legacy style and run them in the appropriate way, etc.
- DeprecatedHooks.php: For marking hooks deprecated and verifying if one is deprecated
- DeprecatedHooksTest.php: Unit tests for DeprecatedHooks.php
- Hooks.php: register() will now additionally register hooks with handlers in new HooksContainer.php. getHandlers() will be a legacy wrapper for calling the newer HookContainer::getHandlers()
- MediaWikiServices.php: Added getHookContainer() for retrieving HookContainer singleton
- ExtensionProcessor.php: modified extractHooks() to be able to extract new style handler objects being registered in extension.json
- ServiceWiring.php: Added HookContainer to list of services to return
- HookContainerTest.php: Unit tests for HookContainer.php
- ExtensionProcessorTest.php: Moved file out of /unit folder and now extends MediaWikiTestCase instead of MediaWikiUnitTestCase (as the tests are not truly unit tests). Modified existing tests for ExtensionProcessor::extractHooks() to include a test case for new style handler
Bug: T240307
Change-Id: I432861d8995cfd7180e77e115251d8055b7eceec
Add public, protected or private to function missing a visibility
Enable the tests folder for the phpcs sniff
Change-Id: Ibefce76ea9984c47e08c94889ea2eafca7565e2c
This changeset implements T89432 and related tickets and is based on exploration
done at the Prague Hackathon. The goal is to identify tests in MediaWiki core
that can be run without having to install & configure MediaWiki and its dependencies,
and provide a way to execute these tests via the standard phpunit entry point,
allowing for faster development and integration with existing tooling like IDEs.
The initial set of tests that met these criteria were identified using the work Amir did in
I88822667693d9e00ac3d4639c87bc24e5083e5e8. These tests were then moved into a new subdirectory
under phpunit/ and organized into a separate test suite. The environment for this suite
is set up via a PHPUnit bootstrap file without a custom entry point.
You can execute these tests by running:
$ vendor/bin/phpunit -d memory_limit=512M -c tests/phpunit/unit-tests.xml
Bug: T89432
Bug: T87781
Bug: T84948
Change-Id: Iad01033a0548afd4d2a6f2c1ef6fcc9debf72c0d
Deprecated since 1.25, very few callers. Having to grep for 2
ways of calling a hook is harmful not only because it wastes developers'
time, but also because it makes it possible to forget to search for the
old way, resulting in mistakes. Better get rid of this.
Change-Id: Iab28bd5758475b780c2016847881757d64973862
When used, any hook that tries to abort the event by returning something
other than true or null, will result in a run-time exception.
To make it easier to introduce this opt-in flag, still allow explicit
'true' returns from existing callers.
Also factor out the common code between these methods into a new
private method callHook().
Bug: T173615
Change-Id: I94c7ab656bd1a046410681a810c0a3fd4f72a2e5
Essentially rewrote Hooks::run() to get rid of the ridiculous
four levels of indentation. Also made some slight adjustments
to fix rare edge cases (for example, moved set_error_handler
after wfProfileIn in case Profiler triggers an error).
Change-Id: Iafdd4ceedac067b49ac597359ac456f4617da9e8
Fix almost all occurences of the following sniffs:
Generic.CodeAnalysis.UselessOverridingMethod.Found
Generic.Formatting.NoSpaceAfterCast.SpaceFound
Generic.Functions.FunctionCallArgumentSpacing.SpaceBeforeComma
Generic.Functions.OpeningFunctionBraceKernighanRitchie.BraceOnNewLine
Generic.PHP.LowerCaseConstant.Found
PSR2.Classes.PropertyDeclaration.ScopeMissing
PSR2.Files.EndFileNewline.TooMany
PSR2.Methods.MethodDeclaration.StaticBeforeVisibility
Change-Id: I96aacef5bafe5a2bca659744fba1380999cfc37d
Until now, Hooks::run() would execute hooks registered via $wgHooks, but
Hooks:isRegistered() would not consider them and Hooks::getHandlers() would
not return them. That is inconsistent and misleading. This change aims to
make the methods of the Hooks class behave consistently, and allows them
to be used as a generic way of interacting with hooks.
Change-Id: I39bd5de2bc8ccbe8df729446363960af9d29b0be
