tomohulk / WinSCP

WinSCP PowerShell Wrapper Module
GNU General Public License v3.0
156 stars 30 forks source link

Sync-WinSCPPath is not downloading newer versions #76

Closed meepsie closed 6 years ago

meepsie commented 6 years ago

Issue Description

I have written a script to mirror the McAfee update directory to a local server. However I have found that if the file already exists on the local server then the newer file on the McAfee FTP server is not downloaded. New files are correctly downloaded and timestamped and obsolete files on the local directory are correctly removed.

I have tried some debugging myself and here's what I have found:

  1. If I use the .Net DLL directly and the $session.SynchronizeDirectories() then newer files are downloaded.
  2. But with exactly the same settings using Sync-WinSCPPath newer files are not downloaded. I am very confused because the module file Sync-WinSCPPath.ps1 just looks like it wraps the .Net DLL function so I don't understand why the .Net assembly script works and the PowerShell wrapper doesn't. I have included the 2 scripts below. They are very basic with just enough commands to replicate the issue.

To replicate the issue:

  1. Create a local directory C:\NAIMirror
  2. Install the WinSCP module - Install-Package -Name WinSCP
  3. Run either script which will populate the local NAIMirror directory
  4. Run (Get-Item -Path C:\NAIMirror\avvdat.ini).LastWriteTime = (Get-Item -Path C:\NAIMirror\avvdat.ini).LastWriteTime.AddDays(-5)
  5. Run WinSCP module script or WinSCP .Net Assembly script
  6. Repeat 4 and 5 alternating to the other script and observe the file timestamp after each script

My 2 Scripts

.Net Assembly script that works

# Load WinSCP .NET assembly
Add-Type -Path "C:\Program Files (x86)\WindowsPowerShell\Modules\WinSCP\5.13.1.2\lib\WinSCPnet.dll"

$sessionOptions = New-Object WinSCP.SessionOptions -Property @{
        Protocol = [WinSCP.Protocol]::Ftp
        HostName = "ftp.nai.com"
        UserName = "anonymous"
 }
$session = New-Object WinSCP.Session

# Connect
$session.Open($sessionOptions)

# Synchronize files
$synchronizationResult = $session.SynchronizeDirectories(
            [WinSCP.SynchronizationMode]::Local, "C:\NAIMirror", "commonupdater2", $True, $True)
Write-Host "IsSuccess :",$synchronizationResult.IsSuccess
$session.Dispose()

WinSCP Module script that fails

Import-Module -Name WinSCP

$sessionOption = New-WinSCPSessionOption -HostName ftp.nai.com -Protocol Ftp
$sessionOption.UserName = "anonymous"

New-WinSCPSession -SessionOption $sessionOption

Sync-WinSCPPath -Mode Local -LocalPath "C:\NAIMirror" -RemotePath "commonupdater2"

Remove-WinSCPSession

Expected Output

The avvdat.ini file timestamp should be the same as the timestamp on the file hosted on the ftp.nai.com after either script is run but only after the .Net assembly script is this true.

Actual Output

Using WinSCP-Powershell does not download the newer file and the local file LastWriteTime does not change. I have actually found that if the local (destination) file exists then WinSCP Powershell module never downloads the file. So if the local file is newer than the source file and the Mirror switch is included then the file should be overwritten but isn't. Using the Remove switch works correctly but when "-Mode Local" is set, the output does not list removed files. This is a minor observation and not related to the actual issue of not downloading files that exist in the destination.

WinSCP-PowerShell Version

WinSCP Module version : 5.13.1.2

Environment

I have tried this on Windows 10 / Windows 7 / Windows Server 2012 R2 with WMF 5 and WMF 5.1 installed. My Windows 7 is: Name : ConsoleHost Version : 5.0.10586.117 InstanceId : 1421b7c4-a9b8-420a-aee8-c8fc558d3806 UI : System.Management.Automation.Internal.Host.InternalHostUserInterface CurrentCulture : en-GB CurrentUICulture : en-US PrivateData : Microsoft.PowerShell.ConsoleHost+ConsoleColorProxy DebuggerEnabled : True IsRunspacePushed : False Runspace : System.Management.Automation.Runspaces.LocalRunspace

