Declare one variable at a time
Add some documentation
Add some whitespace for readability
Replace some globals with retrieval from MainConfig
Should be a no-op
Change-Id: Ie58e56d61808d00fea99c35cd1c83a30d6aa28af
A terminating line break has not been required in wfDebug() since 2014,
however no migration was done. Some of these line breaks found their way
into LoggerInterface::debug() calls, where they mess up the formatting
of the debug log.
So, remove terminating line breaks from wfDebug() and
LoggerInterface::debug() calls.
Also:
* Fix the stripping of leading line breaks from the log header emitted
by Setup.php. This feature, accidentally broken in 2014, allows
requests to be distinguished in the log file.
* Avoid using the global variable $self.
* Move the logging of the client IP back to Setup.php. It was moved to
WebRequest in the hopes that it would not always be needed, however
$wgRequest->getIP() is now called unconditionally a few lines up in
Setup.php. This means that it is put in its proper place after the
"start request" message.
* Wrap the log header code in a closure so that variables like $name do
not leak into global scope.
* In Linker.php, remove a few instances of an unnecessary second
parameter to wfDebug().
Change-Id: I96651d3044a95b9d210b51cb8368edc76bebbb9e
Migrate all callers of Hooks::run() to use the new
HookContainer/HookRunner system.
General principles:
* Use DI if it is already used. We're not changing the way state is
managed in this patch.
* HookContainer is always injected, not HookRunner. HookContainer
is a service, it's a more generic interface, it is the only
thing that provides isRegistered() which is needed in some cases,
and a HookRunner can be efficiently constructed from it
(confirmed by benchmark). Because HookContainer is needed
for object construction, it is also needed by all factories.
* "Ask your friendly local base class". Big hierarchies like
SpecialPage and ApiBase have getHookContainer() and getHookRunner()
methods in the base class, and classes that extend that base class
are not expected to know or care where the base class gets its
HookContainer from.
* ProtectedHookAccessorTrait provides protected getHookContainer() and
getHookRunner() methods, getting them from the global service
container. The point of this is to ease migration to DI by ensuring
that call sites ask their local friendly base class rather than
getting a HookRunner from the service container directly.
* Private $this->hookRunner. In some smaller classes where accessor
methods did not seem warranted, there is a private HookRunner property
which is accessed directly. Very rarely (two cases), there is a
protected property, for consistency with code that conventionally
assumes protected=private, but in cases where the class might actually
be overridden, a protected accessor is preferred over a protected
property.
* The last resort: Hooks::runner(). Mostly for static, file-scope and
global code. In a few cases it was used for objects with broken
construction schemes, out of horror or laziness.
Constructors with new required arguments:
* AuthManager
* BadFileLookup
* BlockManager
* ClassicInterwikiLookup
* ContentHandlerFactory
* ContentSecurityPolicy
* DefaultOptionsManager
* DerivedPageDataUpdater
* FullSearchResultWidget
* HtmlCacheUpdater
* LanguageFactory
* LanguageNameUtils
* LinkRenderer
* LinkRendererFactory
* LocalisationCache
* MagicWordFactory
* MessageCache
* NamespaceInfo
* PageEditStash
* PageHandlerFactory
* PageUpdater
* ParserFactory
* PermissionManager
* RevisionStore
* RevisionStoreFactory
* SearchEngineConfig
* SearchEngineFactory
* SearchFormWidget
* SearchNearMatcher
* SessionBackend
* SpecialPageFactory
* UserNameUtils
* UserOptionsManager
* WatchedItemQueryService
* WatchedItemStore
Constructors with new optional arguments:
* DefaultPreferencesFactory
* Language
* LinkHolderArray
* MovePage
* Parser
* ParserCache
* PasswordReset
* Router
setHookContainer() now required after construction:
* AuthenticationProvider
* ResourceLoaderModule
* SearchEngine
Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
Carefully, I checked usage of these methods in our code bases and
added the best possible visibilities to them. Not sure if I missed
something but let me know if I did.
Used private & public where suitable for the various methods. As for
__toString(), this is a magic method, so should be public per PHP docs.
Change-Id: Ie0987f4a984cac2f5eb1d9e21a305ad9467a8eb2
The method User::isBlocked() attempts to answer two questions:
(1) Does the user have a block?
(2) Is the user prevented from performing this action?
The method can answer #1, but it cannot answer #2. Since User::getBlock() can
also answer #1, this method is redundant. The method cannot answer #2 because
there is not enough context in order to answer that question.
If access is being checked against a Title object, all access checks can be
performed with PermissionManager:userCan() which will also check the user's
blocks.
If performing all access checks is not desirable, using
PermissionManager::isBlockedFrom() is also acceptable for only checking if the
user is blocked. This method does *not* determine if the action is allowed,
only that the user's block applies to that Title.
If access is being checked without an existing Title, User::getBlock() can be
used to get the user's block. Then Block::appliesToRight() can be used to
determine if the block applies explicitly to a right (or returns null if
it is unknown or false if explicitly allowed). If the user is creating a new
Title, but the text of the title is not yet known (as in the case of Wikibase),
access should be checked with Block::appliesToNamespace().
Bug: T209004
Change-Id: Ic0ad1b92e957797fee8dcd00bd1092fe69fa58f1
Added missing line-break in email notification specifically after
page summary or immediately before $PAGEMINOREDIT (for minor edits).
In addition, if the edit is not marked as a "minor edit", avoid the
line break.
Bug: T176595
Change-Id: I68cc249c3546eb7fed0834063f15304e3c245e55
This was originally a global search and replace. I manually checked all
replacements and reverted them if (due to the lack of type hints) either
null (that would be 0 when counted) or a Countable object can end in the
variable or property in question.
Now this patch only touches places where I'm sure nothing can break.
For the sanity of the honorable reviewers this patch is exclusively touching
negated counts. You should not find a single `!== []` in this patch, that
would be a mistake.
Change-Id: I5eafd4d8fccdb53a668be8e6f25a566f9c3a0a95
Extensions that have hooks for
UpdateUserMailerFormattedPageStatus (to provide additional
pageStatus indicators) need a way to make sure that, when their
hook is called in SendWatchlistemailNotification, they only
notifications handling their pageStatus indicator. This allows them
to fetch the protected property.
Bug: T175104
Change-Id: I2beb5b3576f2cd739dab16b9adf613c96636edd3
If email is not successfully sent, return Status (with appropriate
status error) and also, if successfully sent, return Status::newGood().
Change-Id: I602e30641b88614087d9aff83cf8c93e32af7b81
Otherwise you get something like this:
"Editor's summary: interwiki fix This is a minor edit"
It's not entirely clear that the editor's summary has ended and the minor edit
text has been added by the software.
Change-Id: Iab79b5f8a1f6f319e9fd284835b7a4e86c3f0394
It's unreasonable to expect newbies to know that "bug 12345" means "Task T14345"
except where it doesn't, so let's just standardise on the real numbers.
Change-Id: I6f59febaf8fc96e80f8cfc11f4356283f461142a
Always treat this as on and simplify the code.
This will also make it easier to move updateWatchlistTimestamp() into
the EnotifNotifyJob class to avoid query timeouts.
Change-Id: I8ceaa42cdcfe3ad00a26368be6a73052be329045
Also make sure that normal watchlist functionality
works for the $wgUsersNotifiedOnAllChanges, for
example changes should marked 'bold' since last visit.
Bug: T69041
Change-Id: I7d53f07ed3dceb5b1c541091fd69beca3fb3e94f
Added an option to UserMailer::sendMail() to allow adding arbitrary
headers to emails.
Bug: T58315
Change-Id: I01a60430bf39f6bd104269b7246767f016eb9cd5
$replyto and $contentType should now be passed as an array of $options.
This will make it easier to add more options in the future without
having a long list of optional parameters.
Change-Id: I2c38bb438bd01e0ed2552024a40311f3e8e2dc08
In actuallyNotifyOnPageChange, the $wgEnotifUserTalk is inside
a check involving $wgEnotifMinorEdits, so it won't fire if
it's a minor edit and $wgEnotifMinorEdits is false.
Change-Id: I6576cb1735db5d9288257e6877b8755450d0dacd
Previously, private data could be leaked on private wikis where
this configuration is common. To prevent this, do not send email
notifications on page changes for blocked watchers and blocked
talk page owners if $wgBlockDisablesLogin is true.
Bug: T54453
Change-Id: I17408e1038ae10ee01eab747591ef2d9c86b106e
Xhprof generates this data now. Custom profiling of various
sub-function units are kept.
Calls to profiler represented about 3% of page execution
time on Special:BlankPage (1.5% in/out); after this change
it's down to about 0.98% of page execution time.
Change-Id: Id9a1dc9d8f80bbd52e42226b724a1e1213d07af7
Pulls one pure method with no state from EmailNotification into a
public static method. This is done so that extensions (Flow, maybe
others) can replace the Email notifications with alternative ones (Echo)
while still updating the watchlist.
A better solution might be to extricate watchlist update from email
notifications, but this seems like a reasonable first step.
Bug: 66876
Change-Id: Iae213b87706c447b880244711e7747954423bb69