flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.28k stars 826 forks source link

flarum install should never nuke an existing DB #4018

Open sbourdeauducq opened 2 weeks ago

sbourdeauducq commented 2 weeks ago

Current Behavior

It is obviously not acceptable for flarum install to drop entire database tables, such as the user tables, if they exist.

Steps to Reproduce

php flarum install on an existing DB.

Expected Behavior

flarum install exercises restraint if data already exists in the database, instead of wantonly deleting everything.

Screenshots

No response

Environment

Whatever

Output of php flarum info

No response

Possible Solution

No response

Additional Context

No response

sbourdeauducq commented 2 weeks ago

Proposed resolution: if any of the tables already exists, flarum install leaves the database untouched and exits with an error.

luceos commented 1 week ago

Under normal circumstances, and from what I've seen when trying to run install on an existing database, is that Flarum actually errors with the fact that the settings table already exists.

Is it possible this "nuking" is due to something on your end or in your hosting environment?

fsagbuya commented 5 days ago

I encountered similar issues when attempting to install Flarum on an existing DB:

  1. I had an existing Flarum database with the following content:
    
    mysql> SELECT content FROM posts;
    +----------------------+
    | content              |
    +----------------------+
    | <t><p>Hi</p></t>     |
    | <t><p>Hello!</p></t> |
    +----------------------+
    2 rows in set (0.00 sec)
2. Using both the CLI command `flarum install` and the web installer, I received errors indicating tables already exist:

SQLSTATE[42S01]: Base table or view already exists: 1050 Table 'flags' alre
ady exists (SQL: create table flags (id int unsigned not null auto_incr
ement primary key, post_id int unsigned not null, type varchar(255) not
null, user_id int unsigned null, reason varchar(255) null, reason_det ail varchar(255) null, time datetime not null) default character set utf
8mb4 collate 'utf8mb4_unicode_ci' engine = InnoDB)

3. After these errors, I checked the database. While the tables remained, their contents were empty:

mysql> SELECT content FROM posts; Empty set (0.00 sec)


This behavior suggests the issue is likely with Flarum's installation process rather than a specific hosting environment. I tested this on Linux PopOS VM with `nginx` and `MySQL`.
luceos commented 4 days ago

Ok I can confirm this. The logic is contained in the install.dump in the migrations folder. It seems every table create statement has a drop exists check before it.

@flarum/core I don't think this is a wise solution, do we want to remove these for v2?

SychO9 commented 4 days ago

we shouldn't modify the install dumps, instead we need to add a check before installFromSchema is executed in the same conditional statement.

        if (! $migrator->repositoryExists() && ! $migrator->installFromSchema($this->path, $this->driver)) {
            $migrator->getRepository()->createRepository();
        }

https://github.com/flarum/framework/blob/eb6e599df113921a9f807ed47a5e80192e92fb05/framework/core/src/Install/Steps/RunMigrations.php#L36