Conversation
NewFTPServer/FTPServer/Server.cs
Outdated
There was a problem hiding this comment.
Здесь нужно и в AcceptTcpClientAsync передавать cancellationToken, иначе listener никогда непозволит серверу остановиться, если к серверу никто не подключиться
NewFTPServer/FTPServer/Server.cs
Outdated
There was a problem hiding this comment.
Зачем подчеркивания? :)
В Питоне, например, модификаторов видимости нет и это обосновано.
Здесь с маленькой буквы -- поле, с большой - свойство
NewFTPServer/FTPServer/Server.cs
Outdated
There was a problem hiding this comment.
Лучше сразу читать по строке, чтобы следовать протоколу (команды разделяются символом перевода строки) и не убирать потом \n
NewFTPServer/FTPServer/Server.cs
Outdated
There was a problem hiding this comment.
Зачем здесь StringBuilder? Можно все то же самое без него сделать включая получение первого символа строки
NewFTPServer/FTPServer/Server.cs
Outdated
There was a problem hiding this comment.
Соединения лучше сразу закрывать после того, как сервер ответил. Иначе остаются бесполезные открытые соединения, в которые никто не пишет и из которых никто не читает
NewFTPServer/TestsFTP/TestsFTP.cs
Outdated
There was a problem hiding this comment.
Не, это низкоуровневая многопоточность. В обычной жизни (когда не надо писать тредпул) лучше использовать более высокоуровневые Task-и.
Здесь, например, Task.Run.
И надо останавливать сервер в конце теста, если он такой метод (stop) предоставляет
NewFTPServer/FTPClient/Client.cs
Outdated
There was a problem hiding this comment.
Не, это инициирует синхронное подключение. Тут надо использовать конструктор без параметров и далее ConnectToAsync
NewFTPServer/FTPClient/Client.cs
Outdated
There was a problem hiding this comment.
| await stream.WriteAsync(Encoding.UTF8.GetBytes("2 " + filePath + "\n")); | |
| await stream.WriteAsync(Encoding.UTF8.GetBytes($"2 {filePath}\n")); |
| } | ||
|
|
||
| private static async Task<string> GetResultFromStream(NetworkStream stream, string method) | ||
| { |
There was a problem hiding this comment.
Зачем передавать method, если он никак не используется? :)
И здесь некоторое неследование протоколу: считывать ответ надо до символа конца строки в случае list-запроса; считывать сначала длину ответа, а потом ответ, используя эту длину, для get-запроса.
NewFTPServer/FTPClient/Client.cs
Outdated
There was a problem hiding this comment.
Кстати, в случае Get запроса лучше передавать от вызывающего поток и туда считывать содержимое файла, полученное от сервера. Либо с помощью CopyToAsync, либо с помощью небольшого промежуточного буффера.
Чтобы файл сразу в память не читать, так как он большой может быть. Пусть вызывающий решает, что с ним делать
| Task.WaitAll(tasks.ToArray()); | ||
| foreach (var client in clients) | ||
| { | ||
| client.Close(); |
There was a problem hiding this comment.
"Соединения лучше сразу закрывать после того, как сервер ответил" -- не исправлено
NewFTPServer/FTPServer/Server.cs
Outdated
There was a problem hiding this comment.
Не, стало только хуже :)
Чтобы не совершать пребразование байты -> строка -> байты, можно сначала отправить длину сообщения (с помощью отдельного WriteAsync), потом текст (с помощью отдельного WriteAsync), а потом сделать Flush
| class Program | ||
| { | ||
| static async Task Main() |
| { | ||
| var buffer = new byte[4096]; | ||
|
|
||
| var sizeResult = await stream.ReadAsync(buffer, 0, buffer.Length); |
There was a problem hiding this comment.
Уже лучше, но здесь всё-таки надо сначала считывать длину ответа (до пробела), а потом ответ, используя эту длину
| /// </summary> | ||
| /// <param name="filePath">The relative path of the file from the specified path on the server</param> | ||
| public async Task<string> Get(string filePath) | ||
| { |
There was a problem hiding this comment.
Замечание про запись в поток актуально :)
| [TearDown] | ||
| public void Teardown() | ||
| { | ||
| server.Stop(); |
There was a problem hiding this comment.
Тесты падают. Похоже потому что server.Stop() единожды запрашивает остановку, и больше в силу реализации этот сервер уже запуститься не может
Одно из решений: запускать для каждого теста свой сервер на своем порту
3 participants
No description provided.