JsonSchemaValidator fails to validate pages where an array within an array is used
Closed, ResolvedPublic2 Estimated Story Points
Actions

Assigned To

Authored By

	Urbanecm_WMF
	Feb 29 2024, 2:47 PM

Description

On my local setup, the MediaWiki:GrowthExperimentsHomepage.json page cannot pass validation (using 53944021e580432ca28ae7fe4500b3f4b1837cb4 from the example repository). Its content is:

{
	"GEHomepageSuggestedEditsIntroLinks": {
		"create": "foo",
		"image": "bar"
	}
}

shell.php fiddling:

> \MediaWiki\MediaWikiServices::getInstance()->get('CommunityConfiguration.ProviderFactory')->newProvider('NewcomersHomepage')->loadValidConfiguration()->getErrors()
= [
    [
      "type" => "error",
      "message" => "communityconfiguration-schema-validation-error",
      "params" => [
        "GEHomepageSuggestedEditsIntroLinks",
        "Array value found, but an object is required",
        [
          "property" => "GEHomepageSuggestedEditsIntroLinks",
          "pointer" => "/GEHomepageSuggestedEditsIntroLinks",
          "message" => "Array value found, but an object is required",
          "constraint" => "type",
          "context" => 1,
        ],
      ],
    ],
  ]

>

Removing "type": "object" from the JSON schema makes the page to pass validation successfully.

Details

	Subject	Repo	Branch	Lines +/-
	JsonSchemaValidator: Properly convert config into stdClass data	mediawiki/extensions/CommunityConfiguration	master	+16 -2
	WikiPageLoader: do not use FORCE_ASSOC when fetching config	mediawiki/extensions/CommunityConfiguration	master	+2 -2

Customize query in gerrit

Event Timeline

Urbanecm_WMF created this task.Feb 29 2024, 2:47 PM

Restricted Application added a project: Growth-Team. · View Herald TranscriptFeb 29 2024, 2:47 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 1007630 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/CommunityConfiguration@master] JsonSchemaValidator: Properly convert config into stdClass data

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

Change 1007575 had a related patch set uploaded (by Urbanecm; author: Sergio Gimeno):

[mediawiki/extensions/CommunityConfiguration@master] WikiPageLoader: do not use FORCE_ASSOC when fetching config

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

In a discussion in Slack, @Sgs said:

The issue is coming from the FormatJson::FORCE_ASSOC flag in \CommunityConfiguration\Store\WikiPage\Loader->fetchConfig() calls to

FormatJson::parse(). The flag converts all "type": "object" properties in the JSON schema to arrays. That “tampers” the data shape and makes it invalid. Tentative fix 1007575. There’s a second FORCE_ASSOC usage in ApiEdit.php that we might want to review.

Based on this advice (thanks Sergio!), I realized the issue actually is that (object)$config from the JsonSchemaValidator class does not work recursively, cf. https://stackoverflow.com/questions/19272011/how-to-convert-an-array-into-an-object-using-stdclass. This can be fixed by writing our own conversion logic (for array -> stdClass) and using that before the validation.

While this would still force us to have an array as the top level data type, I think that makes absolute sense (everything that's not a single variable provider will need to start with an object either way). Staying with array also means less changes.

Based on the above, I uploaded https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CommunityConfiguration/+/1007630.

Urbanecm_WMF claimed this task.Feb 29 2024, 2:53 PM

Urbanecm_WMF triaged this task as High priority.

Urbanecm_WMF edited projects, added Growth-Team (Sprint 8 (Growth Team)); removed Growth-Team.

Urbanecm_WMF moved this task from Backlog to Migrate Special:EditGrowthConfig on the CommunityConfiguration board.

Urbanecm_WMF set the point value for this task to 2.

Urbanecm_WMF moved this task from Incoming to Code Review on the Growth-Team (Sprint 8 (Growth Team)) board.

Change 1007575 abandoned by Sergio Gimeno:

[mediawiki/extensions/CommunityConfiguration@master] WikiPageLoader: do not use FORCE_ASSOC when fetching config

Reason:

Solved in I91aa8808307c36abb393574a85c4c6883028ce42

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

Change 1007630 merged by jenkins-bot:

[mediawiki/extensions/CommunityConfiguration@master] JsonSchemaValidator: Properly convert config into stdClass data

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

Maintenance_bot removed a project: Patch-For-Review.Feb 29 2024, 4:30 PM

> $s = \MediaWiki\MediaWikiServices::getInstance()->get('CommunityConfiguration.ProviderFactory')->newProvider('NewcomersHomepage')->loadValidConfiguration()
= StatusValue {#7074
    +value: [
      "GEHomepageSuggestedEditsIntroLinks" => [
        "create" => "foo",
        "image" => "bar",
      ],
    ],
    +success: [],
    +successCount: 0,
    +failCount: 0,
    +statusData: null,
  }

> $s->isOK()
= true

> $s->getErrors()
= []

> $s->getValue()
= [
    "GEHomepageSuggestedEditsIntroLinks" => [
      "create" => "foo",
      "image" => "bar",
    ],
  ]

>

It works now! :)

JsonSchemaValidator fails to validate pages where an array within an array is usedClosed, ResolvedPublic2 Estimated Story PointsActions

Description

Details

Event Timeline

JsonSchemaValidator fails to validate pages where an array within an array is used
Closed, ResolvedPublic2 Estimated Story Points
Actions