[Blueprints v2] Constraint the Blueprint bundle format #142
Conversation
| if ( ! empty( $wpConstraintErrors ) ) { | ||
| throw new BlueprintExecutionException( 'Invalid WordPress version constraint from CLI override: ' . implode( '; ', $wpConstraintErrors ) ); | ||
| } | ||
| $validator = new \WordPress\Blueprints\Validator\BlueprintValidator( $this->blueprintArray, $this->blueprintExecutionContext ); |
There was a problem hiding this comment.
Nice idea separating it! I've been experimenting with co-locating that logic with parsing the Blueprint and it had some ups and downs.
The downside here is that it's separated from parsing the Blueprint. The upside here is that it's separated from parsing the Blueprint. Let's roll with a separate validator and see how it goes.
There was a problem hiding this comment.
The downside here is that it's separated from parsing the Blueprint. The upside here is that it's separated from parsing the Blueprint.
This. I think that if we can make the parsing logic collect all errors before bailing out while keeping a reasonably nice code, then we should go with that. I'm looking into that.
| $errors = []; | ||
| $rootPointer = '/bundle/wp-content'; | ||
|
|
||
| // Top-level allowed directories within wp-content |
There was a problem hiding this comment.
I wonder if there's a reason to allow arbitrary directories 🤔 As in, themes, langs etc. would still have to be in specific paths, but would it be useful to allow, say, my-custom-dir here? I don't know, it's an open question – I wonder if there are sites or plugins out there that rely on those. If yes, we can adjust the spec.
Also – it's not blocking here. If anyone reports a compelling use-case later on, we can easily relax this constraint.
| $errors[] = new ValidationError( | ||
| $pointer, | ||
| 'unexpected-language-subdir', | ||
| sprintf('Unexpected directory under languages: %s. Allowed: plugins/, themes/, or .po/.mo files at root.', $entry) |
There was a problem hiding this comment.
Would .pot/.mot be useful here? Or not? Any .json files we expect? Again, I only have questions, not answers.
There was a problem hiding this comment.
Perhaps we should tolerate any files here, even .png, to support cases where you just zip a WordPress site and call that a Blueprint. However, if you want to refer to a translation file from Blueprint.json, that file must live in wp-content/languages. If I'm contradicting the Blueprints spec here, let's discuss and adjust the spec if needed.
There was a problem hiding this comment.
So what I'm saying is that the validation is more about confirming the .json file refers to allowed paths than about constraining what is allowed in those paths. In a scenario where we treat a URL as a Blueprint, say https://adamziel.com/my-demo-site/, we don't even have the ability to list files inside a directory.
There was a problem hiding this comment.
Thanks for jumping in! Indeed, this validation is very strictly focused on the directory structure, without looking at the references in the JSON file. I think it will be much better to only validate if the referenced paths are correct and then only output some warning that there are some unused (non-referenced) files.
|
I'm exploring an alternative with more Blueprint parsing & validation collocation in #144. |
WIP
Closes #132.