Skip to content

Commit

Permalink
XmlConfiguration: minimize lifetime of instances
Browse files Browse the repository at this point in the history 
This is intended to prevent the loss of changes made to the configuration file
between it being read into the XmlConfiguration instance and saving that instance.
Also, this removes the unnecessary global state.

mythcontext.cpp: use XmlConfiguration::k_default_filename and the default constructor.
  • Loading branch information
@ulmus-scott @bennettpeter
ulmus-scott authored and bennettpeter committed Nov 18, 2022
1 parent 722368e commit dcdc3bb
Show file tree
Hide file tree
Showing 19 changed files with 116 additions and 140 deletions.
28 changes: 17 additions & 11 deletions mythtv/libs/libmyth/backendselect.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// -*- Mode: c++ -*-
#include "backendselect.h"

#include <utility>

#include <QEventLoop>

Expand All @@ -12,13 +15,16 @@
#include "libmythui/mythuistatetype.h"
#include "libmythupnp/mythxmlclient.h"

#include "backendselect.h"

BackendSelection::BackendSelection(
MythScreenStack *parent, DatabaseParams *params,
XmlConfiguration *pConfig, bool exitOnFinish) :
MythScreenStack *parent,
DatabaseParams *params,
QString config_filename,
bool exitOnFinish
) :
MythScreenType(parent, "BackEnd Selection"),
m_dbParams(params), m_pConfig(pConfig), m_exitOnFinish(exitOnFinish)
m_dbParams(params),
m_config_filename(std::move(config_filename)),
m_exitOnFinish(exitOnFinish)
{
if (exitOnFinish)
{
Expand Down Expand Up @@ -46,15 +52,15 @@ BackendSelection::~BackendSelection()
}

BackendSelection::Decision BackendSelection::Prompt(
DatabaseParams *dbParams, XmlConfiguration *pConfig)
DatabaseParams *dbParams, const QString& config_filename)
{
Decision ret = kCancelConfigure;
MythScreenStack *mainStack = GetMythMainWindow()->GetMainStack();
if (!mainStack)
return ret;

auto *backendSettings =
new BackendSelection(mainStack, dbParams, pConfig, true);
new BackendSelection(mainStack, dbParams, config_filename, true);

if (backendSettings->Create())
{
Expand Down Expand Up @@ -109,12 +115,12 @@ void BackendSelection::Accept(MythUIButtonListItem *item)

if (ConnectBackend(dev))
{
if (m_pConfig)
{
auto config = XmlConfiguration(m_config_filename);
if (!m_pinCode.isEmpty())
m_pConfig->SetValue(kDefaultPIN, m_pinCode);
m_pConfig->SetValue(kDefaultUSN, m_usn);
m_pConfig->Save();
config.SetValue(kDefaultPIN, m_pinCode);
config.SetValue(kDefaultUSN, m_usn);
config.Save();
}
CloseWithDecision(kAcceptConfigure);
}
Expand Down
10 changes: 5 additions & 5 deletions mythtv/libs/libmyth/backendselect.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
#define BACKENDSELECT_H

#include <QMutex>
#include <QString>

// MythTV
#include "libmythbase/configuration.h"
#include "libmythui/mythscreentype.h"
#include "libmythupnp/upnpdevice.h"

Expand Down Expand Up @@ -44,11 +44,11 @@ class BackendSelection : public MythScreenType
kCancelConfigure = 0,
kAcceptConfigure = +1,
};
static Decision Prompt(
DatabaseParams *dbParams, XmlConfiguration *pConfig);
static Decision Prompt(DatabaseParams *dbParams, const QString& config_filename);
// TODO return a DatabaseParams, use a bool& parameter

BackendSelection(MythScreenStack *parent, DatabaseParams *params,
XmlConfiguration *pConfig, bool exitOnFinish = false);
QString config_filename, bool exitOnFinish = false);
~BackendSelection() override;

bool Create(void) override; // MythScreenType
Expand All @@ -72,7 +72,7 @@ class BackendSelection : public MythScreenType
void CloseWithDecision(Decision d);

DatabaseParams *m_dbParams {nullptr};
XmlConfiguration *m_pConfig {nullptr};
QString m_config_filename;
bool m_exitOnFinish;
ItemMap m_devices;

Expand Down
145 changes: 69 additions & 76 deletions mythtv/libs/libmyth/mythcontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#ifdef _WIN32
#include "libmythbase/compat.h"
#endif
#include "libmythbase/configuration.h"
#include "libmythbase/dbutil.h"
#include "libmythbase/exitcodes.h"
#include "libmythbase/mythdate.h"
Expand Down Expand Up @@ -116,8 +117,6 @@ class MythContextPrivate : public QObject
DatabaseParams m_dbParams; ///< Current database host & WOL details
QString m_dbHostCp; ///< dbHostName backup

