Prevent upgrade error should a plugin get disabled during the Matomo 4 upgrade #16468

Needs Review 4.0.0
@tsteur opened this Pull Request on September 24th 2020 Member

Could probably also remove the file again from core/API/NoDefaultValue.php and simply always define it here. This prevents showing an error like class Proxy already defined.

What happened?

  • Matomo 4 code is being downloaded and updated
  • If a plugin is not compatible, this will deactivate a plugin
  • This triggers the event PluginManager.pluginDeactivated
  • While will update the tracking code and also Tag Manager regenerate all containers
  • One of those, or the DateFormatProvider might trigger an API call using Request::processRequest
  • This will load Proxy::getInstance()
  • This will use our container to create a proxy instance
  • It will do a new Proxy()
  • It will execute the __constructor in Proxy class.
  • This will do a $this->noDefaultValue = new NoDefaultValue()
  • Now comes the tricky part!
    • Background: In Matomo 3 the Proxy.php defined two classes in one file. class Proxy and class NoDefaultValue.
    • This means in Matomo 3 in composer it has a config 'Piwik\\API\\NoDefaultValue' => $baseDir . '/core/API/Proxy.php',
  • Because no proxy class was created before the plugin being deactivated, and the code was already updated by then, we would load the Proxy class from the Matomo 4 code which does no longer contain NoDefaultValue in the same file but in a separate file core/API/NoDefaultValue.php.
  • This means when creating the new proxy instance, the NoDefaultValue class did not exist yet as it's no longer in the same file as Proxy.php
  • Composer will try to locate the class
  • It still has the reference 'Piwik\\API\\NoDefaultValue' => $baseDir . '/core/API/Proxy.php', because the autoloader was loaded with Matomo 3 configuration. It should have done 'Piwik\\API\\NoDefaultValue' => $baseDir . '/core/API/noDefaultValue.php',
  • As a result it will try to load core/API/Proxy.php again. Not to load the Proxy class but to load the NoDefaultValue class.
  • This leads then to this error:Cannot redeclare class Piwik\API\Proxy, because the name is already in use.

The only workaround was to add the class back to Proxy.php. Then the updated no longer resulted in an error. I verified this by patching the Proxy.php as in this PR, building a custom release zip, and then testing the upgrade.

Here's the backtrace that led to this error:

