Commit graph

38 commits

Author SHA1 Message Date
James D. Forrester
4f2d1efdda Coding style: Auto-fix MediaWiki.Classes.UnsortedUseStatements.UnsortedUse
Change-Id: I94a0ae83c65e8ee419bbd1ae1e86ab21ed4d8210
2020-01-10 09:32:25 -08:00
James D. Forrester
5e9fca47b9 Coding style: Auto-fix MediaWiki.Usage.PHPUnit*
Change-Id: I86fc55a4fc8ceafe368692173211bbcd6d8581d7
2020-01-10 10:17:12 +00:00
daniel
2a763bfc67 DatabaseTest: fix provider for testIsWriteQuery
testIsWriteQuery() never worked. It was using a non-existing provider,
which caused the test to be skipped with a warning. Access to the
protected method under test also needed fixing.

Change-Id: If35789027f2d6c44a7079e68a1c07c86be72d42b
2020-01-07 21:39:39 +01:00
Daimona Eaytoy
29156b4c84 rdbms: Account for leading spaces in isWriteQuery
Bug: T241034
Change-Id: I313e81dddeb9faef1780c9780fb575a5f8376bf4
2019-12-30 21:24:06 +00:00
Daimona Eaytoy
7c9e3db1e6 Fixes for PHPUnit 8 compatibility
Bug: T192167
Change-Id: Ic14f5debc53e55d67146dc96279d26dfd52b4000
2019-12-10 17:02:06 +00:00
Tim Starling
673d496f2d Have Database::addQuotes() pass through bare integers without quoting
Quotes started being added to integers in r4984 (August 2004). Before
that, is_numeric() was used to determine whether to add quotes, so
quotes were omitted from numeric strings, which is obviously wrong.

The idea here is to use the type of the variable to hint to the database
as to whether quotes are needed. The results are somewhat inconsistent,
since some callers do not convert numeric strings obtained from user
input to integers. That makes it a more conservative change. Callers can
opt out of unquoted integers by casting them to string.

The reason for doing this is that quoting integers turns out to be not
as harmless as originally assumed. We found a case of it confusing the
MariaDB query planner, causing inappropriate indexes to be used.

I also made addQuotes() consistently return a string, instead of
returning an integer for boolean values. This was already the case for
MySQL, but it seems like a good idea everywhere.

Bug: T238378
Change-Id: I70473280f542ee5ecd79e187f580807410fbd548
2019-11-18 11:40:28 +11:00
Max Semenik
48a323f702 tests: Add explicit return type void to setUp() and tearDown()
Bug: T192167
Depends-On: I581e54278ac5da3f4e399e33f2c7ad468bae6b43
Change-Id: I3a21fb55db76bac51afdd399cf40ed0760e4f343
2019-10-30 14:31:22 -07:00
James D. Forrester
83d76f4cb5 phpcs: Enable MediaWiki.Commenting.PhpunitAnnotations.ForbiddenExpectedException* and make pass
Change-Id: I63f97497714a32236268be6965c5e181dade6c58
2019-10-14 12:48:48 -07:00
Thiemo Kreuz
e4272518f7 tests: Replace PHPUnit's loose assertEquals(false) with assertFalse()
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
2019-10-04 00:30:36 +00:00
Thiemo Kreuz
32a429e8c4 tests: Prefer assertSame() when comparing the integer 0
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
2019-09-19 15:35:23 +00:00
Amir Sarabadani
4d10bb14e8 Drop Oracle and Mssql
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
2019-08-14 11:31:41 +00:00
Aaron Schulz
023c73f612 rdbms: normalize Database open() code and error handling
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
2019-07-26 15:24:28 +01:00
Aaron Schulz
ef38199422 rdbms: fix IDatabase::setLBInfo() handling of null and allow clearing keys
Change-Id: I20cb799b54cabb1172940f8ece93b7f45d7cf0ba
2019-07-15 03:56:16 +00:00
Legoktm
4e35134f7a Revert "Separate MediaWiki unit and integration tests"
This reverts commit 0a2b996278.

Reason for revert: Broke postgres tests.

Change-Id: I27d8e0c807ad5f0748b9611a4f3df84cc213fbe1
2019-06-13 23:00:08 +00:00
Máté Szabó
0a2b996278 Separate MediaWiki unit and integration tests
This changeset implements T89432 and related tickets and is based on exploration
done at the Prague Hackathon. The goal is to identify tests in MediaWiki core
that can be run without having to install & configure MediaWiki and its dependencies,
and provide a way to execute these tests via the standard phpunit entry point,
allowing for faster development and integration with existing tooling like IDEs.

