Silverstripe-framework: [ORM] SS_DATABASE_SUFFIX doesn't work

[kinglozzer]

Documented here: https://docs.silverstripe.org/en/4/getting_started/environment_management/#core-environment-variables, CoreKernel doesn’t take it into account at all

[tractorcow]

It seems to do so here.

   protected function getDatabasePrefix()
    {
        return Environment::getEnv('SS_DATABASE_PREFIX') ?: '';
    }

    protected function getDatabaseName()
    {
        // ...

        // Check environment
        $database = Environment::getEnv('SS_DATABASE_NAME');

        if ($database) {
            return $this->getDatabasePrefix() . $database;
        }

Let me know if there's something wrong here and I can re-open this issue. ;)

[tractorcow]

quick test

SS_DATABASE_PREFIX="superprefix_"
SS_DATABASE_NAME="myname"

dev/build:

Building database superprefix_myname using SilverStripe\ORM\Connect\MySQL 5.5.5-10.2.8-MariaDB

[kinglozzer]

Hahaha, doh, I meant suffix! 🤦‍♂️

[tractorcow]

Oh....

[tractorcow]

I can see that if we fixed this, it could possibly leave a lot of websites with a broken database, as the previously un-suffixed databases would suddenly work.

What if we changed this in 5.x to work again?

[kinglozzer]

Yeah 5.x is fine by me

[dhensby]

oh man, I'm not so sure about leaving this out of the 4.x line... if someone "fixed" it by keeping the suffix defined and added it to the DB name too... well, they are asking for trouble..

[tractorcow]

Yes but if we fixed it in 4.x, it would result in an upgrade having a double-suffixed database (and apparent blank one would be created).

[dhensby]

Yes but if we fixed it in 4.x, it would result in an upgrade having a double-suffixed database (and apparent blank one would be created).

how, though? by someone doing what I said and double defining it?

[tractorcow]

If you have

SS_DATABASE_NAME="mydb_suffix"
SS_DATABASE_SUFFIX="_suffix"

If we "fixed" this bug, the full DB name would switch from mydb_suffux to mydb_suffix_suffix wouldn't it?

[dhensby]

yep, but that's pretty mental set up ;)

[sminnee]

This feels like something we could do in a minor change, with a message along the lines of "if you had SS_DATABASE_SUFFIX present and you were relying on it not working, watch out"

Perhaps we could have a check if the database-without-suffix exists and the database-with-suffix does not, to throw an explanatory warning: "as of 4.x, SS_DATABASE_SUFFIX has been fixed, and your system appears to rely on its nonexistence".

A bit messy but would guard against the impact of the change?

[kinglozzer]

Pull request here: https://github.com/silverstripe/silverstripe-framework/pull/8470