content: Refactor and fix various bugs in JsonContent
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 3 Dec 2014 02:10:50 +0000 (02:10 +0000)
committerOri.livneh <ori@wikimedia.org>
Wed, 17 Dec 2014 23:04:24 +0000 (23:04 +0000)
commite4f84af980c0e1054b7cf101c00b8c858dd6c562
treee1846cd51ffa19284e0dae533497f721235d6e66
parentd34a6ca677399892237d94b139ed14e3e3e52077
content: Refactor and fix various bugs in JsonContent

Follows-up d2a82fcb60. These issues weren't previously exposed
as nothing uses JsonContent by default in core, and the extensions
using it (e.g. EventLogging) lock it down very early. As are
most of these methods were never really put to use (they were
called after the extension does its superset of checking, or
too early and WikiPage ignores it).

Bug fixes

* Empty JSON object was converted to an array by PST conversion.
  The beautifyJSON method is intended for prettify purposes but
  actually modified the content stored in the database and made
  it no longer roundtrip ({} != []).

  We can't change getJsonData to return an object since it's
  a public method and people use it as an array. So we can't cast
  it to a PHP object as that would break back-compat.

  Turns out the class doesn't even support non-objects anyway (a
  primitive in JSON can trivially cause a fatal as it wasn't
  consistently considered invalid, though it didn't actually fatal
  due to some lucky spaghetti code in WikiPage).

* Fix beautifyJSON by checking for empty objects to prevent
  implicit {} to [] conversion.

* Add isValid() check to fillParserOutput() as it's called early
  on. Otherwise it throws a warning that 'foreach' (in objectTable)
  iterates over null. In practice it doesn't matter since the
  entire parser output is rejected when WikiPage eventually
  checks isValid (through Content::prepareSave).

* Consider all non- (PHP) array values invalid instead of just
  non-null values.

Enhancements

* Display message "Empty object" instead of a completely blank page
  for an empty object.

* Display message "Empty object" or "Empty array" instead of an
  empty table cell.

* Render arrays as a list of values (without indices).

* Remove italics from table cells for values. The monospace font
  should be enough. It also offsets it from the "Empty"
  placeholders (which are italicised).

Refactoring and clean up

* Use FormatJson::parse so that we can use Status to distinguish
  between null parse result and thus reliably cache it.

  Ideally we wouldn't need to cache it, but right now this code
  is pulled apart and called in so many strange ways that we end
  up calling this several times.

* Improve fairly meaningless test (testBeautifyJson) that was
  calling FormatJson in its data provider, exactly what the method
  being tested did. It also provided the test with data that could
  never end up in normal usage (a PHP-style associated array with
  implied numerical indices).

* Document that this class rejects non-array values.

* Document the problem with WikiPage assumming PST can run on any
  content. WikiPage fundamentally still assumes wikitext, in that
  there's no concept of invalid content.

* Fix incorrect documentation for getJsonData's return value
  (It may return null.)

* Fix incorrect documentation for beautifyJSON's return value.
  (It never returned boolean.)

Bug: T76553
Change-Id: Ifed379ba4674a8289b554a95953951886bf2cbfd
includes/content/JsonContent.php
languages/i18n/en.json
languages/i18n/qqq.json
resources/src/mediawiki/mediawiki.content.json.css
tests/phpunit/includes/content/JsonContentTest.php