The initial set of tests that met these criteria were identified using the work Amir did in
I88822667693d9e00ac3d4639c87bc24e5083e5e8. These tests were then moved into a new subdirectory
under phpunit/ and organized into a separate test suite. The environment for this suite
is set up via a PHPUnit bootstrap file without a custom entry point.

You can execute these tests by running:
$ vendor/bin/phpunit -d memory_limit=512M -c tests/phpunit/unit-tests.xml

Bug: T89432
Bug: T87781
Bug: T84948
Change-Id: Iad01033a0548afd4d2a6f2c1ef6fcc9debf72c0d
2019-06-13 22:56:31 +02:00
Aaron Schulz
b5c0e927a4 rdbms: add another null db/schema sanity check to DatabaseDomain
Change-Id: Id75a47a3dee97b43c586faf06208dffcc30c9e6e
2019-03-28 19:18:56 -07:00
Aaron Schulz
c4e284f113 rdbms: codify DatabaseDomain table "_" prefix convention
Alos simplify isCompatible() slightly and make the string
case in convertToString() explicit.

Change-Id: Ifb61bb5fb012491520525bbebfbde2269fa55b52
2019-03-26 21:04:51 +00:00
Brad Jorsch
c5a5b02240 Database: Allow selectFieldValues() to accept SQL fragments
The documentation says "This must be a valid SQL fragment", but as
written it breaks if given anything other than a field name. It's easy
enough to fix by adding an alias to the internal select() call.

Bug: T201781
Change-Id: I76428af6d3aadc266254fdb24109a0ac2db3761f
2018-10-17 22:21:40 +00:00
Aaron Schulz
fe0af6cad5 rdbms: Database::selectDB() update the domain and handle failure better
LoadBalancer uses Database::getDomainId() for deciding which keys to use
in the foreign connection handle arrays. This method should reflect any
changes made to the DB selection.

If the query fails, then do not change domain field. This is the sort of
approach that LoadBalancer is expects in openForeignConnection(). Also,
throw an exception when selectDB() fails.

The db/schema/prefix fields of Database no longer exist in favor of just
using the newer currentDomain field.

Also:
* Add IDatabase::selectDomain() method and made selectDB() wrap it.
* Extract the DB name from sqlite files if not explicitly provided.
* Fix inconsistent open() return values from Database subclasses.
* Make a relationSchemaQualifier() method to handle the concern of
  omitting schema names in queries. The means that getDomainId() can
  still return the right value, rather than confusingly omitt the schema.
* Make RevisionStore::checkDatabaseWikiId() account for the domain schema.
  Unlike d2a4d614fc, this does not incorrectly assume the storage is
  always for the current wiki domain. Also, LBFactorySingle sets the local
  domain so it is defined even in install.php.
* Make RevisionStoreDbTestBase actually set the LoadBalancer local domain.
* Make RevisionTest::testLoadFromTitle() account for the domain schema.

Bug: T193565
Change-Id: I6e51cd54c6da78830b38906b8c46789c79498ab5
2018-10-10 12:03:30 -07:00
Aaron Schulz
5358c41e86 rdbms: use Database::getDomainId for transaction logging calls
Change-Id: I360ca633b145dc8e510bb00b542ac44933283943
2018-08-15 01:29:39 -07:00
Aaron Schulz
d16bfb0a30 rdbms: fix Sqlite::tableExists() method to avoid STATUS_TRX_ERROR
Sqlite used the base implementation of trying a SELECT 1 query and
seeing if it failed. Instead, make it use the sqlite_master table.
Also remove the base version of that method since it would always
cause this problem and all subclasses have proper implementations.

Make LoadBalancerTest::assertWriteAllowed() more explicit and add
more assertions there.

Change-Id: I6c7b0bea8894c45dfe8931748d6687f0e5d1e101
2018-06-22 22:33:45 +00:00
Aaron Schulz
8880a25112 rdbms: make runOnTransactionIdleCallbacks() reset DBO_TRX on exceptions
Change-Id: Ibbb2a3ebf9dd970772ee704aa643a3843f20a3b5
2018-05-25 23:40:47 +00:00
Aaron Schulz
b6cd5421b9 rdbms: rename onTransactionIdle() to onTransactionCommitOrIdle()
This is clearer and is consistent with onTransactionPreCommitOrIdle()

Change-Id: I3a34a0e9adea69ec55ed6ddfef47703e31e7c3b5
2018-05-09 21:07:06 +00:00
Aaron Schulz
5bfa77f8d9 rdbms: enforce and improve LBFactory/LoadBalancer callback handling
* Handle the case where an onTransaction* callback for one handle
  adds more onTransaction* callbacks to a different handle. Instead
  of supporting only a short chain of such callbacks, try to resolve
  the whole chain by using a loop in LoadBalancer and LBFactory.
