[mantisbt-dev] Updating code in install functions

[Damien Regad]

I've been thinking at Paul's recent changes in some upgrade functions
(e.g. moving from serialized to json)

I have come to the conclusion that it's incorrect to alter existing
upgrade functions, as it potentially creates situations where we can no
longer know the state of the system and have incorrectly upgraded data.

For example, assume the following:
- upgrade function F() is called as part of version 2 (call it F()v2).
- F() is modified as part of version 3 (call that F()v3)

Admin A installs v1, then upgrades straight to v3; data gets modified by
F()v3.

Admin B installs v1, then upgrades to v2; data gets modified by F()v2.
Admin upgrades to v3, but data is still as of F()v2...

I believe the proper way to handle such situations where an upgrade
function needs to be changed, is to

1. create a NEW install function
2. add a NEW upgrade step

9dfc5fb6edb6da1e0324ceac3a27a727f2b23ba7 would have to be amended.

[P Richards]

Give me 2 hours to pop to work and go for a swim and I'll send a PR with a
different approach - as even my 'new' approach is incorrect, and this email
hadn't arrived last night.

The original upgrade function is incorrect, and equally, the approach I've
used doesn't make life easier either.

Paul

-----Original Message-----
From: Damien Regad [mailto:***@mantisbt.org]
Sent: 05 September 2014 23:34
To: Mantisbt-***@lists.sourceforge.net
Subject: [mantisbt-dev] Updating code in install functions

I've been thinking at Paul's recent changes in some upgrade functions (e.g.
moving from serialized to json)

I have come to the conclusion that it's incorrect to alter existing upgrade
functions, as it potentially creates situations where we can no longer know
the state of the system and have incorrectly upgraded data.

For example, assume the following:
- upgrade function F() is called as part of version 2 (call it F()v2).
- F() is modified as part of version 3 (call that F()v3)

Admin A installs v1, then upgrades straight to v3; data gets modified by
F()v3.

Admin B installs v1, then upgrades to v2; data gets modified by F()v2.
Admin upgrades to v3, but data is still as of F()v2...

I believe the proper way to handle such situations where an upgrade function
needs to be changed, is to

1. create a NEW install function
2. add a NEW upgrade step

9dfc5fb6edb6da1e0324ceac3a27a727f2b23ba7 would have to be amended.


----------------------------------------------------------------------------
--
Slashdot TV.
Video for Nerds. Stuff that matters.
http://tv.slashdot.org/

[Damien Regad]

Post by P Richards
Give me 2 hours to pop to work and go for a swim and I'll send a PR with a
different approach - as even my 'new' approach is incorrect, and this
email hadn't arrived last night.

I assume that https://github.com/mantisbt/mantisbt/pull/293 is your response
to this discussion ?

[P Richards]

Sourceforge mailing lists seem to be playing up again and missing emails.

I've not yet had the mail avetis or the mail I sent regarding filters - but
I've had your reply to avetis' mail ;/

Anyway, PR293 is fix for schema update process to ensure that we don't rely
on filter api. We still need to do some tidy up in Mantis to deal with the
fact that the previous filter upgrade was incorrect - but the filter code
upgrades the filters, so that's actually dealt with anyway.

In short, we have 2 filter changes - a) orginal filter thing that renamed
fields and set filters to latest version b) json "container" changes

I attempted to change/fix with the json container changes what's actually a
bug in the original filter changes:

The original filter changes "upgraded" the filters from v4 to v5 and renamed
the fields required between those two steps, and once it had done, set the
filter to the value of the filter version. The second part of this behaviour
is incorrect - we are now on version 8 (or 9). Therefore, filters that were
version 4/5, and were upgraded straight to the previous version 8, will not
receive the 'upgrade steps' to filters for version 6 or 7 until we next make
a filter change. This is incorrect behaviour.

Separately to that, the code for upgrading the filters was placed in the DB
upgrade step - as we use filter_ensure_valid to do filter version update
(e.g. from 6 to 7), upgrade filters stored in tokens, cookies, URL etc.
filter_ensure_valid needs to contain the filter upgrade steps in the db
layer to ensure that filters stored in cookies etc are also correctly
upgraded (as I believe query_store.php stores the value of a cookie filter
back to the db when saving current filter into the database).

[Damien Regad]

Post by P Richards
Sourceforge mailing lists seem to be playing up again and missing emails.

Seems to be just you - I'm getting all messages without any issues.

Anyway, thanks for the detailed response and explanation. I will test the
updated code to ensure it resolves the regression issue, and properly
upgrades my test DBs.

[Damien Regad [mailto:]]

When implementing the json container changes, I attempted to fix the above
by calling filter_ensure_valid - this was obviously a mistake as it leaves
you in a state where you are relying on the latest code and a partially
upgraded database. Hence my first thought, of making the filter changes a
single upgrade step (and the previous patch/commits). Whilst that's OK, and
fixes the issue for now, it leaves the same problem - as soon as we do a
database upgrade step, we need to evaluate whether it might break
filter_ensure_valid.

It also leaves a second possible issue that when filter_ensure_valid relies
on user configuration settings for example config_get( 'show_sticky_issues'
) - so filter_ensure_valid should be called by the end user to ensure their
personal/project settings are included correctly.

Having concluded the above, the best thing to do in schema/install.php is
not to try and upgrade the filter in some way - but to let our existing
mechanisms handle the version upgrade. That leaves the remaining question of
whether it would be feasible to store an upgrade filter back into the
database - at the moment the filter api doesn't lend itself easily to doing
that. Something that also isn't helped by the fact we don't always use our
own API within the code for viewing filters.

Paul


-----Original Message-----
From: Damien Regad [mailto:***@mantisbt.org]
Sent: 08 September 2014 09:33
To: Mantisbt-***@lists.sourceforge.net
Subject: Re: [mantisbt-dev] Updating code in install functions

Post by P Richards
Give me 2 hours to pop to work and go for a swim and I'll send a PR
with a different approach - as even my 'new' approach is incorrect,
and this email hadn't arrived last night.

I assume that https://github.com/mantisbt/mantisbt/pull/293 is your response
to this discussion ?




----------------------------------------------------------------------------
--
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce Perforce version control.
Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk