Changes in migrator in 3.1.10-RC1 / 3.2.0-RC2

Discussion forum for Extension Writers regarding Extension Development.
User avatar
Elsensee
Development Team Member
Development Team Member
Posts: 94
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 » Sat Aug 20, 2016 2:30 am

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.

User avatar
3Di
Registered User
Posts: 11865
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milano - Frankfurt
Name: Marco
Contact:

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

Post by 3Di » Sat Aug 20, 2016 2:45 am

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.
Want to compensate me for my interest? Donate
Please PM me only to request paid works. Thx.
Extensions, Scripts, MOD porting, Update/Upgrades

User avatar
Elsensee
Development Team Member
Development Team Member
Posts: 94
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 » Sat Aug 20, 2016 3:03 am

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.

User avatar
3Di
Registered User
Posts: 11865
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milano - Frankfurt
Name: Marco
Contact:

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

Post by 3Di » Sat Aug 20, 2016 3:17 am

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 ?¿?
Want to compensate me for my interest? Donate
Please PM me only to request paid works. Thx.
Extensions, Scripts, MOD porting, Update/Upgrades

User avatar
Elsensee
Development Team Member
Development Team Member
Posts: 94
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 » Sat Aug 20, 2016 3:32 am

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.

User avatar
3Di
Registered User
Posts: 11865
Joined: Mon Apr 04, 2005 11:09 pm
Location: Milano - Frankfurt
Name: Marco
Contact:

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

Post by 3Di » Sat Aug 20, 2016 3:35 am

Indeed, just answers for the posterity here. Thanks.
Want to compensate me for my interest? Donate
Please PM me only to request paid works. Thx.
Extensions, Scripts, MOD porting, Update/Upgrades

User avatar
david63
Jr. Extension Validator
Posts: 13143
Joined: Thu Dec 19, 2002 8:08 am
Location: Lancashire, UK
Name: David Wood
Contact:

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

Post by david63 » Sat Aug 20, 2016 7:08 am

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
VSE
Extensions Development Coordinator
Extensions Development Coordinator
Posts: 4484
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 VSE » Sat Aug 20, 2016 2:56 pm

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')),
      )),
   );
} 
Official phpBB Extensions My Extensions & MODs
Please do not PM me for support.
Dictated but not read.

User avatar
david63
Jr. Extension Validator
Posts: 13143
Joined: Thu Dec 19, 2002 8:08 am
Location: Lancashire, UK
Name: David Wood
Contact:

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

Post by david63 » Sat Aug 20, 2016 3:05 pm

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
Development Team Member
Development Team Member
Posts: 94
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 » Sat Aug 20, 2016 8:12 pm

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.

User avatar
Sajaki
Registered User
Posts: 1319
Joined: Mon Mar 02, 2009 1:41 pm
Name: Andreas
Contact:

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

Post by Sajaki » Wed Oct 19, 2016 8:23 pm

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).

User avatar
Tarantino
Registered User
Posts: 424
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 » Wed Oct 19, 2016 8:52 pm

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
Recognised Extension Developer
Posts: 20431
Joined: Wed Jun 22, 2005 4:33 pm
Location: Your display
Name: Rich McGirr
Contact:

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

Post by RMcGirr83 » Wed Oct 19, 2016 10:31 pm

That's bad as it would require additional changes to an already existing migration file which is frowned upon by the validation team.
In times of change, learners inherit the earth, while the learned find themselves beautifully equipped to deal with a world that no longer exists - Eric Hoffer
Former Modifications/Extensions Team Member | My extensions
Appreciate the extensions/mods/support then buy me a beer
All requests for support via PM will be ignored

User avatar
Paul
Infrastructure Team Leader
Infrastructure Team Leader
Posts: 23721
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 » Thu Oct 20, 2016 7:24 am

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

User avatar
Tarantino
Registered User
Posts: 424
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 » Thu Oct 20, 2016 11:06 am

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”

Who is online

Users browsing this forum: No registered users and 8 guests

cron