* Add sanity checks to enforce the proper call order of LoadBalancer
  transaction methods, such as those that execute callbacks. This is
  the order that LBFactory already uses. Use ROUND_ERROR for problems
  that can ruin the instance state. Such problems require rollback.
* Correct setTrxEndCallbackSuppression() calls in beginMasterChanges()
  that were making tests fail.
* Make Database handle callback suppression for FLUSHING_ALL_PEERS
  instead of making LoadBalancer/LBFactory have to manage it.
* Simplify finalizeMasterChanges() given that suppression does not
  actually effect runOnTransactionPreCommitCallbacks().
* Make dangling callback warning in Database::close work properly.
* Actually use $fname in flushReplicaSnapshots().
* Use DBTransactionError instead of DBExpectedError in some places
  where stages fail.
* Fix failing testGetScopedLock() unit tests so everything passes.

Add more comments to setTransactionListener and onTransactionIdle.

Change-Id: I6a25a6e4e5ba666e0da065a24846cbab7e786c7b
2018-04-30 21:55:25 +00:00
Aaron Schulz
c8085ad43f rdbms: make IDatabase::onTransaction* methods pass the DB handle for convenience
Change-Id: Ia45a26830d62326b103593268fbf34c907783c90
2018-04-24 16:45:11 -07:00
Aaron Schulz
20777d7399 rdbms: make cancelAtomic() handle callbacks and work with DBO_TRX
Make transaction callbacks aware of cancelled sections. If the
statements of a section are reverted via cancelAtomic(), then the
dependant callbacks are now cancelled as well. Any callbacks for
onTransactionResolution(), which does not depend on COMMIT, will
see the triggering event as a ROLLBACK, since the unit of work it
was part of was rolled back.

Also fix the handling of topmost atomic sections with DBO_TRX.
These still need their own savepoint to make cancelAtomic() work.

Follow-up to 52aeaa7a5.

Change-Id: If4d455c98155283797678cfb9df31d5317dd91a2
2018-04-19 12:43:01 -07:00
Aaron Schulz
d4c31cf841 rdbms: clean up DBO_TRX behavior for onTransaction* callbacks
* Make onTransactionIdle() wait until any transaction round
  is gone, even if there is no SQL transaction active. This
  is what onTransactionPreCommitOrIdle() already does.
* Decouple "transaction round mode" (DBO_TRX) from whether a
  round is active via a 'trxRoundId' LB info field. If rounds
  are enabled, but not is started, then the transaction state
  should be interpreted as "idle".
* Improve related documentation.
* Add more related unit tests.

Change-Id: I3ab18f577ec0375897fcb63f18f4ee2deeb436e9
2018-04-03 23:16:45 -07:00
Aaron Schulz
b0ee0a8d9f rdbms: clean up DBO_TRX behavior for onTransactionPreCommitOrIdle()
* Make sure cancelled onTransactionPreCommitOrIdle() callbacks do not
  run if a transaction round is rolled back and then a second round is
  committed. LoadBalancer::rollbackMasterChanges() now always calls
  rollback(), which in turn always cleans up such callbacks.
* Remove error logging for rollback() calls when trxLevel = 0; this is
  harmless and is sometimes hard to avoid in error handling anyway.
* Add more related unit tests.

Change-Id: I6bdefe8bf8b6630fc252b5bbafe4808758ba1684
2018-03-20 01:11:24 +00:00
Aaron Schulz
434d5a6321 rdbms: allow construction of Database objects without connecting
* Database::factory() supports a $connect parameter, that defaults
  to NEW_CONNECTED (current behavior) but can also be NEW_UNCONNECTED.
* Add tests asserting the type of various instances returned from
  Database::factory().
* Clean up sqlite "conn" field handling to handle cases of it
  not being set, just as other classes do.
* Add some comments about the return type of doQuery().

Change-Id: Ic0837cfdb35326c2045133d664abd29043d48c03
2018-03-08 19:47:35 -08:00
Timo Tijhof
a211db0f73 rdbms: Restore test for Database::setFlag()
Follows-up b4eb1feed0, which inadvertendly replaced the setFlag()
test with the clearFlag() test.

Also move the test to the DatabaseTest.php file given it's only
executing and covering base class.

Change-Id: I2f4ed6c4eeba845eb67013e1ab7d2b2bde863119
2018-03-08 04:32:21 +00:00
Umherirrender
63d96c15fd build: Updating mediawiki/mediawiki-codesniffer to 16.0.0
Change-Id: I59b59f79bbf3ce4feff3b3a20c1c31bc16370531
2018-02-17 13:29:13 +01:00
Aaron Schulz
c9ad7037ce rdbms: do not bother making DBO_TRX transactions in IDatabase::lock()
Named locks are session-level constructs and this transaction agnostic.
Also make lockIsFree() a bit more consistent when the thread has the
lock itself.

