Оцените код передачи файла через QTcpSocket

[vebmaster]

Здравствуйте.
Потребовалось сделать передачу файла через QTcpSocket. Нагуглил много разных вариантов, которые меня запутали.
Первым способом сделал через передачу длины блока и затем сам блок. Но потом нашёл более продвинутый способ через транзакции, который появился в Qt 5.7. На нём и сделал, асинхронно.
Оцените пожалуйста, правильно ли и на сколько грамотно сделано? Если есть какие то замечания, буду рад узнать их. Спасибо.

Передаю в несколько переменных:
- тип сетевого пакета (файл, сообщение или другое)
Если файл, то:
- имя файла
- размер файла в байтах
- блок данных
- и в конце текстовое сообщение

Код передачи файла:

Код:

enum PacketType
{
    TYPE_NONE = 0,
    TYPE_MSG = 1,
    TYPE_FILE = 2,
};

void TcpClient::socketSendMessage()
{
    QDataStream stream(m_pTcpSocket);
    stream.setVersion(QDataStream::Qt_DefaultCompiledVersion);

    stream << PacketType::TYPE_FILE;

    QString fileName("/mnt/d/1.png");
    QFile file(fileName);
    QFileInfo fileInfo(file);
    qint64 fileSize = fileInfo.size();

    stream << fileName;
    stream << fileSize;
	
    int countSend = 0;

    if (file.open(QFile::ReadOnly))
    {
        while(!file.atEnd())
        {
            QByteArray data = file.read(32768*8);
            stream << data;
            countSend++;
        }
        qDebug() << Tools::getTime() << "_CLIENT: ------------------------ countSend FINAL: " << countSend;
    }

    file.close();

    qDebug() << Tools::getTime() << "_CLIENT: send file ok";

    QString testStr("TEST_MESSAGE");
    stream << testStr;
}

Код получения файла:
Заголовочный сервера:

Код:

#ifndef MYTCPSERVER_H
#define MYTCPSERVER_H

#include <QObject>
#include <QTcpServer>
#include <QTcpSocket>
#include "global.h"
#include <QFile>


class MyTcpServer : public QObject
{
    Q_OBJECT

public:
    explicit MyTcpServer(QObject *parent = nullptr);
    ~MyTcpServer();

    int number;
    QString str;

public slots:
    void slotNewConnection();
    void slotServerRead();
    void slotClientDisconnected();
    void onSocketReceiveMessage();
    void startServer();

private:
    QTcpServer * mTcpServer;
    QTcpSocket * mTcpSocket;
    qint64 sizeReceivedData;
    QString fileCopy;
    PacketType packetType;

    QString filePath;
    qint64 fileSize;
    QString testStr;
    QByteArray tmpBlock;
    int countSend;

    bool receiveFile(QDataStream &stream);
};

#endif // MYTCPSERVER_H

В конструкторе:

Код:

    packetType = PacketType::TYPE_NONE;
    filePath.clear();
    fileSize = 0;
    testStr.clear();
    sizeReceivedData = 0;
    tmpBlock.clear();
    countSend = 0;

Слот получения сообщения:

Код:

void MyTcpServer::onSocketReceiveMessage()
{
	if (!mTcpSocket || !mTcpSocket->bytesAvailable())
		return;

	qDebug() << Tools::getTime() << "SERVER: --------------------new-----------------------";
	qDebug() << Tools::getTime() << "SERVER: onSocketReceiveMessage: bytesAvailable" << mTcpSocket->bytesAvailable();

	QDataStream stream(mTcpSocket);
	stream.setVersion(QDataStream::Qt_DefaultCompiledVersion);

	// Считывание PacketType
	if (packetType == PacketType::TYPE_NONE) {
		stream.startTransaction();
		stream >> packetType;
		if (!stream.commitTransaction()) {
			qDebug() << Tools::getTime() << "SERVER: packetType - FAIL commitTransaction";
			return;
		}
		qDebug() << Tools::getTime() << "SERVER: type:" << packetType;
	}

	if (packetType == PacketType::TYPE_MSG)
	{
		//
	}
	else if (packetType == PacketType::TYPE_FILE)
	{
		//====================================================
		// Получение filePath

		if (filePath.isEmpty()) {
			stream.startTransaction();
			stream >> filePath;
			if (!stream.commitTransaction()) {
				qDebug() << Tools::getTime() << "SERVER: filePath - FAIL commitTransaction";
				return;
			}
			qDebug() << Tools::getTime() << "SERVER filePath:" << filePath;
		}

		//====================================================
		// Получение fileSize

		if (!fileSize) {
			stream.startTransaction();
			stream >> fileSize;
			if (!stream.commitTransaction()) {
				qDebug() << Tools::getTime() << "SERVER: fileSize - FAIL commitTransaction";
				return;
			}
			qDebug() << Tools::getTime() << "SERVER: fileSize:" << fileSize;
		}

		//====================================================
		// Получение файла

		if (sizeReceivedData != fileSize)
		{
			filePath = this->fileCopy; // временная замена имени файла
			QFile file(filePath);
			file.open(QFile::Append);

			// Работа с файлом в цикле "пока в сокете есть данные"
			while (!mTcpSocket->atEnd())
			{
				//====================================================
				// Получение tmpBlock

				stream.startTransaction();
				stream >> tmpBlock;

				if (!stream.commitTransaction()) {
					qDebug() << Tools::getTime() << "SERVER: tmpBlock - FAIL commitTransaction";
					break;
				}

				qint64 toFile = file.write(tmpBlock);

				sizeReceivedData += toFile;
				countSend++;
				
				tmpBlock.clear();

				if (sizeReceivedData == fileSize)
					break;

			} // while (!mTcpSocket->atEnd())

			file.close();

		} // if (sizeReceivedData != fileSize)

		if (sizeReceivedData != fileSize)
			return;

		qDebug() << Tools::getTime() << "SERVER: sizeReceivedData END: " << sizeReceivedData;
		qDebug() << Tools::getTime() << "SERVER fileSize ORIG:" << fileSize;
		qDebug() << "SERVER: countSend FINAL: " << countSend;


		//====================================================
		// Получение testStr

		if (testStr.isEmpty()) {
			stream.startTransaction();
			stream >> testStr;
			if (!stream.commitTransaction()) {
				qDebug() << Tools::getTime() << "SERVER: testStr - FAIL commitTransaction";
				return;
			}
			qDebug() << Tools::getTime() << "SERVER: testStr:" << testStr;
		}

		qDebug() << Tools::getTime() << "SERVER: END - bytesAvailable:" << mTcpSocket->bytesAvailable();

		// Очистка переменных
		filePath.clear();
		fileSize = 0;
		tmpBlock.clear();
		sizeReceivedData = 0;
		testStr.clear();
		countSend = 0;

	} // else if (packetType == PacketType::TYPE_FILE)
}

