Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Discussion forum for Extension Writers regarding Extension Development.
User avatar
Elsensee
Former Team Member
Posts: 122
Joined: Sat May 07, 2011 11:04 am
Location: Hamburg, Germany
Name: Oliver Schramm
Contact:

Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Elsensee »

As of the next released phpBB version (3.1.10-RC1, 3.2.0-RC2), the migrator will skip if statements returned by update_data() on revert just as if they were custom methods. if statements returned by revert_data() are of course unaffected.

Previously, these if statements were somehow executed when reverting, which could lead to unexpected results.

If you did rely on this (really unexpected) behaviour, please fix your migrations. Otherwise you don't have to take action.
Development Team Member
I don't make bugs - I make features
User avatar
3Di
Former Team Member
Posts: 16032
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milan (IT) Frankfurt (DE)
Name: Marco
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by 3Di »

Assuming there will be just a 3.1.10-RC. (Hopefully ;) ).

Has, all of this, been previously discussed somewhere where we can lurk at?

Some clear example and/or documentation update is always appreciated by the posterity. :)

Thanks.
Please PM me only to request paid works. Thx.
Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user
Extensions, Scripts, MOD porting, Update/Upgrades
Looking for a specific feature or alternative option? We will rock you! 🚀
User avatar
Elsensee
Former Team Member
Posts: 122
Joined: Sat May 07, 2011 11:04 am
Location: Hamburg, Germany
Name: Oliver Schramm
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Elsensee »

There was no discussion about this, since it isn't possible to predict what the extension author wants to execute on revert in case of an if. You can't invert the condition as that might not be always true and you can't invert the call since this isn't always true for all. Thus, we just avoid it altogether now.

Also, this was just an information since it is possible that this was the intended case in the past too, it just wasn't coded that way.

The usage is just about the same.
On revert only the non-revertable migration steps from update_data() have to be included in revert_data().
Non-revertable migration steps are custom methods and (this is new! ;) ) steps that are only executed when a certain condition is true.

So, on revert this update_data():

Code: Select all

public function update_data()
{
	return array(
		array('config.add', array('foobar', 1)),
		array('if', array(
			true,
			array('permission.add', array('some_data')),
		)),
		array('config.remove', array('foobar')),
		array('custom', array(array($this, 'foo_bar'))),
	);
}
will result in these steps being executed in this order: (the steps will be reversed as you can see)

Code: Select all

array('config.reverse', array('remove', 'foobar')),
array('config.reverse', array('add', 'foobar', 1)),
which means that you have to add the custom call and the conditional step to the revert_data() if this is necessary.
Development Team Member
I don't make bugs - I make features
User avatar
3Di
Former Team Member
Posts: 16032
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milan (IT) Frankfurt (DE)
Name: Marco
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by 3Di »

Sounds OK. Thanks for making it even more clear to us. :)

One last question though, it seems that the Authors of some extensions are in the need/urge to update those and re-submit them the CDB and/or update the related development repositories/topics once phpBB 3.1.10-RC/3.2.0-RC2 will be out.

Or is it just a thought of mine and there is some kind of " BC " which I am not aware of ?¿?
Please PM me only to request paid works. Thx.
Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user
Extensions, Scripts, MOD porting, Update/Upgrades
Looking for a specific feature or alternative option? We will rock you! 🚀
User avatar
Elsensee
Former Team Member
Posts: 122
Joined: Sat May 07, 2011 11:04 am
Location: Hamburg, Germany
Name: Oliver Schramm
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Elsensee »

I assume the number of the affected extensions will be very very small.

As I said, it is only important for those who relied on this behaviour. (But you really should have not relied on this as it is really hard to predict what will happen) All others will just benefit from this change and don't have to do anything.
Development Team Member
I don't make bugs - I make features
User avatar
3Di
Former Team Member
Posts: 16032
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milan (IT) Frankfurt (DE)
Name: Marco
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by 3Di »

Indeed, just answers for the posterity here. Thanks.
Please PM me only to request paid works. Thx.
Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user
Extensions, Scripts, MOD porting, Update/Upgrades
Looking for a specific feature or alternative option? We will rock you! 🚀
User avatar
david63
Registered User
Posts: 18324
Joined: Thu Dec 19, 2002 8:08 am
Location: Lancashire, UK
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by david63 »

Based on this topic I use this code in many of my extensions

Code: Select all

class version_1_0_0 extends \phpbb\db\migration\migration
{
	public function update_data()
	{
		$update_data = array();

		$update_data[] = array('config.add', array('version_friendsandfoes', '1.0.0'));

		if ($this->module_check())
		{
			$update_data[] = array('module.add', array('acp', 'ACP_CAT_USERGROUP', 'ACP_USER_UTILS'));
		}

		$update_data[] = array('module.add', array(
			'acp', 'ACP_USER_UTILS', array(
				'module_basename'	=> '\david63\friendsandfoes\acp\friendsandfoes_module',
				'modes'				=> array('main'),
			),
		));

		return $update_data;
	}

	protected function module_check()
	{
		$sql = 'SELECT module_id
				FROM ' . $this->table_prefix . "modules
    			WHERE module_class = 'acp'
        			AND module_langname = 'ACP_USER_UTILS'
        			AND right_id - left_id > 1";

		$result		= $this->db->sql_query($sql);
		$module_id	= (int) $this->db->sql_fetchfield('module_id');
		$this->db->sql_freeresult($result);

		// return true if module is empty, false if has children
		return (bool) !$module_id;
	}
}
So are you now saying that this will not work in the future? If so how should it be handled and how do I handle the already installed extensions?

