This was noticed when looking into T364056: CommunityConfiguration: saving an empty value for numerical input does not behave correctly.

Hi @Michael,

Thank you for filling the task! I do agree it is challenging to work with objects as defaults as of now, but it should be already possible to use objects as default values (we made that possible in T363412). As of now, the following does what you'd expect it to:

class ExampleSchema extends JsonSchema {

	public const CCExampleObject = [
		self::TYPE => self::TYPE_OBJECT,
		self::DYNAMIC_DEFAULT => true,
	];

	public static function getDefaultCCExampleObject(): stdClass {
		return (object)[ 'Foo' => 1, 'Bar' => 2 ];
	}
}

With that schema, the defaults work correctly:

> \MediaWiki\MediaWikiServices::getInstance()->get('CommunityConfiguration.ProviderFactory')->newProvider('FooBar')->getStore()->loadConfiguration()->getValue() # nothing stored in the on-wiki page
= {#7120}

> \MediaWiki\MediaWikiServices::getInstance()->get('CommunityConfiguration.ProviderFactory')->newProvider('FooBar')->loadValidConfiguration()->getValue()->CCExampleObject # the object has a default
= {#7119
    +"Foo": 1,
    +"Bar": 2,
  }

>

I also uploaded a MR to the example repository, which adds an example object, including a default value for it.

In the callback, it is also possible to construct stdClass directly if so desired (rather than casting). Instead of true, it is also possible to point self::DYNAMIC_DEFAULT to a callable (possibly one within the schema class), which might make the origin of the default more clear.

That being said: While this approach works technically-speaking, it is definitely far from obvious. Because of that, I kind of hesitate from declaring the dynamic defaults approach to be the correct way of setting objects as defaults. But if nothing else, it currently works, and it can be considered as an approach 4.

Let me comment on the other approaches from the description as well, so that we can decide which one should be our approach-of-choice (and document the decision):

Approach 1: The first and naive approach would be to define the value on the sub-property of the object:
This plain doesn't do anything.

That is actually deliberate. Especially with objects and references, having nested defaults gets tricky really quickly, and for that reason, I do not think we should support it. For example, what should the following do:

	public const CCExampleObject = [
		self::TYPE => self::TYPE_OBJECT,
		self::PROPERTIES => [
			'numberOnObject' => [
				self::TYPE => self::TYPE_NUMBER,
				self::REQUIRED => true,
				self::DEFAULT => 25,
			],
		],
                self::DYNAMIC_DEFAULT => true,
	];

	public static function getDefaultCCExampleObject() {
		return (object)['NumberOnObject' => 20];
	}

Should the default value for NumberOnObject be 20, or 25? Should this result in an error? Etc. When all is in a single file, it is relatively easy to understand what is happening, but if we pointed to a reference, things could get confusing real quick, I think.

When implementing support for defaults, we decided to not support defaults outside of the top level to avoid this kind of problems; see the patch's commit message.

Approach 2: The next approach would be to define the default values on the object as an associative array:

I kind of like this approach. It is far easier to discover than what we currently have, and there is no possible ambiguity between empty object and empty array, as the TYPE constant will tell us what the expected interpretation is. Well, in most cases at least; self::TYPE => [ self::TYPE_OBJECT, self::TYPE_ARRAY ], self::DEFAULT => [] is a perfectly valid schema, and there is no way to determine whether the default should be an array or an object (I do realize this is a very unlikely usage, but we should at least be aware this is a possibility).

It should also be possible to implement without any Core changes; changing JsonSchemaBuilder::getDefaultsMap() should be sufficient to make this happen. Of course, if we decide this approach should be used for any (JSON) schemas in MediaWiki, then changing ReflectionSchemaSource makes a lot of sense, but I am unsure if approach 2 would be a good decision for all other users of ReflectionSchemaSource.

Approach 3: Trying to follow the error message and casting to object:
[...]
I'm not sure if there is a realistic way to have a class/object inside a class constant.

(Unfortunately?), PHP's objects are always mutable by definition, which does not tend to align well with constants. There is a PHP RFC that proposes immutable classes/objects, and if it does get accepted, then we could have objects in constants, but that would require a change within the PHP language. So, no realistic way for now, unfortunately.

urbanecm opened https://gitlab.wikimedia.org/repos/growth/community-configuration-example/-/merge_requests/17

Add example object to the ExampleSchema

migr merged https://gitlab.wikimedia.org/repos/growth/community-configuration-example/-/merge_requests/17

Add example object to the ExampleSchema

The dynamic default approach makes sense for indeed dynamic defaults. I think the problem statement may still be valid and true for JsonSchemaTrait, see https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/includes/Settings/Source/JsonSchemaTrait.php#241. I would find fair to fix that upstream inspecting the schema type. Does this make sense? cc @daniel

Cross-posting feedback from technical checking meeting, ConfigSchemaAggregator may contain some useful "hacks" on deciding how to process empty default values like [] for arrays and objects.

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

[mediawiki/extensions/CommunityConfiguration@master] JsonSchemaBuilder: Accept arrays as defaults for TYPE_OBJECT

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

In T365045#9808546, @Sgs wrote:

Cross-posting feedback from technical checking meeting, ConfigSchemaAggregator may contain some useful "hacks" on deciding how to process empty default values like [] for arrays and objects.

Perhaps it's time to evaluate alternative libraries for schema validation. Maybe there is s omething out there that requires fewer hacks and workarounds.

Change #1035725 merged by jenkins-bot:

[mediawiki/extensions/CommunityConfiguration@master] JsonSchemaBuilder: Accept arrays as defaults for TYPE_OBJECT

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