As designed triggerStyleDataRebuild in setup fails to add custom fields

[Arty]

Affected version

2.0.0 DP 10

This is sample code from setup:

    public function installStep1()
   {
      $this->schemaManager()->alterTable('xf_style', function(Alter $table)
      {
         $table->addColumn('test', 'tinyint')->setDefault(0);
      });
    \XF::repository('XF:Style')->triggerStyleDataRebuild();
   }

There is also entity class that adds test field to entity.

After that code is ran, styles list in registry doesn't contain "test" column, so using this code to get list of styles:

		$styles = \XF::registry()->get('styles');

doesn't return 'test' column.

I think problem is caused by this code in XF\Repository\Style:

	public function triggerStyleDataRebuild()
	{
		$this->app()->service('XF:Style\Rebuild')->rebuildFullParentList();

It immediately rebuilds styles list, before style entity is extended, therefore entity doesn't contain "test" field. It should be ran after add-on installation is complete and classes are extended.

 

[Mike]

There are some changes that I don't believe are in DP10 but the short of it would really be that it'd be your responsibility to trigger it after install or upgrade. There are new setup methods that represent this explicitly.

 

[Arty]

How? It requires entity to be extended, which is done after add-on installation. Wouldn't it make more sense for full style rebuild job to take care of that?

 

[Chris D]

In beta 1 there is a new postInstall and postUpgrade method in the AbstractSetup class which you can extend for custom behaviours. That'd probably be the ideal place to trigger it.

For now, you could try this.

\XF::runOnce('SomeUniqueKey', function()
{
    \XF::repository('XF:Style')->triggerStyleDataRebuild();
});

That will make it pretty much the last thing to run in the request. Though I'm not 100% sure it will help. Worth a try though. I have a couple of other ideas if that doesn't work.

 

[Arty]

Nice. During execution of that method, will classes extended by add-on be extended? Like in code above, will style entity have 'test' field? If that's the case, that's exactly what I'm looking for.

 

[Chris D]

Well, I’m not sure.

Certainly by that point the extension will be added and the relevant caches been rebuilt so, I think so.

When you have the postInstall method in beta 1 you can probably run it there without the runOnce closure I think.

 

[Mike]

This is now resolved more consistently: during installation, upgrade and uninstall steps, you cannot rely on listeners or extensions for your add-on being active as we explicitly disable them to prevent inconsistent behavior and other problems. To rectify this, we have added postInstall and postUpgrade methods to the AbstractSetup class which run after all of the latest data is imported and thus do run with listeners and extensions enabled. There is also a new onActiveChange in AbstractSetup which will be called when a user explicitly toggles the active state. Upgrading an add-on will also automatically enable it (this postUpgrade and postInstall should always run when an add-on is enabled); in this situation, onActiveChange won't be triggered since you would generally trigger these sorts of rebuilds in postUpgrade anyway.

So in this regard, the report here is roughly something we'd consider as designed.

Now saying all that, the particular code being referenced here has problems because it has interactions with XenForo upgrades. Like in XF1, an XF upgrade happens in a context where add-ons don't run. Otherwise, it is possible for an add-on to entirely block XF upgrading. (Another situation that would apply is if someone disables listeners via config.php and does something that triggers a cache rebuild.) This is going to lead to the cache not having the data you expect.

In this situation, the best approach would be to not modify the style cache directly. Instead, when it's being rebuilt, you can use the simple cache system to cache the extra data you need. You can then inject this data into the style setup process (such as by extending XF\Style or the "style.cache" entry in the container). This is a little more complex, but it should be safer in certain scenarios.