Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

add upgrade code for persistence layer upgrades #5431

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

sudden6
Copy link
Member

@sudden6 sudden6 commented Nov 11, 2018

This is the rest of the changes for finishing #5410

These are not yet final so don't waste your time reviewing it.


This change is Reviewable

@sudden6 sudden6 force-pushed the feat/paths2 branch 2 times, most recently from 5755028 to 8ca11f4 Compare November 19, 2018 15:34
@sudden6
Copy link
Member Author

sudden6 commented Nov 19, 2018

Based on #5440 this integrates the upgrade code to move the persistent data to the new places.

Once merged, this fixes #5410

@sudden6 sudden6 changed the title [WIP] Feat/paths2 add upgrade code for persistence layer upgrades Nov 19, 2018
{
assert(!curVersion.isEmpty());

if(curVersion < "1.17.0" && !upgradeFrom_1_16_3()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version strings can be compared wrong. "10.1" < "1.2"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspected this solution is to simple to be good -_-

@Diadlo
Copy link
Member

Diadlo commented Nov 20, 2018

I suggest instead of hardcored if(curVersion < "1.17.0" && !upgradeFrom_1_16_3()) + if(curVersion < "1.18.0" && !upgradeFrom_1_17_0()) etc. use something like this

struct Version {
    int major, minor, patch;
    bool operator<(const Version& other);
};

class IVersionUpdater {
    Version getVersion() virtual = 0;
    bool apply() virtual = 0;
};

class VersionUpdater_1_16_3 : IVersionUpdater {
    Version getVersion() { return {1, 16, 3}; }
    bool apply() { /* something */ }
};

std::vector<IVersionUpdater> PersistenceUpgrader::updates = {
    VersionUpdater_1_16_3(),
    VersionUpdater_1_17(), 
    /* ... */
};

bool PersistenceUpgrader::doUpdate() {
    for (auto update : updates) {
        if (update.getVersion() < currentVersion) {
            if (!update.apply()) {
                return false;
            }
        }
    }
}

ADDED: Instead of abstract class IVersionUpdater with extra memory allocation etc., you can use

struct VersionUpdater {
    Version version;
    std::function<void()> apply;
};


std::vector<VersionUpdater> PersistenceUpgrader::updates = {
    { {1, 16, 3}, apply_from_1_16_3 },
    { {1, 17, 0}, apply_from_1_17_0 },
    /* ... */
};

@Diadlo
Copy link
Member

Diadlo commented Nov 20, 2018

Or even better use not version, but incremental number. Otherwise you can't do two persistence update in one release. In this case you just need to apply PersistenceUpgrader::updates from currentId until the end

@sudden6
Copy link
Member Author

sudden6 commented Nov 21, 2018

Thanks for the feedback, I'll rework this and then update it.

This provides a central place to perform incremental upgrades in our
persistence layer. At the same time we can keep the code separated from
the operational code base.
With this commit the upgrade process is hooked up and executed on every
startup of qTox.

What's missing is the integration of the new paths in qTox.
The current implementation was broken and wrote to the non portable
directories even in portable mode.

Since Paths now handles this correctly we can remove the Settings part
and UI.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants