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

Php Blueprints do not pass validation #83

Open
reimic opened this issue Mar 19, 2024 · 7 comments
Open

Php Blueprints do not pass validation #83

reimic opened this issue Mar 19, 2024 · 7 comments
Labels
blocker bug Something isn't working V1

Comments

@reimic
Copy link
Collaborator

reimic commented Mar 19, 2024

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
Copy link
Collaborator

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

@adamziel adamziel mentioned this issue Mar 19, 2024
@reimic
Copy link
Collaborator Author

reimic commented Mar 19, 2024

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
Copy link
Collaborator

What would be a suitable solution here?

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

@adamziel
Copy link
Collaborator

adamziel commented Mar 19, 2024

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

@reimic
Copy link
Collaborator Author

reimic commented Jun 8, 2024

@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
Copy link
Collaborator

MayPaw commented Jun 8, 2024

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

@MayPaw
Copy link
Collaborator

MayPaw commented Jun 16, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Something isn't working V1
Projects
None yet
Development

No branches or pull requests

3 participants