Deprecations in schema- and namespace-related APIs

[morozov]

Q	A
Type	improvement, deprecation
BC Break	no

See #4503.

1. PostgreSQLSchemaManager::getExistingSchemaSearchPaths() and ::determineExistingSchemaSearchPaths() have been marked internal and will be made protected in 4.0.0. They are only used by the schema manager itself and are not part of the API.
2. AbstractSchemaManager::dropSchema() has been added to implement the test properly.
3. AbstractSchemaManager::listSchemaNames() has been added as a replacement for AbstractSchemaManager::listNamespaceNames() and PostgreSQLSchemaManager::getSchemaNames().

Reasoning behind the naming changes:

The "namespace" term is internal to PostgreSQL and isn't part of the ANSI SQL vocabulary:

A namespace is the structure underlying SQL schemas

Schemas are supported by most if not all platforms supported by the DBAL.

Reasoning behind the design changes:

The new AbstractSchemaManager::listSchemaNames() method have been implemented directly in the schema manager instead of using the getListSchemataSQL() → getPortableSchemaList() → getPortableSchemaDefinition() approach. This approach has the following flaws:

1. Although getListSchemataSQL(): string would be part of the abstract API, it could be used only within a given platform or even its version, so it's not really part of the abstraction. Also, such methods are untestable unless tested as part of the schema manager.
2. getPortableSchemaDefinition() would expect a list<string,mixed> and return a list<string> but again, it couldn't accept a row set from another platform. Furthermore, neither the API nor default the default implementation of such methods make any sense:

   dbal/src/Schema/AbstractSchemaManager.php

   Lines 710 to 718 in f18aee0

   	/**
   	* @param mixed $database
   	*
   	* @return mixed
   	*/
   	protected function _getPortableDatabaseDefinition($database)
   	{
   	return $database;
   	}

AbstractSchemaManager::dropSchema() has been implemented using the implementation from AbstractPlatform in order to be able to reuse it go generate migrations.

[greg0ire]

PHP 7.3 allows you to use indention for nowdoc here and below 🙂

[morozov]

I have mixed feelings about this feature:

1. The left margin of the block is not clearly visible.
2. It's usually a program within a program, so it's fine that it's not indented the same as the outer one.

So far, it's consistent with the rest of the codebase (except for #4483). Let's reconsider this at the coding standard level first.

[greg0ire]

dataproviders return signature should match the return iterable<array{…}>, where the … part is build from the phpdoc of the method that use it, so here return iterable<array{callable(AbstractSchemaManager): list<string>}>?

[morozov]

ERROR: InvalidReturnType - tests/Functional/Schema/SchemaManagerFunctionalTestCase.php:180:16 - The declared return type 'iterable<mixed, array{callable(Doctrine\DBAL\Schema\AbstractSchemaManager):list}>' for Doctrine\DBAL\Tests\Functional\Schema\SchemaManagerFunctionalTestCase::listSchemaNamesMethodProvider is incorrect, got 'Generator<int, array{impure-Closure(Doctrine\DBAL\Schema\AbstractSchemaManager):array<array-key, string>|impure-Closure(Doctrine\DBAL\Schema\AbstractSchemaManager):list}, mixed, void>' (see https://psalm.dev/011)
* @return iterable<array{callable(AbstractSchemaManager): list}>

I agree in general but it would be much better if it was validated by the static analysis. Otherwise, it helps only when implementing the data provider but isn't used by the caller.

Similarly to the above comment, let's get it sorted out at the project level and apply consistently. Psalm at level 1 should be able to report (and maybe fix) some of such issues.