Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flarum install should never nuke an existing DB #4018

Open
sbourdeauducq opened this issue Aug 31, 2024 · 5 comments
Open

flarum install should never nuke an existing DB #4018

sbourdeauducq opened this issue Aug 31, 2024 · 5 comments
Labels
Milestone

Comments

@sbourdeauducq
Copy link
Sponsor

sbourdeauducq commented Aug 31, 2024

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
Copy link
Sponsor Author

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

@luceos
Copy link
Member

luceos commented Sep 4, 2024

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
Copy link

fsagbuya commented Sep 12, 2024

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)

  1. 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)
  1. 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
Copy link
Member

luceos commented Sep 12, 2024

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
Copy link
Member

SychO9 commented Sep 13, 2024

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();
        }

if (! $migrator->installFromSchema($this->path, $this->driver)) {

@SychO9 SychO9 added this to the 2.0 milestone Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants