Configuration caches #435

ulmus-scott · 2021-12-23T22:51:29Z

Checklist

contribution does not duplicate one of our existing pull requests
contribution is in a branch rebased against master
code compiles successfully without errors
code follows the MythTV Coding Standards
commits are logically organised and have good commit messages

@stuarta I had hoped to do more work on this, regarding the XmlConfigurations use, but this should be an improvement over what you did.

The configuration caches are currently in libmyth/mythcontext (private).

See the commit messages for details. None of the UPnP stuff was ever saved.

ulmus-scott · 2021-12-23T23:18:57Z

~~Compiles, but not working. I'm investigating.~~
Overload resolution failure, ~~renamed function to correct~~. Added the missing overload.

mythtv/libs/libmythupnp/eventing.cpp

mythtv/libs/libmythupnp/httprequest.cpp

mythtv/libs/libmythupnp/mmulticastsocketdevice.cpp

mythtv/libs/libmythupnp/ssdp.cpp

mythtv/libs/libmythupnp/upnpcds.cpp

mythtv/libs/libmythupnp/upnptasknotify.cpp

mythtv/libs/libmythupnp/upnptasksearch.cpp

mythtv/libs/libmythupnp/upnputil.cpp

mythtv/programs/mythfrontend/mediarenderer.cpp

mythtv/programs/mythfrontend/mythfexml.cpp

mythtv/libs/libmythupnp/configuration.cpp

mythtv/libs/libmyth/mythcontext.cpp

ulmus-scott · 2021-12-30T05:46:31Z

Rebased and force pushed. (I meant to merge some of the commits together before pushing, but I forgot. It is probably clearer while in progress to not do that kind of rebasing, anyway.)

See 381a3c9 for my solution for the non-DB settings. It works fine when I tested it.

Re: "UPnP/DescXmlPath" in mythfexml.cpp
stuarta:

The settings that are of relevance should be migrated to DB configuration items, unless they are required to gain access to get the config items out of the DB. This involves a read config.xml, and save to DB cycle rather than just changing the code to request everything out of the DB.

I think that is beyond the scope of this pull request.

ulmus-scott · 2022-01-07T23:12:50Z

Rebased onto master and added more logging.

ulmus-scott · 2022-01-12T21:11:58Z

WriteDelayedSettings.patch.txt LOG_ALERT to make searching the log easier.
@stuarta
I could not verify that WriteDelayedSettings() works because I could not trigger appending a delayed setting. This is probably because I was testing on a combined backend/frontend.

GetSettings() does return the default given.

WriteDelayedSettings() is called a number of times. However,

mythtv/mythtv/libs/libmythbase/mythdb.cpp

Line 966 in 4d3f06e

if (!gCoreContext->IsUIThread())

should probably be renamed to IsCoreContextThread.

ulmus-scott · 2022-02-07T21:31:24Z

Compilation failure from

mythtv/mythtv/programs/mythbackend/servicesv2/v2config.cpp

Line 80 in 377e00f

const DatabaseParams params = gCoreContext->GetDatabaseParams();

Now converted to GetMythDB().

@stuarta The changes are otherwise identical.

Several commits will need to be squashed before merging this PR.

ulmus-scott · 2022-03-01T04:54:22Z

Rebased onto master with the commits re-ordered and squashed.

The only difference is an extra newline in configuration.h and the removal of the void parameters in XmlConfiguration::(Save|Load).

ulmus-scott · 2022-07-01T18:00:15Z

@stuarta I have split out #598 because it was unrelated and I am currently rebasing this onto master.

ulmus-scott · 2022-07-02T19:53:52Z

@stuarta I have now rebased this onto master and made a number of changes versus the previous version, including no longer using reversion commits. The overall changes are similar.

ulmus-scott · 2022-07-02T19:57:54Z

configuration.h was a public header when it was part of libmythupnp, and unless you see a reason it needs to be public, I'm fine keeping it a private header as it is currently in libmythbase.

