| Tgr added a comment. |
It might help with the terminology to differentiate between the history parent (the preceding row in the page history, ie. the current verion of the page before saving the edit) and the logical parent (the revision the user based their edit on, e.g. the one pointed to by the oldid parameter, or the rollback target). There are then four possible kinds of parents:
- history parent as seen by the client (last at the time of starting the edit) - this is EditPage::$editRevId / EditPage::getBaseRevision()
- actual history parent (differs from the previous one if there were concurrent edits) - this is rev_parent_id
- logical parent as seen by the client (oldid, or last version at the time of starting the edit if the user was not editing an oldid, or the conflicting version, if the user is doing manual edit conflict resolution) - this is EditPage::$oldId (except it's only set when it's different from the current revision at time of starting the edit)
- actual logical parent (same as previous one except after a successful automatic edit conflict resolution, in which case it points to the (non)conflicting revision) - this is EditPage::getParentRevId() (and EditPage::$parentRevId when set)
$baseRevId in WikiPage::doEditContent and PageContentSaveComplete and $baseId in NewRevisionFromEditComplete are probably meant to be the logical parent (actual, since they are only called after automatic edit conflict resolution), although given the limited documentation and that the paramterer is not actually passed in the one case where it would be relevant, it's hard to tell. At least FlaggedRevs seems to assume instead that it is the revision the edit is identical to (rollback target or null revision parent), which is the current behavior.
PageUpdater::$baseRevId is documented as "the last revision known to the client" but is set after automatic edit conflict resolution, at which point that is not relevant anymore, so it probably should be the actual logical parent. PageUpdater::grabParentRevision() is the history parent as seen in the early phase of the edit submission logic (same as the actual history parent if the edit actually gets saved; might differ from the latest version of the page at the time of attempting the DB write, but in that case the save will fail).
The semantics [of parentRevId and getParentRevId()] as described matches the rev_parent_id
Not really, when editing an old version, parentRevId points to that version (the code is confusingly written; $out->addHTML( Html::hidden( 'parentRevId', $this->getParentRevId() ) ) does the real work), while rev_parent_id (in its current incarnation) always points to the previous DB row for the same title.
$baseRevId of doEditContent: The revision ID this edit was based off, if any. Not stored, but passed to some hooks. Doesn't seems to be used for anything by core.
It seems that some hook implementations interpret it as "the ID of the revision this edit is identical to" (ie. rollback target or null edit parent). Which is the current behavior but probably not the intent (although the hook documentation is minimal so it's hard to tell).
rev_parent_id field: The ID of the revision that was current immediately before an edit. Used to display size differences in the page history (compare T193690)
Might be used for other things as well as the revision object is passed to some hooks.
It looks like EditPage editRevId/getBaseRevision() and parentRevId/getParentRevId() should be equivalent until the edit is actually saved.
getBaseRevision() is the last revision at the time of starting the edit. getParentRevId() is the logical parent. They are only equivalent if the edit was based on the latest revision.
It looks like EditPage's oldid and WikiPage's $baseRevId should be equivalent when oldid != 0. When oldid == 0, I believe $baseRevId should be editRevId.
See the FlaggedRevs part of this comment for why that's not necessarily a good idea.
It looks like there's a possible race where edits A→B→C could have C's rev_parent_id as A even though merge3() used B's content.[1]
rev_parent_id is set in PageUpdater::makeNewRevision() based on PageUpdater::grabParentRevision(), which is frozen early in saveRevision() (possibly earlier, due to in-process caching of WikiPage::getRevision()). There cannot be any race after that; PageUpdater::doModify() uses WikiPage::lockAndGetLatest() which gets the current DB value and locks it against later races, and that gets compared to the frozen value. So that part is OK.
PageUpdater::hasEditConflict() would compare the frozen value with PageUpdater::getBaseRevisionId(), which prevents conflicts in theory. It is never actually called though, so as long as the concurrent edit happens after the edit conflict detection in EditPage and before PageUpdater::saveRevision() freezing the parent, nothing will stop it. Also, for that to work, the check would have to be against EditPage::$parentRevId but that does not match how getBaseRevisionId() is documented (as the task description already notes).
Cc: Tgr, GoranSMilovanovic, gerritbot, Anomie, CCicalese_WMF, Aklapper, daniel, Gaboe420, Versusxo, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Lahi, PDrouin-WMF, Gq86, Baloch007, E1presidente, Ramsey-WMF, Cparle, Darkminds3113, SandraF_WMF, Bsandipan, Lordiis, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, QZanden, Tramullas, Acer, LawExplorer, Lewizho99, JJMC89, Maathavan, Agabi10, Susannaanas, Aschroet, Fjalapeno, Jane023, Wikidata-bugs, PKM, Base, matthiasmullie, aude, Ricordisamoa, Lydia_Pintscher, Fabrice_Florin, Raymond, Steinsplitter, Mbch331, Ltrlg
_______________________________________________ Wikidata-bugs mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