[qate]

рабочий пример (проект на gihub) былобы удобнее оценивать

[vebmaster]

рабочий пример (проект на gihub) былобы удобнее оценивать

Добавил на https://github.com/vebmaster/QtFileTransferViaSocket

[navrocky]

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

1) Название класса некорректное, более правильное FileSender
2) У твоей синхронной функции почему-то закомментирован waitForBytesWritten, это как раз для блокирующей отправки должно быть.
3) Почему параметрически не передается имя файла и адрес сервера?
4) Повторный вызов метода отправки надо блокировать (кидать ошибку)
5) Нет нормальной обработки ошибок
6) Не все переменные инициализированы в конструкторе, это приведет к крэшам

И в целом кривой подход. Я бы переиспользовал одно TCP соединение, поэтому надо вынести создание и коннект QTcpClient из этого класса, отправлять файл пакетами, реализовать очередь пакетов на отправку, дополнить отправляемые пакеты размером, чтобы можно было отделить данные одного пакета от другого.

[vebmaster]

Ну сам код конечно ужасно спроектирован

Спасибо, можете добавить пару замечаний по поводу того, что именно не так.

[navrocky]

Ответил развернуто выше

[vebmaster]

2) У твоей синхронной функции почему-то закомментирован waitForBytesWritten, это как раз для блокирующей отправки должно быть.

Закоментил, потому что и так отправляет хорошо. Как говорят, если всё работает хорошо - ничего не трогай.

3) Почему параметрически не передается имя файла и адрес сервера?

Черновой вариант.

4) Повторный вызов метода отправки надо блокировать (кидать ошибку)

Не понял. Где именно кидать ошибку и зачем.

6) Не все переменные инициализированы в конструкторе, это приведет к крэшам

Если посмотреть main.cpp, то можно заметить, что я каждый экземпляр ввожу в отдельный поток.
А если указатель класса инициализировать в конструкторе, то объект создастся не в новом потоке, а в основном (из main.cpp)

Я бы переиспользовал одно TCP соединение

Так в клиенте и сервере везде создаётся по одному сокету и в него пишется через QDataStream.

отправлять файл пакетами, реализовать очередь пакетов на отправку, дополнить отправляемые пакеты размером, чтобы можно было отделить данные одного пакета от другого

Можете показать пример, не совсем понимаю как это пакетами.
Пакеты шлёт уже сам TCP/IP. Это уже другой уровень. Или вы про что то другое?

[alumnus]

Добрый день! Я то же сейчас разбираюсь в этой теме. Пока плохо получается.
Подскажите, пожалуйста, я не совсем поняла, если у вас и клиент и сервер в одном проекте, то как они разносятся на разное железо?
Ведь передача файлов подразумевает удаленную передачу.

[vebmaster]

если у вас и клиент и сервер в одном проекте

Здравствуйте.
Да, клиент и сервер в одном проекте, т.к. цель данного проекта/кода - написание и проверка корректной передачи файла и данных.
Дальше вы уже сами разносите методы и классы куда вам надо. А это только пример передачи файлов.
Я же для тестов делаю так:
собираю 2 бинарника и разношу по серверам
1) сборка сервера: в main.cpp оставляете только код для создания сервера, клиент комментируете.

Код:

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    MyTcpServer *server = new MyTcpServer;
    QThread *threadServer = new QThread;
    QObject::connect(threadServer, &QThread::started, server, &MyTcpServer::startServer);
    QObject::connect(threadServer, &QThread::finished, server, &MyTcpServer::deleteLater);
    server->moveToThread(threadServer);
    threadServer->start();

    //QThread::msleep(100);

    //TcpClient *client = new TcpClient("127.0.0.1", 1111);
    //QThread *threadClient = new QThread;
    //QObject::connect(threadClient, &QThread::started, client, &TcpClient::startClient);
    //QObject::connect(threadClient, &QThread::finished, client, &TcpClient::deleteLater);
    //client->moveToThread(threadClient);
    //threadClient->start();

    return a.exec();
}

2) Для клиента наоборот.

Или просто создайте два проекта и собирайте каждый отдельно.

https://wiki.qt.io/WIP-How_to_create_a_simple_chat_application - вот тут гляньте пример с транзакциями.
https://stackoverflow.com/a/39149727/7306569 - и тут интересный вариант с транзакциями.

[alumnus]

Спасибо!