froger-me / wp-packages-update-server

WP Packages Update Server - Run Your Own Update Server for Plugins and Themes
GNU General Public License v3.0
141 stars 39 forks source link

Delete and Delete All Packaes not work #6

Closed ghost closed 5 years ago

ghost commented 5 years ago

Hello,

I'm trying your plugin and I realized that all the options to delete a plugin do not work. I tried on Linux where I could have a permissions problem and also on Windows but the problem persists.

With WP_DEBUG set no warning or error is displayed.

Do you have any idea?

froger-me commented 5 years ago

Hello @adrix71 ! Thanks for the notice.

Could you create a small plugin and hook into the action wppus_deleted_package, like below, and check the log? I could not reproduce the issue personally.

add_action( 'wppus_deleted_package', 'test_wppus_delete_package', 10, 3 );
function test_wppus_delete_package( $result, $type, $slug ) {

    if ( $result ) {
        error_log( 'SUCCESS - package with slug ' . $slug . ' of type ' . $type . ' deleted from the file system.');
    } else {
        error_log( 'FAILURE - package with slug ' . $slug . ' of type ' . $type . ' could NOT be deleted from the file system.');
    }
}

FYI: internally, the plugin uses WP_Filesystem() and its global $wp_filesystem from WordPress Core to call $wp_filesystem->delete( $package_path ) (called in wp-content/plugins/wp-plugin-update-server/inc/class-wp-plugin-update-server.php on line 171 and defined in wp-admin/includes/wp-filesystem-direct.php on line 279). The delete method uses the @unlink fucntion - that's right, with warnings silenced with the @.

If nothing is output in the logs, then the package was not found for some reason. If the logs shows succes, as far as WordPress (and by extension the plugin) is concerned, the file was deleted. If the log shows failure, the plugin and more specifically WordPress could not delete the file, and some warning might have been silenced.

ghost commented 5 years ago

Logs do not show any output. Now I try to investigate further.

froger-me commented 5 years ago

Ok, so in this case it seems to mean that $wp_filesystem->is_file( $package_path ) on line 178 of wp-content/plugins/wp-plugin-update-server/inc/class-wp-plugin-update-server.php returned false, that is to say that the path in $package_path variable does not point to a file as far as WordPress is concerned. Maybe editing the plugin's code base to determine the content of $package_path with a log trace, and checking the file system (existence of the file, permissions, etc) may help.

ghost commented 5 years ago

I installed the plugin directly from WordPress.org but where you have indicated to me nothing makes you think of something to delete the files:

wp-content/plugins/wp-plugin-update-server/inc/class-wp-plugin-update-server.php on line 171 is correct ?

If it's correct maybe I'm doing something wrong because, as you can see in the screen, the content of my file is completely different.

Maybe I'm using a wrong version of the plugin?

cattura

The only occurrence of $wp_filesystem->delete($package_path) is found in class-wppus-update-server.php on line 181.

cattura

The php unlink() method, with or without error suppressor, is used a total of 23 times - 5 are comments. inside the wp-content/plugins/wp-plugins-update-server directory project and you use it three times only in the download_remote_package() method.

cattura

In no other file of your plugin is referred to methods of cancellation, at least I have not found.

I'm doing something wrong? I'm using a wrong plugin.

ghost commented 5 years ago

Maybe I found the problem. Making a simple echo statment on the $pachage_path variable immediately after the line

$package_path = trailingslashit( $this->packageDirectory ) . $slug . '.zip';

in the remove_package() method line gives me this output

C:/web/www/envato/wp-content/wppus//packages/C:/web/www/envato/wp-content/wppus/packages/omega-server-manager.zip

which is certainly not valid. This does not explain why on Linux it gives me the same problem.

Please note that this output can only be obtained by clicking on the Delete All Packages button. If I try to delete the file from the links or using the bulk delete I do not get any output, as if the remove_package() method was not even called.

For completeness it seems that the value of packageDirectory is incorrect. In fact, in the remove_package() method I have these values:

