3

Resolved

Infoset Upgrade wipes the site settings

description

This happens if there is a setting having NULL value, (see code in Upgrade/InfoSetController)

reader["HomePage"] is DbNull.Value (which you cannot cast to String) and the lambada fails, exception is catched, reader is closed. Consequently the table is dropped and you end up empty handed.

What is the HomePage setting anyway? I don't remember seeing it in the Admin UI...
_upgradeService.ExecuteReader("SELECT * FROM " + _upgradeService.GetPrefixedTableName("Settings_SiteSettingsPartRecord"),
                (reader, connection) => {
                    site.HomePage = (string)reader["HomePage"];
                    site.PageSize = (int)reader["PageSize"];
                    site.PageTitleSeparator = (string)reader["PageTitleSeparator"];
                    site.ResourceDebugMode = (ResourceDebugMode)reader["ResourceDebugMode"];
                    site.SiteCulture = (string)reader["SiteCulture"];
                    site.SiteName = (string)reader["SiteName"];
                    site.SiteSalt = (string)reader["SiteSalt"];
                    site.SiteTimeZone = (string)reader["SiteTimeZone"];
                    site.SuperUser = (string)reader["SuperUser"];
                });

            _upgradeService.ExecuteReader("DROP TABLE " + _upgradeService.GetPrefixedTableName("Settings_SiteSettingsPartRecord"), null);
Cheers

comments

x0r wrote Dec 8, 2013 at 7:32 PM

Well, you can't do this either - unless you are casting from an int - which i wondered long time ago; why orchard stores the enums as strings instead of int?
site.ResourceDebugMode = (ResourceDebugMode)reader["ResourceDebugMode"];

Piedone wrote Dec 8, 2013 at 9:08 PM

It would be a lot more interesting if the UpgradeService would indeed operate on lambadas :-).

The home page is a mystery for me too (never seen it used anywhere). It should either be removed altogether or that line should contain a null-check. It would be great if somebody would chime on to tell what the heck this property is. Maybe it's from back when setting a content item as the home page worked a bit differently?

Regarding ResourceDebugMode: storing enums as strings is the adviced way. As strings the values are human-readable and are not prone to errors due to changes in the enum's values' order. If that cast fails (however I think NHibernate should parse the string there automatically...) then there should be an Enum.Parse() instead.

I once tried this migration, I wonder why it worked then...

x0r wrote Dec 9, 2013 at 7:08 AM

You got me there with the lambada expressions :D

How many people would really look into db to track the enum value? (ok, i do) But there is a trade-off between the readibility and performace gained when deserializing. Though good point about numerical values beeing more error prone.

x0r wrote Dec 9, 2013 at 7:36 AM

It also wouldn't hurt to check if the table exists at all. Migration fails on akismet_antispam (because i don't use it), but you can't complete the whole proces, because on the second run it fails on another table that isn't there (specifically, settings)

x0r wrote Dec 9, 2013 at 7:43 AM

I tried to debug the migration for the sake of my local version beign usable for the parts i use, but it also fails on Orchard_OutputCache_CacheSettingsPartRecord
VaryQueryStringParameters, VaryRequestHeaders, IgnoredUrls are NULL

Piedone wrote Dec 11, 2013 at 8:53 AM

As a general fail safe UpgradeService could re-throw exceptions from the delegate so subsequent statements (like table drops) are not executed if there is an error.

Piedone wrote Jan 28 at 4:03 PM

Fixed in changeset 2bb8f9f7b5c7b53cb24425c7a9cc8cefea56d526

Piedone wrote Jan 28 at 4:04 PM

** Closed by Piedone 01/28/2014 9:04AM

pszmyd wrote Mar 10 at 11:03 PM

Reopened this. Site settings are still partially wiped out (base URL, site name etc - all that belongs to SiteSettings2Record) when upgrading to 1.x.

Piedone wrote Mar 11 at 12:20 AM

Negative for me, all of the sites I upgrades had these settings correctly migrated.

sebastienros wrote Mar 14 at 7:47 PM

open ? close ?

CSADNT wrote Mar 14 at 8:29 PM

clopen

sebastienros wrote Mar 21 at 1:06 AM

No problem. I have modified with custom values, upgraded, and the custom values are correctly kept.

Please provide a repro.

IntranetFactory wrote Mar 23 at 3:08 PM

I assume that I just got this problem. I've just updated a project to the recent 1.x. In the Settings_SiteSettingsPartRecord the settings still exist, but _contentManager.Get<ISite>(siteId, VersionOptions.Published) doesn't return the values. It returned: BaseUrl: null
HomePage: null
MaxPageSize: 0
PageSize: 10
PageTitleSeparator: null
ResourceDebugMode: FromAppSetting
SiteCalendar: null
SiteCulture: null
SiteName: null
SiteSalt: null
SiteTimeZone: null
SuperUser: null
Nothing was written to the log so I assume nothing failed.

sebastienros wrote Mar 24 at 5:54 PM

The procedure to upgrade is:
  • add the "administrator" role to an account you will use to upgrade. at least, ensure the role you are using has "SiteOwner" permission
  • install 1.x on your current website
  • log into the dashboard using the account with "SiteOwner" permission
  • enable the Upgrade module
  • on each tab related to 1.8, click on the buttons to upgrade the contents
You are done

IntranetFactory wrote Mar 24 at 7:00 PM

That worked. Using the regular super user account didn't work, the super user can't even reach admin after the update is installed - so ensuring that another account with permissions assigned is created before the update is installed was the essential step I didn't now.

CSADNT wrote Mar 24 at 7:45 PM

Why the regular Site owner doesn't work ?

IntranetFactory wrote Mar 24 at 8:12 PM

In my test SuperUser was null -> so all the permission checks which grant hardcoded permissions to SuperUser failed and my SuperUser account therefore couldn't even reach the Dashboard to start the upgrade.

Another problem was that the theme which was used failed as well as var themeName = _siteThemeService.GetSiteTheme(); is also null. So a check for themeName.Name therefore also failed.

CSADNT wrote Mar 24 at 8:16 PM

Thank I will check this.
But it is bugs, isn't it ?

IntranetFactory wrote Mar 24 at 9:17 PM

I didn't expect such problems to upgrade from 1.7.2 to 1.8 - I personally think it shouldn't be 1.8 but 2.0 given the fundamental change of the InfoSets.

I won't really classify it a bug, it was "just" a lack of documentation and a kind of surprise that SuperUser can't reach the Dashboard any longer.

CSADNT wrote Mar 24 at 9:33 PM

That's really breaking, I had not understood we were loosing the old super user when migrating ?

IntranetFactory wrote Mar 24 at 11:22 PM

It's not really lost - but until the upgrade is finished the SuperUser has lost all permissions, so it behaves like a regular user. That's probably the reason that an additional user needs to be created with "SiteOwner" permission BEFORE the new version is applied.

sebastienros wrote Mar 25 at 12:31 AM

This will be the upgrade method. I have tried my best but it's not doable otherwise. It's should be very by following the upgrade steps, and the benefits are really worth it.