revolution icon indicating copy to clipboard operation
revolution copied to clipboard

Transport package validator running multiple times

Open BobRay opened this issue 1 year ago • 8 comments

Bug report

Summary

In the Captcha package, there is one validator, it prints a message that appears three times when the package is installed or re-installed.

Step to reproduce

Install or reinstall the current version of captcha

Observed behavior

This appears in the console (the log message is in the validator): Running PHP Validator. Running PHP Validator. Running PHP Validator.

I believe I've seen this with resolvers as well. Unless I'm doing something wrong (entirely possible), this may be happening regularly. Without a progress message, it would not be noticed.

It's happening in MODX 2 and MODX 3. I don't think this happened with earlier versions of MODX and the package has not changed for a few years.

Expected behavior

Validator should run once.

Code

Here's the relevant code in the transport package. It is not inside a loop.

/* @var $c modPlugin */
$c= $modx->newObject('modPlugin');
$c->set('id',1);
$c->set('name', 'Captcha');
$c->set('description', 'CAPTCHA Plugin');
$c->set('category', 0);
$c->set('plugincode', file_get_contents($sources['source_core'] . '/plugin.captcha.php'));

/* create a transport vehicle for the data object */

/* add plugin events */
$modx->log(xPDO::LOG_LEVEL_INFO,'Packaging in Plugin Events...'); 
$events = include $sources['data'].'plugin.events.php';
if (is_array($events) && !empty($events)) {
    $c->addMany($events);
} else {
    $modx->log(xPDO::LOG_LEVEL_ERROR,'Could not find plugin events!');
}
$modx->log(xPDO::LOG_LEVEL_INFO,'Plugin Events Packaged...'); 

$modx->log(xPDO::LOG_LEVEL_INFO,'Setting Package Attributes...'); 
$attributes= array(
    xPDOTransport::ABORT_INSTALL_ON_VEHICLE_FAIL => true,
    xPDOTransport::UNIQUE_KEY => 'name',
    xPDOTransport::PRESERVE_KEYS => false,
    xPDOTransport::UPDATE_OBJECT => true,
    xPDOTransport::RELATED_OBJECTS => true,
    xPDOTransport::RELATED_OBJECT_ATTRIBUTES => array (
       'PluginEvents' => array(
           xPDOTransport::PRESERVE_KEYS => true,
           xPDOTransport::UPDATE_OBJECT => true,
           xPDOTransport::UNIQUE_KEY => array('pluginid','event'),
       ),
    ),
);
$modx->log(xPDO::LOG_LEVEL_INFO,'Package Attributes Set...'); 
$modx->log(xPDO::LOG_LEVEL_INFO,'Creating Vehicle...'); 
$vehicle = $builder->createVehicle($c, $attributes);

$vehicle->validate('php',array(
    'type' => 'php',
    'source' => $sources['build'] . 'preinstall.script.php'
));

$vehicle->resolve('file',array(
    'source' => $sources['source_assets'],
    'target' => "return MODX_ASSETS_PATH . '/components/';",
));
$vehicle->resolve('file',array(
    'source' => $sources['source_core'],
    'target' => "return MODX_CORE_PATH . '/components/';",
));

$builder->putVehicle($vehicle);

BobRay avatar Jul 13 '22 22:07 BobRay

To be honest I'm not really familiar with the code around transport packages but maybe the code below needs to be changed?

$vehicle->validate('php',array(
    'type' => 'php',
    'source' => $sources['build'] . 'preinstall.script.php'
));

Into

$vehicle->validate('php',array(
    'source' => $sources['build'] . 'preinstall.script.php'
));

JoshuaLuckers avatar Jul 19 '22 11:07 JoshuaLuckers

I think the validator might run for each object in the vehicle - in this case I'm guessing 1 plugin and 2 plugin events = 3 runs of the validator?

Mark-H avatar Jul 19 '22 11:07 Mark-H

Mark is correct. The validator runs for each object installed from the vehicle, as _installIObject() is called recursively for this purpose.

opengeek avatar Jul 19 '22 14:07 opengeek

That was my suspicion as well, but why would we want a single validator to run over and over. This doesn't happen with resolvers does it?

BobRay avatar Jul 19 '22 18:07 BobRay

That was my suspicion as well, but why would we want a single validator to run over and over. This doesn't happen with resolvers does it?

Solid question. Resolvers only execute if parentObject is null, so no, this does not happen with resolvers.

opengeek avatar Jul 19 '22 18:07 opengeek

I can confirm this!

I have a PHP and MODX version validator in my install-package and it runs multiple times. As this is a month old problem - is there any solution for this?

gadgetto avatar Nov 17 '22 09:11 gadgetto

It's avoidable via several solutions. Probably the easiest is to modify the validator so it checks to see if it has run before, and returns if it has. One way to do that is to set a $_SESSION variable $_SESSION['my_validator_has_run'].

If (isset($_SESSION[''my_validator_has_run']) && $_SESSION[''my_validator_has_run'] === true) {
    return true;
} else {
    $_SESSION[''my_validator_has_run'] = true;
}

/* Rest of your code here */

Be sure to put the code at the top, above the switch statement.

You should probably clear that $_SESSION variable in a Resolver that runs at the end of the install.

You could also create a file somewhere with the name my_validator_has_run for the same thing and use file_exists() see if the validator has run already.

BobRay avatar Apr 09 '24 04:04 BobRay

I wanted to make another comment on the main issue here. In a transport file I want most of my validators to run up front (e.g., to check for a necessary library). In the build file, I create a category for my extra first, then I add all the elements to it, then put the category into the vehicle. The elements, as related objects, some along for the ride. Normally, a validator attached to this vehicle will run once for the category and once for every element in that category. Sometimes that's a lot of objects.

The validator is usually just a binary check to see if some library is installed, or something similar that needs to run once and aborts the install if it fails.

My question is, does anyone intentionally use the same validator to check every object in a vehicle during installation? If not, it would make sense to have it work the way resolvers work.

BobRay avatar Apr 09 '24 04:04 BobRay