$slug = C:/web/www/envato/wp-content/wppus/packages/omega-server-manager this is correct.

$this->packageDirectory = C:/web/www/envato/wp-content/wppus//packages this is wrong, note the double slash before packages

$package_path = C:/web/www/envato/wp-content/wppus//packages/C:/web/www/envato/wp-content/wppus/packages/omega-server-manager.zip obviously this is also wrong

Update:

The paths are set in the original Wpup_UpdateServer class and in fact modify them to become correct. Likewise, the cancellation does not take place. Why ?

Update 1: I have found a work around for the Delete All Packages options. In the remove_package() simple set the $package_path without the trailingslahit( $this->packageDirectory), since the $slug variable already contains the full path minus the zip extension.

Example:

$package_path` = $slug . '.zip';

work and delete the file correctly but only for this options. The bulk delete and the link on the file name not work again.

froger-me commented 5 years ago

Hi @adrix71 ! Indeed, bad copy/paste of filename - definitely class-wppus-update-server.php. Thanks for the investigation.

As for the rest, I'll do a full trace of the delete methods tomorrow and keep you updated (23:30 Beijing time now).

ghost commented 5 years ago

OK thank you very much. In the meantime, I continue to investigate. Let's see if we can solve because the plugin is valid and very useful.

See you tomorrow

ghost commented 5 years ago

Last update for now.

Out of curiosity I gave a nice arrangement to the permissions on the linux box and now the cancellation options work properly. Who knows what the hell I had done before.

On Windows it still does not work. It happened to me with another plugin that did not recognize the path on windows and I had developed a class with a static method that establishes the operating system in use and based on that creates the correct path.

I look for it and do copy paste.

froger-me commented 5 years ago

Hello @adrix71 Glad to see you found a solution for UNIX systems. Sorry for not replying earlier.

As for the double slash in the $this->packageDirectory variable, it is not critical for UNIX systems - however, I found where the issue is and I fixed the code base.

Also, I was confused with what's happening on Windows file systems. Indeed, $slug = 'C:/web/www/envato/wp-content/wppus/packages/omega-server-manager' feels odd here, because it should be $slug = 'omega-server-manager. Internally, the code is using DIRECTORY_SEPARATOR to break the path, extract the last fragment, and remove the extension (line 466 of class-wppus-update-manager.php) - so it should be file system agnostic. In any case, I've changed how the slug is determined, using basename PHP function ("duh!", yeah, I know...) after reading here.

Feel free to update to v1.4.13 and let me know!

ghost commented 5 years ago

First of all, thank you for solving the problem. Now it works correctly even on Windows.

The problem of properties set up as ../../ in the class UpdateServer it was a known problem for me. I had already used this class a few years ago and I had also talked about it with Yahnis (original author) who never answered. It's nice to see that someone answers.

I wanted to ask you a question. In the past I used this class to manage updates of themes and plugins, but I've never been able to find the efficient way to use it to create a page of add-ons available for a specific plugin.

The problem is that WP Update Server works at the level of a single plugin or theme. The instance of the UpdateChecker class is created by passing the server IP. the slug and little else. It can also be created by sending the path of a json file.

However the main problem is that the instance that searches for updates is always relative to a single package (theme or plugin). To create my add-ons page I modified the dispatcher (and all its properties) to read a new action called get_addons, with the slug parameter to be passed to the corresponding new methods.

In practice, a query args like this:

?action=get_addons&slug=theme-o-plugin-slug&version=theme-o-plugin-version

However, as a system it has become fragile and unmanageable when needs have grown and in addition to add-ons, patches and translations have taken over.

So what I wanted to ask you is this. How could one do to add this feature?

I forgot one thing. I remember that wp update server allowed to do something like this:

Easy to set up.

Unfortunately with your plugin it does not work. I have also tried to the wp-content/wppus directory but without success.

Did you change anything?

Best regards

froger-me commented 5 years ago

Hi @adrix71 There seem to be 2 new questions here, I'm creating 2 new issues to keep the discussion on topic.

See you there!