Edit: Just to add that if there is an if within an already existing migration file then it cannot be changed as migration files cannot be changed retrospectively
David
Remember: You only know what you know and - you don't know what you don't know!
My CDB Contributions | How to install an extension
I will not be accepting translations for any of my extensions in Github - please post any translations in the appropriate topic.
No support requests via PM or email as they will be ignored
User avatar
MattF
Extensions Development Coordinator
Extensions Development Coordinator
Posts: 5250
Joined: Sat Jan 17, 2009 9:37 am
Location: Los Angeles, CA
Name: Matt Friedman
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by MattF »

So basically, what we're saying here is if you use an "if" in update_data like:

Code: Select all

public function update_data()
{
   return array(
       array('if', array(
         true,
         array('permission.add', array('some_data')),
      )),
   );
}
If you rely on the (unintended) behavior that when uninstalling an extension, that change got automatically reverted (the permission would be removed in the example, if the condition were still true), this will no longer be reverted automatically, so you have to do it yourself in a revert_data method (which was always the intended behavior anyways).

Code: Select all

public function revert_data()
{
   return array(
       array('if', array(
         true,
         array('permission.remove', array('some_data')),
      )),
   );
} 
Formerly known as VSEMy ExtensionsPlease do not PM me for support.
User avatar
david63
Registered User
Posts: 18324
Joined: Thu Dec 19, 2002 8:08 am
Location: Lancashire, UK
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by david63 »

So, just to clarify, what I am currently doing is still OK
David
Remember: You only know what you know and - you don't know what you don't know!
My CDB Contributions | How to install an extension
I will not be accepting translations for any of my extensions in Github - please post any translations in the appropriate topic.
No support requests via PM or email as they will be ignored
User avatar
Elsensee
Former Team Member
Posts: 122
Joined: Sat May 07, 2011 11:04 am
Location: Hamburg, Germany
Name: Oliver Schramm
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Elsensee »

VSE wrote:So basically, what we're saying here is if you use an "if" in update_data like:

Code: Select all

public function update_data()
{
   return array(
       array('if', array(
         true,
         array('permission.add', array('some_data')),
      )),
   );
}
If you rely on the (unintended) behavior that when uninstalling an extension, that change got automatically reverted (the permission would be removed in the example, if the condition were still true), this will no longer be reverted automatically, so you have to do it yourself in a revert_data method (which was always the intended behavior anyways).

Code: Select all

public function revert_data()
{
   return array(
       array('if', array(
         true,
         array('permission.remove', array('some_data')),
      )),
   );
} 
With one little note for your example:
That step was just executed as-is. So the permission would be still added if the condition was true.
Development Team Member
I don't make bugs - I make features
User avatar
Sajaki
Registered User
Posts: 1371
Joined: Mon Mar 02, 2009 1:41 pm
Location: Amsterdam
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Sajaki »

Recent topics seems to be affected by this change.

I get an install error when enabling the ext : "A required module does not exist: RECENT_TOPICS_EXT"

I think it's due to this line (was coded by Paybas).
Tarantino
Registered User
Posts: 797
Joined: Sat Feb 18, 2012 1:51 pm
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Tarantino »

I was guessing that it was by this change, but they told me it was not xD

In any case lol the solution I would say is the same as mine, and my messages from here was moved to here:
viewtopic.php?f=461&t=2389746&p=14538996#p14540471

And as you can see on that link your problem comes for the lines below:

Code: Select all

// Add new modules
			array('module.add', array(
				'acp',
				'ACP_CAT_DOT_MODS',
				'RECENT_TOPICS'
			)),
Just delete that.

And maybe the code:

Code: Select all

array('module.add', array(
				'acp',
				'RECENT_TOPICS',
				array(
					'module_basename'    => '\paybas\recenttopics\acp\recenttopics_module',
					'modes'    => array('recenttopics_config'),
				),
			)),
should become:

Code: Select all

array('module.add', array(
				'acp',
				'ACP_CAT_DOT_MODS',
				array(
					'module_basename'    => '\paybas\recenttopics\acp\recenttopics_module',
					'modes'    => array('recenttopics_config'),
				),
			)),
Best Regards
User avatar
RMcGirr83
Former Team Member
Posts: 21493
Joined: Wed Jun 22, 2005 4:33 pm
Location: Your display
Name: Rich McGirr

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by RMcGirr83 »

That's bad as it would require additional changes to an already existing migration file which is frowned upon by the validation team.
Appreciate the extensions/mods/support then buy me a beerImage
Former Modifications/Extensions Team Member | My extensions | github | All requests for support via PM will be ignored
Paul
Infrastructure Team Leader
Infrastructure Team Leader
Posts: 26815
Joined: Sat Dec 04, 2004 3:44 pm
Location: The netherlands.
Name: Paul Sohier
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Paul »

RMcGirr83 wrote:That's bad as it would require additional changes to an already existing migration file which is frowned upon by the validation team.
In this specific case we will accept it :)
Knock knock
Race condition
Who's there?

My BlogMy Photosmy phpBB Extensionscustom phpBB work & Development
Tarantino
Registered User
Posts: 797
Joined: Sat Feb 18, 2012 1:51 pm
Contact:

Re: Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Post by Tarantino »

So, all validated extensions will be fixed by the extensions team? :) Thats sounds fair enough and good news.
david63 wrote:So, just to clarify, what I am currently doing is still OK
David as far I've tested your code doesn't work no more. :(
But it's not about the if thing that was talked here. Is about the module.add. something changed on module.add.

Best regards
Post Reply

Return to “Extension Writers Discussion”