array (
  0 => 
  array (
    'file' => '/home/vendor/composer/ClassLoader.php',
    'line' => 444,
    'function' => 'include',
  ),
  1 => 
  array (
    'file' => '/home/vendor/composer/ClassLoader.php',
    'line' => 322,
    'function' => 'Composer\\Autoload\\includeFile',
  ),
  2 => 
  array (
    'function' => 'loadClass',
    'class' => 'Composer\\Autoload\\ClassLoader',
    'type' => '->',
  ),
  3 => 
  array (
    'file' => '/home/core/API/Proxy.php',
    'line' => 43,
    'function' => 'spl_autoload_call',
  ),
  4 => 
  array (
    'file' => '/home/vendor/php-di/php-di/src/DI/Definition/Resolver/ObjectCreator.php',
    'line' => 140,
    'function' => '__construct',
    'class' => 'Piwik\\API\\Proxy',
    'type' => '->',
  ),
  5 => 
  array (
    'file' => '/home/vendor/php-di/php-di/src/DI/Definition/Resolver/ObjectCreator.php',
    'line' => 70,
    'function' => 'createInstance',
    'class' => 'DI\\Definition\\Resolver\\ObjectCreator',
    'type' => '->',
  ),
  6 => 
  array (
    'file' => '/home/vendor/php-di/php-di/src/DI/Definition/Resolver/ResolverDispatcher.php',
    'line' => 58,
    'function' => 'resolve',
    'class' => 'DI\\Definition\\Resolver\\ObjectCreator',
    'type' => '->',
  ),
  7 => 
  array (
    'file' => '/home/vendor/php-di/php-di/src/DI/Container.php',
    'line' => 303,
    'function' => 'resolve',
    'class' => 'DI\\Definition\\Resolver\\ResolverDispatcher',
    'type' => '->',
  ),
  8 => 
  array (
    'file' => '/home/vendor/php-di/php-di/src/DI/Container.php',
    'line' => 131,
    'function' => 'resolveDefinition',
    'class' => 'DI\\Container',
    'type' => '->',
  ),
  9 => 
  array (
    'file' => '/home/core/Container/StaticContainer.php',
    'line' => 80,
    'function' => 'get',
    'class' => 'DI\\Container',
    'type' => '->',
  ),
  10 => 
  array (
    'file' => '/home/core/API/Proxy.php',
    'line' => 48,
    'function' => 'get',
    'class' => 'Piwik\\Container\\StaticContainer',
    'type' => '::',
  ),
  11 => 
  array (
    'file' => '/home/core/API/Request.php',
    'line' => 266,
    'function' => 'getInstance',
    'class' => 'Piwik\\API\\Proxy',
    'type' => '::',
  ),
  12 => 
  array (
    'file' => '/home/core/API/Request.php',
    'line' => 513,
    'function' => 'process',
    'class' => 'Piwik\\API\\Request',
    'type' => '->',
  ),
  13 => 
  array (
    'file' => '/home/plugins/LanguagesManager/LanguagesManager.php',
    'line' => 143,
    'function' => 'processRequest',
    'class' => 'Piwik\\API\\Request',
    'type' => '::',
  ),
  14 => 
  array (
    'file' => '/home/plugins/Intl/DateTimeFormatProvider.php',
    'line' => 128,
    'function' => 'uses12HourClockForCurrentUser',
    'class' => 'Piwik\\Plugins\\LanguagesManager\\LanguagesManager',
    'type' => '::',
  ),
  15 => 
  array (
    'file' => '/home/plugins/Intl/DateTimeFormatProvider.php',
    'line' => 111,
    'function' => 'uses12HourClock',
    'class' => 'Piwik\\Plugins\\Intl\\DateTimeFormatProvider',
    'type' => '->',
  ),
  16 => 
  array (
    'file' => '/home/plugins/Intl/DateTimeFormatProvider.php',
    'line' => 83,
    'function' => 'getTimeFormat',
    'class' => 'Piwik\\Plugins\\Intl\\DateTimeFormatProvider',
    'type' => '->',
  ),
  17 => 
  array (
    'file' => '/home/core/Date.php',
    'line' => 731,
    'function' => 'getFormatPattern',
    'class' => 'Piwik\\Plugins\\Intl\\DateTimeFormatProvider',
    'type' => '->',
  ),
  18 => 
  array (
    'file' => '/home/plugins/TagManager/Model/BaseModel.php',
    'line' => 39,
    'function' => 'getLocalized',
    'class' => 'Piwik\\Date',
    'type' => '->',
  ),
  19 => 
  array (
    'file' => '/home/plugins/TagManager/Model/Container.php',
    'line' => 380,
    'function' => 'formatDate',
    'class' => 'Piwik\\Plugins\\TagManager\\Model\\BaseModel',
    'type' => '->',
  ),
  20 => 
  array (
    'file' => '/home/plugins/TagManager/Model/Container.php',
    'line' => 214,
    'function' => 'enrichContainer',
    'class' => 'Piwik\\Plugins\\TagManager\\Model\\Container',
    'type' => '->',
  ),
  21 => 
  array (
    'file' => '/home/plugins/TagManager/Model/Container.php',
    'line' => 118,
    'function' => 'getContainer',
    'class' => 'Piwik\\Plugins\\TagManager\\Model\\Container',
    'type' => '->',
  ),
  22 => 
  array (
    'file' => '/home/plugins/TagManager/TagManager.php',
    'line' => 255,
    'function' => 'generateContainer',
    'class' => 'Piwik\\Plugins\\TagManager\\Model\\Container',
    'type' => '->',
  ),
  23 => 
  array (
    'file' => '/home/core/Context.php',
    'line' => 75,
    'function' => 'Piwik\\Plugins\\TagManager\\{closure}',
    'class' => 'Piwik\\Plugins\\TagManager\\TagManager',
    'type' => '->',
  ),
  24 => 
  array (
    'file' => '/home/plugins/TagManager/TagManager.php',
    'line' => 256,
    'function' => 'changeIdSite',
    'class' => 'Piwik\\Context',
    'type' => '::',
  ),
  25 => 
  array (
    'file' => '/home/core/Access.php',
    'line' => 635,
    'function' => 'Piwik\\Plugins\\TagManager\\{closure}',
    'class' => 'Piwik\\Plugins\\TagManager\\TagManager',
    'type' => '->',
  ),
  26 => 
  array (
    'file' => '/home/plugins/TagManager/TagManager.php',
    'line' => 264,
    'function' => 'doAsSuperUser',
    'class' => 'Piwik\\Access',
    'type' => '::',
  ),
  27 => 
  array (
    'function' => 'regenerateReleasedContainers',
    'class' => 'Piwik\\Plugins\\TagManager\\TagManager',
    'type' => '->',
  ),
  28 => 
  array (
    'file' => '/home/core/EventDispatcher.php',
    'line' => 141,
    'function' => 'call_user_func_array',
  ),
  29 => 
  array (
    'file' => '/home/core/Piwik.php',
    'line' => 775,
    'function' => 'postEvent',
    'class' => 'Piwik\\EventDispatcher',
    'type' => '->',
  ),
  30 => 
  array (
    'file' => '/home/plugins/CustomPiwikJs/TrackerUpdater.php',
    'line' => 142,
    'function' => 'postEvent',
    'class' => 'Piwik\\Piwik',
    'type' => '::',
  ),
  31 => 
  array (
    'file' => '/home/plugins/CustomPiwikJs/CustomPiwikJs.php',
    'line' => 35,
    'function' => 'update',
    'class' => 'Piwik\\Plugins\\CustomPiwikJs\\TrackerUpdater',
    'type' => '->',
  ),
  32 => 
  array (
    'function' => 'updateTracker',
    'class' => 'Piwik\\Plugins\\CustomPiwikJs\\CustomPiwikJs',
    'type' => '->',
  ),
  33 => 
  array (
    'file' => '/home/core/EventDispatcher.php',
    'line' => 141,
    'function' => 'call_user_func_array',
  ),
  34 => 
  array (
    'file' => '/home/core/Piwik.php',
    'line' => 775,
    'function' => 'postEvent',
    'class' => 'Piwik\\EventDispatcher',
    'type' => '->',
  ),
  35 => 
  array (
    'file' => '/home/core/Plugin/Manager.php',
    'line' => 529,
    'function' => 'postEvent',
    'class' => 'Piwik\\Piwik',
    'type' => '::',
  ),
  36 => 
  array (
    'file' => '/home/plugins/CoreUpdater/Updater.php',
    'line' => 308,
    'function' => 'deactivatePlugin',
    'class' => 'Piwik\\Plugin\\Manager',
    'type' => '->',
  ),
  37 => 
  array (
    'file' => '/home/plugins/CoreUpdater/Updater.php',
    'line' => 164,
    'function' => 'disableIncompatiblePlugins',
    'class' => 'Piwik\\Plugins\\CoreUpdater\\Updater',
    'type' => '->',
  ),
  38 => 
  array (
    'file' => '/home/plugins/CoreUpdater/Controller.php',
    'line' => 160,
    'function' => 'updatePiwik',
    'class' => 'Piwik\\Plugins\\CoreUpdater\\Updater',
    'type' => '->',
  ),
  39 => 
  array (
    'function' => 'oneClickUpdate',
    'class' => 'Piwik\\Plugins\\CoreUpdater\\Controller',
    'type' => '->',
  ),
  40 => 
  array (
    'file' => '/home/core/FrontController.php',
    'line' => 593,
    'function' => 'call_user_func_array',
  ),
  41 => 
  array (
    'file' => '/home/core/FrontController.php',
    'line' => 168,
    'function' => 'doDispatch',
    'class' => 'Piwik\\FrontController',
    'type' => '->',
  ),
  42 => 
  array (
    'file' => '/home/core/dispatch.php',
    'line' => 34,
    'function' => 'dispatch',
    'class' => 'Piwik\\FrontController',
    'type' => '->',
  ),
  43 => 
  array (
    'file' => '/home/index.php',
    'line' => 27,
    'args' => 
    array (
      0 => '/home/core/dispatch.php',
    ),
    'function' => 'require_once',
  ),
)
@tsteur commented on September 25th 2020 Member

Actually it's still good to have the class in the separate file as well so it works without issues in the documentation generator etc.

@tsteur commented on September 25th 2020 Member

fyi @sgiehl merging this one

This Pull Request was closed on September 25th 2020
Home | Back
Powered by GitHub Issue Mirror
Privacy Policy