Moving this to "atching" on the CPT board. The proposal sounds good, but it seems to me this should be owned by Discovery-Search.

Cheers Daniel. Even if CPT isn't going to do it, mind chiming in about the below? I don't know if Discovery-Search are necessarily going to have the time to work on this any time soon, so I might JFDI so we can get it in 1.35 as the LTS

Any suggestion from anyone with how to proceed? It seems to me changing the format of $wgSearchType/$wgSearchTypeAlternatives is probably complications waiting to happen. Generally, the docs have the user set $wgSearchType in LocalSettings.php, and in the cases of like CirrusSearch, after the wfLoadExtension(), so expecting them to set $wgSearchType['CirrusSearch'] = 'CirrusSearch\CirrusSeach...

So that leaves some sort of mapping elsewhere. Another $wg/config mapping that can be set in extension.json (though, attributes feels more sensible, but it's not an extension depending on each other)? A hook? Some sort of Service addition/override?

That all being said, just putting a top level item in extension.json... SearchMapping (not fussed on actual resultant name) and it taking something like...

"SearchMapping": { 'displayname': 'Namespaced\\Class\\Name' }

and then in the real world...

"SearchMapping": { 'CirrusSeach': 'CirrusSearch\\CirrusSearch' }

Feels the right way forward

I guess then in SearchEngineFactory::create() we can use the mapping... Renaming the variables to reflect some sort of reality (class becomes $name or similar, and then lookup $name in $wgSearchMappingVariableThing` and use that if there's a result

	/**
	 * Create SearchEngine of the given type.
	 * @param string|null $type
	 * @return SearchEngine
	 */
	public function create( $type = null ) {
		$configuredClass = $this->config->getSearchType();
		$alternativesClasses = $this->config->getSearchTypes();

		$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
		if ( $type !== null && in_array( $type, $alternativesClasses ) ) {
			$class = $type;
		} elseif ( $configuredClass !== null ) {
			$class = $configuredClass;
		} else {
			$class = self::getSearchEngineClass( $lb );
		}

		if ( is_subclass_of( $class, SearchDatabase::class ) ) {
			return new $class( $lb );
		} else {
			return new $class();
		}
	}

Using phab as a rubber duck.. :D

Change 592650 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Add $wgSearchMappings

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

Change 592656 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/CirrusSearch@master] Add "SearchMappings" to extension.json

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

This looks like the handler/registry pattern we are already using elsewhere. In the past, we have generally used a global variable for such a mapping (e.g. $wgContentHandlers). Using an ExtensionRegistry attribute is the better solution, since it avoids global state. In any case, the implementation should not be specified as a class name, but an "object spec" as supported by ObjectFactory. See how MediaWiki\Rest\EntryPoint instantiates the Router for an example of how this could work.

In T250977#6084927, @daniel wrote:

This looks like the handler/registry pattern we are already using elsewhere. In the past, we have generally used a global variable for such a mapping (e.g. $wgContentHandlers). Using an ExtensionRegistry attribute is the better solution, since it avoids global state. In any case, the implementation should not be specified as a class name, but an "object spec" as supported by ObjectFactory. See how MediaWiki\Rest\EntryPoint instantiates the Router for an example of how this could work.

ExtensionRegistry attributes in the extension schema are documented as

Registration information for other extensions

So it doesn't seem right to abuse this for stuff in MW core

I'll have a look at the ObjectFactor/object spec stuff and see if I make some adjustments

Obviously we need to keep the old style new $wgSearchType style to keep that back compatability, but certainly for this "mapping" we should be able to do something "better"

In T250977#6085341, @Reedy wrote:

I'll have a look at the ObjectFactor/object spec stuff and see if I make some adjustments

Done

Change 592650 merged by jenkins-bot:
[mediawiki/core@master] search: Add 'SearchMappings' attribute to map canonical name to PHP class

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

Change 592656 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Add "SearchMappings" to extension.json

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

Change 593582 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] registration: Add SearchMappings to CORE_ATTRIBS

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

Change 593582 merged by jenkins-bot:
[mediawiki/core@master] registration: Move SearchMappings to CORE_ATTRIBS from NOT_ATTRIBS

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

Change 596417 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Add RELEASE-NOTES for extension.json SearchMappings

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

Change 596417 merged by jenkins-bot:
[mediawiki/core@master] Add RELEASE-NOTES for extension.json SearchMappings

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