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
We're finally to the point of making the massive alter to the `revision`
table that we've been building up to for 2.5 years now.
Changes here are:
* Drop `rev_text_id`, `rev_content_model`, and `rev_content_format` that
MCR obsoleted.
* Drop `ar_text_id`, `ar_content_model`, and `ar_content_format` that
MCR obsoleted.
* Replace `rev_comment` with `rev_comment_id`.
* Replace `rev_user` and `rev_user_text` with `rev_actor`, plus
associated index changes.
Future patches will make the code changes to migrate data from
`revision_actor_temp` and `revision_comment_temp` into the new
`revision` columns.
Bug: T251343
Bug: T184615
Bug: T215466
Change-Id: I18071a2fe45907a0cf1b0fefebd96a97a2dacb7b
Change Ic6f98d8fb changed RevisionStore::getRevisionByTitle to convert
the passed LinkTarget to a Title using Title::newFromLinkTarget, to
address bug T206498.
Unfortunately, this breaks when RevisionStore is operating in the context
of a foreign wiki. In this scenario, when RevisionStoreRecord::__construct
calls Title::getArticleID on the Title instance to perform a sanity check
of the page ID against the page ID reference stored in the revision table
for the given revision record, then Title::getArticleID will lazily fetch
the page ID on the local, rather than the foreign wiki. This results in
an InvalidArgumentException thrown by RevisionStoreRecord::__construct.
This patch changes the logic to only resolve the LinkTarget to a Title
when operating in the context of the local wiki, similar to the checks
that RevisionStore::getTitle and friends are already performing.
It also adds a regression test to verify that
RevisionStore::getRevisionByTitle does not try to access the local wiki
DBLoadBalancer when called in the context of a foreign wiki.
Bug: T248756
Change-Id: I76090fa2907c2db8e23a62959346db531c6d9cb2
When for some reason we can't determine the title for a revision
in the batch, this should not trigger a fatal TypeError, but handled
gracefully, with helpful information included in the error message.
Bug: T205936
Change-Id: I0c7d2c1fee03d1c9208669a9b5ad66612494a47c
See codesearch - only deployed call outside of core is in flaggedrevs,
and already passes a RevisionRecord:
https://codesearch.wmflabs.org/deployed/?q=-%3EupdateRevisionOn%5C(&i=nope&files=&repos=
Also fixed a use of Revision::newFromId in orphans.php
Bug: T249561
Bug: T249021
Change-Id: I5933a278de8645b7005c11026c87ae27c0373770
Sometimes, an edit is done with a Title object that has gone
out of sync with the database after a page move. In this case,
we should re-load the current page ID from the database,
instead of failing the update hard.
Bug: T246720
Bug: T204793
Bug: T221763
Bug: T225366
Change-Id: If7701205ec2bf4d4495349d3e67cf53d32ee8357
This script scans for content blobs that can't be loaded due to
database corruption, and can change their entry in the content table
to an address starting with "bad:". Such addresses cause the content
to be read as empty, with no log entry. This is useful to avoid
errors and log spam due to known bad revisions.
The script is designed to scan a limited number of revisions from a
given start date. The assumption is that database corruption is
generally caused by an intermedia bug or system failure which will
affect many revisions over a short period of time.
Bug: T205936
Change-Id: I6f513133e90701bee89d63efa618afc3f91c2d2b
Before:
> 1. 1140ms to run …McrRevisionStoreDbTest:testLoadRevisionFromTimestamp
> 2. 1140ms to run …McrRevisionStoreDbTest:testGetRevisionByTimestamp
> …
After:
> …
> 36. 137ms to run …McrRevisionStoreDbTest:testGetRevisionByTimestamp
> 40. 135ms to run …McrRevisionStoreDbTest:testLoadRevisionFromTimestamp
Bug: T225730
Change-Id: I716bcf127761ef01d7ccfe10a79c08bea33ebc97
They naturally belong in RevisionLookup. They return Revision,
so should be replaced anyway.
Bug: T246284
Change-Id: Ie5c478e4667ca0e773186b9cb8a319cd09145112
As well hard-deprecate RevisionStore::listRevisionSizes.
It was marked as @deprecated, but the release for the
deprecation was not specified. Let's say it's 1.35.
Additionally, in order to avoid temporary code duplication,
listRevisionSizes now uses getRevisionSizes and ignores the
database object injected into it. This is ok since we're
hard-deprecating the method and all the usages have been
removed.
Bug: T246284
Change-Id: Ifad1c25a0af892b88fce492b2d34c8cf71279b70
Part of the soft deprecated revision class, no known callers outside
of EditPage, which is updated, and tests, which hide the deprecation
Bug: T246284
Change-Id: I099cb93a12f3a1d9a720e18e3236374321ce7b0c
Add new ILBFactory::setDomainAliases() method for injection database
domain aliases and call it in MWLBFactory::setDomainAliases().
Also:
* Remove overkill "last db/section" caching in LBFactoryMulti
* Clean up some LBFactoryMulti code comments
* Split out separate MWLBFactoryTest test file
Change-Id: If180a58c61178969ca7587c4a06b8786574c7254
Added:
- ContentHandlerFactory
Tests:
- PHPUnit
Changed
- Calls of changed and deprecated
- DI for some service/api
Deprecated:
- ContentHandler::* then similar to ContentHandlerFactory
- ContentHandler::getForTitle
- ContentHandler::$handlers
Bug: T235165
Change-Id: I59246938c7ad7b3e70e46c9e698708ef9bc672c6
This patch tries to make assertions in tests more readable by using more
self-documenting assertions as provided by modern PHPUnit versions. Among
a few others, these two main changes are done:
* I found a lot of assertions with the expected value being the *second*
parameter. I did not changed all of them. Only some that can be replaced
with assertNull() and such.
* I try to replace all `assertTrue( is_…() )` with dedicated assertions.
Change-Id: I1fc72188fbd0edacf13886e7f9a9eacbd85f13c2
The SlotRecord passed to RevisionStore::insertRevisionOn may have its sha1 and size
already set by the caller. This is useful when restoring archived re
visions, but apparently sometimes goes wrong, leading to a revision
being recorded with the hash and size of its parent revision.
This patch introduces a sanity check that ensures that the revision's
hash and size matches the main slot's hash and size. If there are
additional slots, the check is skipped.
This does not fix the bug as reported, but should prevent any new
revisions to be inserted with mismatching meta-data, and provide us with
a stack trace that should help with finding the original cause of the
issue.
Bug: T239717
Change-Id: I4947731364925fef1c77cabcfd144fd2c07bf9db
In several places, we're including rc_timestamp or other fields in a
query selecting on rc_this_oldid because there was historically no index
on the column.
The needed index was created by I0ccfd26d and deployed by T202167, so
let's remove the hacks.
Bug: T139012
Bug: T239772
Change-Id: Ic99760075bde6603c9f2ab3ee262f5a2878205c7
When restoring revisions that have an empty user name associated with
them, force "Unknown user" to be used instead, even if the actor table
has an entry for the empty user name.
Bug: T236624
Change-Id: I31edd5f7d89d9b43806ad18e96f5e93cff0f8c6f
* Move Title::{get,count}AuthorsBetween to RevisionStore
* Use it in REST count handler to to provide 'from' and 'to'
support for editors count.
Bug: T235896
Bug: T235666
Change-Id: Ib7947ae64b82871ceb4a3097a0ea03c0ba218c7c
* Add 'from' and 'to' query parameters to history count endpoint,
in case we are counting 'edits'. The support for parameters will be
expanded in the future, given that 'editors' is on the way.
* Refactor RevisionStore::countRevisionsBetween to be able to omit
starting or ending revision.
Bug: T235666
Change-Id: I1fc3f2d4e422f17ccfc99664ea35e210c13ade5f
Move the DBO_TRX init logic out of Database::__construct() and into
LoadBalancer since the later already handles setting and clearing this
flag based on transaction rounds starting and ending.
Add 'lazyMasterHandle', 'topologyRole', and 'topologicalMaster' parameters
to Database::factory() and inject them via LoadBalancer all at once in order
to avoid worrying about call order. Move some type casting code to
Database::__construct().
Add IDatabase::getTopologyRole()/getTopologicalMaster().
Use constants for getLBInfo()/setLBInfo() for better usage tracking and
typo resistance.
Change-Id: I437ce434326601e6ba36d9aedc55db396dfe4452