dotps1 commented 6 years ago

Could you try one thing? in your script using the assembly, these are your args: [WinSCP.SynchronizationMode]::Local, "C:\NAIMirror", "commonupdater2", $True, $True The first $true is for Removing obsolete files and the second $true is for Mirror Mode. However, in your command using the module, you are not specifying those same options. You would need to do:

Sync-WinSCPPath -Mode Local -LocalPath "C:\NAIMirror" -RemotePath "commonupdater2" -Remove -Mirror

because those parameters are both of type Switch in the logic its looking for $Remove.IsPresent and $Mirror.IsPresent which means in the module command, those are $false. Please let me know if that helps. Thanks for the feed back.

meepsie commented 6 years ago

Sorry for the inconsistency. I have tried to compare the different ways to use both the .Net assembly and the POSH module and just happened to post with the differences in the code. I have added the switches and it makes no difference. I have also removed the switches/parameters from both scripts and as this is the core function of the synchronization, new and newer files should be synchronized, the issue is still present with the POSH module.

So in summary: If the mode is Local and a file exists in the destination then the source file is not downloaded when using the POSH module. Using the .Net Assembly no issue exists with the core functionality or when using the mirror switch where older files in the source are also downloaded.

The one scenario I cannot test but for completeness and maximum information for you I would like to is if the mode is remote. Does the Posh module upload files that exist in the destination (remote) file system?

Although I have good scripting skills such as vbscript and powershell, I don't know how to debug a module i.e step through the process, so I am struggling to help further. If you need me to do anything else (I have VS2017 on one PC) then I'd really like to help where I can.

Thanks for the module it certainly makes my script easier for my other admins to understand but until I can get the sync working through the module I'll have to use the assembly directly.

dotps1 commented 6 years ago

very strange, ill have to dig into this when I get a few minutes. Here is essentially the command being run:

$WinSCPSession.SynchronizeDirectories(
    $Mode, $localPathInfo.FullName, $RemotePath, $Remove.IsPresent, $Mirror.IsPresent, $Criteria, $TransferOptions
)

So the command should be exactly the same as calling it directly, the way you are with the assembly. Ill try to look it this later today. Thanks for the very thourogh information!

dotps1 commented 6 years ago

Ok, so I found the issue. When initializing the -Criteria, parameter, I am creating a default [WinSCP.SynchronizationCriteria] as the value, IE: $Criteria = (New-Object -TypeName WinSCP.SynchronizationCriteria). Well, according to the WinSCP Docs, that should default to a value of Time: https://winscp.net/eng/docs/library_session_synchronizedirectories. However that is not what is happening:

PS C:\> New-Object WinSCP.SynchronizationCriteria
None

So, what is happening, is the first time you sync, its working because the file does not exist, IE its using the value of None, its just making sure the same files exist on both locations. But the next time, it doesn't care because the file exists. So, you can add the -Criteria Time Parameter and Value to your code:

Sync-WinSCPPath -Mode Local -LocalPath "C:\NAIMirror" -RemotePath "commonupdater2" -Remove -Mirror -Criteria Time

I could change the initialization of that object to default to Time. @martinprikryl what are you thoughts on that? Should the object be defaulting to Time, rather than None?

martinprikryl commented 6 years ago

SynchronizationCriteria is an enumeration type. It cannot have a default value. It's the criteria parameter of SynchronizeDirectories that has SynchronizationCriteria.Time as a default value.

dotps1 commented 6 years ago

Thanks @martinprikryl, based on that I will default the value to Time so the behavior matches that of the method in the assembly. @meepsie I'm not going to do a release just for this, so in the meantime use the -Criteria Time Parameter and Value. I will make the change and it will be part of the next release of the assembly.

thanks.

meepsie commented 6 years ago

Many thanks @dotps1. It's really nice to know why, so I greatly appreciate your time in explaining what's going on. I agree with your decision to update the initialization to maintain synergy with the assembly and thanks for the working solution, my script will be much simpler as a result. Keep up the great work.