MyFTP by MikePuzanov · Pull Request #4 · MikePuzanov/HomeworkSem3

MikePuzanov · 2021-10-26T20:19:19Z

No description provided.

yurii-litvinov

Что-то проектные файлы вообще не выложены :) И ConsoleApp1 как имя проекта не годится. И тестов нет.

yurii-litvinov · 2021-11-06T18:18:30Z

MyFTP/MyFTP/Program.cs

+        static void Main(string[] args)
+        {
+
+        }


Не, я хочу две консольные программы --- для сервера и для клиента. Чтобы я мог запустить сервер, и затем запустить клиент и подконнектиться к серверу.

yurii-litvinov · 2021-11-06T18:20:18Z

MyFTP/ConsoleApp1/Client.cs

+
+namespace ConsoleApp1
+{
+    public class Client


Нужны комментарии

yurii-litvinov · 2021-11-06T18:21:14Z

MyFTP/ConsoleApp1/Client.cs

+
+        public async Task<(string name, bool isDir)[]> List(string path)
+        {
+            using var client = new TcpClient(_host, _port);


Не-а, это заставит TcpClient синхронно подключаться к хосту, так что если у него не выйдет, процесс затянется до наступления таймаута, а всё это время поток будет висеть. Надо ConnectAsync

yurii-litvinov · 2021-11-06T18:22:11Z

MyFTP/ConsoleApp1/Client.cs

+
+            for (int i = 0; i < size; i++)
+            {
+                var name = await reader.ReadLineAsync();


По протоколу разделители --- пробелы, поэтому сервер вообще не может папки с пробелами правильно обрабатывать. Но протокол есть протокол, от него нельзя отступать, потому что Вы решили, что так правильнее.

yurii-litvinov · 2021-11-06T18:23:37Z

MyFTP/ConsoleApp1/Client.cs

+                throw new FileNotFoundException();
+            }
+
+            var content = new byte[size];


Не надо грузить файл в память целиком, файлы могут быть очень большие. stream.CopyToAsync вполне ок.

yurii-litvinov · 2021-11-06T18:24:47Z

MyFTP/MyFTP/Client.cs

+        /// <summary>
+        /// запрос на листинг файлов в папке по пути
+        /// </summary>
+        public async Task<(string name, bool isDir)[]> List(string path)


Кстати, все длительные операции должны быть прерываемы, так что и List, и Get должны принимать CancellationToken

yurii-litvinov · 2021-11-06T18:25:42Z

MyFTP/MyFTP/Server.cs

+            while (!_cancellationToken.IsCancellationRequested)
+            {
+                var client = await _listener.AcceptTcpClientAsync();
+                Working(client);


Многопоточность не поддержана, пока обслуживают одного клиента, второй не может подключиться. А надо

yurii-litvinov · 2021-11-06T18:26:32Z

MyFTP/MyFTP/Server.cs

+            var stream = client.GetStream();
+            var reader = new StreamReader(stream);
+            var writer = new StreamWriter(stream);


Они все IDisposable, так что должны быть using. client тоже, кстати

yurii-litvinov · 2021-11-06T18:27:39Z

MyFTP/MyFTP/Server.cs

+                    await Get(writer, path, stream);
+                    break;
+                default:
+                    throw new ArgumentException();


Так сервер упадёт, а клиент об этом ничего не узнает. Надо исключение бросать не сюда, а сообщать клиенту по сети, что ошибка протокола.

yurii-litvinov · 2021-11-06T18:28:08Z

MyFTP/MyFTP/Server.cs

+            {
+                await writer.WriteLineAsync("-1");
+                await writer.FlushAsync();
+            }


И дальше продолжили работать как ни в чём не бывало :)

yurii-litvinov · 2021-12-01T18:19:33Z

MyFTP/MyFTPClient/MyFTPClient.csproj

+
+    <PropertyGroup>
+        <OutputType>Exe</OutputType>
+        <TargetFramework>net5.0</TargetFramework>


Можно уже на .net 6 и C# 10 переходить, там всякие крутые штуки типа file-scoped namespaces и глобальных юзингов.

yurii-litvinov · 2021-12-01T18:19:58Z

MyFTP/MyFTPServer/Dockerfile

@@ -0,0 +1,18 @@
+FROM mcr.microsoft.com/dotnet/runtime:5.0 AS base


yurii-litvinov · 2021-12-01T18:21:09Z

MyFTP/MyFTP.Test/MyFTPTest.cs

+            _server = new Server("127.0.0.1", 80);
+            _client = new Client(80, "127.0.0.1");


Как-то оно хаотично, у одного сначала IP, потом порт, у второго наоборот

yurii-litvinov · 2021-12-01T18:22:06Z

MyFTP/MyFTP.Test/MyFTPTest.cs

+        public void GetInvalidFileNameTest()
+        {
+            Assert.Throws<AggregateException>(() => _client.Get("Text.txt", _fileStream, _cancellationToken).Wait());
+            _server.StopServer();


Это стоит в TearDown написать, а то если вдруг тест не пройдёт и минует StopServer, остальные тесты, скорее всего, даже не запустятся, потому что порт занят.

yurii-litvinov · 2021-12-01T19:42:14Z

MyFTP/MyFTP.Test/MyFTPTest.cs

+        [Test]
+        public async Task ListInvalidFileNameTest()
+        {
+            Assert.Throws<AggregateException>(() => _client.Get("Text.txt", _fileStream, _cancellationToken).Wait());


Бывает Assert.ThrowsAsync

yurii-litvinov · 2021-12-01T19:50:50Z

MyFTP/MyFTPServer/Program.cs

+            Console.WriteLine("Введите IP сервер:");
+            var ipAddress = Console.ReadLine();
+            Console.WriteLine("Введите порт сервера:");
+            var port = int.Parse(Console.ReadLine());


Тут тем более. Сервер в принципе не предполагает какой-то интерактивности, поэтому только аргументы командной строки, никаких ReadLine-ов :)

yurii-litvinov · 2021-12-01T19:51:09Z

MyFTP/MyFTPServer/Program.cs

+                    command = Console.ReadLine();
+                }
+            }
+            catch (Exception e)


Suggested change

catch (Exception e)

catch (Exception)

yurii-litvinov · 2021-12-01T19:51:46Z

MyFTP/MyFTPServer/Server.cs

+    public class Server
+    {
+        private readonly TcpListener _listener;
+        private readonly CancellationTokenSource _cancellationToken = new ();


Так это Source или токен? Мне кажется, Вы путаете фабрику и продукт. См. https://stackoverflow.com/questions/14215784/why-cancellationtoken-is-separate-from-cancellationtokensource

yurii-litvinov · 2021-12-01T19:53:36Z

MyFTP/MyFTPServer/Server.cs

+            while (!_cancellationToken.IsCancellationRequested)
+            {
+                var client = await _listener.AcceptTcpClientAsync();
+                Working(client);


Working нигде не await-ится, что огорчает компилятор и даёт возможность остановить сервер до того, как он успел ответить клиенту, что может привести к порче данных. Тут можно было сложить таск, который вернёт Working, в список, и потом в конце заawait-ить их все.

yurii-litvinov · 2021-12-01T19:55:30Z

MyFTP/MyFTPServer/Server.cs

+            foreach (var dir in directories)
+            {
+                result += $" {dir} true";
+            }


Конкатенировать строки в цикле правильнее StringBuilder-ом. Без него это одна большая утечка памяти, квадратичная от размера строки.

MyFTP/MyFTP.Test/MyFTPTest.cs

+using System.Threading.Tasks;
+using NUnit.Framework;
+
+public class Tests


yurii-litvinov · 2022-03-07T13:31:01Z

MyFTP/MyFTP.Test/MyFTPTest.cs

+    [Test]
+    public void GetInvalidFileNameTest()
+    {
+        Assert.ThrowsAsync<AggregateException>(() => Task.Run(() =>  _client.Get("Text.txt", _fileStream, _cancellationToken).Wait()));


Task.Run(...).Wait() --- это практически то же самое, что просто ..., эта конструкция довольно бессмысленна. На что намекает в частности то, что тут лямбда не асинхронная.

MyFTP/MyFTP.Test/MyFTPTest.cs

+        Assert.AreEqual(result, result2);
+        File.Delete(destination);
+    }
+}


MyFTP/MyFTPClient/Client.cs

+/// <summary>
+/// класс клиента для общения с сервером
+/// </summary>
+public class Client


yurii-litvinov · 2022-03-07T13:36:50Z

MyFTP/MyFTPClient/Client.cs

+        var size = Convert.ToInt32(infoArray[0]);
+        if (size == -1)
+        {
+            throw new Exception();


Ой, нет, кидать Exception запрещено :) Это слишком общее исключение

yurii-litvinov · 2022-03-07T13:38:49Z

MyFTP/MyFTPClient/Client.cs

+        }
+        if (size == -1)
+        {
+            throw new ArgumentException();


Это тоже. ArgumentException --- это когда нам неверные параметры передали

yurii-litvinov · 2022-03-07T13:40:03Z

MyFTP/MyFTPClient/Program.cs

+global using System.IO;
+
+var client = new Client(args[0], Convert.ToInt32(args[1]));
+var request = Console.ReadLine().Split(' ');


Передавать аргументы в консольное приложение не через args, а с помощью ReadLine --- нельзя, замучаетесь скрипты писать

yurii-litvinov · 2022-03-07T13:40:38Z

MyFTP/MyFTPClient/Program.cs

+
+    if (request[0] == "2")
+    {
+        using (var fstream = new FileStream(request[2], FileMode.OpenOrCreate))


Тут лучше просто using var, отдельный scope тут не нужен

yurii-litvinov · 2022-03-07T13:40:55Z

MyFTP/MyFTPClient/Program.cs

+                Console.WriteLine($"{file.Item1} {file.Item2}");
+            }
+        }
+        catch (Exception)


Ловить просто Exception тоже нельзя, кстати

yurii-litvinov · 2022-03-07T13:41:25Z

MyFTP/MyFTPServer/Program.cs

+try
+{
+    var server = new Server(args[0], Convert.ToInt32(args[1]));
+    await server.StartServer();


Есть ли какой-нибудь способ его остановить? :)

MyFTP/MyFTP.Test/MyFTPTest.cs

+using System.Threading.Tasks;
+using NUnit.Framework;
+
+public class Tests


yurii-litvinov · 2022-04-01T09:38:13Z

MyFTP/MyFTP.Test/MyFTPTest.cs

+        _client = new Client("127.0.0.1", 80);
+        _fileStream = new MemoryStream();
+        _cancellationToken = new ();
+        _= _server.StartServer();


Зачем присваивание в никуда? :) И пробела не хватает

yurii-litvinov · 2022-04-01T09:50:22Z

MyFTP/MyFTPClient/Program.cs

+    {
+        var client = new Client(args[0], Convert.ToInt32(args[1]));
+        var token = new CancellationToken();
+        while (args[0] != "exit" || !token.IsCancellationRequested)


args[0] != "exit" выглядит довольно странно. Будто args[0] как-то может поменяться :)

yurii-litvinov · 2022-04-01T09:51:16Z

MyFTP/MyFTPClient/Program.cs

+
+            if (args[0] == "2")
+            {
+                using (var fstream = new FileStream(args[2], FileMode.OpenOrCreate))


Можно using var без скобок

MyFTP/MyFTPServer/Program.cs

+        {
+            var server = new Server(args[0], Convert.ToInt32(args[1]));
+            await server.StartServer();
+            Console.WriteLine("Введите !exit, чтобы остановить сервер");


MyFTP/MyFTPServer/Program.cs

+    {
+        try
+        {
+            var server = new Server(args[0], Convert.ToInt32(args[1]));


MyFTP/MyFTPServer/Program.cs

+            while (command != "!exit")
+            {
+                command = Console.ReadLine();
+            }


yurii-litvinov · 2022-04-01T09:58:32Z

MyFTP/MyFTPServer/Server.cs

+    /// </summary>
+    private async Task Working(TcpClient client)
+    {
+        using var stream = client.GetStream();


client реализует IDisposable, так что надо сначала using на него сделать

yurii-litvinov

Да, вот теперь всё правильно, хоть и неаккуратно. Зачтена.

yurii-litvinov · 2022-04-05T11:01:59Z

MyFTP/MyFTP.Test/MyFTPTest.cs

+    [Test]
+    public void ParallelClientConnection()
+    {
+        var clients = new Task[2];


Ну, два клиента — это не очень убедительно, было бы их хотя бы 200... Но ладно.

yurii-litvinov · 2022-04-05T11:03:00Z

MyFTP/MyFTPClient/Client.cs

+        var data = new List<(string, bool)>();
+
+
+
+        for (int i = 1; i < infoArray.Length; i += 2)


Suggested change

var data = new List<(string, bool)>();

for (int i = 1; i < infoArray.Length; i += 2)

var data = new List<(string, bool)>();

for (int i = 1; i < infoArray.Length; i += 2)

yurii-litvinov · 2022-04-05T11:03:42Z

MyFTP/MyFTPClient/Client.cs

+        {
+
+            var isDir = Convert.ToBoolean(infoArray[i + 1]);


Suggested change

{

var isDir = Convert.ToBoolean(infoArray[i + 1]);

{

var isDir = Convert.ToBoolean(infoArray[i + 1]);

yurii-litvinov · 2022-04-05T11:05:08Z

MyFTP/MyFTPClient/Program.cs

+        var token = new CancellationToken();
+        Console.WriteLine("1 - List — листинг файлов в директории на сервере\nНапример, 1 ./Test/Files\n");
+        Console.WriteLine("2 - Get — скачивание файла с сервера\n2 ./Test/Files/file1.txt\n");
+        Console.WriteLine("Введите !exit, чтобы остановить сервер\n");


Мы клиент, зачем останавливать сервер

lykwer added 3 commits October 22, 2021 21:51

add service

5368267

add client and test

db8e152

correct code and delete file

89ed803

yurii-litvinov requested changes Nov 6, 2021

View reviewed changes

fix code

850857e

yurii-litvinov requested changes Dec 1, 2021

View reviewed changes

lykwer added 9 commits December 6, 2021 23:31

fix

2ca4b93

add global.json

72fe38b

fix

5f15a7e

fix tests

a32e6ac

correct

e85010e

try fix

9426a17

change

31c11e0

delete files

f470b1f

update appveyor.yml

a4ba4e9

yurii-litvinov requested changes Mar 7, 2022

View reviewed changes

lykwer added 2 commits March 14, 2022 12:58

not all fix

41a5664

fix code

82e0df4

yurii-litvinov requested changes Apr 1, 2022

View reviewed changes

lykwer added 2 commits April 4, 2022 00:13

fix bugs

ec465c8

fix test

e3361df

yurii-litvinov approved these changes Apr 5, 2022

View reviewed changes

		@@ -0,0 +1,18 @@
		FROM mcr.microsoft.com/dotnet/runtime:5.0 AS base

		_server = new Server("127.0.0.1", 80);
		_client = new Client(80, "127.0.0.1");

		var data = new List<(string, bool)>();



		for (int i = 1; i < infoArray.Length; i += 2)

Comments

Conversation

MikePuzanov commented Oct 26, 2021

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurii-litvinov Nov 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

yurii-litvinov Nov 6, 2021 •

edited

Loading