Semanticmediawiki: Behaviour of namespaces changed in 3.1, e.g. $smwgNamespacesWithSemanticLinks

[krabina]

Setup and configuration

• SMW version:3.1
• MW version:1.31
• PHP version:7,2

Issue

The issue showed up when using the CommentStreams extension. What happens is that in a working environment with SMW <3.1 and working CommentStreams, after upgrading to SMW 3.1.x CommentsStremas still worked, but no properties were set.

There are two workarounds for this.

1. set $smwgNamespacesWithSemanticLinks[844] = true;
   (this step is usually not required, but helps as a workround).
   BTW, documentation says it should be: $smwgNamespacesWithSemanticLinks[NS_COMMENTSTREAMS] = true;
   (accoring to CommentStreams namespaces)
   But this results in PHP Notice: Use of undefined constant NS_COMMENTSTREAMS - assumed 'NS_COMMENTSTREAMS' as described in here (even tough enableSemantics is clearly before this)
2. load the extension CommentStreams before SMW

Steps to reproduce

1. Install CommentStreams in a SMW 3.1.x installation, invoke it after SMW, then no properties are being saved.
2. Move enabling CommentStremas in LocalSettings.php before calling SMW and properties will get saved,

@kghbln indicated, this could be related to #4119

[kghbln]

A third alternative will be to add define( 'NS_COMMENTSTREAMS', 844 ); prior to invoking any extension.

Since things like this happen also in other contexts (namepace permissions) I suspect that something may be wrong with the extension registration when it comes to registering the namespace constant. Namespace IDs seem to work.

[cicalese]

The namespaces are created and $wmwgNamespacesWIthSemanticLinks is set by CommentStreams in an extension registration callback [0].

In SMW 3.0, it did not matter whether CommentStreams was loaded with wfLoadExtension() before or after calling enableSemantics(). In SMW 3.1, CommentStreams must be loaded before enableSemantics() is called. The error only occurs in SMW 3.1 if CommentStreams is loaded after enableSemantics() is called.

It should not be necessary to do any additional configuration, since the configuration happens within the CommentStreams extension. I updated the CommentStreams documentation to indicate that it must currently be loaded before enableSemantics(). But, ideally, it would be best if SMW could return to the prior behavior where the order did not matter.

[0] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/CommentStreams/+/master/includes/CommentStreamsHooks.php#334

[mwjames]

Maybe I have time to write a more thorough analysis later but the issue here is due to how ExtensionRegistry resolves the internal queue of registered extensions which works after the FIFO principle therefore there shouldn't be any surprise that when you add something to the bottom that it will not be recognized by something at the top (I'm talking about the order of invocation in LocalSettings.php),

[mwjames]

In SMW 3.0, it did not matter whether CommentStreams was loaded with wfLoadExtension() before or after calling enableSemantics(). In SMW 3.1, CommentStreams must be loaded before enableSemantics() is called. The error only occurs in SMW 3.1 if CommentStreams is loaded after enableSemantics() is called.

Let's review how wfLoadExtension and the ExtensionRegistry works, the registration has three entry points LocalSettings.php, the callback and ExtensionFunctions section in the extension.json.

When you add wfLoadExtension (or for that matter enableSemantics) in LocalSettings.php then it is registered with the ExtensionRegistry queue. Now, after LocalSettings.php has been loaded in its entirety, the ExtensionRegistry will start its registration process in the form of FIFO which means the first extension registered in the queue will also the first to be registered with MediaWiki, the hook system and any GLOBALS it manipulates.

While adding each extension after another, the ExtensionRegistry will call out to the callback section (see the extension.json) to provide the possibility for some "free" registration of variable manipulation of GLOBALS.

The ExtensionFunctions section is the last which gets called after all extensions have been registered through the ExtensionRegistry making sure anything registered with the function runs at the latest point of entry.

