Use the stricter comparisons from assertSame instead of
assertEquals.
In some cases this revealed a test that was asserting a value
different from the one actually returned:
* testSetAndGet:
The test failed with the test case (null, 3), where it tries
to store the value `null`, but gets back `false`, which is wrong.
This appears to be a genuine bug, filed as T234583.
Change-Id: Ic3dcc7fa2e8749b0f2d68917a8ac728dda26b6ca
It's a bit late given that just earlier this week we removed
almost all uses of `@embed` (T121730). So as an optimisation, this
won't have much impact now.
Bug: T234582
Bug: T121730
Change-Id: I5729c8b059504244ac7fb0b515df9c29909b9d5e
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
The benefit of using count() is that the test would still succeed if
the return vfalue is not an array, but an iterable object. It seems
this is not needed.
Change-Id: I23529f6990aebe0cce86e236a21820fe74993204
assertEquals( null, … ) still succeeds when the actual value is 0, false,
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( null ). 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. assertNull()
and assertSame( null ) are functionally identical.
Change-Id: I92102e833a8bc6af90b9516826abf111e2b79aac
In masterPosWait(), only $pos will have the known active domain/server set
since it usually comes from getMasterPos(). However, the reference position,
from getReplicaDB(), does not have the active domains set since querying
gtid_domain_id on the replica would be incorrect and getting a connection
to the master could be expensive.
Remove obsolete hacks for jobs that used to store master positions.
Also, use the regular Database::query() method for stylistic consistency.
Bug: T224422
Change-Id: I41bbb9f337e46451aa17788dbd446db4a213a5a7
These type hints make it easier to follow the code paths and understand
what code is actually under test here. A few of these tests appear to
be misplaced, as they don't cover any code in the DatabaseMysqlBase
class.
This also makes all @covers tags absolute so more tools can understand
them (e.g. PHPStorm doesn't).
Change-Id: I43c69be994d6094239954ea662801c1e7e8e2e41
assertSame() is guaranteed to not do any type conversion. This can be
critical when acciden tially comparing, for example, 0 to 0.0.
Change-Id: Iffcc9bda69573623ba14af655dcd697d0fcce525
There's no function to check if a string is a valid regex, and doing it
via preg_match requires error suppression, often performed with @. This
isn't however a good practice, and suppressing errors via functions is
not a one line solution. Adding an utility function would probably be a
benefit for several use cases (try to grep '@preg_match' for a few of
them).
Change-Id: I257a096319f1ec13441e9f745dcd22545fdd5cc6
Enforce this pattern for the remaining LoadBalancer methods.
Carry this over into Database::close() to decide how loud the
error handling should be.
In LBFactory, clean up ownership of newMainLB()/newExternalLB().
The should have a null owner if called from outside the class
since the LBFactory does not track nor care about them anymore
in that case. Disable newMainLB() for LBFactorySingle as it
makes no sense and was broken.
Also remove some redundant abstract LBFactory methods that
just duplciate ILBFactory.
Bug: T231443
Bug: T217819
Depends-On: I7ed5c799320430c196a9a8e81af901999e2de7d0
Change-Id: I90b30a79754cfcc290277d302052e60df4fbc649
Among other things, this removed mention of MediaWiki classes from the
library and adds a README.md that attempts to define constraints for
interoperability between libraries producing MessageValues and formatter
implementations that are expected to handle them.
This also renames "TextParam" to "ScalarParam", as that seems a more
accurate name for the class.
Change-Id: I264dd4de394d734a87929cf4740779e7b7d0e04a
An injectable service interface for message formatting, somewhat
narrowed compared to Message.
Only the text format is implemented in this framework so far, with
getTextFormatter() returning a formatter that converts to the text
format. Other formatters could be added to MessageFormatterFactory.
Bug: T226598
Change-Id: Id053074c1dbcb692e8309fdca602f94a385bca0c
Make LoadBalancer::reallyOpenConnection() handle initializing DBO_TRX
instead of Database::__construct().
Also:
* Avoid having the "catch" block appear like it returns a
half-constructed Database.
* Use the variable name $conn instead of $db to be consistent
throughout the class. Only send Database::__construct() parameters
that it recognizes instead of mixing in setLBInfo() data.
Change-Id: Iffc3d1d0713051a164adb51a4c4ee12e4ac887c3
Make the default $init value for incrWithInit() be $value.
This is far less suprising and also makes the operation
easier to replicate without conflicts.
Make decr() definitions more explicit since various cache
drivers do not handle negative incr() values (e.g. memcached).
Change-Id: I2b8d642656cc91c841abbd7a55d97eba101b027a
Add MediumSpecificBagOStuff::getValueOrSegmentList() helper method.
Also:
* Use $keysMissing variable correctly in CachedBagOStuff::getMulti()
to avoid extra overhead.
* Optimize mergeViaCas() when the current value matches the new one.
Change-Id: I5c4bd74379bc459216ac0278150ce3aecff3b851
These are in preparation for making a TempFSFileFactory service, thus
the odd break-up into two files. I split it into a separate commit so
that we could verify that the same tests pass before and after the
conversion to service.
Tests cover everything except getUsableTempDirectory() (which I don't
see how to test), and register_shutdown_function()-related stuff (which
seems actually impossible to test without starting a new PHP process).
Change-Id: If61b7ea3e332adc2bceefc8e6879a9e9443c99dd
This will throw when trying to create a service while already in the
process of creating that same service, i.e., if there's a circular
service dependency. This would have saved me a whole bunch of debugging
time. :)
Change-Id: Id148d4f221f35f4069f3e0ab0069d13ca271df3d
After approval of RFC T191231, we are going to drop oracle and mssql
and it will be possible to bring back the support using the abstract schema
Adding to release notes will be done in a follow-up
Bug: T230418
Change-Id: I90bd5cfcc3e18011b193c965fdb1fa54675040b5
Partly a follow-up to 88640fd902.
Use real time in changeTTL() tests to fix all remaining
failures for BagOStuff sub-classes.
Change-Id: I537d665d6c8770a68a5a79233a913f1714881dfb
Use real time for testing absolute expirations with changeTTL().
Otherwise, backends like memcached or redis will fail since
they do not use the mock time.
Also:
* Make SqlBagOStuff actually override changeTTLMulti() by
using the right method name
* Check TTL_INDEFINITE more explicitly for clarity
* Rename TTL conversion methods for clarity
* Use isRelativeExpiration() in MemcachedBagOStuff
Change-Id: I9365ceb31d4e7bef65906363d42b8c3020a66346
Mainly:
* Use oci_new_connect() for Oracle to avoid broken connection reuse
similar to the PGSQL_CONNECT_FORCE_NEW flag in DatabasePostgres
* Set 'client_min_messages' unconditionally for PostgreSQL
* Factor out Database::getConnectExceptionAndLog() helper method
* Use the same style of query() calls in DatabaseOracle::open() as
the other subclasses
* Make sure the Database driver handle field is null on failure
instead of false for sanity
Also:
* Disallow changing of Database handle DBO_* flags after construction
where it does not make sense to change them
* Do not mention DBO_* flags meant for non-config use in $wgDBservers
* Ignore DBO_PERSISTENT for SQLite if DBO_TRX is also set for sanity
* Remove $wgDBOracleDRCP variable to discourage careless automatic
setting of DBO_PERSISTENT that breaks LoadBalancer assumptions
Change-Id: Iea948f7f872294ea8fc5d897fc10c9d29b7141d5
unpack() actually returns an array with indexes starting from 1, not
zero, so unpack(...)[0] gives a notice and always returns null. It is
lucky that ZIPs normally have zero-length comments, so this would have
had little impact on file type detection aside from log spam.
Also, add a check to make sure the unpack() will not read beyond
the end of the file. Without this, unpack() could generate a warning.
The bug was introduced by me in f12db38048.
Add tests. The test files were generated by appending an EOCDR signature
and some extra bytes to 1bit-png.png.
Bug: T223728
Change-Id: I6fab63102d1d8eea92cdcce5ab6d1eb747a0a890
