Last modified: 2013-08-03 20:30:19 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T54072, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 52072 - Annotation serialization is inconsistent and sometimes incorrect
Annotation serialization is inconsistent and sometimes incorrect
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Annotator (Other open bugs)
unspecified
All All
: Unprioritized normal (vote)
: ---
Assigned To: richa jain
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-26 01:54 UTC by Matthew Flaschen
Modified: 2013-08-03 20:30 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Matthew Flaschen 2013-07-26 01:54:23 UTC
There are two separate ways to serialize, and one of them is incorrect for ranges.

Search serializes the entire rows array at once, which includes all annotation contents.  However, Read explicitly lists certain fields.  Read has a bug.  When the database has:

{"ranges":[{"start":"/p[1]","startOffset":118,"end":"/p[1]","endOffset":134}],"quote":"The local legend","text":"Is the legend accurate?"}

it is serialized by ApiAnnotatorRead as:

{"id":14,"text":"Is the legend accurate?","quote":"The local legend","ranges":{"start":"/p[1]","startOffset":118,"end":"/p[1]","endOffset":134}}


Note how the array of ranges is stripped of its brackets.

It would be good to filter the annotation (only allowing valid fields).  However,  I don't think that's a blocker, since FormatJson can only serialize JSON, not arbitrary objects.

So to simply serialize the entire object in Read, you can loop like:

https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FEventLogging.git/3985fbf7d20c9c2ef47fb561b418a62618ff1177/includes%2FApiJsonSchema.php#L112

You still need to explicitly add the ID, but that will handle the ranges correctly, and everything else.
Comment 1 Matthew Flaschen 2013-07-26 02:14:46 UTC
This is also necessary to handle multiple ranges in a single annotation, which is allowed; that's why it's an array.
Comment 2 Matthew Flaschen 2013-07-26 05:25:29 UTC
The best way to avoid Search and Read being inconsistent is to use a base class or helper class to process the annotations (adding id, and probably user, from column data into the JSON).
Comment 3 Matthew Flaschen 2013-07-30 01:40:43 UTC
I recommend you use a simple version of the repository pattern for this.  I'll comment further at https://gerrit.wikimedia.org/r/#/c/75106/
Comment 4 Gerrit Notification Bot 2013-08-03 19:10:40 UTC
Change 75106 had a related patch set uploaded by Mattflaschen:
Added the update class

https://gerrit.wikimedia.org/r/75106
Comment 5 Gerrit Notification Bot 2013-08-03 20:25:18 UTC
Change 75106 merged by jenkins-bot:
Added the update class

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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links