Clean up some technical debt; use MutableRevisionRecord instead of
manually constructing a Revision from an array, remove last uses of
RevisionStoreDbTestBase::revisionToRow and remove the method.
Each file can be reviewed separately (except that the removal of
revisionToRow depends on replacing its usage)
Bug: T246284
Change-Id: I0bdc069b21a5c41ef8f9e972c5b17ff189d4a741
There is native support for all of this now in PHP, thanks to changes
and additions that have been made in later versions. There should be no
need any more to ever use call_user_func() or call_user_func_array().
Reviewing this should be fairly easy: Because this patch touches
exclusivly tests, but no production code, there is no such thing as
"insufficent test coverage". As long as CI goes green, this should be
fine.
Change-Id: Ib9690103687734bb5a85d3dab0e5642a07087bbc
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
This setting has been effectively constant since 2008. In modern code
we should be using a ParserFactory instead to customize Parser creation
and not calling the Parser constructor directly (T236811).
Because the ParserFactory is cached, which freezes the current value of
the content language and other options, we need to reset the ParserFactory
object when running parser tests (T248977). Thanks to
Peter Ovchyn <peter.ovchyn@speedandfunction.com> for first uncovering this
issue and suggesting a fix in I4203bf7719a8555a09b72cdb5b1ae7a6e1505acf.
Code search:
https://codesearch.wmflabs.org/deployed/?q=wgParserConf&i=nope&files=&repos=https://codesearch.wmflabs.org/deployed/?q=ParserConf&i=nope&files=&repos=
Bug: T248977
Bug: T236811
Depends-On: I97d58750c91b06eeca5d810509becdf53a39cc95
Depends-On: Idf59cd54146d31c1c32883f4318e6a0bf60e1a8a
Change-Id: I787f22ea9bf59a049b13631ba6974866a1300988
Some tests were failing after Hooks::clear() was deprecated in
the new HookContainer.php, and needed to have the deprecation
warning hidden to pass.
Bug: T250102
Change-Id: I59784e7972517488ba4cb8d0bad8d09a6f534f48
Disabling tidy has been deprecated since 1.33. This cleans up the code
paths which still used untidy output.
Bug: T198214
Change-Id: I821ef3b8f59b272d983583d407b2f0794fe1e791
Done:
* Replace LanguageConverter::newConverter by LanguageConverterFactory::getLanguageConverter
* Remove LanguageConverter::newConverter from all subclasses
* Add LanguageConverterFactory integration tests which covers all languages by their code.
* Caching of LanguageConverters in factory
* Make all tests running (hope that's would be enough)
* Uncomment the deprecated functions.
* Rename FakeConverter to TrivialLanguageConverter
* Create ILanguageConverter to have shared ancestor
* Make the LanguageConverter class abstract.
* Create table with mapping between lang code and converter instead of using name convention
* ILanguageConverter @internal
* Clean up code
Change-Id: I0e4d77de0f44e18c19956a1ffd69d30e63cf51bf
Bug: T226833, T243332
Remove the deprecated Preprocessor_DOM class, which was hard-deprecated
in 1.34. This begins to simplify parser configuration and reduce redundant
code paths, but I've left two things for cleanup in a future patch:
1. The `preprocessorClass` configuration option to the parser, exposed
in `$wgParserConf`, ServiceWiring, ParserFactory, etc. There is no reason
for this to be exposed as configurable, but I've left this clean up to a
future patch.
2. The `$wgMaxGeneratedPPNodeCount` configuration, exposed also in
ParserOptions. Only Preprocessor_DOM calculated this count, and since
we are only using Preprocessor_Hash now, this configuration has no effect.
But since this value was exposed in ParserOptions and elsewhere, I've
deprecated where needed but left this clean up to a future patch.
Bug: T204945
Change-Id: I727f003f9a42d0c92bcbcce8a8289d5af6cd1298
Not only the logic was wrong, it also completely failed to include
the second parameter both in doc comment and exception message,
leading to a lot of confusion.
Change-Id: Icc585214e94c82142ac585c748d1038769cd4db2
The following public methods were renamed and made private; the old name
is hard-deprecated and calls the new renamed private method:
Parser::doMagicLinks() => handleMagicLinks()
Parser::doDoubleUnderscore() => handleMagicLinks()
Parser::doHeadings() => handleHeadings()
Parser::doAllQuotes() => handleAllQuotes()
Parser::replaceExternalLinks() => handleExternalLinks()
Parser::replaceInternalLinks() => handleInternalLinks()
Parser::replaceInternalLinks2() => handleInternalLinks2()
Parser::getVariableValue() => expandMagicVariable()
Parser::initialiseVariables() => initializeVariables()
Parser::formatHeadings() => finalizeHeadings()
Parser::test{Pst,Preprocess,Srvus}() => fuzzTest{Pst,Preprocess,Srvus}()
Additionally, the following methods are not used externally, but are
used outside the Parser class by core code. They have been marked
@internal:
Parser::doQuotes() (used by {{#displaytitle}}),
Parser::getExternalLink{Rel,Attribs}() (used by Linker),
Parser::normalizeLinkUrl() (used by Special:LinkSearch and elsewhere).
Parser::{brace,arg,extension}Substitution() (used by PPFrame)
Code search query:
https://codesearch.wmflabs.org/deployed/?q=do%28MagicLinks%7CDoubleUnderscore%7CHeadings%7CAllQuotes%29%7Creplace%28ExternalLinks%7CInternalLinks%28%7C2%29%29%7CgetVariableValue%7CinitialiseVariables%7CformatHeadings%7Ctest%28Pst%7CPreprocess%7CSrvus%29%7CdoQuotes%7CgetExternalLink%28Rel%7CAttribs%29%7CnormalizeLinkUrl%7C%28brace%2Carg%2Cextension%29Substitution&i=nope&files=&repos=
Bug: T236810
Change-Id: I19a43ffc5dcfdd2981b51079c33422c964acb076
Add public, protected or private to function missing a visibility
Enable the tests folder for the phpcs sniff
Change-Id: Ibefce76ea9984c47e08c94889ea2eafca7565e2c
This removes most of the pre-actor user and user_text columns, and the
$wgActorTableSchemaMigrationStage setting that used to determine
whether the columns were used.
rev_user and rev_user_text remain in the code, as on Wikimedia wikis the
revision table is too large to alter at this time. A future change will
combine that with the removal of rev_comment, rev_content_model, and
rev_content_format (and the addition of rev_comment_id and rev_actor).
ActorMigration's constructor continues to take a $stage parameter, and
continues to have the logic for handling it, for the benefit of
extensions that might need their own migration process. Code using
ActorMigration for accessing the core fields should be updated to use
the new actor fields directly. That will be done for in a followup.
Bug: T188327
Change-Id: Id35544b879af1cd708f3efd303fce8d9a1b9eb02
100% test coverage of code that appears to be working and used, in both
LockManagerGroupFactory and also LockManagerGroup. Where possible I
wrote it as unit tests. One preexisting code path seems to be broken and
I marked the test as skipped. Two methods look unused and perhaps not
especially helpful, so I didn't write tests for them yet in case we want
to just get rid of them instead.
Change-Id: Iaa7354f31c451b87773468609c674a3bf1d4382f
After I0d7dacee3, wfGetDB returns a connection reference instead of
a real connection, which means it can't be passed to CloneDatabase
anymore, and testcase / test runner classes should avoid it.
Bug: T228928
Change-Id: I42be4214e33a4d4f8d53fafc925c8d708b4c0106
This can be used to avoid double parsed on save if the prior output
can be reused in-spite of involving a self content reference.
Change-Id: Idcd30a3fa3f7012dac76ce8bbf46625453ae331f
The Preprocessor_DOM implementation doesn't interact well with PHP memory
profiling, and has some limitations not present in the Preprocessor_Hash
implementation (see T216664). There is no reason to keep around two
versions of the preprocessor: it just complicates on-going wikitext
feature development.
Hard deprecate use of Preprocessor_DOM, so we can remove the redundant
code in a future release.
Bug: T204945
Depends-On: Id38c9360e4d02b570996dbf7a660f964f02f1a2c
Change-Id: Ica5d1ad5b1e677542962fc36d582a793f941155e
MWNamespace::clearCaches() has been removed entirely, along with the
$rebuild parameter to MWNamespace::getCanonicalNamespaces(). The rest of
MWNamespace is deprecated.
Diff best viewed with -C1 so git notices that NamespaceInfo is a copy of
MWNamespace.
Depends-On: Icb7a4a2a5d19fb1f2453b4b57a5271196b0e316d
Depends-On: Ib3c914fc99394e4876ac9fe27317a1eafa2ff69e
Change-Id: I1a03d4e146f5414ae73c7d1a5807c873323e8abc
This allows us to break a circular dependency in the patches for T208070.
It will be reverted as soon as T208070 is merged.
Bug: T208070
Change-Id: I3da235cb83efa424f0cf1cf4fc7233240fcdf6b2
It's a temporary feature flag not included in any release, just
removing it outright. The functonality will now be always enabled.
Bug: T205040
Change-Id: Ia9da82e6f6b2d270f1790a99fc8c35ad5e6aee5e
This removes most of the pre-CommentStore text columns, and the
$wgCommentTableSchemaMigrationStage setting that used to determine
whether the columns were used.
rev_comment remains in the code, as on Wikimedia wikis the revision
table is too large to alter at this time. A future change will combine
that with the removal of rev_user_text, rev_content_model, and
rev_content_format (and the addition of rev_comment_id and rev_actor).
CommentStore's constructor continues to take a $stage parameter, and
continues to have the logic for handling it, for the benefit of
extensions that might need their own migration process.
Bug: T166733
Change-Id: I1479c73774e01ead1490adf6128f820c09bce9d4
Remex is pure PHP so there is no reason to use an external tidy any
more. Configuration variables and implementation classes were
deprecated in 1.32 or earlier. We've kept only $wgTidyConfig
which can be used for experimental features or debugging Remex.
Bug: T198214
Change-Id: I99d48f858d97b6e1d1e6cd76a42c960cc2c61f9f
Future parsers will not support the output generated with tidy disabled.
Parser tests using untidied output will also be deprecated (and
rewritten) in a follow-up patch.
No new release notes necessary since user-visible tidy configuration
was deprecated previously (in 1.32), and individual methods which had
disabled tidy during execution were individually release-noted as they
were updated.
Bug: T198214
Depends-On: I0f417f75a49dfea873e9a2f44d81796a48b9f428
Depends-On: If5c619cdd3e7f786687cfc2ca166074d9197ca11
Change-Id: I592e0e0dfef7d929f05c60ffe4d60e09725b39cc
For historical reasons, we used a complicated test to try to determine
if a test is "parsoid only" based on the presence of the string "parsoid"
in the parser test options clause. But really test clauses should be
marked up specifically as html/php or html/parsoid, we don't need to
play hacky games any more. Remove some unnecessary code and simplify
the test logic.
Change-Id: Ia331e7e12a75aa19aeb96601712f169f2aa75bf9
Previously, they were always displayed in defult language unless
forced explicitly in wikitext, e.g. [[File:Foo.svg|lang=ru]].
This change adds a feature flag that would enable always trying to
display in page language.
* If enabled, Parser will pass a new parameter - 'pagelang' - to
the media handler.
* SvgHandler uses page language when determining what language to
render the image in.
* 'pagelang' can always be overridden by 'lang'.
* If no translation in page language is available, the default
language (English) will be used for thumbnail URLs, to prevent
cluttering media storage and HTTP caches with useless copies.
Performance: this requires accessing image's metadata during parsing.
My testing indicates there were no code path where this wasn't the
case already, so no performance hit is expected, however we should
still keep an eye on page save performance.
Bug: T205040
Change-Id: I348840ef405e1370cc0c17d69051bce30153c9c0
