Make WordPress Core

Opened 5 years ago

Last modified 6 days ago

#43162 new enhancement

Deleting a site from a multisite network leaves orphaned database tables and files

Reported by: bluep92877's profile bluep92877 Owned by:
Milestone: 6.3 Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: 2nd-opinion needs-testing has-patch has-unit-tests
Focuses: multisite Cc:

Description

Looking at the table structure of a typical multisite install, I see that tables for individual sites within the network are prefixed with wp_{siteid}_ tablename. When I delete a site from the from the network, all standard WP tables for that particular site ID are deleted; however, several plugin tables are not.

As well, looking at the uploads/sites directory, the directory for that site (represented by its ID) is also still there along with all its files (media and other uploads).

an earlier ticket:30673 suggested that this should be caught when uninstalling a plugin from the master site. But, I'd have to disagree. Most uninstall scripts use a type of {TABLEPREFIX} before its table names. This would delete the tables for every site in the network, wouldn't it? and that's not what the goal would be.

Aside from the plugin issue are the media files that are also orphaned. Media file uploads is part of core, and that's not something a plugin author could approach.

When deleting a site from the network, this is irrevocable. Wordpress even create an extra confirmation page upon delete saying so just to make sure. If that's the case, and this is by design, what's the point of keeping these extra files and tables if they are never going to be used again?

Shouldn't all site specific db tables and all files in the uploads directory be deleted when a site is deleted? Or, perhaps as an enhancement, on the Delete Site confirmation page, could we have two checkboxes: "1) delete all tables associated with this site" and "2) delete all files associated with this site"

Currently I have a script setup to alert me as to when a site is deleted. I then open up a cli to manually delete the db tables for that install, and then its files from the uploads directory. This doesn't feel like the intended workflow to handle the orphaned elements of the site.

Attachments (3)

Screen Shot 2023-01-31 at 4.46.15 PM.png (19.4 KB) - added by carl-alberto 4 months ago.
muconfirm.png (19.4 KB) - added by carl-alberto 4 months ago.
4172.diff (3.7 KB) - added by carl-alberto 3 weeks ago.
Trying to add a patch

Download all attachments as: .zip

Change History (19)

#1 @mdifelice
5 years ago

Hello. I do think that when you uninstall a plugin it will only delete the tables that it created for that specific site. Of course it depends on the plugin, and whether the plugin was network activated or only activated on the specific site.

Either way, I think that it would be a good addition what you propose. About the tables, it would be nice if WP listed all those orphan tables before deleting the site and ask the user what to do with them: delete or keep them.

About the media files, I think that each site has a particular folder assigned to them, and simply deleting it it would remove all the associated media.

Another doubt that I have is whether WP really deletes a site or it keeps like in a hidden status. Because if that is the case the idea is that the site could be recover at any time and in those cases files and extra tables should be kept.

Regards.

#2 @johnjamesjacoby
5 years ago

  • Keywords 2nd-opinion added
  • Type changed from defect (bug) to enhancement
  • Version changed from 4.9.2 to 3.0

Hey @bluep92877, thanks for opening this issue up, and congrats on your first ticket here on WordPress Trac.

I think you've brought up good points, and these are all areas where clarification and improvements are necessary.

Re: #30673, I agree that "uninstall" in the context of a multisite installation implies that all relevant data is removed from all sites, which seems heavy handed under most circumstances, though I can imagine why that might be necessary.

Plugins can (and should) use the wpmu_drop_tables filter to add their custom database tables to the array of tables that are dropped when a site is deleted.

Database tables and Uploads are both pretty sacred, IMO. If we are deleting one, we can safely delete the other. Dropping large database tables can be costly on the database server in a similar fashion to unlinking a directory. It looks like wpmu_delete_blog() does try to delete both the database tables and the uploads directory. Maybe something is wrong with the $drop logic, or something else is in the way?

