View Issue Details

IDProjectCategoryView StatusLast Update
0017605mantisbtupgradepublic2014-12-08 02:07
Reporterdregad Assigned To 
PriorityhighSeverityblockReproducibilityalways
Status closedResolutionfixed 
Product Version1.3.0dev 
Target Version1.3.0-beta.1 
Summary0017605: Regression issue with upgrade step 184
Description

Issue 0017809 / Pull request https://github.com/mantisbt/mantisbt/pull/225/ converts config entries from serialized to json_encoded.

This introduced a regression in the upgrade process, at step 184. When upgrading an existing system, this triggers errors as follows

UNHANDLED ERROR TYPE (4096)
Argument 1 passed to columns_remove_invalid() must be of the type array, null given, called in /home/dregad/dev/mantisbt/core/custom_function_api.php on line 302 and defined
Please use the "Back" button in your web browser to return to the previous page. There you can correct whatever problems were identified in this error or select another action. You can also click an option from the menu bar to go directly to a new section.
Full path: /home/dregad/dev/mantisbt/core/columns_api.php
Line: 412

After some research, this occurs as install_stored_filter_migrate() calls filter_ensure_valid_filter(), which eventually reaches columns_remove_invalid(), where config_get() is called. At that stage, the data stored in the config table is still serialized, but the function (line 177) calls json_decode(), which returns NULL.

I'm not sure what's the right way to fix this, please have a look.

TagsNo tags attached.

Relationships

related to 0017806 closedvboctor Upgrade your installation consistently failing 
related to 0017809 closedgrangeway Store config entries json encoded 

Activities

grangeway

grangeway

2014-08-20 12:32

reporter   ~0041101

in fact, wait this isn't due to the json stuff

Release marker: 1.2.1 - 1.2.15

$g_upgrade[184] = array( 'UpdateFunction', 'stored_filter_migrate' );

184 should be an old schema update

193-195 is latest - I'm thinking I can guess at fix though..

grangeway

grangeway

2014-08-20 12:37

reporter   ~0041102

Ok, right I understand what you mean.

grangeway

grangeway

2014-08-20 12:55

reporter   ~0041103

Ok,

Got a possible fix for this - as soon as Victor adds public key to mantisbt.org i'll test against a backup of that database from our sanitised version script, as that should give me enough old filter examples to test the script against.

Not sure if you want to leave this assigned to me or assign this issue to victor as a reminder.

Paul

dregad

dregad

2014-08-21 03:28

developer   ~0041107

in fact, wait this isn't due to the json stuff

It is. As mentioned, it breaks as json_decode() returns null when decoding serialized data. install_stored_filter_migrate() was updated by you in commit 9dfc5fb6 [1].

The problem is that config_get() uses the new code with json_decode, while the db still contains not-upgraded data.

I sent you a mail with details to download a dump.

[1] https://github.com/mantisbt/mantisbt/blame/master/core/install_helper_functions_api.php#L428

grangeway

grangeway

2014-08-21 18:44

reporter   ~0041109

Don't suppose you have any older aka 1.1/1.0 schema's knocking about from the old days as well?

grangeway

grangeway

2014-08-21 18:56

reporter   ~0041110

Damien:

https://github.com/grangeway/mantisbt/commit/87af01368c58b67875b892f82230f38bc7b13ac3 contains approach i'm thinking of taking.

a) shouldn't allow users to "config" a variable we use in schema updates
b) need to be more explicit and not call functions like columsn_get_all or whatever it is (different schema update) as over time these might change
c) make the filter migration upgrade step a function that is 'replaced' and always upgrades to latest filter
d) once B above has been done (assuming it's feasible, can probably remove the config_cache_flush i added

grangeway

grangeway

2014-08-22 17:14

reporter   ~0041113

Damien,

I've submitted a PR, as well as the fix for filter issue i've:

a) updated install_update_history_long_custom_fields to use ['name'] instead of [0]

b) replaced columns_get_standard() with an explicitly listed array in install_update_history_long_custom_fields (this is to avoid upgrade issues in future if we ever redefine columns and someone updates an old DB.

c) I've not evaluated with the config_cache_flush() is required after config update, but it seems sensible to leave in place just in case for future.

Ideally, i'd like to not call filter_ensure_valid from the schema updates - i.e. schema updates should really not rely on mantis internals which might change, but I think in the case if filters, we might be safe to do this and just bump a single function for future updates.

dregad

dregad

2014-09-05 19:05

developer   ~0041181

Paul, this needs fixing because the bug makes it impossible to use a DB with existing data: upgrade fails, and without upgrade my view page is broken.

Please see my comments in the PR.

dregad

dregad

2014-10-27 08:17

developer   ~0041674

See also possibly related PR https://github.com/mantisbt/mantisbt/pull/293

Related Changesets

MantisBT: master 8ececccc

2014-08-30 17:17

Paul Richards


Details Diff
Fix 0017605: Regression issue with upgrade step 184 Affected Issues
0017605
mod - admin/schema.php Diff File
mod - config_defaults_inc.php Diff File
mod - core/constant_inc.php Diff File
mod - core/filter_api.php Diff File
mod - core/install_helper_functions_api.php Diff File
mod - docbook/Admin_Guide/en-US/Configuration.xml Diff File
mod - search.php Diff File
mod - view_all_set.php Diff File