Before a test is started there are no services, there is no need to
reset services in setUp().
After a test finished all created services are destroyed by
MediaWikiIntegrationTestCase and does not need own teardown.
overrideConfigValue() also resets services
Change-Id: I41c71866ec6d93cd2b7e3dd45269e0972ed6a273
Changes to the use statements done automatically via script
Addition of missing use statement done manually
Change-Id: Iae45fa269363be8ee05c598ea6926514ce817762
Before this method name gets baked forever into the 1.42 release, rename
the ParserOutput::setIndexedPageProperty() and ::setUnindexedPageProperty()
methods to ::setNumericPageProperty() and ::setUnsortedPageProperty() to
try to address some confusion about whether the *presence* of the page
property is still indexed (it is!), in contrast to whether there's an
additional "sort key" associated with the *value* assigned to the page
property.
This naming is compatible with the feature request in T357783 to have
the sort key and property value specified independently. The new
method signature in that case would be:
...setSortedPageProperty( string $name, string $value, int|float $sortKey )
Although PHP 8.0 will throw a TypeError if a non-numeric type is coerced
to numeric using `0 + ...`, use an explicit is_numeric check to obtain
the same behavior in PHP 7.x.
Change-Id: Ia94c192c429d0482c58467bed787fd2e0aca052f
Deprecate non-string values to ::setPageProperty(), which introduce easy
traps for programmers to fall into. Instead if page properties are intended
to be indexed, use the new ::setIndexedPageProperty() instead. Also add
::setUnindexedPageProperty() for symmetry, with a tighter string type on
the value.
Bug: T305158
Bug: T350224
Change-Id: I8a39a7c90341dfee932aa819c9a0a637a8782f69
Use the same strategy for setting up interwiki links for the
LinksUpdateTest that is used in ParserTestRunner. This is more
robust against service initialization order and doesn't import
unrelated interwiki prefixes from the host wiki during testing.
Change-Id: I6b2141ea8299fc8ca3db996104fbafff68276c55
Non-scalar values passed to ParserOutput::setPageProperty() have never
"worked"; they've been stringified (and null has been stored as an empty
string). Emit a warning so we can fail harder in future releases.
Bug: T305158
Depends-On: Ib36787d04c0ca713587dc8b814ca1c5a827f6f72
Change-Id: I38234084fdc7427ca577bb33a7fce1541581188d
Update cases where one of the IConnectionProvider methods is called
immediately.
This doesn't really change anything, but I hope it helps promote
getConnectionProvider() as the common way to do this.
Follow-up to 8604c384f6.
Change-Id: Id0e7d02bab0c570343c2b1f03c70b44ee39db112
Broadened the argument type to allow passing LinkTarget to:
* ParserOutput::addCategory()
* ParserOutput::addLanguageLink()
* ParserOutput::addLink()
* ParserOutput::addImage()
* ParserOutput::addTemplate()
This allows for a tighter interface with Parsoid's
ContentMetadataCollector class and avoids errors caused by passing the
wrong form of string title ("text" with spaces versus "dbkey" with
underscores).
There are a few performance problems remaining after this patch, which
only apply to use by Parsoid (not the legacy parser):
1. ::addLink() does inefficient db requests to fetch the page id for
each link if the optional $id parameter is not passed. These lookups
should be deferred and a LinkBatch used. (The legacy parser always
passes $id.)
2. ::addTemplate() similarly requires $page_id (and $rev_id) to be
passed, so is not currently usable by Parsoid.
3. ::addLanguageLink() uses Title::getFullText() which is not present
in LinkTarget and is currently implemented as a full Title lookup.
This is not an issue for the legacy parser, because it already has a
Title object so the lookup is a no-op, but could be improved for
Parsoid's use.
Bug: T296023
Change-Id: If21ec8563c8a619bdde7c0cb6534bb9009480a21
Using LinkFilter functions can repair some values, but that would never
removes invalid values from the externallinks table.
When LinkFilter get bug fixes on how to handle domain or path there is
no way to refresh the links on the externallinks table (even not with
refreshlinks.php) as it compares the url rebuilded from the database
with the value from the parser. But rebuilding the url from database
must not match, when reversing the url to find back the database row.
Just use the raw values internally.
Bug: T350476
Change-Id: I60e61c229c6923427585a445a08e6130d265519d
This was done automatically using the
`Universal.WhiteSpace.CommaSpacing` sniff, which will be included in the
next release of the MW PHPCS config.
Some of these have been adjusted manually where the autofix broke
vertical alignment.
Change-Id: I54a4668d8a2759b9d7de47742c943a535a04e211
Already dropped from production
Also dropping FixExtLinksProtocolRelative as it's not useful anymore and
it has been run in previous releases so it's not worth fixing.
Bug: T312666
Change-Id: I1dd6e704b34e685ada6e316da11243d10827d769
Follows-up I98d2ecc987, I7555c9b6b5, I6d845bdfbb, I69b5385868, I4c7d826c7e,
I1287f3979a, which widened the `@covers` annotations of other suites:
> We lose useful coverage and spend valuable time keeping these tags
> accurate through refactors (or worse, forget to do so).
>
> I've audited each test to confirm it is a general test of the
> subject class, where adding any called methods would be an accepted
> change, thus widening it is merely a no-op that clarifies intent
> and reduces maintenance. I am not disabling the "only track coverage
> of specified subject" benefits, nor am I claiming coverage in
> in classes outside the subject under test.
>
> Tracking tiny details per-method wastes time in keeping references
> in sync during refactors, time to realize (and fix) when people
> inevitably don't keep them in sync, time lost in finding uncovered
> code to write tests for only to realize it was already covered but
> not yet claimed, etc.
Change-Id: I133c7b707aab7ceb4f2ecd3be38bd4bd1b194143
* Add MediaWikiIntegrationTestCase::newSelectQueryBuilder(), which
creates a subclass of SelectQueryBuilder with assert methods.
* Migrate most callers of assertSelect() to this new query builder
interface.
Bug: T311866
Change-Id: I7392b37988067020d5f684276320dae0a474631a
We renamed many classes to be namespaced, but the `@covers` and
`@coversDefaultClass` annotations weren't updated properly.
PHPUnit didn't support short cover annotations with `use` statements,
these didn't trigger any errors yet, because they are class alias.
This patch is populated by a modified version of PhpunitAnnotationsSniff.
Change-Id: I6c602290a30099239b17d2dc0d67b1488b4eaeeb
Notable changes:
* In SqlBagOStuff::getConnectionFromServerInfo, only two loggers were
injected. The rest implicitly got a NullLogger due to being absent.
These are now effectively unsilenced.
* Database::__construct() required almost all parameters, even the
loggers. I've wanted to move some of DatabaseFactory into the ctor
here for a while. In order to make this change not a breaking
change, the new 'logger' parameter is optional with NullLogger as
default. This allowed some of the test cases, which were simply
passing NullLogger, to be fixed by passing nothing instead of
passing the new option name.
The Database class is behind a dozen layers of indirection for
real use, so this will still be injected just fine (DBF, LB, LBF,
MWLBF, etc.).
* In LegacyLogger, the handling for $wgDBerrorLog was previously
limited to DBConnection and DBQuery. This now includes errors
from other (generally, newer) parts of Rdbms as well, which were
previously missing.
This only affects sites (typically CI and dev setup) where
$wgDBerrorLog is used, as opposed to the more common
$wgDebugLogGroups by-channel configuration.
* TransactionProfiler gets its logger injected in a rather odd way,
via entrypoints (MediaWiki.php, ApiMain.php, and MaintenanceRunner)
as opposed to service wiring. This is kept as-is for now.
* In LBFactoryTest, in particular testInvalidSelectDBIndependent2,
there are cases that intentionally produce failures of which
the result is then observed. In CI we assert that dberror.log is
empty so instead of adding the missing logger fields to that
LBFactory instance, the only one set (replLogger) is removed.
The alternative is to set 'logger' now, which would naturally
cause CI failures due to unexpected entries coming through to
non-mocked error log.
Bug: T320873
Change-Id: I7ca996618e41b93f488cb5c4de82000bb36e0dd3
The day has gone. Still keeping the code as the schema changes are not
done in production but the data migration has been finished.
Bug: T299417
Change-Id: I906e069a63d1dae14924c72318b22b16244371d6
This is a quick find & replace of calls to the deprecated method
ParserOptions::newCanonical() when the context is the string literal
'canonical'. This can be safely replaced by called newFromAnon().
Change-Id: If7bb68459b11e0c5f5de188f10fdae85ad1a78bf
When fetching a key from an array, PHP converts numeric strings to
integers. This led to an incorrect category update query.
So:
* In LinksTable subclasses, cast all string-like array keys to string.
* Add a unit test which confirms that no integers leak into link IDs.
* Add a fully integrated regression test for T301433.
The integration test and 7 of the link ID cases are confirmed to fail
if Amir's patch is reverted.
Bug: T301433
Change-Id: I8d19443607121b3efcafb82096bcff18c41035df
* In PagePropsTable, compare values after converting to string. In the
old code this was implicitly done by array_diff_assoc().
* Also convert to string before inserting, since
ParserOutput::setPageProperty() is documented as allowing null, but
the field is not nullable, so the query failed.
* Add a regression test which confirms that no write queries are done on
the second invocation of LinksUpdate.
Bug: T299662
Change-Id: I3f7d5800bf27a47cb57be80bdba13792fe3caae6
Running cat_pages + 1 and cat_pages - 1 for the same rows/categories is
uneeded.
Cannot skip the whole update on move, because by use of parser functions
there could be new categories after cross namespace page moves.
Follow-Up: Ifaa4051a736b01cda82fbd890325e0751ce041f8
Change-Id: I188d0224f1a1d43746f657159e0ed8c80b8646cd
Since LinksUpdate was already run after a page move, it was just a
matter of propagating a move flag down to it, and resolving the
differences between the move case and the edit case.
This should be faster since it avoids writing to the links tables twice
on a page move.
Change-Id: Ifaa4051a736b01cda82fbd890325e0751ce041f8
* In LinksUpdate, put each table in its own class, so that the
special code for each table has a more elegant place to go.
* Use generators to abstract and flatten out the underlying ParserOutput
data structures, without requiring the data to be copied. This allows
the incremental logic which was previously duplicated for each table,
i.e. computing insertions and deletions from the old and new state,
to be factored out. The per-table batch classes act as container
abstractions for the underlying data.
Bug: T263437
Depends-On: I472f4a023969bfde6298eb56112c16d2ae842199
Change-Id: Id40be19a8caa7b4669911927964cfd6755c25238
Each table at least needs an added, deleted and unmodified link in order
to fully exercise the incremental update code.
Disable INSERT IGNORE during testing, since this can hide an omitted
delete query.
Change-Id: Iaa9f32ea5171da4f983379d7ff313bfdf1cb0b2f
The ::getProperty() naming is too generic and doesn't clearly indicate
that these are "page properties" (which have their own table in the DB).
As part of refactoring a clean API out of ParserOutput which can be used
by Parsoid, clean up the naming here.
Soft-deprecation in this patch, there are a handful of external users
which need to be cleaned up before we hard-deprecate.
Bug: T287216
Change-Id: Ie963eea5aa0f0e984ced7c4dfa0fd65d57313cfa
