Fetch text from a site as QString

[realroot]

How can I fetch text from a site as QString in a function of a C++ class?
Can I do it using Qt classes as QNetworkAccessManager etc.?

[JonB]

@realroot
It looks reasonable. Although your

QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);

will work personally I would do the new in the MyClass constructor, to the line above where you have moved the connect() like @SGaist said. But maybe that's just me. In any case I believe your code is now acceptable.

[posktomten]

Hello!@realroot
I have made a library that reads a text string from a text file on a web server.
My library compares the loaded character string (which contains the latest version number) to the current version.

You might be able to get some help if you check the part of the library that reads the text string.
https://gitlab.com/posktomten/libcheckforupdates

[posktomten]

The library
https://gitlab.com/posktomten/libcheckforupdates/-/tree/master/bibliotek/code?ref_type=heads

[ChrisW67]

@realroot said in Fetch text from a site as QString:

Can I do it using Qt classes as QNetworkAccessManager etc.?

Yes. Most of the logic is in this example.

[realroot]

QNetworkAccessManager qnam;
   QScopedPointer<QNetworkReply, QScopedPointerDeleteLater> reply;
   reply.reset(qnam.get(QNetworkRequest(QUrl(
       "https://...file"))));
   QByteArray bytes = reply->readAll();
 QString s = QString::fromUtf8(bytes));

It's empty.
I just want to fetch the webpage as text no need to open files etc.

[JonB]

@realroot said in Fetch text from a site as QString:

It's empty.

Yes, it would (likely) be. You are calling readAll() too early. QNetworkAccessManager::get() only starts the process of getting the response content

Posts a request to obtain the contents of the target request and returns a new QNetworkReply object opened for reading which emits the readyRead() signal whenever new data arrives.

It is asynchronous. See https://doc.qt.io/qt-6/qnetworkaccessmanager.html#details for an example of what you should be doing. You need to act on the QNetworkReply's readyRead() or finished() signals, there you will be able to read the data returned.

[SGaist]

Hi,

Beside what @JonB, you code sample also has an object lifetime issue. You need to ensure it lasts longer than the query call.

[realroot]

If I need to act on signals() does that mean that I have to connect my C++ class to do that?

connect(manager, &QNetworkAccessManager::finished,
        this, &MyClass::replyFinished);

To fix the lifetime can I do this?

QString s = QString::fromUtf8(reply->readAll());

[JonB]

@realroot
Yes, after your connect() (or you could have connected the reply object) you should be able to reply->readAll() in slot.

No to second, that's not the issue. The reply needs to outlive where you do the get(), till (at least) the finished(). You won't want to use a QScopedPointer, that will destroy it. And of course the QNAM must also be kept in existence.

[realroot]

QNetworkAccessManager *manager = new QNetworkAccessManager(this);
   connect(manager, &QNetworkAccessManager::finished,
       this, &MyClass::replyFinished);

   manager->get(QNetworkRequest(QUrl(
       "https://...file")));

// Class public slot function:

void replyFinished(QNetworkReply *reply) {
   if (reply->error() == QNetworkReply::NoError) {
       QByteArray data = reply->readAll();
       QFile file(<file>);
       if (file.open(QIODevice::WriteOnly)) {
           QTextStream out(&file);
           out << data;
           file.close();
           doSomething(<file>);
       }
   }
   reply->deleteLater();
}

Thanks, this is working. Is it safe now?

[SGaist]

When are you creating your manager object ? Based only on your code, it seems you will be creating it many times though you only need one instance during the lifetime of your application.

[realroot]

In the function there is no more code:

void MyClass::downloadFile() {
QNetworkAccessManager *manager = new QNetworkAccessManager(this);
   connect(manager, &QNetworkAccessManager::finished,
       this, &MyClass::replyFinished);

   manager->get(QNetworkRequest(QUrl(
       "https://...file")));
}

Should I make a manager instance as private member of MyClass?

[JonB]

@realroot said in Fetch text from a site as QString:

In the function there is no more code:

It is not a question of whether this function has more code. It is a question of whether MyClass::replyFinished() completes all processing of the reply/downloading the file. Which I imagine it does.

Should I make a manager instance as private member of MyClass?

Yes. And do not allocate it more than once! You could alternatively allocate manager within the class instead of with new, i.e. QNetworkAccessManager manager; as a member variable would work. And don't forget you need to call reply->deleteLater() in MyClass::replyFinished(QNetworkReply *reply). As per https://doc.qt.io/qt-6/qnetworkaccessmanager.html#details

Note: After the request has finished, it is the responsibility of the user to delete the QNetworkReply object at an appropriate time. Do not directly delete it inside the slot connected to finished(). You can use the deleteLater() function.

[SGaist]

In addition to what @JonB wrote, the fact that you pass a parent to your manager object only ensures that it will get destroyed when the parent gets destroyed.

What you currently have is a variant of memory leak since you create new instances of QNetworkAccessManager every time you call that function and they will only get destroyed when your MyClass instance will as well.

[realroot]

I see thanks.
If I declare it as QNetworkAccessManager manager; I have errors so I made it like this:

private:
    QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);

void MyClass::downloadFile() {
   connect(m_manager, &QNetworkAccessManager::finished,
       this, &MyClass::replyFinished);

   manager->get(QNetworkRequest(QUrl(
       "https://...file")));
}

void MyClass::replyFinished(QNetworkReply *reply) {
   if (reply->error() == QNetworkReply::NoError) {
       QByteArray data = reply->readAll();
       QFile file(<file>);
       if (file.open(QIODevice::WriteOnly)) {
           QTextStream out(&file);
           out << data;
           file.close();
           doSomething(<file>);
       }
   }
   reply->deleteLater();
}

[SGaist]

@realroot said in Fetch text from a site as QString:

I see thanks.
If I declare it as QNetworkAccessManager manager; I have errors so I made it like this:

private:
    QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);

void MyClass::downloadFile() {
   connect(m_manager, &QNetworkAccessManager::finished,
       this, &MyClass::replyFinished);

Move that connect to the constructor of your class. Otherwise each time you call downloadFile you will create a new connection which means that the slot will be called an additional time.

[realroot]

Then it should be so I believe:

class MyClass : public QAbstractListModel {
    Q_OBJECT
public:
    MyClass(QObject *parent = nullptr) : QAbstractListModel(parent) {
        connect(m_manager, &QNetworkAccessManager::finished,
                this, &MyClass::downloadFinished);
    }
private:
    QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);

void MyClass::downloadFile() {
    m_manager->get(QNetworkRequest(QUrl(
       "https://...file")));
}

It's working at least.

[realroot]

If something is wrong let me know, thanks.

[JonB]

@realroot
It looks reasonable. Although your

QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);

will work personally I would do the new in the MyClass constructor, to the line above where you have moved the connect() like @SGaist said. But maybe that's just me. In any case I believe your code is now acceptable.

[realroot]

@JonB Like this?

public:
            MyClass(QObject *parent = nullptr) : QAbstractListModel(parent) {
                m_manager = new QNetworkAccessManager(this);
                connect(m_manager, &QNetworkAccessManager::finished,
                        this, &MyClass::downloadFinished);
            }