Openlayers Upgrade Path for 0.x to 2.x (and ensure that schema version is correct)

The upgrade from 0.x to 2.x doesn't require a schema update, but the upgrade from 1.x to 2.x does.

The errors are caused by a misnumbered update hook that is trying to run the update code when it's not necessary to do so. This is probably all a result of the confusing break in version numbers.

Installations using 0.x should be at schema 6002. This patch uses the same update code that is used in the 0.x branch.

I think we need to plan this out a little bit. This is not a usual situation. We have the 0.x and 1.x branches both going to the 2.x branch (and the 0.x is not going through the 1.x). These will be two distinct upgrade paths.

Which means we need to name our update functions accordingly, as well as probably put in some manual checks to see what the current version is.

Current update hooks:

0.x branch has:
openlayers_update_6001
openlayers_update_6002

1.x branch has:
openlayers_cck_update_6100

2.x branch has:
openlayers_update_6200

The above patch should not change the name of the update function in 2.x. The 6200 update function should do a check to see what the current version is and update from there.

It should be assumed that the person has the most updated version of 1.x to 2.x. I think this is the only real sustainable way to handle the branches the way they are.

@zzolo, agreed. I will adjust this patch. I didn't realize what schema version the 1.x users were on. Thanks!

Just to note, the JavaScript error mentioned above can be caused by a missing layer type.

It's unfortunate that the schema version is set to to 2199 immediately prior to executing openlayers_update_6200() because there is no elegant way to tell if the schema is being upgraded from 0.x or a 1.x using drupal_get_schema_versions() or even a direct DB query to the system table. Both 0.x and 1.x will just be 2199.

The solution I arrived at is a little awkward, but it should be sufficient. I'm implementing one last update hook for the 0.x version that sets a $openlayers_avoid_schema_upgrade global variable to TRUE. Only 0.x installations will run this hook so the global will only be set if upgrading from 0.x.

openlayers_update_6200() now checks for this global variable and does not run the update if it is TRUE.

Will, this seems like a valid solution. What if updating is done via batch and for some reason 6003 is done on a separate call from 6200 (not sure how possible this is)? So, would it make more sense to use a Drupal variable?

Another excellent point. Updated the patch to use a drupal variable.

Added a return to the new update hook.

Your newest patch actually goes back to using a global variable.

Also, it would make a lot more sense if we called the variable something different. We are not avoiding a schema upgrade, we are simply choosing what actions to take given the current state of the schema. Maybe something like: openlayers_upgrade_from_0x

Another thing, it just occurred to me that this method might not work. Currently, the openlayers 1.x branch has no update functions, meaning that it is at 0 and the new 6003 update will be applied to both branches. I think the best solution here is to put in an arbitrary update function in the 1.x branch, forcing the schema to 6100. This might also alleviate the need for the arbitrary 6003 update as well. (please note that I have not tested this)

If the schema_version for 1.x installs is 0 then the original patch on this issue should work, right? 1.x installs starting at zero would run the 6001 and 6002 update hooks, but 0.x installs won't.

I would just need to add an empty openlayers_update_6200() hook to bring the schema up to 6200 for everyone.

Regardless of the above possibility, I've attached a new fixed patch.

I tested the patch in #1 upgrading from both 1.x and 0.x and it works without any problems. This is because 1.x users have 0 for their schema_version.

Great, thanks for testing this. Committed.

http://drupal.org/cvs?commit=325858

I am confused (and have not tested), but it seems like the patch in comment #1 is the one that was committed, so how does that work. Currently (what is committed), it looks like there are two update functions 6001 and 6002 in the 2.x version.

The problem here is that we can no longer add update functions to the 1.x branch if needed. We need to be bringing both branches to 62XX; there should not be any update functions in the 2.x branch less than 6200. I am not sure why we abandoned the idea of using a variable to circumvent the current situation. Or even just added an update function in the 1.x branch.

Nonetheless, this ticket should be reserved for the 0.x to 2.x upgrade and dealing with the nuances of the separate upgrade paths, and I am starting a new ticket for the 1.x to 2.x upgrade. The schema may be easy to change but the map data structures are completely different:
#710610: Upgrade Path for 1.x to 2.x

If the only corner case of the patch is that the OpenLayers 1.x branch cannot add its own schema updates, I don't think that's a big problem. As you said yourself, 1.x is essentially stuck in beta and isn't seeing much development. And, as stated everywhere, the upgrade path is manual but documented.

Just because you don't foresee it happening doesn't mean we need to completely disallow the possibility that the 1.x version will need an update or security fix that requires an update hook. That's just bad project management. I'll work on this this weekend.

Hey zzolo, the approach you outline in #8 seems reasonable, and we could document that users need to update to the newest version of OpenLayers 1.x before updating to 2.x. The original understanding was that 1.x was not going to have schema updates because of stability at 1.x, but adding an empty update hook will do just what you describe and will provide an upgrade path in those cases. Waiting on a patch.

The basic gotcha here is that there can't be an update hook from 1.x -> 2.x as long as 1.x is still continued. Zzolo hinted at that in #8. To put it in different words, as long as the schema of 1.x is not frozen we cannot write update hooks for it.

