WordPress / blueprints-library

31 stars 7 forks source link

Php Blueprints do not pass validation #83

Open reimic opened 7 months ago

reimic commented 7 months ago

During e2e tests implementation I've uncovered that Php Blueprints do not pass validation. See this test run.

This blueprint:

$blueprint = BlueprintBuilder::create()
    ->withWordPressVersion( 'https://wordpress.org/latest.zip' )
    ->toBlueprint();

...results in this error:

Array
(
    [/constants] => Array
        (
            [0] => The data (array) must match the type: object
        )

)

Same applies to this blueprint:

$blueprint = BlueprintBuilder::create()
    ->addStep( ( new MkdirStep() )->setPath( 'dir1' ) )
    ->addStep( ( new RmStep() )->setPath( 'dir1' ) )
    ->addStep( ( new MkdirStep() )->setPath( 'dir2' ) )
    ->toBlueprint()
adamziel commented 7 months ago

Aha, that's Opis not liking constants defaulting to array() – it needs to be an \ArrayObject() instance.

reimic commented 7 months ago

Ok. For experiment's sake I've modified the constructor in the BlueprintBuilder:

public function __construct() {
   $this->blueprint = new Blueprint();
   $this->blueprint->setConstants( new \ArrayObject() );
   $this->blueprint->setSiteOptions( new \ArrayObject() );
}

And it did move on. (Which, in my case, is an error wile executing step downloadWordPress.) But let's stick to the topic of this issue. What would be a suitable solution here? This, some other way to make those params ArrayObjects or rather the opis validation accepting array()?

adamziel commented 7 months ago

What would be a suitable solution here?

What you've done is spot on. That, and removing array() as the default value.

adamziel commented 7 months ago

...actually, perhaps setting the default to (object) array() would also work 🤔

reimic commented 5 months ago

@maypaw - e2e tests are also blocked here, could you take a look? This is also a case where a quick unblocking fix exists, but a bit more effort is needed to solve the problem comprehensively.

MayPaw commented 5 months ago

@reimic I'll check this out. Looks like it may be connected to the issues with the blueprint compiling scripts as well.

MayPaw commented 4 months ago

I have looked into this issue and it seems to me that the suggested solution in BlueprintBuilder.php addresses the outcome, but not the source of the problem. I think that the cause lies somewhere in autogenerate_models.php script and how the types from schema.json are interpreted and then assigned as the default value. Improving this seems like cleaner option.

However, to be honest, I don't really know how the jane-php/json-schema or nette/php-generator generate the models, so I'll look into that first and analyse how an ArrayObject can be assigned instead of an array.