That said, permanent deletion is so scary; I'm always hesitant to give anyone that power (even super admins.) 1 click to ruin it all is a lot of responsibility, even 2 or 3 clicks with warning signs and notices.


I think erring on the side of caution here is OK. Only delete stuff if we are 100% certain it's OK to do so, and leave it untouched if there is any doubt. This might result in orphaned tables or files, but I think that's better than blindly deleting them without explicit say-so.

I guess, now that I type this, we could (should) do a quick check on that delete-site confirmation screen that says "Here are all of the database tables this site has, and it contains 2GB of uploads. Are you sure you want to delete all of this stuff?"

#3 follow-ups: @scamartist26
5 years ago

For what it's worth, we have this same issue on our multisite install. I do not see any reason to keep tables around at all. We created this little guy as a workaround:

<?php

/**
 * Cleanup orphaned tables during site deletion
 *
 * @param $blog_id
 * @param $drop
 */
add_action( 'delete_blog', 'delete_blog_43162', 10, 2 );
function delete_blog_43162( $blog_id, $drop ) {

    if ( true == $drop ) {

        /**
         * SELECT all tables relating to a specific blog id and add them to wpmu_drop_tables
         */
        global $wpdb;
        $prep_query = $wpdb->prepare( "SELECT table_name FROM information_schema.TABLES WHERE table_name LIKE %s;", $wpdb->esc_like( "{$wpdb->base_prefix}{$blog_id}_" ) . '%' );
        $table_list = $wpdb->get_results( $prep_query, ARRAY_A );

        add_filter( 'wpmu_drop_tables', function ( $filter_list ) use ( $table_list ) {

            foreach( $table_list as $index => $data ) {
                $filter_list[] = $data['table_name'];
            }

            return array_unique( $filter_list );

        });
    }
}

#4 in reply to: ↑ 3 @waynmeyer
3 years ago

  • Keywords needs-testing added

Replying to scamartist26:

For what it's worth, we have this same issue on our multisite install. I do not see any reason to keep tables around at all. We created this little guy as a workaround:

<?php

/**
 * Cleanup orphaned tables during site deletion
 *
 * @param $blog_id
 * @param $drop
 */
add_action( 'delete_blog', 'delete_blog_43162', 10, 2 );
function delete_blog_43162( $blog_id, $drop ) {

    if ( true == $drop ) {

        /**
         * SELECT all tables relating to a specific blog id and add them to wpmu_drop_tables
         */
        global $wpdb;
        $prep_query = $wpdb->prepare( "SELECT table_name FROM information_schema.TABLES WHERE table_name LIKE %s;", $wpdb->esc_like( "{$wpdb->base_prefix}{$blog_id}_" ) . '%' );
        $table_list = $wpdb->get_results( $prep_query, ARRAY_A );

        add_filter( 'wpmu_drop_tables', function ( $filter_list ) use ( $table_list ) {

            foreach( $table_list as $index => $data ) {
                $filter_list[] = $data['table_name'];
            }

            return array_unique( $filter_list );

        });
    }
}

This worked like a charm, please could we have this added to core its vital especially for multisite. Alternative would be as @mdifelice stated, simply ask an admin when they deleting the subsite if they would like to delete orphaned tables with a checkbox next to each table for custom selection, when an admin wishes to retain data then the choice should be available as to what tables are chosen by the admin.

Bigup to @scamartist26 for assisting with the snippet.

#5 in reply to: ↑ 3 @deepakdcc
19 months ago

Last edited 19 months ago by deepakdcc (previous) (diff)

#6 @carl-alberto
4 months ago

To add on as a temporary solution in deleting the corresponding subsite's uploads folder

<?php
add_action( 'wp_delete_site', 'wpmu_delete_blog_uploads_folder' );

/**
 * Deletes the corresponding uploads folder of a subsite.
 *
 * @return boolean Return true if successful.
 */
