linuxserver / davos

Web-based FTP automation for Linux servers.
MIT License
167 stars 15 forks source link

"Delete Host File" doesn't work in some cases #16

Closed Sneakyrom closed 7 years ago

Sneakyrom commented 7 years ago

Hey,

unfortunately there seems to be an issue with the "Delete Host File" feature. I am still trying to figure out when exactly and why the issue occurs, but here is what I found out.

When you download two folders with several files inside, while having the "Delete Host File" checkbox activated the removal of the folder fails and the 2nd folder does not get downloaded.

In this exacmple I tried to download two folders called [HorribleSubs].Naruto.Shippuuden.-.489.[720p] and [HorribleSubs].Naruto.Shippuuden.-.489.[720p]2 wich contained 3 files each.

Here is the debug log:

2017-03-26 00:22:10.801 - DEBUG - [ProgressListener] - Progress downloaded: 81.07043679903812% 2017-03-26 00:22:22.691 - INFO - [FilesAndFoldersTranferStrategy] - Successfully downloaded file. 2017-03-26 00:22:22.691 - INFO - [FilesAndFoldersTranferStrategy] - Running post download actions on [HorribleSubs].Naruto.Shippuuden.-.489.[720p] 2017-03-26 00:22:22.691 - DEBUG - [TransferStrategy] - Running actions... 2017-03-26 00:22:22.691 - INFO - [MoveFileAction] - Executing move action: Moving [HorribleSubs].Naruto.Shippuuden.-.489.[720p] to /download/ 2017-03-26 00:22:22.693 - INFO - [MoveFileAction] - File successfully moved! 2017-03-26 00:22:22.693 - DEBUG - [TransferStrategy] - Finished running actions... 2017-03-26 00:22:22.694 - INFO - [SFTPConnection] - Deleting remote file at path: /files/completed/[HorribleSubs].Naruto.Shippuuden.-.489.[720p] 2017-03-26 00:22:22.694 - DEBUG - [SFTPConnection] - Path is for a directory, so calling channel#rmdir() 2017-03-26 00:22:22.818 - DEBUG - [SFTPConnection] - channel threw exception. Assuming file not deleted 2017-03-26 00:22:22.818 - ERROR - [DownloadFilesWorkflowStep] - Unable to complete download. Error was: Unable to delete file on remote server 2017-03-26 00:22:22.818 - DEBUG - [DownloadFilesWorkflowStep] - Stacktrace io.linuxserver.davos.transfer.ftp.exception.DownloadFailedException: Unable to delete file on remote server at io.linuxserver.davos.transfer.ftp.connection.SFTPConnection.deleteRemoteFile(SFTPConnection.java:174) ~[classes!/:?] at io.linuxserver.davos.schedule.workflow.DownloadFilesWorkflowStep.runStep(DownloadFilesWorkflowStep.java:53) [classes!/:?] at io.linuxserver.davos.schedule.workflow.FilterFilesWorkflowStep.runStep(FilterFilesWorkflowStep.java:69) [classes!/:?] at io.linuxserver.davos.schedule.workflow.ConnectWorkflowStep.runStep(ConnectWorkflowStep.java:36) [classes!/:?] at io.linuxserver.davos.schedule.workflow.ScheduleWorkflow.start(ScheduleWorkflow.java:44) [classes!/:?] at io.linuxserver.davos.schedule.RunnableSchedule.run(RunnableSchedule.java:43) [classes!/:?] at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:1.8.0_121] at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) [?:1.8.0_121] at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) [?:1.8.0_121] at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) [?:1.8.0_121] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [?:1.8.0_121] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [?:1.8.0_121] at java.lang.Thread.run(Thread.java:745) [?:1.8.0_121] Caused by: com.jcraft.jsch.SftpException: Directory is not empty at com.jcraft.jsch.ChannelSftp.throwStatusError(ChannelSftp.java:2833) ~[jsch-0.1.50.jar!/:?] at com.jcraft.jsch.ChannelSftp.rmdir(ChannelSftp.java:2109) ~[jsch-0.1.50.jar!/:?] at io.linuxserver.davos.transfer.ftp.connection.SFTPConnection.deleteRemoteFile(SFTPConnection.java:164) ~[classes!/:?] ... 12 more 2017-03-26 00:22:22.821 - INFO - [DownloadFilesWorkflowStep] - Clearing current queue and will still continue to next step 2017-03-26 00:22:22.821 - DEBUG - [SFTPClient] - Disconnecting from channel 2017-03-26 00:22:22.821 - DEBUG - [SFTPClient] - Disconnecting from session 2017-03-26 00:22:22.823 - INFO - [ScheduleWorkflow] - Finished schedule run: Download completed Files from FTPServer 2017-03-26 00:22:22.823 - DEBUG - [RunnableSchedule] - Workflow finished 2017-03-26 00:22:22.823 - DEBUG - [RunnableSchedule] - Saving newly scanned files against schedule

