To my knowledge in all the places I'm touching in this patch the new
code is functionally identical to the old one.
Change-Id: I0ffa96d2f9cb9bf932f68b689244051c96c17ad9
Class Revision is deprecated and in this patch, replaces
usage with appropriate classes; RevisionRecord, RevisionStore,
etc.
Bug: T221163
Change-Id: Icfc85167a636bef95daab236ab80113c1a3cf41b
When deleting echo_notification or echo_email_batch rows, also delete
the corresponding echo_event rows if no other echo_notification rows
or echo_email_batch rows refer to them.
Bug: T221262
Change-Id: I416ff107bd000a0cfac102408c993f8bec2d0286
* Clear echo_event before the test. Otherwise it contains
a 'thank-you-edit' event that offsets everything.
* Use standard $this->db
* Use standard ~~~~ signature
Bug: T67336
Change-Id: Idd09aa107d01038d03b689d9086f65a9fb7b653a
It was sort of checked in an implicit and roundabout way, in the sense
that the preferences page checks this and forces the relevant
preferences to on or off. But this was difficult to discover, and it's
not clear if this works correctly when defaults change.
Instead, explicitly check whether the requested notify type is available
for the event's category. Also rewrite the entire method to use
array_filter().
Change-Id: I9502a42fc705b2e56ef5edab433f1804f2924359
When pages are deleted or undeleted, their associated
Echo notifications also need to be marked as deleted
or undeleted. This is done by looking up the
echo_target_page table.
With this change, echo_event.event_page_id is also included
in the search. This way, all the events that don't use
target pages are also moderated when needed.
Bug: T217452
Change-Id: I277fca68ce088ab564e76ed9dfb2134ab3be4c4a
When the location on disk of an icon is known,
load its content and turn it into a data uri
for the <img> tag.
Otherwise (when it's only configured with a url),
use the url in the <img> tag.
Bonus: the rasterized icons were a bit blurry
and they are now crisp.
Bug: T55479
Change-Id: I8b5d7f09d8181f22927b8d4712505212ea1a10b1
PHPUnit 4 on HHVM is unable to create a mock for a class that uses variadic
arguments in one of its methods. Details at I0e027f5ec66 in core.
To work around this, IDatabase will continue to document it the old-fashioned
way so that Phan can analyse it correctly, and also so that PHPUnit can
mock it properly.
Change-Id: I4b8f2b8a8bb54b71d8b4836c3663f1a646ece1bd
This trait was using private fields from its host classes
($this->event, $this->language). It created a weird coupling
situation where the host class uses the trait and the trait
uses the host class. Effectively a circular dependency.
Also, phan is complaining about it in
I65ae6adc10941c05a2646e551b1baa829e4e8654
Change-Id: Ib2796b7ca62ecd7ece19583d7ca83e4252a5d878
These work now that I4c0d629da254ac2aaf31aae35bd2efc7bc064ac6 is merged.
Bug: T194632
Bug: T218388
Change-Id: If8031d488a67c423c7d4f4d3bf0f5e4126d3212a
Note, per T194632#5018823 it appears that replacing instances of
Benutzer:Schnark with User:Schnark cause the test to pass, so we should re-add
this test case once we've figure out why that's happening.
Bug: T194632
Change-Id: I8000c037c515283e46d4b530f8d91bdef35c7ace
Also remove fetchNewestByUserBundleHash() because it's now unused,
and remove the echo_notification_user_hash_timestamp index which
existed specifically to support that function's DB query.
Bug: T143763
Change-Id: I74be8f156bc14d0e189d328953d17dc26cdb697b
No longer used in the new bundling system.
Also removes indexes that contain bundle_base.
Bug: T143763
Bug: T131415
Change-Id: Ibf94cdc471a11cb14995fee6a55af0d227b50aa5
This patch contains a series of different clean-ups in test classes.
Some documentation is added as well as soft and hard type hints. Note
all this is exclusively done in tests. So if the CI is fine with it,
it can't be wrong. Right? ;-)
Change-Id: Ibcf1f65f48ac0fb41837c47672dddfd70302e9fd
The @expectedException annotation got deprecated in PHPUnit 7.5, and
removed in PHPUnit 8.0. This was done because the annotation does have
two disadvantages:
* The class name is encoded in string, where it is not easy to find for
all IDEs and tools.
* it did not allow to say exactly *when* the exception is expected.
Change-Id: I96862a18874f36355e817accd64d8703c1965c86
The codebase already used the …::class feature in many places. So this
is more for consistency than anything. The …::class feature makes it
much easier to do refactoring in the future.
Note this patch is exclusively touching tests. That should make it
relatively easy to review this. As long as the CI is fine with it, it
should be ok. Right? ;-)
Change-Id: I4d2adee76b4adbc83b2061161fd4e863ba833fcb
Specify which notification types allow notifying the event agent in
$wgEchoNotifications, and stop specifying it in the event_extra data.
Putting 'notifyAgent' => true in event_extra will still work, but is
discouraged.
Change-Id: I4f558654ec23757dd4ecd6986eb3e9a5593f5386
These are updated in deferred updates and should not rely on the same
User instance being used in those updates. This also avoids convoluted
logic in User to set the new edit count for various cases.
Change-Id: I0ad3d17107efc7b0e59f1dd54d5733cd1572a2b7
* Reduce responsibility of resolve() methods to only supplying
the resolves values.
Moved logic for populating the cache and clearing the queue
to the base class, and made 'lookups' private.
* The second parameter to LocalCache::add() is unused, and never passed.
Removed to avoid confusion.
* The getTargets() method is unused. Removed.
* The getLookups() method is unused. Removed.
* The internal 'lookups' member was being used both for its keys and its
values, but never at the same time. This seemed risky, especially in
EchoRevisionLocalCache::resolve() where the associative array was passed
directly to the 'where' clause of IDatabase::select(), which shouldn't
espect keys when creating the 'IN' clause.
Using only values would keep value types flexible, but would require
use of the less efficient in_array().
Keeping both keys and values and calling array_values() would work.
Using only keys also works and is simpler, so long only ints are used.
* The tests were swapping 'targets' MapCacheLRU with a HashBagOStuff.
Following-up 4939bff7, this was forgotten, but works because the two
called methods (get and set) exist in both, but still seems odd.
Fixed by using TestingAccessWrapper to act on the existing object
instead of swapping it out.
* Improved tests by asserting more of the observed behaviour and impact.
Change-Id: I530eeac8bf3b407b8c633e0e20c7d35cc49f7a9f
This fixes some issues I found while updating this code base, e.g.
this removes types a method really does not return.
Change-Id: I19457e7bf88945eec958bf53e0b76a7585715a45
There are about 200 of such generic "array" type hints in this code base,
the majority in @param tags. I started with what I found most relevant:
@var and @return tags. I might continue working on this later, but
wanted to stop for now to keep this patch moderately small.
Change-Id: Iff0d9590a794ae0f885466ef6bb336b0b42a6cd3
This cache was only used so that, if we're told to clear the newtalk
flag and we already know there are no edit-user-talk notifications, we
won't try to delete them. But that's not a good justification for such a
confusingly-written cache that would have been hard to convert to
getWithSetCallback(), and I'm concerned that using cached data to make
this decision could lead to inconsistencies.
Also remove the notifCountHasReachedMax() check, which made no sense: if
the user has >99 notifications, that is no justification for not marking
their user talk notifications as read when they visit their user talk
page. Whether the displayed notification count will change has no
bearing on whether these notifications should be marked as read (and now
that bundled notifications are counted individually, the displayed count
actually could change).
Bug: T164860
Change-Id: I3ff5c9b31307839b9336bd8856015db9baa52fad
Also test it more meaningfully by setting up a mock database and
asserting that the right DELETE queries are issued.
Change-Id: Id39723b92118e98d9c9f0cd7381e9396dce67c17
This sets $wgLanguageCode appropriately. It will also be necessary when
$wgContLang becomes a service, in which case setting the global directly
will not work correctly.
Bug: T200246
Change-Id: I4aaf1c641ec6abef214eb96c0e4b42a67488ac00
Makes so many things simpler and robust.
Bug: T198935
Change-Id: Ia836f8f497cae8599f85cf86a7f6b299cd012e81
Depends-On: Iff63da0d215585cfcf083e7f7ec8ed45d5b77301
To use WANObjectCache correctly in a multi-DC-safe way, we need to use
getWithSetCallback() to read data, and call delete() when it changes.
NotifUser's caching of notification counts and timestamps relied
heavily on set() calls, and so wasn't multi-DC-safe.
Changes in this commit:
* Rather than caching counts/timestamps in separate cache keys, and
using separate cache keys for each section (alert/message/all), put
all this data in an array and store that in a single cache key.
This reduces the number of cache keys per user per wiki from 6 to 1.
* Similarly, use a single global cache key per user. The global check
key for the last updated timestamp is retained, so we now have
2 global cache keys per user (down from 7)
* Remove preloading using getMulti(), no longer needed
* Move computation of counts and timestamps into separate compute
functions (one for local, one for global), and wrap them with
a getter that uses getWithSetCallback().
* Use TS_MW strings instead of MWTimestamp objects internally, to
simplify comparisons and max() operations.
* Make existing getters wrap around this new getter. They now ignore
their $cached and $dbSource parameters, and we should deprecate/change
these function signatures.
* In resetNotificationCounts(), just delete the cache keys. In global
mode, also recompute the notification counts and put them in the
echo_unread_wikis table. We could also set() the data into the cache
at this point, but don't, because you're not supposed to mix set() and
getWithSetCallback() calls and I don't want to find out what happens
if you do.
Bug: T164860
Change-Id: I4f86aab11d50d20280a33e0504ba8ad0c6c01842
All files containing more than one PHP class were split into
multiple files.
extension.json was updated to match new class locations.
phpcs `OneObjectStructurePerFile.MultipleFound` rule was
re-enabled.
Bug: T177809
Change-Id: I6fc2ec9cc35e6bac5a7c44d94b0f1b1b40e6dba5
When intval() fails, the function returns a zero. We should remove
the failures from the blacklist.
Bug: T178512
Change-Id: I89ad680a287da16c2fbd6aa4d53a725142429144
Otherwise, if $list->getValues() contains the number 0,
any non-numerical string will match, because 'foo'==0 is true.
This, in combination with a broken maintenance script that had
inserted 0s into some users' blacklist, broke all notifications
for those users.
Bug: T177825
Change-Id: If8700b4d0de0fdba876eb9d5cc4997e185dfeb3c
With 349457, anytime a revision is created, a corresponding record
is created in the new ip_changes table. This may cause tests to fail
if they don't drop the ip_changes table between individual tests.
https://gerrit.wikimedia.org/r/#/c/349457/
Change-Id: I48f0b64f19f9582b40540fa1b42a39d281979625
The return value from the method is only suitable for passing to
$db->insert(). To get the inserted ID, you need to call $db->insertId()
even if $db->nextSequenceValue() returned non-null.
Bug: T164900
Change-Id: I466fd372804927b3ad72125c7a69d253bd7a24f8
Other tests (possibly from other enabled extensions) could also,
by chance, create Echo notifications for the user MWEchoThankYouEditTests
uses. In order to those tests be reliable, they should make sure
there is no notification data in the database prior to running tests.
Bug: T161087
Change-Id: I870a50b1f831795731235fa8ec97477b3e470b50
When content is changed and the change contains the signature
of the user, the method checking for reasonable mentions in
that changes did not consider multiple signatures.
The patch fixes that and adds a test for it.
Bug: T154406
Change-Id: I86303f42e97d16c68e3235b0e2d13542ceedf1fe
This speeds up the tests from 2 mins to 50 seconds for me.
(with a slow setup, no caching etc.)
Bug: T158120
Change-Id: I8adb6c8fe783d1be8841a1139bb141da3b046f9d
For the used diff engines an empty/new page is equivalent
to an empty line. When adding content that includes an
empty line the content will be split into two parts. The
interpreted diff will then consist of one addition, one
copied line and another addition.
Is the ping in the first addition and the signature in the
second addition the DiscussionParser does not send mentions.
The patch introduces a special case when interpreting diffs
if content is added to empty content. This will also skip
execution off diff engines since the interpretation is trivial.
Bug: T155998
Change-Id: Id5e44bc3245940c1f77c80f036db637756542552
Purge all cache when unread count changes and
repopulate on read.
Also fix client-side estimation when
marking a foreign notification as read.
Bug: T151389
Change-Id: I62def3d40a5640e26c234bb0335bc506dbf864a0
