Page MenuHomePhabricator

wbeditentity does not return entity data when adding form
Closed, ResolvedPublic

Description

You can create a form using wbeditentity:

(new mw.Api()).postWithToken('csrf', {
  action:'wbeditentity',
  'new': 'form',
  data: JSON.stringify({
    lexemeId: 'L1',
    representations: { en: { language: 'en', value: 'test form' } },
    grammaticalFeatures: []
  })
})

However, the API response doesn’t contain the full entity data:

{
  "entity": {
    "claims": {},
    "id": null,
    "type": "form",
    "lastrevid": 1449
  },
  "success": 1
}

It doesn’t even contain the form ID, which would otherwise at least let you issue a wbgetentities request for the data. (You can try to work around this by getting the full entity data for the lexeme and assuming the last form is the one you added, but that’s susceptible to a race condition.)

(wbeditentity doesn’t yet support senses, but with some WIP patches to set up the wiring I’m seeing the same problem there, too.)


Current status: the form ID is now returned, and nothing else. That’s enough to get the form ID with a separate wbgetentities request, though.

Event Timeline

The null ID in the response is a big hint to what’s happening: ResultBuilder is trying to serialize a NullFormId or DummyFormId. Both of those classes override serialize():

public function serialize() {
	throw new LogicException( 'Shall never be called' );
}

But they don’t override getSerialization(), and that’s the function which ResultBuilder uses, so it just sees the uninitialized internal serialization field: hence the null. If you copy the serialize() implementation to getSerialization(), you get an error response instead (from DummyFormId, not NullFormId, for what it’s worth).

So I assume that when the initial BlankForm is turned into a real Form in Lexeme::addOrUpdateForm(), the real Form is never returned back to EditEntity/ModifyEntity. You can see this in the implementation of wbladdform as well, in fact – AddForm::getFormWithMaxId() does some ugly acrobatics to get the form that was just created:

private function getFormWithMaxId( Lexeme $lexeme ) {
	// TODO: This is all rather nasty
	$maxIdNumber = $lexeme->getForms()->maxFormIdNumber();
	// TODO: Use some service to get the ID object!
	$formId = new FormId( $lexeme->getId() . '-F' . $maxIdNumber );
	return $lexeme->getForm( $formId );
}

So it turns out there are a lot more places that call getSerialization(), and fixing that would be a lot of work :/ the assumption is that an entity ID is allocated very early in the edit process, and then everything else can operate on that entity ID. Forms and senses break that assumption, at least the way they’re currently implemented. (To be honest, I now think it would be easier to restructure the form/sense saving process to satisfy that assumption than to reorganize all the other code to no longer assume the existence of a proper entity ID.)

However, we can at least make sure the entity ID is returned in the result. I’ll upload a patch set to achieve that.

Change 447651 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseLexeme@master] Store real form/sense ID in BlankForm/BlankSense

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

Change 447652 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseLexeme@master] Make addOrUpdateForm/-Sense return the new Form/Sense

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

Change 447652 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Make addOrUpdateForm/-Sense return the new Form/Sense

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

Change 447651 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Store real form/sense ID in BlankForm/BlankSense

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

The current status is that the ID is returned, but not much else:

{
  "entity": {
    "claims": {},
    "id": "L1-F5",
    "type": "form",
    "lastrevid": 406081
  },
  "success": 1
}

That’s enough to get the rest of the data with a separate wbgetentities call, but it would still be better to return the full entity data. Should I update the task description to reflect the current situation or open a new task for that?

This seems to be resolved with the recent wbeditentity result change:

request
(new mw.Api()).postWithToken('csrf', {
  action:'wbeditentity',
  'new': 'form',
  data: JSON.stringify({
    lexemeId: 'L1483',
    representations: { en: { language: 'en', value: 'test form' } },
    grammaticalFeatures: []
  })
})
response
{
  "entity": {
    "id": "L1483-F1",
    "representations": {
      "en": {
        "language": "en",
        "value": "test form"
      }
    },
    "grammaticalFeatures": [],
    "claims": {},
    "type": "form",
    "lastrevid": 537540
  },
  "success": 1
}