Updated the closure in WatchedItemStore::resetNotificationTimestamp() to return
an array obtained from MapCacheLRU->toArray(), instead of an object.
Updated the two places that fetch from the cache to read cache entries in either format,
for backwards compatibility.
Added testResetNotificationTimestamp_stashItemTypeCheck test to
WatchedItemStoreUnitTest which verifies correct behaviour with
cache values supplied by the test case.
Bug: T282105
Change-Id: Ibfe4c205ffdf7af7087f6f862004b95b29bdc394
Stash values will change from a MapCacheLRU to an array, to avoid a
warning. Implement the read side of the change so that the write side
can be safely reverted.
Bug: T282105
Change-Id: I8d60ae709004a8aba9ad916e1a5fd05406a05984
WatchlistExpiryJob is being queued in some edits and they are being pushed directly.
This means in some edits, the appserver opens a request, communicate to
eventgate and then closes it, adding around 40ms to the time to save the edit.
You can see it in flamegraph of EditAction:
https://performance.wikimedia.org/arclamp/svgs/daily/2021-09-30.excimer-wall.all.fn-EditAction.svgz
It's not an urgent job and can be pushed later through deferred update
instead. The important thing is that it gets batched through all other
jobs being pushed.
Originally added in Ica8ab92837c38.
Bug: T292048
Bug: T255502
Change-Id: I2b0612052d6b34545f50d9cd9a28da50f14a16b4
For both LinkTarget and PageReference, just extract
namespace and dbkey and pass those in the array
of parameters to Job::__construct().
Allows a bunch of simplification to WatchedItemStore.
Bug: T291531
Change-Id: Id150d0c62af38d4b3d17e5698866127c6e04717e
This helps phan to detect unreachable code and also impossible types
after the functions.
It helps phan to avoid false positives for array keys
when the keys are checked before
Bug: T240141
Change-Id: I895f70e82b3053a46cd44135b15437e6f82a07b2
These not only make the code more robust, but also help a lot when
writing unit tests: if a method is return-typehinted and its class is
mocked, the mock method will automatically return a mock of its declared
return type. Otherwise it will return null, and developers are forced to
manually mock the method if the return value is used by the SUT in a way
that doesn't accept null.
Depends-On: I628fcb1807133390c7b9b47984f512f5b1ae58d0
Depends-On: I7080bc505f5838b2f51a368da562104e206063b0
Change-Id: I59068cfed10aabf6c6002f9e9312a6ef6e7e9441
- Add some documentation
- Add a bunch of typehints
- Add helper method ::modifyForExpiry to reduce duplication
around queries that need to be modified if watchlist expiration
is enabled, inspired by ChangeTags::modifyDisplayQuery
- Add some whitespace for readability
- Remove documentation stating that some parameters to
private methods were deprecated being a LinkTarget rather
than a PageIdentity - the deprecation is in the public
interface of the class, and in the methods that call
these private helper methods, we don't deprecate the internal
calls
Should be a no-op
Change-Id: I80088ca8238898741f04c1e9ea854168f252e174
Currently, we have a UserIdentity, we retrieve the user id,
and then we create a User object based on the id. Instead,
create the User object based on the UserIdentity directly,
so that if the UserIdentity happens to already be a full User
object no change is needed.
This allows us to simplify WatchedItemStoreUnitTest by
not needing to tailor the mock UserFactory to the specific
user ids that we are using in each test case.
Change-Id: I52918b4f4c7d44086ea4fcdabdbfe14197c06373
Use array_filter with ARRAY_FILTER_USE_KEY instead of retrieving
and filtering keys separately and make the code cleaner. This
resolves a fixme remark in the code.
Change-Id: I6e9f08522cbef1318be541982ca8c2a359fbe046
array_fill_keys() was introduced in PHP 5.2.0 and works like
array_flip() except that it does only one thing (copying keys) instead
of two things (copying keys and values). That makes it faster and more
obvious.
When array_flip() calls were paired, I left them as is, because that
pattern is too cute. I couldn't kill something so cute.
Sometimes it was hard to figure out whether the values in array_flip()
result were used. That's the point of this change. If you use
array_fill_keys(), the intention is obvious.
Change-Id: If8d340a8bc816a15afec37e64f00106ae45e10ed
Instead of using a PermissionManager
Required some changes to the creation of
the mock users in WatchedItemQueryServiceUnitTest,
simplified those changes by combining the various
helper methods that returned user mocks.
Change-Id: Id8382a5884133013e3da0d3318a41f8c451c3039
Should call the same ::countWatchersMutiple on the
actual store, not ::countVisitingWatchersMultiple
Has been broken since NoWriteWatchedItemStore was
first introduced, see 1ff58fc746
Change-Id: I20ab9a130f5bfe8b888ad08ea5852f13a03184f8
Replaces ::enqueueWatchlistExpiryJob which is hard deprecated
New method checks if watchlist expiration is enabled, and if
so enqueues the job based on the $wgWatchlistPurgeRate,
instead of forcing calls to handle that logic
Change-Id: I07a96941efd1a240846284d5c86db66c6ba45156
Required some fixes:
* getMockJobQueueGroup - add a parameter to not mock the
behavior of calling 'lazyPush'. By default, the mock
has a callback to just run the job being pushed, but
some tests don't want to run the job
* WatchedItemStore::updateNotificationTimestamp - use
$this->deferredUpdatesAddCallableUpdateCallback to allow
testing the updates
Change-Id: I4d35266340e1c44fe2154198de416ad93bfedf1d
This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.
Created by I25a17fb22b6b669e817317a0f45051ae9c608208
Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
Sometimes when moving pages, a row for the target page may already exist
in the watchlist_expiry table. The exact scenario that's causing the
prod errors could not be repo'd in tests or locally, but using REPLACE
should be a safe solution.
Tests updated to actually verify the expiry of the new title, if
present, is overriden with the old one following the page move.
Bug: T271971
Change-Id: Id4d46fcdea4816e5714d266985ba86fdc6450cfb
The expiry is returned as a separate key, 'watchlistexpiry', to match
other APIs, and because some clients might expect 'watched' to be a
boolean (or blank string depending on the formatversion).
Bug: T268834
Change-Id: I227d6ed42e70ba1ddec0139e8198f536dfba0b46
Normalize watchlist expiry field so that it is set to
ConvertibleTimestamp and the expiry value validation is
handled by the WatchedItem class.
Bug: T260868
Bug: T260009
Change-Id: I3ef31900cfbe7bce23c5ebe1db777a5137ea6167
Fix up display of success messages after action=watch is
submitted. This adds two new messages for when the remaining
watch period is less than a day, and fixes the display of
the other six messages. The factors for which message is used
are:
* watchlist expiry feature flag;
* article vs talk page;
* temporarily vs permanently watched; and
* hours or whole days remainig in watch period.
Bug: T259055
Change-Id: Ice3a30028d0bec5c5f4e4b9ce9e95b6553bdabdf
This commit makes three changes:
1) Re-fetches the expiry before caching WatchedItems in addWatch().
Sometimes (as with action=rollback) addWatch() is called without an
expiry, even though one currently exists. So we must query for the
preexisting expiry to ensure it persists in the process cache,
otherwise the star in Vector will be full when it should be a
half-star.
2) Removes caching of WatchedItems within addWatchBatchForUser(). We do
not want to fix#1 above within addWatchBatchForUser() as this would
mean we'd need to fetch expiries en masse, when seemingly only a
single WatchedItem needs the process cache (the page you're viewing).
Bulk caching was added in Ie4b69c985815a77b70692db0c4dbf52e1a6a018d
solely for the benefit of a single page (T28292), so it seems safe to
move the caching to addWatch().
3) Changes User::addWatch() to use WatchedItemStore::addWatch() instead
of addWatchBatchForUser(). This is to ensure the WatchedItem is
stored in the process cache for the aforementioned reasons.
Accordingly the docblock for addWatchBatchForUser() now encourages
use of addWatch() for a single page/talk page.
Bug: T259379
Change-Id: I3a071f4fbc28fcc96ca7768e31c6810ff7b8e605
These were never meant to be part of the public interface and should not
ever have been marked with @since. They're only useful for constructing
the respective objects, which no outside users should be doing.
Change-Id: I86e01272d46fc72af32172d8a12b9180971d4613
Update watchlist expiry language indicators (dropdowns, tooltips, watch
via star, watch via edit):
* Change "Permanently" to "Permanent"
* Change "XX days left" and "Expires in X days" to "XX days left in
watchlist"
* Update the "Click.." sentence by star to "Click to remove it."
* Change watch period for a page that is less than "1 day left" to "A
few hours left"
Bug: T253135
Bug: T255632
Change-Id: I114c6f77e86ad81b1810fedcd49f52c88700ca16
Expiries are copied post-send so as to not slow anything down.
Note the expiry feature has been disabled in some unit tests that mock
database queries, simply because these tests are so hard to maintain and
are very fragile. The logic introduced with this patch is covered by the
integration tests.
Bug: T257259
Change-Id: I4223eaa6782a319fb684acf656f54b88a92e5288
