It has MapCacheLRU->toArray and MapCacheLRU::newFromArray for this purpose. I guess we forgot to use it!

• Update the closure in WatchedItemStore::resetNotificationTimestamp() to return an array obtained from MapCacheLRU->toArray(), instead of an object
• Update the two places that fetch from the cache to read cache entries in either format, for backwards compatibility. In resetNotificationTimestamp() $current may either be an array or a MapCacheLRU. In getPageSeenTimestamps(), the return value of getWithSetCallback() may either be an array or a MapCacheLRU.
• Add a test to WatchedItemStoreUnitTest which verifies correct behaviour with cache values supplied by the test case.

Forward compatibility of cache reads needs to be considered here: if the patch gets rolled back for some reason, the old code will encounter arrays when expecting MapCacheLRU objects. It may be necessary to first deploy a patch to defend against this situation, then deploy the actual change a week (a train) later.

PS: someone (me) broke something (wikipedia) this way before...

Tagging as "log spam", since this is currently generating over a hundred warnings per minute.

Change 702202 had a related patch set uploaded (by Dmitriy.sky; author: Dmitriy.sky):

[mediawiki/core@master] Update resetNotificationTimestamp and write a test

https://gerrit.wikimedia.org/r/702202

@tstarling Question: in order to test what's in private property WatchedItemStore::stash after resetNotificationTimestamp execution we have to somehow access it and check what's in the bag.
There are several approaches to do it. The question is which one is favorable?

• Implement the getter - WatchedItemStore::getStash()
• Use magic method __get and return $this-> stash by unmanifest key (eg. prefixed with underscore, like _stash) so that private visibility is still making sense
• Utilize Reflection API and change WatchedItemStore::stash visibility only within a test

Or maybe you can suggest a better approach.

In T282105#7184839, @dmitriy.sky wrote:

@tstarling Question: in order to test what's in private property WatchedItemStore::stash after resetNotificationTimestamp execution we have to somehow access it and check what's in the bag.
There are several approaches to do it. The question is which one is favorable?

For $this->stash you already have the object, since it was a constructor parameter. It is $mockCache in your test method. For latestUpdateStash you can use TestingAccessWrapper::newFromObject($store)->latestUpdateStash.

tagging for code jam

Hi @daniel, I'm working on that. Going to wrap up ASAP.

@Tim.abdullin , are you still working on this (or planning to)? If so, that's great and we'll stay out of your way. Or support you however we can, including but not limited to code review.

If things have changed and you're not still working on this, let us know and we'll consider it as a "Code Jam" project (basically an internal Platform Team hackathon) for the week of Sept. 20th.

Hi @BPirkle, I'm planning to work on it this on it this on Thursday and Friday this week.

Change 722111 had a related patch set uploaded (by Tim.abdullin; author: Tim.abdullin):

[mediawiki/core@master] changed WatchedItemStore to cache an array instead of MapCacheLRU

https://gerrit.wikimedia.org/r/722111

Please review

Change 726199 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] WatchedItemStore: forwards compatibility for stash value change

https://gerrit.wikimedia.org/r/726199

Change 726199 merged by jenkins-bot:

[mediawiki/core@master] WatchedItemStore: forwards compatibility for stash value change

https://gerrit.wikimedia.org/r/726199

@tstarling @Pchelolo I noticed some comments under my patchset, should I address them ? Should I continue working on this tasks ?

Change 722111 merged by jenkins-bot:

[mediawiki/core@master] Change WatchedItemStore to cache an array instead of MapCacheLRU

https://gerrit.wikimedia.org/r/722111

Change 702202 abandoned by Umherirrender:

[mediawiki/core@master] Update resetNotificationTimestamp and write a test

Reason:

Superseeded by Ibfe4c205ffdf7af7087f6f862004b95b29bdc394

https://gerrit.wikimedia.org/r/702202