Ended up using
grep -Prl '\->setMethods\(' . | xargs sed -r -i 's/setMethods\(/onlyMethods\(/g'
special-casing setMethods( null ) -> onlyMethods( [] )
and then manual fix of failing test (from PS2 onwards).
Bug: T278010
Change-Id: I012dca7ae774bb430c1c44d50991ba0b633353f1
My personal best practice is to not document @params when there
is a @dataProvider. I mean, these test…() functions are not
meant to be called from anywhere. They do not really need
documentation. @param tags don't do much but duplicate what the
@dataProvider does. This is error-prone, as demonstrated by the
examples in this patch.
This patch also removes @throws tags from tests. A test…() can
never throw an exception. Otherwise the test would fail.
Most of these are found by the not yet released I10559d8.
Change-Id: I3782bca43f875687cd2be972144a7ab6b298454e
For specific actions that do not change the content of
pages, skip the additional permission checks required
for individual user configuration pages. This allows
sysops to view deleted content of such pages,
even though they cannot edit the page.
Bug: T202989
Change-Id: I02e820c818e9e58e6591ec412fc3eca9384e0bb2
Remove broken `testQuickPermissions` tests,
as well as the helper method `runGroupPermissions`
only used in those broken tests. Will follow-up
with the addition of working tests, splitting
to ease review since the new tests will
be very different from the old ones (eg using
a dataProvider)
Change-Id: I4aa9d4b5f8ac4ee74aae3282b221d0bbbb7276a4
Replace existing `testPageRestrictions` method,
which is `@group Broken`, with simpler tests with
direct access to `checkPageRestrictions` via TestingAccessWrapper
Change-Id: Iadd74c1bad95ecdbb89fd5835330a9c5f32432d0
Rewrite tests for `checkUserConfigPermissions`
Replace `runConfigEditPermissions` with direct
access to the private method using TestingAccessWrapper
Change-Id: I05757a0d0249edf7dd39b8c21988f7fa88020b2b
Right now its just reducing the number of calls to
MediaWikiServices::getInstance()->getPermissionManager()
to make the tests a bit more readable
This change should be a no-op
Change-Id: I97c9dd8d057dbb20d07c51a67e17f4734b13723f
Following 23c3c70d7f, soft deprecate the static methods on
DatabaseBlock that have been moved to DatabaseBlockStore:
* ::insert
* ::delete
* ::update
* ::purgeExpired
Update calls to the deprecated methods from core.
Change-Id: I1272eb978594fd4f386bda12cbc24131ad7d882f
A new user right, `delete-redirect`, is added (not given to anyone
by default). At Special:MovePage, if attempting to move to a single
revision redirect that would otherwise be an invalid target (i.e.
doesn't point to the source page), the user is able to delete the
target.
Deletions are logged as `delete/delete_redir2`, and the move is
then logged normally as `move/move`, mirroring current delete and
move logging.
To allow for separate handling by Special:MovePage,
MovePage::isValidMove now returns a fatal status `redirectexists` if
the target isn't valid but passes Title::isSingleRevRedirect.
Otherwise, `articleexists` is returned (as previously). Other callers
that don't intend to treat single revision redirects differently
should treat `redirectexists` the same as `articleexists`.
Currently, this deletion (like normal delete and move) cannot be
done through the move api. Since the deletion is only valid when
moving a page, unlike for normal deletion, deleting redirects with
this right cannot be done via the delete api either.
Bug: T239277
Change-Id: I36c8df0a12d326ae07018046541bd00103936144
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
The new HookContainer.php introduces a scopedRegister() method for
temporarily setting hooks. Let's use that in MediaWikiUnitTestCase
and MediaWikiIntegrationTestCase instead of directly accessing
global $wgHooks to do so.
Also introduces setTemporaryHook() and removeTemporaryHook()
methods in MWIntegrationTestCase for easily adding/removing of
temporary hooks.
Bug: T250300
Change-Id: I8cefd41b66f882c53646b76de76c51f0d8730f72
This reverts commit 8d57c5e32c.
Reason for revert: 'Session' is a final class and cannot be mocked.
Added a comment to explain this.
Change-Id: Ic8bbd82aa668916766ab8434018299a22d31224d
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
The motivations behind this change is T227892 and that a blocker for a System or
Composite block provides no useful information for the end user.
Here is what's changing:
* Move the $blocker property to DatabaseBlock, since this is the only type of
block that can be created by a user.
* Move handling of the 'by' and 'byName' constructor option from AbstractBlock to
DatabaseBlock.
* getBy(), getByName(), are now abstracts methods and each block type have to provide
their own implementation
* getBlocker(), setBlocker() are being deprecated in AbstractBlock and moved as internal
methods into DatabaseBlock
Bug: T227892
Depends-On: Ie2aa00cfec5e166460bcaeddba3c601fe0627c13
Change-Id: I1b17c652b12d98de3d996d309d4d3f22074be411
Update PermissionManagerTest and FormSpecialPageTestCase to use a
mock User object that returns a specified block.
Before this commit, these tests manipulate the public properties
User::mBlock and User::mBlockedby in order to change the user's
block. There is no advantage to doing this over mocking: it is not
more realistic, and is sensitive to changes in the code. These tests
need updating in order to deprecate public access to these properties
(see T229035).
Also, PermissionManager::testUserBlock is a large, complicated method
that tests three different things, so split it into three smaller
methods, each testing one thing:
* testCheckUserBlockActions: A blocked user is blocked/unblocked
correctly from certain actions, depending on their block.
* testCheckUserBlockMessage: The correct block message key and
parameters are returned, depending on the block.
* testCheckUserBlockEmailConfirmToEdit: A user is or isn't asked to
provide email confirmation in order to edit, as specified by the
config.
Change-Id: I0bb9252b476131c2b255d4c503c0dab5dfff94be
The main reasons for adding this service layer are:
* It allows error messages to be more consistent, by defining
a set of reportable information that can describe any block
type and is consistently formatted.
* It decouples formatting from the block classes, removing
their dependency on language, for the most part.
The service provides one public method, getMessage, which
returns a Message object whose key and parameters are
determined by the type of block. This should be used instead
of the deprecated AbstractBlock::getPermissionsError and
AbstractBlock::getBlockErrorParams.
Calls to AbstractBlock::getPermissionsError are replaced in
this patch.
Bug: T227174
Change-Id: I8caae7e30a46ef7120a86a4e5e6f30ae00855063
assertEquals( false, … ) still succeeds when the actual value is 0, null,
an empty string, even an empty array. All these should be reported as a
failure, I would argue.
Note this patch previously also touched assertSame( false ). I reverted
these. The only benefit would have been consistency within this codebase,
but there is no strict reason to prefer one over the other. assertFalse()
and assertSame( false ) are functionally identical.
Change-Id: Ic5f1c7d504e7249002d3184520012e03313137b4