The Configuration DBConfiguration has always been a shim to the database see commit 85bb976 so I have replaced those few uses in mediaserver.cpp with direct GetMythDB() database accesses. However, setting a DBConfiguration to the global Configuration was incorrect since the UPnP code looks for values that are not present in the database, only optionally present in config.xml. Note: I now have UDNs for MediaRenderer, MediaServer, and MasterMediaServer in config.xml. However, I also had UPnP/UDN/* in the settings table of my database.

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.

add const char* overloads to fix overload resolution

for the classes used in this file.

remove unnecessary variable

reordered lines for enhanced clarity prevented an unwanted, inaccurate log line on Android

improve short circuit comparison make the return value actually have meaning, which it never did; see original commit 3fb9d6e from 2012 It now returns if the settings were saved successfully, instead of always returning true, which frankly I'm surprised was never caught (I would have thought this would throw a warning on the compiler or other analysis tools).

by moving the only code using it into the same function and making it local to that function. The only times saveSettingsCache() is called are right there in MythContext::Init() and in mythfrontend::main() immediately before exiting. Thus this change should have no noticeable functional change. For reference, these lines are originally from: deeed3c and 0b9d21a references https://code.mythtv.org/trac/ticket/13239 It is not clear to me from either commit or the referenced issue why the main window is being destroyed in the first place.

It's not much of a class, but this makes its function clearer and prepares MythContext for refactoring into MythGUIContext.

I don't like the pointer use of BackendSelection::Prompt(), but I am not investigating replacing it at this time. It should probably return a DatabaseParams and take by reference as an output a BackendSelection::Decision (enum).

from 3fb9d6e

ulmus-scott · 2022-09-04T20:58:37Z

@stuarta Ping. Rebased onto master to resolve minor conflict from minimum Qt version bump.

bennettpeter · 2022-11-15T15:54:44Z

From the comments this pull seems to follow on to some discussion you had with stuart, which I know nothing about, so I need some context.

Please can you give an overall description of what the purpose is of the pull request, what features of MythTV are affected, and what changes will be visible to the user.

ulmus-scott · 2022-11-15T17:45:22Z

From the comments this pull seems to follow on to some discussion you had with stuart, which I know nothing about, so I need some context.

#435 (comment) is about MythDB::WriteDelayedSettings(), which if I remember correctly, we discussed (privately?) on IRC. Since 2010 with c93be1c and 75cf4e4, you are supposed to be able to write settings to the database before it is connected. Looking over the current changes, I don't think this is relevant to this pull request.

Please can you give an overall description of what the purpose is of the pull request,

I had started work on this before Stuart committed 6ba7c84 and the basic goal was the same: break dependencies on libmythupnp.

However, I went further than he did and also started some code cleanup.

My intention was to prepare to break the dependency of MythContext on libmythupnp, but I'm not sure that is completely possible.

what features of MythTV are affected,

Loading the configuration caches in config.xml for database settings and the frontend settings from cache/contextcache.xml.

and what changes will be visible to the user.

Aside from the extra log messages I added, there should be no visible behavior changes. UDNs for MediaRenderer, MediaServer, and MasterMediaServer will all end up saved in config.xml. Only one of those is currently saved, I think. I have those in my database's settings table because of an earlier version where I (incorrectly) made everything directly access the database.

bennettpeter

The destroying of the main window was to address the case where the xml cache had been deleted and you were starting up before the database was active, and got the wrong GUI setup in your window.

That destroying of the main window is no longer necessary, so this commit is good.

bennettpeter · 2022-11-18T20:02:41Z

Merged into master.

ulmus-scott force-pushed the config-caches1 branch from 5c6ef1d to 2140a6a Compare December 24, 2021 00:22

stuarta reviewed Dec 24, 2021

View reviewed changes

mythtv/libs/libmythupnp/eventing.cpp Outdated Show resolved Hide resolved

stuarta reviewed Dec 24, 2021

View reviewed changes

ulmus-scott commented Dec 26, 2021

View reviewed changes

mythtv/libs/libmythupnp/configuration.cpp Outdated Show resolved Hide resolved

stuarta added this to In progress in UPnP Rework via automation Dec 28, 2021

ulmus-scott commented Dec 29, 2021

View reviewed changes

mythtv/libs/libmyth/mythcontext.cpp Outdated Show resolved Hide resolved

ulmus-scott force-pushed the config-caches1 branch 2 times, most recently from c895b72 to e17a11e Compare December 30, 2021 05:35

ulmus-scott force-pushed the config-caches1 branch from e17a11e to 5bc1efd Compare January 7, 2022 23:09

ulmus-scott force-pushed the config-caches1 branch from 7bf00ce to 7b60a1d Compare February 7, 2022 21:31

ulmus-scott requested a review from stuarta February 9, 2022 03:57

ulmus-scott force-pushed the config-caches1 branch from 7b60a1d to 2d71090 Compare March 1, 2022 04:53

ulmus-scott mentioned this pull request Jun 29, 2022

mythdb: New function GetDatabaseName #598

Closed

5 tasks

stuarta linked an issue Jun 30, 2022 that may be closed by this pull request

Separate upnp, ssdp and webserver / API functionality #329

Open

3 tasks

stuarta removed a link to an issue Jun 30, 2022

Separate upnp, ssdp and webserver / API functionality #329

Open

3 tasks

stuarta mentioned this pull request Jun 30, 2022

Separate upnp, ssdp and webserver / API functionality #329

Open

3 tasks

ulmus-scott force-pushed the config-caches1 branch from 2d71090 to d279859 Compare July 2, 2022 19:50

ulmus-scott added 4 commits September 4, 2022 16:51

XmlConfiguration: add default initializers and constructor

d4c7155

libmythupnp.pro: remove configuration.h missed by 6ba7c84

394b1b4

ulmus-scott added 12 commits September 4, 2022 16:51

XmlConfiguration: Style and variable names

909c530

XmlConfiguration: Use QVariant, overloading, to reduce code duplication

55705b9

add const char* overloads to fix overload resolution

libmythbase/configuration.h: add specified Qt includes

eac6dae

for the classes used in this file.

XmlConfiguration: add more log messages for debugging

097b5e6

mythcontext.cpp: LoadDatabaseSettings part 1

a332aa5

remove unnecessary variable

mythcontext.cpp: LoadDatabaseSettings() part 2

73f90b1

reordered lines for enhanced clarity prevented an unwanted, inaccurate log line on Android

mythcontext.cpp: Spaces and braces

1d971f9

mythcontext.cpp: separate out class GUISettingsCache

aeafe44

It's not much of a class, but this makes its function clearer and prepares MythContext for refactoring into MythGUIContext.

mythcontext.cpp: remove pointless casts

91fb917

I don't like the pointer use of BackendSelection::Prompt(), but I am not investigating replacing it at this time. It should probably return a DatabaseParams and take by reference as an output a BackendSelection::Decision (enum).

mythcontext.cpp: remove support for decade old legacy config.xml format

773691a

from 3fb9d6e

ulmus-scott force-pushed the config-caches1 branch from d279859 to 773691a Compare September 4, 2022 20:57

bennettpeter self-assigned this Nov 14, 2022

bennettpeter reviewed Nov 18, 2022

View reviewed changes

bennettpeter closed this Nov 18, 2022

UPnP Rework automation moved this from In progress to Done Nov 18, 2022

ulmus-scott deleted the config-caches1 branch November 19, 2022 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configuration caches #435

Configuration caches #435

ulmus-scott commented Dec 23, 2021

ulmus-scott commented Dec 23, 2021 •

edited

ulmus-scott commented Dec 30, 2021 •

edited

ulmus-scott commented Jan 7, 2022

ulmus-scott commented Jan 12, 2022

ulmus-scott commented Feb 7, 2022

ulmus-scott commented Mar 1, 2022

ulmus-scott commented Jul 1, 2022

ulmus-scott commented Jul 2, 2022

ulmus-scott commented Jul 2, 2022

ulmus-scott commented Sep 4, 2022

bennettpeter commented Nov 15, 2022

ulmus-scott commented Nov 15, 2022

bennettpeter left a comment

bennettpeter commented Nov 18, 2022

Configuration caches #435

Configuration caches #435

Conversation

ulmus-scott commented Dec 23, 2021

Checklist

ulmus-scott commented Dec 23, 2021 • edited

ulmus-scott commented Dec 30, 2021 • edited

ulmus-scott commented Jan 7, 2022

ulmus-scott commented Jan 12, 2022

ulmus-scott commented Feb 7, 2022

ulmus-scott commented Mar 1, 2022

ulmus-scott commented Jul 1, 2022

ulmus-scott commented Jul 2, 2022

ulmus-scott commented Jul 2, 2022

ulmus-scott commented Sep 4, 2022

bennettpeter commented Nov 15, 2022

ulmus-scott commented Nov 15, 2022

bennettpeter left a comment

Choose a reason for hiding this comment

bennettpeter commented Nov 18, 2022

ulmus-scott commented Dec 23, 2021 •

edited

ulmus-scott commented Dec 30, 2021 •

edited