Change-Id: Ief51196161bbc50c798740f3c738fd0e39880508
2018-02-15 16:32:35 -08:00
Aaron Schulz
6237fd11b6 rdbms: make affectedRows() work more consistently
* Update replace()/upsert() to combine the affected row
  count for the non-native case
* Also make replace() atomic in the non-native case,
  similar to how upsert() already works

Change-Id: I6c9bcba54eca6bcf4a93a9b230aaedf7f36aa877
2018-01-30 20:02:07 -08:00
Kunal Mehta
75160bdd3b Use MediaWikiCoversValidator for tests that don't use MediaWikiTestCase
Change-Id: I8c4de7e9c72c9969088666007b54c6fd23f6cc13
2018-01-01 08:28:02 +00:00
Brad Jorsch
2b2f9e229d Database: Fix degenerate parenthesized joins
The SQL standard supports parenthesized joins like

    a JOIN (b JOIN c ON (...)) ON (...)

But it doesn't support parenthesizing a single table name, i.e. a
one-table "join", like

    a JOIN (b) ON (...)

Detect the degenerate single-table case and omit the parentheses.

Bug: T181674
Change-Id: I82cacd80465092aa67ff19bdcfd6682001bf12ab
2017-11-29 15:42:27 -05:00
Brad Jorsch
4bd3e80e7a Database: Support parenthesized JOINs
SQL supports parentheses for grouping in the FROM clause.[1] This is
useful when you want to left-join against a join of other tables.

For example, say you have tables 'a', 'b', and 'c'. You want all rows
from 'a', along with rows from 'b' + 'c' only where both of those
exist.

 SELECT * FROM a LEFT JOIN b ON (a_b = b_id) JOIN c ON (b_c = c_id)

doesn't work, it'll only give you the rows where 'c' exists.

 SELECT * FROM a LEFT JOIN b ON (a_b = b_id) LEFT JOIN c ON (b_c = c_id)

doesn't work either, it'll give you rows from 'b' without a
corresponding row in 'c'. What you need to do is

 SELECT * FROM a LEFT JOIN (b JOIN c ON (b_c = c_id)) ON (a_b = b_id)

This patch implements this by extending the syntax for the $table
parameter to IDatabase::select(). When passing an array of tables, if a
value in the array is itself an array that is interpreted as a request
for a parenthesized join. To produce the example above, you'd do
something like

 $db->select(
     [ 'a', 'nest' => [ 'b', 'c' ] ],
     '*',
     [],
     __METHOD__,
     [],
     [
         'c' => [ 'JOIN', 'b_c = c_id ],
         'nest' => [ 'LEFT JOIN', 'a_b = b_id' ],
     ]
 );

[1]: In standards as far back as SQL-1992 (I couldn't find an earlier
 version), and it seems to be supported by at least MySQL 5.6, MariaDB
 10.1.28, PostgreSQL 9.3, PostgreSQL 10.0, Oracle 11g R2, SQLite 3.20.1,
 and MSSQL 2014 (from local testing and sqlfiddle.com).

Change-Id: I1e0a77381e06d885650a94f53847fb82f01c2694
2017-10-13 19:12:16 +00:00
Aaron Schulz
f4e0c720a8 rdbms: Ensure onTransactionPreCommitOrIdle() callbacks don't lead transactions
If no writes started a transaction yet, the callback would run
but not commit (by design, joining the request round). Later
writes will then pile on top of it.

The point of this method is to avoid such cases, so this edge
case has been fixed.

Change-Id: I9b44b19261d679de4aff6e44a9cfeb4f684ce02e
2017-07-26 14:28:48 -07:00
Timo Tijhof
ddb7575e4e rdbms: Refactor DatabaseTest
* Move DatabaseTest and DatabaseSQLTest to libs,
  and remove MediaWikiTestCase dependency.

* Refactor DatabaseTest to be a test of the Database abstract class,
  not of whatever current DB backend is configured by LocalSettings.

  - Remove most switches/conditionals and other tests for specific
    database backends. Move those to individual test classes for
    those backends instead.
  - Some tests appear to have been integration tests for the PHP driver
    and/or the db backend itself. Moved to a new DatabaseIntegrationTest.
  - Now that only the abstract Database is invoked, the test runs a bit
    faster (no real connections/queries).

* Add missing @covers tags, and remove or fix broken ones
  (follows-up 26e52f0c49).

Change-Id: I9dc4a558e701d00e95789e7eb8e02926783b65ad
2017-07-20 18:23:37 -07:00
Renamed from tests/phpunit/includes/db/DatabaseTest.php (Browse further)