Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from Sep 25, 2020
Merged

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

merged 2 commits into from Sep 25, 2020

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Sep 24, 2020

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 tsteur added the Needs Review PRs that need a code review label Sep 24, 2020
@tsteur tsteur added this to the 4.0.0 milestone Sep 24, 2020
@tsteur
Copy link
Member Author

tsteur commented Sep 25, 2020

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 tsteur merged commit a01eea3 into 4.x-dev Sep 25, 2020
@tsteur
Copy link
Member Author

tsteur commented Sep 25, 2020

fyi @sgiehl merging this one

@tsteur tsteur deleted the preventupgradeerror branch September 25, 2020 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Needs Review PRs that need a code review
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@tsteur