RESTBase used to emit ETag in the `"<rev_id>/<render_id>" format.
For the benefit of the clients, preserve the formar.
Render ID is a UUIDv1 uniquely identifying the ParserOutput.
In future it would be used as a stashing key for stash deduplication.
At this time I decided to just attach the render ID as extension data
to our fake ParserOutput. Once we integrate Parsoid more into core,
we will likely move it into a ParserOutput property, or even
replace CacheTime::mCacheTime with a UUIDv1, but it's too early for that.
Bug: T268234
Change-Id: Ie604e9c98021d59eb1a17ca65f227e8f234a45be
This reverts commit b98f7a6fc1.
Reason for revert: Breaks Parsoid CI but doesn't seem to run on core patches?
Change-Id: I1eaf1495dce6f6ba78093aacb9475a023a2aabfa
This extracts two helper classes from PageHTMLHandler:
* PageContentHelper for accessing page content. This replaces the
LatestRevisionContentHandler mase class.
* ParsoidHtmlHelper for generating HTML from wikitext using parsoid.
The idea is to decouple the functionality from the REST handlers, so we
can easily mix and match functionality to create a handler for the
new per-revision HTML endpoint.
Bug: T267981
Bug: T267982
Change-Id: I3226833d12e51c959712d642b0195de1fe1ef979
Specifically, remove the conditional processing of
ContentModelChangeConstraint and ChangeTagsConstraint in
favor of early returns within the constraints
Bug: T157658
Change-Id: I377c0e3d9611d2da0a2d0f9bca055d65a1bec7e6
This reverts commit 38ca1b261e.
Reason for revert: Even though API appserver is ready, the REST API traffic is not routed to the correct MW cluster.
Change-Id: I00582e32c87e803c305930dd8de60c38b771b219
The name is only used for local config and internal virtual URLs.
A key use case is having a LocalRepo named "local" on central repo
wiki and a ForeignDBViaLBRepo named "shared" on the client wikis,
the later being a reference to the former. The keys should be be
shared regardless of this naming difference.
Bug: T267668
Change-Id: Ic239bb980740ec5237edc878af4274885034a96f
This reverts commit 1157007658.
Reason for revert: can be reapplied after dependencies are resolved.
Change-Id: I1270853766fd5bf59ed191065b9e52b76e3d9fc9
These are mostly easy fixes. Tests were fixed when that didn't require
any change to the tested code, and moved to /integration otherwise.
MediaWikiUnitTestCase::setTemporaryHook was removed: the
caller should provide a HookContainer, at which point it would just
become a useless wrapper around HookContainer::register. (We don't
really need it to be temporary, if proper DI is used).
The method was only used in the tests touched by this commit.
Change-Id: I2aba02560c41b77eea9dd4bff0e4d1c4bb0da9a2
This reverts commit 4191c9fe31.
Reason for revert: This can not be released yet. It has slipped my mind that Parsoid extension is not enabled on the API MW cluster, thus releasing this will break the html endpoint. This code is good and can be re-reverted once https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/635096 is resolved.
Change-Id: I808be187ae582995e6c1899044b2a7019bf02d32
* Convert everything that is protected to be private,
since the class is not stable to extend
* Remove PasswordReset::$hookContainer, unneeded
Additionally, add missing UserFactory::newFromRow,
for now just a wrapper for User::newFromRow
Bug: T253432
Change-Id: I1de57a08605ff6e0d2be8e276b7fcb08934fb5da
Ugh, my mistake.
Also added a test that should cover this. It fails on the previous
version of code, succeeds after applying this patch.
Bug: T262463
Change-Id: Ifda30daadea5a908505423caaf818b9f88f989ad
The tag is added to reverted edits as described in T254074.
Functionality:
* Adding the mw-reverted tag to reverted edits (duh)
* Limiting the maximum depth of the update through a config variable
(mitigation #2 from T259014).
* Only applying the reverted tag after the edit has been somehow
approved. Only the patrol subsystem currently implements this, but
there's a hook that extensions can use (mitigation #4 from T259014, more
explanation in T259103).
* When performing the delayed update, it is checked whether the reverted
edit was reverted itself. If so, the update is ignored. This is
probably the only way to make the feature work due to the lack of an
explicit "disapproval" mechanism other than reverting.
* The update is also ignored if the revert is marked as deleted.
Technical design:
* The update code is in RevertedTagUpdate.php, which is a deferrable
update, but is not used as such. It's separated to allow for better DI,
testing and better code reusability in the future.
* The update is queued / ran using the Job subsystem. The relevant job
is in RevertedTagUpdateJob.php
* PageUpdater determines whether the edit is approved or not and passes
that to the DerivedPageDataUpdater.
* The BeforeRevertedTagUpdate hook lets extensions decide whether the
update should be ran right away or await approval.
* DerivedPageDataUpdater checks whether the edit is a revert and if so
either enqueues the job (if it's auto-approved) or caches the EditResult
for later use (if it needs approval).
* RevertedTagUpdateManager allows for easy re-enqueueing of the update
for extensions. Thus, it has a very minimal interface.
Other notes:
* The unit testing setup for RevertedTagUpdate is a bit complicated,
but it was the only way I could make this class testable while using
the static ChangeTags class.
Bug: T254074
Depends-On: I86d0e660f0acd51a7351396c5c82a400d3963b94
Change-Id: I70d5b29fec6b6058613f7ac2fb49f9fad9dc8da4
The new service replaces the following public DatabaseBlock methods:
* ::delete
* ::insert
* ::update
* ::purgeExpired
Bug: T221075
Change-Id: I5f95db14b1189fd62d2c2bd5dae2cab0a368f401
This is a follow-up of T259014 which describes issues stemming from
users being able to mark arbitrary edits as undos (mitigation #3).
A "dirty" undo is when the user clicks the undo link and applies some
changes to page's content before saving. By doing so, the user can
set the edit's content to anything, while still marking the edit
with the mw-undo change tag and communicating to extensions that this
was an undo.
With this patch EditPage will only consider edits that had their
content provided by automatic conflict resolution as undos. Anything
other than that can't be reliably considered an undo.
THIS CHANGES THE BEHAVIOR OF EditPage IN A NOTICEABLE MANNER.
Most users should not notice anything different. In my opinion this
change makes EditPage's behavior more sensible and is justified.
Bug: T259014
Change-Id: I9279230303a01461039ae8a4641d9897ce194f73
These four test cases have been broken for months:
- testInsertIgnoreOld
- testInsertIgnoreNew
- testInsertSelectIgnoreOld
- testInsertSelectIgnoreNew
Disabling now to unblock T246358.
Bug: T246358
Bug: T259084
Change-Id: Idd1e920de8759d8795788572139c8528af05bd2c
Page titles used in URL paths, such as the Location header returned
after a page was created, must use the correct encoding for spaces and
pluses.
Bug: T258606
Change-Id: I75e91ac8f8da4eb183a9c8f1a682ea08c2225227
Based on the patch that introduces manual revert detection, this is
to create a software-defined change tag that will be applied to all
edits that restore a page to an exact previous state. This is also
for consistency with mw-undo and mw-rollback that will make a
"reverted" tag appear on reverted edits in the future.
Note about the REST API tests: in the next patch in this chain
I encountered even more issues with comparing returned changed tags
with expectations, so I decided it'd better if we just checked the
change tags applied manually in these tests. Otherwise we can run into
nasty race conditions, as the reverted tag is processed after sending
the response to client in the deferred update. It also makes the test
hard to maintain.
Bug: T256001
Change-Id: Ic367886f39faedcb823222b7d63bf4d5cb236ae9
Create a method in UserFactory to instantiate an anonymous
user with an optional IP address.
Bug: T257464
Change-Id: I557620f9bcd4b646288b4a76b26c4730fccbc3d8
The code for manual revert detection is ran when building the
EditResult and the edit was not marked as a revert yet. It's
based on a SELECT looking for recent edits that match the new
revision's SHA1. There is no index over rev_sha1 field, so we
limit the search to N most recent edits, in order not to cause a
full table scan.
I think this shouldn't break any extensions by setting the
originalRevisionId in situations that weren't expected, almost no
extension uses that anyway:
https://codesearch.wmflabs.org/search/?q=%5C%24originalRevId&i=nope&files=&repos=
Bug: T256001
Change-Id: I4e38b382391b42e252cb66584be1e174cdfe348c
This is to fix issues with reverts being marked incorrectly,
especially when using the undoafter param which, until this patch,
wasn't really supported.
Honestly, EditPage is such a mess that I'm not sure if this is good
or not, but I did a lot of manual testing and it seems to work fine.
WikiPage::doEditContent() now checks whether the provided original
revision really has the same content as the new revision. This was
previously the task of callers, but that doesn't make much sense to
me because:
* This task would fall on EditPage otherwise and it should be more
focused on the UI side of things, without too much worrying about
backend details.
* This would require adding another parameter to WikiPage::
doEditContent(), $undoAfterId. It would be only useful for multiple-
revision undos that are not top revisions. In all other cases it
would be the same as $originalRevId.
* An extra sanity check before applying the value to the PageUpdater
won't hurt. Who knows what crazy ideas extensions might have.
The test cases are almost identical to those written for
McrUndoAction, so I decided to group them in a single file.
This should cover most undo cases.
Bug: T256915
Change-Id: I78641b3de0a012af932ea38265b695362f1f8491
This aims to fix the following:
* Properly handle the undoafter param (it is currently set as the
oldest reverted revision ID, which is BAD)
* Set $originalRevId on undos to let EditResultBuilder check
whether the undo was an exact one and to let extensions know
about the "base" revision for this undo.
Note: $originalRevId was previously set explicitly set to false
here which does not make any sense to me. Setting it to the
revision we base the undo on is the only way to make EditResult's
getters' values consistent and make any sense. According to
PageUpdater's docs, this should only be set when the new revision has
"the exact same content as the given original revision", which is
checked here. I don't necessarily agree with this view, but it's
the spec, so we should probably keep to it.
I also hope it to be a kind of a "reference" implementation for
undos in case someone tries to tidy up the EditPage class in the
future, so I left quite a few comments behind.
I wrote some integration tests for that, should be sufficient for
now. I think.
Bug: T256915
Change-Id: I1c1f8d3b8a6273fbe7b460d140ef468f560456ce
The name change happened some time ago, and I think its
about time to start using the name name!
(Done with a find and replace)
My personal motivation for doing this is that I have started
trying out vscode as an IDE for mediawiki development, and
right now it doesn't appear to handle php aliases very well
or at all.
Change-Id: I412235d91ae26e4c1c6a62e0dbb7e7cf3c5ed4a6
The following endpoints move from v0 to v1:
GET page/{title}/links/language
GET page/{title}
PUT page/{title}
POST page
GET page/{title}/bare
GET page/{title}/html
GET page/{title}/with_html
GET page/{title}/links/media
GET file/{title}
NOTE: after merging this, give SRE a heads up, e.g. by
putting a note on the train ticket when this is merged
(navigate from T254174 as appropriate).
Bug: T255043
Change-Id: I3b8890da901e6312582d9a215c6a647173f16149
Makes it possible to mock static User methods in tests;
actually introducing dependency injection to the User class is left for
the future.
New class has 100% code coverage
Bug: T253432
Change-Id: I0b93da09124d95beafd84e932b214909ce920230