An extension is unable to influence the order of the execution of ExtensionFunctions which means if some extension is accessing a function from another extension (see #4195) any preparation has to be done during the callback to ensure variables are set before something tries to access a function or class.

As outlined in #4195 and due to how ExtensionRegistry works we had to move the registration of connection providers to the callback section which also meant that we are isolating the GLOBALS access at this point because in SMW we don't permit GLOBALS (there are only a few left) to interfere with the program flow which means by the time SMW is registered and the callback is run by the ExtensionRegistry we have looked and isolated access to any smwg* GLOBALS via the Settings class.

It should be clear now from the explanation above why when adding wfLoadExtension( 'CommentStreams' ); after enableSemantics changes to smwgNamespacesWithSemanticLinks (or any other smwg*) will have no effect via a callback because enableSemantics runs before CommentStreams and has therefore already isolated it access to GLOBALS (at the point of callback). The cited [0] as part of the callback runs after that of Semantic MediaWiki which is the reason why adding the invocation before or after eventually makes a difference.

[0] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/CommentStreams/+/master/includes/CommentStreamsHooks.php#334

[mwjames]

I could try to mend the Settings invoked isolation during ExtensionFunctions but I'm not sure that this is a valid approach as we already have instances that rely on a complete Settings object.

[JeroenDeDauw]

I ran into this problem with the Maps extension as well. (https://github.com/JeroenDeDauw/Maps/pull/602#discussion_r378555769)

I concur with @cicalese that restoration of the old behavior would be nice. Otherwise the INSTALL instructions get more complicated for all these extensions doing something with SMW. Users are already having a hard enough time as it is.

[mwjames]

I concur with @cicalese that restoration of the old behavior would be nice. Otherwise the INSTALL instructions get more complicated for all these extensions doing something with SMW. Users are already having a hard enough time as it is.

I added #4558 as a show case but I'm not happy about it because either you use the ExtensionRegistry as-is with all its limitations or you don't, cherry picking some functionality to make up for some lost functionality due to how the queue works is certainly not something that has my support.

[mwjames]

To clarify, adding $smwgNamespacesWithSemanticLinks[FOO] = true before or after enableSemantics in LocalSettings.php doesn't matter because in both cases the setting will be recognized because at this point the extension hasn't been registered __only__ queued. Above issue arises from trying to manipulate $smwgNamespacesWithSemanticLinks from inside of another extension after LocalSettings.php has finished loading all settings.

[JeroenDeDauw]

Ok

I don't want the user to need to add that setting at all though. I assume you saw my pull request on Maps, which describes only a temporary setup.

[mwjames]

I assume you saw my pull request on Maps, which describes only a temporary setup.

Yes, I understand, and I wanted to make sure everyone in this thread understands the circumstances of where the problem originates from.

[cicalese]

@mwjames, thank you for your detailed explanation of the root cause of this issue. Given that setting $smwgNamespacesWithSemanticLinks[FOO] = true in an ExtensionRegistry callback function will not work, is there another way to interact with the Settings object from the callback that will accomplish the desired effect? Perhaps that is what you are referring to in https://github.com/SemanticMediaWiki/SemanticMediaWiki/issues/4505#issuecomment-586565003? The goal is to make it so that an extension can declare a namespace with semantic links in a way that does not have an impact on the user of the extension (i.e. making them set a global or being aware of the order of enabling extensions). It is not necessary to provide a general way of allowing all $smwg... globals to be changed in an ExtensionRegistry callback function. Is the concern that other extensions in their ExtensionRegistry callback functions may require that the list of all namespaces with semantics links already be populated? If so, is there an example of such an extension? Thank you for your additional time in exploring this issue.

[mwjames]

n an ExtensionRegistry callback function will not work, is there another way to interact with the Settings object from the callback that will accomplish the desired effect?

We have a hook that runs before the Settings object is finalized. [0] contains the description and I'd rather avoid introducing #4558 if the hook is sufficient (which should kick-in while you are calling from the callback context in ExtensionRegistry).

[0] https://github.com/SemanticMediaWiki/SemanticMediaWiki/blob/master/docs/technical/hooks/hook.settings.beforeinitializationcomplete.md

[kghbln]

@cicalese Is this information and docu sufficient to adapt the CommentStreams extension to pick up namespaces with semantic links?

[JeroenDeDauw]

Worked in Maps https://github.com/JeroenDeDauw/Maps/commit/31ca87d127e5b66aa02134844bf43678167afa95

[kghbln]

Worked in Maps JeroenDeDauw/Maps@31ca87d

Cool, thus closing.

[JeroenDeDauw]

Eh, while this hook allows doing what I want in Maps, it does not undo the behavior change Bernhard reported.

[kghbln]

Eh, while this hook allows doing what I want in Maps, it does not undo the behavior change Bernhard reported.

Figured that it was similar. Keeping fingers crossed with a portion of optimism. :)

[cicalese]

Sorry for the delay! I have patched CommentStreams as suggested by @mwjames, and the fix works. @krabina / @kghbln , could you please test https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CommentStreams/+/579341 and confirm that it works for you as well? If it does, please +1 it there. Thank you all!

[cicalese]

Thanks for testing, @krabina. Based upon your +1, I have merged the patch to CommentStreams.

[kghbln]

Thanks for testing, @krabina. Based upon your +1, I have merged the patch to CommentStreams.

Thanks a lot, great collaboration. Cool that is also works for CommentStreams and so I do hope consequently also for other consumers.

Looks like we are soon facing a new release for CommentSteams. :) Just also found a pull request on GitHub which may be considered for the release: https://github.com/wikimedia/mediawiki-extensions-CommentStreams/pull/1