While we may be able to find a smarter update hook version name such as hook_update_6199(), there is nothing that guarantees that in the future changes to 1.x will render assumptions in hook_update_6199() obsolete.

Further, there are 2 practical problems with the current hooks hook_update_6001() and hook_update_6002() in the DRUPAL-6--2:

1) They also run on an upgrade from 0.x to 2.x, which causes some barfing when _6001 executes.
2) drupal_get_schema_unprocessed() in openlayers_update_6002 could break in subsequent upgrades when schemas of openlayers tables have changed.

I would suggest to remove the broken update hooks and provide a 1.x -> 2.x upgrade only if 1.x development ceases for good. Until then 1.x -> 2.x upgrades are a migration problem.

Of course, there is an option I haven't mentioned yet: we could add a hook_update_6199() to 2.x upgrading always the latest version of 1.x to 2.x. This would mean that any change to the 1.x schema would need an update of hook_update_6199() in the 2.x schema. I'm not sure whether we really want to maintain such a permanent upgrade path, though - it would make any change to the 1.x schema twice as expensive.

So, alex_b and I just talked some in IRC, though I dont think we came to an actual conclusion. I think its important to actually figure out what the problem is. This is what I think it is:

We cannot reliably determine what current schema openlayers is when we are upgrading to 2.x.

This is caused by either the fact that Drupal does not allow us to that information, or since someone just downloaded the newest 0.x or 1.x, they don't have any schema information.

So, here is my proposal that is by no means the most elegant, but does not box any branch into not having space for upgrades, and we are still using the right numbers for the upgrade hooks.

In the 1.x branch we define a variable in both the hook_enable and a new hook_update_6100 that basically sets a flag. Then, we create our 6200 hook which can do an easy conditional on the variable and do the correct processing. The one thing we have to communicate in the upgrade process is that the user must upgrade to newest 1.x version first (and whatever other manual changes are necessary).

This could be done for 0.x as well, but since we only have two option to be coming form, it is not technically necessary.

As point out by Will in #690964: Schema update hook uses drupal_get_schema_unprocessed()

This should just be included in this ticket:
=================================
drupal_get_schema_unprocessed() should not be used in schema update hooks. In the event that the schema changes in the future, the update hook from the past will load the newest schema and operate with that when it is expecting to use an older "snapshot."

The solution is to make a copy of the schema definition in the update hook so there is a snapshot of what the schema looked like at a given point in time.

This is still fixable now, but will quickly become critical if the schema changes.
=================================

I just thought of this.

We do actually know the schema versions because the schemas are different. For instance, the 1.x branch does not have a layers table at all, so we could use a TABLE EXISTS query to determine what branch we are coming from. I don't support this method because it does actually box in the modules to not allow schema update to a specific way, like adding a layers table to the 1.x branch if that is what we are testing.

#19: right. Same as #17 b). This is one of the two reasons why the existing update hooks in 2.x need to be removed.

#18:

We cannot reliably determine what current schema openlayers is when we are upgrading to 2.x.

We can: We know that every 0.x installation out there has schema version 6002 (look at openlayers.install that has been tagged for 0.x releases, you will find that all 0.x releases have the schema version 6002. Also, the existing (broken) release of 2.x has the schema version 6002.

This is all we need to know.

For a 1.x->2.x upgrade path we would only need to add an openlayers_update_6199() hook to 2.x that upgrades 1.x to 2.x. To avoid that this upgrade runs for 0.x versions too, we add a check in the upgrade that looks for the actual schema version:

function openlayers_update_6199() {
  $ret = array();
  if (drupal_get_installed_schema_version('openlayers') == 6002) {
    return $ret;
  }
  // Do 1.x->2.x upgrade here ....
  return $ret;
}

Further, we need to add an openlayers_update_6200() to make sure that the 2.x version has a defined schema.

The attached patch contains an openlayers_update_6200() and removes the broken hooks _6001 and _6002 from the DRUPAL-6--2 branch.

For the record, the issue for the upgrade path from 1.x -> 2.x is here: #710610: Upgrade Path for 1.x to 2.x, [edit:] the solution around openlayers_update_6199() outlined above is actually subject to the 1.x -> 2.x issue.

Talking with alex_b in IRC. It seems that we can reliably know what the schema version is coming form 0.x, so, similar to the above, we check for that schema version, but only in a 6200 update, since there is no real need for a 6199. Also, remove the 6001 and 6002 for the 2.x branch. Here is what should be in the install file:

function openlayers_update_6200() {
  $ret = array();
  $version = drupal_get_installed_schema_version('openlayers');
  if ($version > 0 && $version < 6100) {
    // 0.x to 2.x update
  }
  else {
    // Do 1.x->2.x upgrade here ....
  }  
  return $ret;
}

http://drupal.org/cvs?commit=327840

All good; thanks all. I have not tested this yet, but it should work fine. The actual upgrade for 1.x to 2.x will be handled here: #710610: Upgrade Path for 1.x to 2.x

Thanks, zzolo.