XmlConfiguration *m_pConfig {nullptr};

bool m_disableeventpopup {false};

MythUIHelper *m_ui {nullptr};
Expand Down Expand Up @@ -243,7 +242,6 @@ MythContextPrivate::MythContextPrivate(MythContext *lparent)

MythContextPrivate::~MythContextPrivate()
{
delete m_pConfig;
if (GetNotificationCenter() && m_registration > 0)
{
GetNotificationCenter()->UnRegister(this, m_registration, true);
Expand Down Expand Up @@ -336,9 +334,6 @@ bool MythContextPrivate::Init(const bool gui,
if (gCoreContext->IsFrontend())
m_needsBackend = true;

// We don't have a database yet, so lets use the config.xml file.
m_pConfig = new XmlConfiguration("config.xml");

// Creates screen saver control if we will have a GUI
if (gui)
m_ui = GetMythUI();
Expand Down Expand Up @@ -407,20 +402,20 @@ bool MythContextPrivate::FindDatabase(bool prompt, bool noAutodetect)

QString failure;

// 1. Either load config.xml or use sensible "localhost" defaults:
// 1. Either load XmlConfiguration::k_default_filename or use sensible "localhost" defaults:
bool loaded = LoadDatabaseSettings();
DatabaseParams dbParamsFromFile = m_dbParams;

// In addition to the UI chooser, we can also try to autoSelect later,
// but only if we're not doing manualSelect and there was no
// valid config.xml
// valid XmlConfiguration::k_default_filename
bool autoSelect = !manualSelect && !loaded && !noAutodetect;

// 2. If the user isn't forcing up the chooser UI, look for a default
// backend in config.xml, then test DB settings we've got so far:
// backend in XmlConfiguration::k_default_filename, then test DB settings we've got so far:
if (!manualSelect)
{
// config.xml may contain a backend host UUID and PIN.
// XmlConfiguration::k_default_filename may contain a backend host UUID and PIN.
// If so, try to AutoDiscover UPnP server, and use its DB settings:

if (DefaultUPnP(failure)) // Probably a valid backend,
Expand Down Expand Up @@ -521,47 +516,40 @@ bool MythContextPrivate::FindDatabase(bool prompt, bool noAutodetect)
return false;
}

/** Load database and host settings from config.xml, or set some defaults.
* \return true if config.xml was parsed
/** Load database and host settings from XmlConfiguration::k_default_filename, or set some defaults.
* \return true if XmlConfiguration::k_default_filename was parsed
*/
bool MythContextPrivate::LoadDatabaseSettings(void)
{
auto config = XmlConfiguration(); // read-only
// try new format first
m_dbParams.LoadDefaults();

m_dbParams.m_localHostName = m_pConfig->GetValue("LocalHostName", "");
m_dbParams.m_dbHostPing = m_pConfig->GetBoolValue(kDefaultDB + "PingHost", true);
m_dbParams.m_dbHostName = m_pConfig->GetValue(kDefaultDB + "Host", "");
m_dbParams.m_dbUserName = m_pConfig->GetValue(kDefaultDB + "UserName", "");
m_dbParams.m_dbPassword = m_pConfig->GetValue(kDefaultDB + "Password", "");
m_dbParams.m_dbName = m_pConfig->GetValue(kDefaultDB + "DatabaseName", "");
m_dbParams.m_dbPort = m_pConfig->GetValue(kDefaultDB + "Port", 0);

m_dbParams.m_wolEnabled =
m_pConfig->GetBoolValue(kDefaultWOL + "Enabled", false);
m_dbParams.m_wolReconnect =
m_pConfig->GetDuration<std::chrono::seconds>(kDefaultWOL + "SQLReconnectWaitTime", 0s);
m_dbParams.m_wolRetry =
m_pConfig->GetValue(kDefaultWOL + "SQLConnectRetry", 5);
m_dbParams.m_wolCommand =
m_pConfig->GetValue(kDefaultWOL + "Command", "");

bool ok = m_dbParams.IsValid("config.xml");
m_dbParams.m_localHostName = config.GetValue("LocalHostName", "");
m_dbParams.m_dbHostPing = config.GetBoolValue(kDefaultDB + "PingHost", true);
m_dbParams.m_dbHostName = config.GetValue(kDefaultDB + "Host", "");
m_dbParams.m_dbUserName = config.GetValue(kDefaultDB + "UserName", "");
m_dbParams.m_dbPassword = config.GetValue(kDefaultDB + "Password", "");
m_dbParams.m_dbName = config.GetValue(kDefaultDB + "DatabaseName", "");
m_dbParams.m_dbPort = config.GetValue(kDefaultDB + "Port", 0);

m_dbParams.m_wolEnabled = config.GetBoolValue(kDefaultWOL + "Enabled", false);
m_dbParams.m_wolReconnect =
config.GetDuration<std::chrono::seconds>(kDefaultWOL + "SQLReconnectWaitTime", 0s);
m_dbParams.m_wolRetry = config.GetValue(kDefaultWOL + "SQLConnectRetry", 5);
m_dbParams.m_wolCommand = config.GetValue(kDefaultWOL + "Command", "");

bool ok = m_dbParams.IsValid(XmlConfiguration::k_default_filename);
if (!ok) // if new format fails, try legacy format
{
m_dbParams.LoadDefaults();
m_dbParams.m_dbHostName = m_pConfig->GetValue(
kDefaultMFE + "DBHostName", "");
m_dbParams.m_dbUserName = m_pConfig->GetValue(
kDefaultMFE + "DBUserName", "");
m_dbParams.m_dbPassword = m_pConfig->GetValue(
kDefaultMFE + "DBPassword", "");
m_dbParams.m_dbName = m_pConfig->GetValue(
kDefaultMFE + "DBName", "");
m_dbParams.m_dbPort = m_pConfig->GetValue(
kDefaultMFE + "DBPort", 0);
m_dbParams.m_dbHostName = config.GetValue(kDefaultMFE + "DBHostName", "");
m_dbParams.m_dbUserName = config.GetValue(kDefaultMFE + "DBUserName", "");
m_dbParams.m_dbPassword = config.GetValue(kDefaultMFE + "DBPassword", "");
m_dbParams.m_dbName = config.GetValue(kDefaultMFE + "DBName", "");
m_dbParams.m_dbPort = config.GetValue(kDefaultMFE + "DBPort", 0);
m_dbParams.m_forceSave = true;
ok = m_dbParams.IsValid("config.xml");
ok = m_dbParams.IsValid(XmlConfiguration::k_default_filename);
}
if (!ok)
m_dbParams.LoadDefaults();
Expand Down Expand Up @@ -639,11 +627,15 @@ bool MythContextPrivate::SaveDatabaseParams(
// only rewrite file if it has changed
if (params != m_dbParams || force)
{
m_pConfig->SetValue(
"LocalHostName", params.m_localHostName);
/* Read in the current file on the filesystem, only setting/clearing as
necessary. This prevents losing changes to the file from between when it
was read at startup of MythTV and when this function is called.
*/
auto config = XmlConfiguration();

config.SetValue("LocalHostName", params.m_localHostName);

m_pConfig->SetBoolValue(
kDefaultDB + "PingHost", params.m_dbHostPing);
config.SetBoolValue(kDefaultDB + "PingHost", params.m_dbHostPing);

// If dbHostName is an IPV6 address with scope,
// remove the scope. Unescaped % signs are an
Expand All @@ -655,36 +647,28 @@ bool MythContextPrivate::SaveDatabaseParams(
addr.setScopeId(QString());
dbHostName = addr.toString();
}
m_pConfig->SetValue(
kDefaultDB + "Host", dbHostName);
m_pConfig->SetValue(
kDefaultDB + "UserName", params.m_dbUserName);
m_pConfig->SetValue(
kDefaultDB + "Password", params.m_dbPassword);
m_pConfig->SetValue(
kDefaultDB + "DatabaseName", params.m_dbName);
m_pConfig->SetValue(
kDefaultDB + "Port", params.m_dbPort);

m_pConfig->SetBoolValue(
kDefaultWOL + "Enabled", params.m_wolEnabled);
m_pConfig->SetDuration(
config.SetValue(kDefaultDB + "Host", dbHostName);
config.SetValue(kDefaultDB + "UserName", params.m_dbUserName);
config.SetValue(kDefaultDB + "Password", params.m_dbPassword);
config.SetValue(kDefaultDB + "DatabaseName", params.m_dbName);
config.SetValue(kDefaultDB + "Port", params.m_dbPort);

config.SetBoolValue(kDefaultWOL + "Enabled", params.m_wolEnabled);
config.SetDuration(
kDefaultWOL + "SQLReconnectWaitTime", params.m_wolReconnect);
m_pConfig->SetValue(
kDefaultWOL + "SQLConnectRetry", params.m_wolRetry);
m_pConfig->SetValue(
kDefaultWOL + "Command", params.m_wolCommand);
config.SetValue(kDefaultWOL + "SQLConnectRetry", params.m_wolRetry);
config.SetValue(kDefaultWOL + "Command", params.m_wolCommand);

// clear out any legacy nodes..
m_pConfig->ClearValue(kDefaultMFE + "DBHostName");
m_pConfig->ClearValue(kDefaultMFE + "DBUserName");
m_pConfig->ClearValue(kDefaultMFE + "DBPassword");
m_pConfig->ClearValue(kDefaultMFE + "DBName");
m_pConfig->ClearValue(kDefaultMFE + "DBPort");
m_pConfig->ClearValue(kDefaultMFE + "DBHostPing");
config.ClearValue(kDefaultMFE + "DBHostName");
config.ClearValue(kDefaultMFE + "DBUserName");
config.ClearValue(kDefaultMFE + "DBPassword");
config.ClearValue(kDefaultMFE + "DBName");
config.ClearValue(kDefaultMFE + "DBPort");
config.ClearValue(kDefaultMFE + "DBHostPing");

// actually save the file
m_pConfig->Save();
config.Save();

// Save the new settings:
m_dbParams = params;
Expand Down Expand Up @@ -1146,7 +1130,8 @@ int MythContextPrivate::ChooseBackend(const QString &error)
LOG(VB_GENERAL, LOG_INFO, "Putting up the UPnP backend chooser");

BackendSelection::Decision ret =
BackendSelection::Prompt(&m_dbParams, m_pConfig);
BackendSelection::Prompt(&m_dbParams, XmlConfiguration::k_default_filename);
// TODO encapuslation: don't use a pointer

EndTempWindow();

Expand Down Expand Up @@ -1226,24 +1211,32 @@ int MythContextPrivate::UPnPautoconf(const std::chrono::milliseconds milliSecond
}

/**
* Get the default backend from config.xml, use UPnP to find it.
* Get the default backend from XmlConfiguration::k_default_filename, use UPnP to find it.
*
* Sets a string if there any connection problems
*/
bool MythContextPrivate::DefaultUPnP(QString& Error)
{
static const QString loc = "DefaultUPnP() - ";
QString pin = m_pConfig->GetValue(kDefaultPIN, "");
QString usn = m_pConfig->GetValue(kDefaultUSN, "");

// potentially saved in backendselect
QString pin;
QString usn;
{
auto config = XmlConfiguration(); // read-only
pin = config.GetValue(kDefaultPIN, QString(""));
usn = config.GetValue(kDefaultUSN, QString(""));
}

if (usn.isEmpty())
{
LOG(VB_UPNP, LOG_INFO, loc + "No default UPnP backend");
return false;
}

LOG(VB_UPNP, LOG_INFO, loc + QString("config.xml has default PIN '%1' and host USN: %2")
.arg(pin, usn));
LOG(VB_UPNP, LOG_INFO,
loc + QString(XmlConfiguration::k_default_filename) +
QString(" has default PIN '%1' and host USN: %2").arg(pin, usn));

// ----------------------------------------------------------------------

Expand Down
8 changes: 0 additions & 8 deletions mythtv/libs/libmythbase/mythcorecontext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
#include "mythplugin.h"
#include "mythmiscutil.h"
#include "mythpower.h"
#include "configuration.h"

#define LOC QString("MythCoreContext::%1(): ").arg(__func__)

Expand Down Expand Up @@ -1805,13 +1804,6 @@ void MythCoreContext::ResetSockets(void)
dispatch(MythEvent("BACKEND_SOCKETS_CLOSED"));
}

XmlConfiguration *MythCoreContext::GetConfiguration()
{
static auto config = XmlConfiguration("config.xml");

return &config;
}

void MythCoreContext::InitPower(bool Create)
{
if (Create && !d->m_power)
Expand Down
4 changes: 0 additions & 4 deletions mythtv/libs/libmythbase/mythcorecontext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
#include "mythsession.h"
#include "qtuplift.h"

class XmlConfiguration;

static constexpr const char * MYTH_APPNAME_MYTHBACKEND { "mythbackend" };
static constexpr const char * MYTH_APPNAME_MYTHJOBQUEUE { "mythjobqueue" };
static constexpr const char * MYTH_APPNAME_MYTHFRONTEND { "mythfrontend" };
Expand Down Expand Up @@ -242,8 +240,6 @@ class MBASE_PUBLIC MythCoreContext : public QObject, public MythObservable, publ
void ResetLanguage(void);
void ResetSockets(void);

static XmlConfiguration* GetConfiguration();

using PlaybackStartCb = void (QObject::*)(void);

/**
Expand Down
Loading

0 comments on commit dcdc3bb

Please sign in to comment.