From 510dd5c047609a07843241c1cfc819ad80acb5ca Mon Sep 17 00:00:00 2001 From: Akshay Joshi Date: Wed, 29 Jul 2020 15:31:38 +0530 Subject: [PATCH] 1) Fixed pgAdmin hang issue when the user clicks on 'View Log' menu option. 2) Fixed some code smell in runtime code. --- runtime/ConfigWindow.h | 2 -- runtime/FloatingWindow.cpp | 18 +++++++++++++-- runtime/FloatingWindow.h | 2 ++ runtime/LogWindow.cpp | 10 ++++---- runtime/Logger.cpp | 2 +- runtime/MenuActions.cpp | 13 +++++------ runtime/MenuActions.h | 2 +- runtime/Runtime.cpp | 32 ++++++++++++++++++-------- runtime/Runtime.h | 12 +++++----- runtime/Server.cpp | 7 ++---- runtime/Server.h | 2 -- runtime/TrayIcon.cpp | 19 +++++++++++++-- runtime/TrayIcon.h | 2 ++ runtime/pgAdmin4.cpp | 47 +++++++------------------------------- runtime/pgAdmin4.h | 7 +++--- 15 files changed, 92 insertions(+), 85 deletions(-) diff --git a/runtime/ConfigWindow.h b/runtime/ConfigWindow.h index 09d79c918..850fda2fa 100644 --- a/runtime/ConfigWindow.h +++ b/runtime/ConfigWindow.h @@ -25,8 +25,6 @@ class ConfigWindow : public QDialog public: explicit ConfigWindow(QWidget *parent = Q_NULLPTR); - bool NeedRestart() { return m_needRestart; }; - signals: void accepted(bool needRestart); void closing(bool accepted); diff --git a/runtime/FloatingWindow.cpp b/runtime/FloatingWindow.cpp index 53a63f720..3d1c10d20 100644 --- a/runtime/FloatingWindow.cpp +++ b/runtime/FloatingWindow.cpp @@ -74,11 +74,11 @@ void FloatingWindow::createActions() connect(m_copyUrlAction, SIGNAL(triggered()), m_menuActions, SLOT(onCopyUrl())); m_configAction = new QAction(tr("C&onfigure..."), this); - m_configAction->setEnabled(true); + m_configAction->setEnabled(false); connect(m_configAction, SIGNAL(triggered()), m_menuActions, SLOT(onConfig())); m_logAction = new QAction(tr("&View log..."), this); - m_logAction->setEnabled(true); + m_logAction->setEnabled(false); connect(m_logAction, SIGNAL(triggered()), m_menuActions, SLOT(onLog())); m_quitAction = new QAction(tr("&Shut down server"), this); @@ -110,6 +110,20 @@ void FloatingWindow::setMenuActions(MenuActions * menuActions) m_menuActions = menuActions; } +// Enable the View Log option +void FloatingWindow::enableViewLogOption() +{ + if (m_logAction != Q_NULLPTR) + m_logAction->setEnabled(true); +} + + +// Disable the View Log option +void FloatingWindow::disableViewLogOption() +{ + if (m_logAction != Q_NULLPTR) + m_logAction->setEnabled(false); +} void FloatingWindow::closeEvent(QCloseEvent * event) { diff --git a/runtime/FloatingWindow.h b/runtime/FloatingWindow.h index 1d805889a..ba37c5485 100644 --- a/runtime/FloatingWindow.h +++ b/runtime/FloatingWindow.h @@ -29,6 +29,8 @@ public: bool Init(); void enablePostStartOptions(); + void enableViewLogOption(); + void disableViewLogOption(); void setMenuActions(MenuActions * menuActions); private: diff --git a/runtime/LogWindow.cpp b/runtime/LogWindow.cpp index 95c46e354..006d68ba9 100644 --- a/runtime/LogWindow.cpp +++ b/runtime/LogWindow.cpp @@ -37,11 +37,11 @@ void LogWindow::LoadLog() ui->lblStatus->setText(tr("Loading logfiles...")); - ui->lblStartupLog->setText(tr("Startup Log (%1):").arg(getStartupLogFile())); - ui->lblServerLog->setText(tr("Server Log (%1):").arg(getServerLogFile())); + ui->lblStartupLog->setText(tr("Startup Log (%1):").arg(g_startupLogFile)); + ui->lblServerLog->setText(tr("Server Log (%1):").arg(g_serverLogFile)); - startupLines = this->readLog(getStartupLogFile(), ui->textStartupLog); - serverLines = this->readLog(getServerLogFile(), ui->textServerLog); + startupLines = this->readLog(g_startupLogFile, ui->textStartupLog); + serverLines = this->readLog(g_serverLogFile, ui->textServerLog); ui->lblStatus->setText(QString(tr("Loaded startup log (%1 lines) and server log (%2 lines).")).arg(startupLines).arg(serverLines)); } @@ -73,7 +73,7 @@ int LogWindow::readLog(QString logFile, QPlainTextEdit *logWidget) log = fopen(logFile.toUtf8().data(), "r"); if (log == Q_NULLPTR) { - logWidget->setPlainText(QString(tr("The log file (%1) could not be opened.")).arg(getServerLogFile())); + logWidget->setPlainText(QString(tr("The log file (%1) could not be opened.")).arg(g_serverLogFile)); this->setDisabled(false); QApplication::restoreOverrideCursor(); return 0; diff --git a/runtime/Logger.cpp b/runtime/Logger.cpp index 20a0ea16a..ed4364954 100644 --- a/runtime/Logger.cpp +++ b/runtime/Logger.cpp @@ -33,7 +33,7 @@ Logger* Logger::GetLogger() { m_pThis = new Logger(); m_Logfile = new QFile; - m_Logfile->setFileName(getStartupLogFile()); + m_Logfile->setFileName(g_startupLogFile); m_Logfile->open(QIODevice::WriteOnly | QIODevice::Text); m_Logfile->setPermissions(QFile::ReadOwner|QFile::WriteOwner); } diff --git a/runtime/MenuActions.cpp b/runtime/MenuActions.cpp index 6c945b26c..2e9e6dbcc 100644 --- a/runtime/MenuActions.cpp +++ b/runtime/MenuActions.cpp @@ -75,15 +75,14 @@ void MenuActions::onConfig() } -void MenuActions::onConfigDone(bool needRestart) +void MenuActions::onConfigDone(bool needRestart) const { - if (needRestart) + if (needRestart && QMessageBox::Yes == QMessageBox::question(Q_NULLPTR, + tr("Shut down server?"), + tr("The pgAdmin 4 server must be restarted for changes to take effect. Do you want to shut down the server now?"), + QMessageBox::Yes | QMessageBox::No)) { - if (QMessageBox::Yes == QMessageBox::question(Q_NULLPTR, - tr("Shut down server?"), - tr("The pgAdmin 4 server must be restarted for changes to take effect. Do you want to shut down the server now?"), - QMessageBox::Yes | QMessageBox::No)) - exit(0); + exit(0); } } diff --git a/runtime/MenuActions.h b/runtime/MenuActions.h index d1eeab3e6..6ab04956c 100644 --- a/runtime/MenuActions.h +++ b/runtime/MenuActions.h @@ -30,7 +30,7 @@ private: ConfigWindow *m_configWindow = Q_NULLPTR; public slots: - void onConfigDone(bool needRestart); + void onConfigDone(bool needRestart) const; protected slots: void onNew(); diff --git a/runtime/Runtime.cpp b/runtime/Runtime.cpp index 022956ecd..9eb964625 100644 --- a/runtime/Runtime.cpp +++ b/runtime/Runtime.cpp @@ -208,7 +208,7 @@ bool Runtime::alreadyRunning() if (is_running) { - QFile addressFile(getAddressFile()); + QFile addressFile(g_addressFile); addressFile.open(QIODevice::ReadOnly | QIODevice::Text); QTextStream in(&addressFile); QString url = in.readLine(); @@ -273,7 +273,7 @@ QSplashScreen * Runtime::displaySplash(QApplication *app) // Get the port number we're going to use -quint16 Runtime::getPort() +quint16 Runtime::getPort() const { quint16 port = 0L; @@ -347,7 +347,7 @@ FloatingWindow * Runtime::createFloatingWindow(QSplashScreen *splash, MenuAction // Server startup loop -Server * Runtime::startServerLoop(QSplashScreen *splash, FloatingWindow *floatingWindow, TrayIcon *trayIcon, int port, QString key) +Server * Runtime::startServerLoop(QSplashScreen *splash, FloatingWindow *floatingWindow, TrayIcon *trayIcon, quint16 port, QString key) { bool done = false; Server *server; @@ -374,6 +374,12 @@ Server * Runtime::startServerLoop(QSplashScreen *splash, FloatingWindow *floatin delete server; + // Enable the View Log option for diagnostics + if (floatingWindow) + floatingWindow->enableViewLogOption(); + if (trayIcon) + trayIcon->enableViewLogOption(); + // Allow the user to tweak the Python Path if needed m_configDone = false; @@ -387,6 +393,12 @@ Server * Runtime::startServerLoop(QSplashScreen *splash, FloatingWindow *floatin // Wait for configuration to be completed while (!m_configDone) delay(100); + + // Disable the View Log option again + if (floatingWindow) + floatingWindow->disableViewLogOption(); + if (trayIcon) + trayIcon->disableViewLogOption(); } else { @@ -410,16 +422,16 @@ void Runtime::onConfigDone(bool accepted) // Start the server -Server * Runtime::startServer(QSplashScreen *splash, int port, QString key) +Server * Runtime::startServer(QSplashScreen *splash, quint16 port, QString key) { Server *server; splash->showMessage(QString(QWidget::tr("Starting pgAdmin4 server...")), Qt::AlignBottom | Qt::AlignCenter); Logger::GetLogger()->Log("Starting pgAdmin4 server..."); - QString msg = QString(QWidget::tr("Creating server object, port:%1, key:%2, logfile:%3")).arg(port).arg(key).arg(getServerLogFile()); + QString msg = QString(QWidget::tr("Creating server object, port:%1, key:%2, logfile:%3")).arg(port).arg(key).arg(g_serverLogFile); Logger::GetLogger()->Log(msg); - server = new Server(this, port, key, getServerLogFile()); + server = new Server(this, port, key, g_serverLogFile); Logger::GetLogger()->Log("Initializing server..."); if (!server->Init()) @@ -504,9 +516,9 @@ void Runtime::checkServer(QSplashScreen *splash, QString url) // Create the address file -void Runtime::createAddressFile(QString url) +void Runtime::createAddressFile(QString url) const { - QFile addressFile(getAddressFile()); + QFile addressFile(g_addressFile); if (addressFile.open(QIODevice::WriteOnly)) { addressFile.setPermissions(QFile::ReadOwner|QFile::WriteOwner); @@ -517,7 +529,7 @@ void Runtime::createAddressFile(QString url) // Open a browser tab -void Runtime::openBrowserTab(QString url) +void Runtime::openBrowserTab(QString url) const { QString cmd = m_settings.value("BrowserCommand").toString(); @@ -594,7 +606,7 @@ bool Runtime::shutdownServer(QUrl url) } -void Runtime::delay(int milliseconds) +void Runtime::delay(int milliseconds) const { QTime endTime = QTime::currentTime().addMSecs(milliseconds); while(QTime::currentTime() < endTime) diff --git a/runtime/Runtime.h b/runtime/Runtime.h index a0834f48c..b5bd2b026 100644 --- a/runtime/Runtime.h +++ b/runtime/Runtime.h @@ -36,7 +36,7 @@ public: bool alreadyRunning(); bool go(int argc, char *argv[]); - void delay(int milliseconds); + void delay(int milliseconds) const; bool shutdownServer(QUrl url); private: @@ -47,14 +47,14 @@ private: void setupStyling(QApplication *app); void configureProxy(); QSplashScreen *displaySplash(QApplication *app); - quint16 getPort(); + quint16 getPort() const; TrayIcon *createTrayIcon(QSplashScreen *splash, MenuActions *menuActions); FloatingWindow *createFloatingWindow(QSplashScreen *splash, MenuActions *menuActions); - Server *startServerLoop(QSplashScreen *splash, FloatingWindow *floatingWindow, TrayIcon *trayIcon, int port, QString key); - Server *startServer(QSplashScreen *splash, int port, QString key); + Server *startServerLoop(QSplashScreen *splash, FloatingWindow *floatingWindow, TrayIcon *trayIcon, quint16 port, QString key); + Server *startServer(QSplashScreen *splash, quint16 port, QString key); void checkServer(QSplashScreen *splash, QString url); - void createAddressFile(QString url); - void openBrowserTab(QString url); + void createAddressFile(QString url) const; + void openBrowserTab(QString url) const; QString serverRequest(QUrl url, QString path); bool pingServer(QUrl url); diff --git a/runtime/Server.cpp b/runtime/Server.cpp index 30340c999..8c64f2b5e 100644 --- a/runtime/Server.cpp +++ b/runtime/Server.cpp @@ -68,10 +68,8 @@ Server::Server(Runtime *runtime, quint16 port, QString key, QString logFileName) Py_NoUserSiteDirectory=1; Py_DontWriteBytecodeFlag=1; - PGA_APP_NAME_UTF8 = QString("pgAdmin 4").toUtf8(); - // Python3 requires conversion of char * to wchar_t *, so... - const char *appName = PGA_APP_NAME_UTF8.data(); + const char *appName = QString("pgAdmin 4").toUtf8(); const size_t cSize = strlen(appName)+1; m_wcAppName = new wchar_t[cSize]; mbstowcs (m_wcAppName, appName, cSize); @@ -177,8 +175,7 @@ Server::Server(Runtime *runtime, quint16 port, QString key, QString logFileName) if (!pythonHome.isEmpty()) { - pythonHome_utf8 = pythonHome.toUtf8(); - const char *python_home = pythonHome_utf8.data(); + const char *python_home = pythonHome.toUtf8().data(); const size_t home_size = strlen(python_home) + 1; m_wcPythonHome = new wchar_t[home_size]; mbstowcs (m_wcPythonHome, python_home, home_size); diff --git a/runtime/Server.h b/runtime/Server.h index a6f8b2b72..1a5d17e54 100644 --- a/runtime/Server.h +++ b/runtime/Server.h @@ -47,11 +47,9 @@ private: // Application name in UTF-8 for Python wchar_t *m_wcAppName = Q_NULLPTR; - QByteArray PGA_APP_NAME_UTF8; // PythonHome for Python wchar_t *m_wcPythonHome = Q_NULLPTR; - QByteArray pythonHome_utf8; }; #endif // SERVER_H diff --git a/runtime/TrayIcon.cpp b/runtime/TrayIcon.cpp index 5d8054dcb..16e0eaa59 100644 --- a/runtime/TrayIcon.cpp +++ b/runtime/TrayIcon.cpp @@ -78,11 +78,11 @@ void TrayIcon::createActions() connect(m_copyUrlAction, SIGNAL(triggered()), m_menuActions, SLOT(onCopyUrl())); m_configAction = new QAction(tr("&Configure..."), this); - m_configAction->setEnabled(true); + m_configAction->setEnabled(false); connect(m_configAction, SIGNAL(triggered()), m_menuActions, SLOT(onConfig())); m_logAction = new QAction(tr("&View log..."), this); - m_logAction->setEnabled(true); + m_logAction->setEnabled(false); connect(m_logAction, SIGNAL(triggered()), m_menuActions, SLOT(onLog())); m_quitAction = new QAction(tr("&Shut down server"), this); @@ -109,6 +109,21 @@ void TrayIcon::enablePostStartOptions() m_quitAction->setEnabled(true); } +// Enable the View Log option +void TrayIcon::enableViewLogOption() +{ + if (m_logAction != Q_NULLPTR) + m_logAction->setEnabled(true); +} + + +// Disable the View Log option +void TrayIcon::disableViewLogOption() +{ + if (m_logAction != Q_NULLPTR) + m_logAction->setEnabled(false); +} + void TrayIcon::setMenuActions(MenuActions * menuActions) { m_menuActions = menuActions; diff --git a/runtime/TrayIcon.h b/runtime/TrayIcon.h index 7d8025441..a03feb5da 100644 --- a/runtime/TrayIcon.h +++ b/runtime/TrayIcon.h @@ -26,6 +26,8 @@ public: void Init(); void enablePostStartOptions(); + void enableViewLogOption(); + void disableViewLogOption(); void setMenuActions(MenuActions * menuActions); private: diff --git a/runtime/pgAdmin4.cpp b/runtime/pgAdmin4.cpp index 12a22aeb0..a749565ba 100644 --- a/runtime/pgAdmin4.cpp +++ b/runtime/pgAdmin4.cpp @@ -15,9 +15,9 @@ #include // Global vars for caching and avoing shutdown issues -QString g_startupLogFile; -QString g_serverLogFile; -QString g_addressFile; +const QString g_startupLogFile = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation) + ("/pgadmin4.startup.log"); +const QString g_serverLogFile = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation) + (QString("/pgadmin4.%1.log").arg(getExeHash())); +const QString g_addressFile = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation) + (QString("/pgadmin4.%1.addr").arg(getExeHash())); int main(int argc, char * argv[]) { @@ -30,46 +30,15 @@ int main(int argc, char * argv[]) return runtime->go(argc, argv); } - -// Get the filename for the startup log -QString getStartupLogFile() -{ - if (g_startupLogFile == "") - g_startupLogFile = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation) + ("/pgadmin4.startup.log"); - - return g_startupLogFile; -} - - -// Get the filename for the server log -QString getServerLogFile() -{ - if (g_serverLogFile == "") - g_serverLogFile = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation) + (QString("/pgadmin4.%1.log").arg(getExeHash())); - - return g_serverLogFile; -} - - -// Get the filename for the address file -QString getAddressFile() -{ - if (g_addressFile == "") - g_addressFile = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation) + (QString("/pgadmin4.%1.addr").arg(getExeHash())); - - return g_addressFile; -} - - // Cleanup the address and log files void cleanup() { - qDebug() << "Removing:" << getAddressFile(); - QFile addrFile(getAddressFile()); + qDebug() << "Removing:" << g_addressFile; + QFile addrFile(g_addressFile); addrFile.remove(); - qDebug() << "Removing:" << getServerLogFile(); - QFile logFile(getServerLogFile()); + qDebug() << "Removing:" << g_serverLogFile; + QFile logFile(g_serverLogFile); logFile.remove(); } @@ -78,4 +47,4 @@ void cleanup() QString getExeHash() { return QString(QCryptographicHash::hash(QCoreApplication::applicationFilePath().toUtf8(),QCryptographicHash::Md5).toHex()); -} \ No newline at end of file +} diff --git a/runtime/pgAdmin4.h b/runtime/pgAdmin4.h index 1a259586d..51d511736 100644 --- a/runtime/pgAdmin4.h +++ b/runtime/pgAdmin4.h @@ -18,9 +18,10 @@ // Global function prototypes int main(int argc, char * argv[]); -QString getStartupLogFile(); -QString getServerLogFile(); -QString getAddressFile(); +extern const QString g_startupLogFile; +extern const QString g_serverLogFile; +extern const QString g_addressFile; + QString getExeHash(); void cleanup();