function wpmu_delete_blog_uploads_folder() {

                require_once ABSPATH . 'wp-admin/includes/class-wp-filesystem-base.php';
                require_once ABSPATH . 'wp-admin/includes/class-wp-filesystem-direct.php';

                $upload_dir = wp_upload_dir(); // This function already calls the subsite's upload folder path automatically.

                $fileSystemDirect = new WP_Filesystem_Direct( false );

                return $fileSystemDirect->rmdir( $upload_dir['basedir'], true );

}

It seems @johnjamesjacoby is right that there is a ready function wpmu_delete_blog which also calls the wp_delete_site which also calls wp_uninitialize_site and supposedly deletes the corresponding uploads folder. Those functions might need to be revisited as it seems to be logical to delete those along when a subsite is deleted, otherwise a busy multisite can easily be bloated with unnecessary files or possibly have something like a checkbox for confirmation like what @mdifelice proposed, something like the attached image and might contain warnings that it may take awhile for large db/uploads:

Last edited 4 months ago by carl-alberto (previous) (diff)

This ticket was mentioned in PR #3951 on WordPress/wordpress-develop by carl-alberto.


4 months ago
#7

  • Keywords has-patch added

Update the wp_uninitialize_site function to delete the uploads folder and custom db tables created by plugins

Trac ticket: https://core.trac.wordpress.org/ticket/43162

#8 @carl-alberto
4 months ago

  • Keywords has-patch removed

Found the function that is supposed to delete the database table as well as the uploads folder which seems not to be working anymore https://github.com/WordPress/wordpress-develop/blob/186abb264d6c97dc955ecc787f29c12be3ffb33c/src/wp-includes/ms-site.php#L828-L866

to get this issue moving, opened up a PR to use the built in WP FS system functions

https://github.com/WordPress/wordpress-develop/pull/3951

This ticket was mentioned in PR #4172 on WordPress/wordpress-develop by @carl-alberto.


3 months ago
#9

  • Keywords has-patch has-unit-tests added

Update the wp_uninitialize_site function to delete the uploads folder and custom db tables created by plugins

Trac ticket: https://core.trac.wordpress.org/ticket/43162

#10 @SergeyBiryukov
3 weeks ago

  • Milestone changed from Awaiting Review to 6.3

Found this ticket after a coding session with @aristath and @poena, where we noticed that wp_uninitialize_site() calls unlink() and rmdir() directly, instead of using the Filesystem API. Fixing that seems like a good first step here.

This particular code appears to be introduced in mu:1069.

Last edited 3 weeks ago by SergeyBiryukov (previous) (diff)

@carl-alberto
3 weeks ago

Trying to add a patch

#11 @carl-alberto
3 weeks ago

Patch would be using the Filesystem API to delete the associated folder related to the subsite being deleted, deletes all the related tables for that subsite and it also contains the unit test. Hopefully I did it correctly :)

This ticket was mentioned in Slack in #core by carl-alberto. View the logs.


8 days ago

This ticket was mentioned in PR #4534 on WordPress/wordpress-develop by @carl-alberto.


7 days ago
#13

Patch would be using the Filesystem API to delete the associated folder related to the subsite being deleted, deletes all the related tables for that subsite and it also contains the unit test

Trac ticket: https://core.trac.wordpress.org/ticket/43162

This ticket was mentioned in PR #4537 on WordPress/wordpress-develop by @carl-alberto.


6 days ago
#14

fixing the orphaned uploads folder

https://core.trac.wordpress.org/ticket/43162

This ticket was mentioned in PR #4537 on WordPress/wordpress-develop by @carl-alberto.


6 days ago
#15

fixing the orphaned uploads folder

https://core.trac.wordpress.org/ticket/43162

This ticket was mentioned in PR #4538 on WordPress/wordpress-develop by @carl-alberto.


6 days ago
#16

fixing the failing php unit test for multisite

https://core.trac.wordpress.org/ticket/43162

Note: See TracTickets for help on using tickets.