Edit: I tried to find the issue but from the code it seems that only single files are supported at this point. I think the api calls behave like "rm" and "rmdir" and we would need something like "rm -r folder".

If I get that right we would just need to copypaste the "download" method and change every download command to the corresponding rm command. Unfortunately my programming skills have seen better days (sued to be pretty good with Fortran) and actually I have no clue how to use github correctly. It would be very nice if someone could take over from here.

Edit2: Here is a vague implementation of the fix:

`public void delete(FTPFile file, String localFilePath) {

    String path = FileUtils.ensureTrailingSlash(file.getPath()) + file.getName();
    String cleanLocalPath = FileUtils.ensureTrailingSlash(localFilePath);

    try {

        if (file.isDirectory())
            deleteDirectoryAndContents(file, cleanLocalPath, path);
        else
            channel.rm(path);

    } catch (SftpException e) {
        throw new DownloadFailedException("Unable to download file " + path, e);
    }
}`

`private void deleteDirectoryAndContents(FTPFile file, String localDownloadFolder, String path) throws SftpException {

    List<FTPFile> subItems = listFiles(path).stream().filter(removeCurrentAndParentDirs()).collect(Collectors.toList());

    String fullLocalDownloadPath = FileUtils.ensureTrailingSlash(localDownloadFolder + file.getName());

    for (FTPFile subItem : subItems) {

        String subItemPath = FileUtils.ensureTrailingSlash(subItem.getPath()) + subItem.getName();

        if (subItem.isDirectory()) {

            String subLocalFilePath = FileUtils.ensureTrailingSlash(fullLocalDownloadPath);
            deleteDirectoryAndContents(subItem, subLocalFilePath, FileUtils.ensureTrailingSlash(subItemPath));
        }

        else {

            channel.rm(subItemPath);
        }
    }
}`

Since I am getting old I have no clue how to commit this or how to use the Logger correctly but in theory you just need to append this to "SFTPConnection.java" and replace "channel.rmdir(path)" with "delete(file, localFilePath)"

Note that you would need a dummy filepath with this old mans solution. If you want to make it pretty you would have to delete all remenants of the lazy copypasta.

I hope that kind of helps.

I'm out. Cheers

JoshStark commented 7 years ago

Ah, I was afraid this might be the case. I'll write a detailed acceptance test for this and see if I can implement the fix in the next couple of days. Apologies.

JoshStark commented 7 years ago

This has now been fixed in the 2.1.2 release. Please can you verify it works as expected. I have written some very high level acceptance tests and can confirm it works in a controlled environment.

Sneakyrom commented 7 years ago

No appoligies needed whatsoever, you spend your free time developping software that is available to everyone for free use. This application will spare me quite a few hours fiddling around with setting up various scripts to archive a similar effect. So the very least I can to is try to help you out as good as I can.

The new code looks way better, I'd also say it should work now. I will report back how it works on my setup when I get home again (out of the country for a week).