Daemon doesn't have the opportunity to recreate the config directories if they are deleted when it is already running

[Chris Wilson]

The development testing section of the wiki (https://wiki.ubuntu.com/Accomplishments/GetInvolved/Testing#Development_Testing) advises the reader to delete their accomplishments config settings before testing so as to provide a sterile environment in which to do it. Recreation of these directories would be carried out by the daemon on startup, however if it is already running then the config files will go uncreated and the view will crash on startup. See bug #1031876 for details.

I'm not familiar enough with how the viewer communicates with the daemon to be sure, but I think a suitable solution would be thus:

1) The viewer checks for the existence of the desired files and directories.
2) Upon not finding them it would ask the daemon to create them
3) The viewer would listen out for the daemon to indicate that it's successfully recreated the directories
4) A failure result should be handled gracefully

[Jono Bacon (jonobacon)]

Well spotted, and thanks for the bug report. I agree that we should always check to see if the config exists and if not just recreate them with default values.

[Jono Bacon (jonobacon)]

Just as a note, this is a daemon bug so re-assigning.

[Chris Wilson (notgary-deactivatedaccount)]

A change was required in the accomplishments viewer to I'm assigning that in addition to the daemon.

[Rafał Cieślak (rafalcieslak256)]

I have been thinking about these two bugs extensively, and it looks to me that 1031876 seems to me like misunderstanding, and either is an issue in the testing documentation, or is a problem of slightly different nature. However, this (1031908) is a valid bug in my opinion.

Having given a closer look at 1031876, one can notice that the *viewer* never crashes due to lack of the config file (daemon on the other hand does so). The viewer will face problems when ~/.cache/accomplishment is emptied while the daemon is running, as it has no trophy icons cache available. The viewer will not have problems because of ~/.config/accomplishments is cleared, as it never accesses it, it's the daemon who takes care about that.

And therefore, I consider _this_ (1031908) bug as a valid problem. 1031876, however, is related to a slightly different issue - the viewer crashes on startup when *icon cache* is missing, not when *config files* are missing. That means your branch [1] does not exactly what is needed to fix this issue, as it makes the viewer ask the daemon to recreate config files if needed. What actually needs to be done is that the viewer should - in such case - ask the daemon to recreate trophy icons cache. Ideally, it should ask it to do so only when it's actually missing, not blindly requesting this on every startup, as this may (when the accomplishments collection grows up) significantly increase the time the viewer needs to start.
Another thing that bothers me in that branch is the use of DBus signals. Note that the viewer will execute config_files_initialised(...) not only on startup, but every time corresponding signal is set. That means if I run another instance of the viewer, config files will get (pointlessly) initialized, and the first instance will misbehave, as it will partially re-initialize too, as config_files_initialised(...) will be called. [And by the way, it seems to me you have accidentally left dbusapi.init_config_files() commented out].

Now about the daemon - I do agree it should recreate the config files if they are not present. It seems logical that it should happen when the file is needed - so when one of the API's config access function is called. Your other MP [2] does it another way - it prepares the daemon to be able to recreate these files when the viewer starts. As I explained in previous paragraph - it's not needed in such case. This should happen when whoever uses the API functions that access the configuration stored in file (this may not necessarily be the viewer, it may be any other application, and that other application would simply use that API call, without having to remember to run init_config_files() before [and waiting for it to complete!]).
And there is something else I need to complain about your branch - the init_config_files() method is really confusing. It does a lot of different unrelated initialization things, and potentially modifies significant variables. We try to keep our API as clean as possible, and structuring methods by what they actually do is a key part of that - therefore I would prefer if the function that is meant to initialize config files would do just that.

Because of all these re...

[Chris Wilson (notgary-deactivatedaccount)]

Thanks for the feedback Rafal. From your comment, I've identified these as the work items for these two MPs. Do they sound OK to you?

For the viewer issue:

* Only check for the existence of the trophy icon cache at startup
    * If not found, recreate it
* Move unnecessary initialisation steps from init_config_files() back into the viewer's constructor

For daemon issue:

* Check for the existence of the config files only when they are actually needed.

[Chris Wilson (notgary-deactivatedaccount)]

I'm really sorry but I'm not going to be able to work on this just now.