Conversation
Signed-off-by: Kaspars Dambis <hi@kaspars.net>
Signed-off-by: Kaspars Dambis <hi@kaspars.net>
Signed-off-by: Kaspars Dambis <hi@kaspars.net>
Signed-off-by: Kaspars Dambis <hi@kaspars.net>
Signed-off-by: Kaspars Dambis <hi@kaspars.net>
| // Score artifacts based on match. | ||
| $score_artifact = function ( $artifact ) use ( $langs ) { | ||
| $score = 0; | ||
| $lang = strtolower( $artifact->lang ?? '' ); |
There was a problem hiding this comment.
Account for missing lang property which is allowed per spec.
| return false; | ||
| } | ||
| $artifact = pick_artifact_by_lang( $release->artifacts->package ); | ||
| $artifact = isset( $release->artifacts->package ) ? pick_artifact_by_lang( $release->artifacts->package ) : null; |
There was a problem hiding this comment.
Should this return false for the check below?
There was a problem hiding this comment.
Functionally speaking, null and false are both falsy so they'll both fail the truthy $artifact check below as expected.
Semantically speaking, this code is saying "The release artifact is..." (object|null), rather than "This has a release artifact" (bool), so null makes more sense to me when no release artifact exists.
Edit: pick_artifact_by_lang() returns stdClass|null, so this maintains a consistent fallback value type.
| if ( $url === $release->artifacts->package[0]->url ) { | ||
| $content_type = $release->artifacts->package[0]->{'content-type'}; | ||
| $artifact = array_find( | ||
| $release->artifacts->package ?? [], |
There was a problem hiding this comment.
This PR normalizes to an array of artifacts for a newly created ReleaseDocument via its ::from_data() method.
However, since the Transient API is a persistent cache mechanism, is may be possible that an existing pre-1.5.0 CACHE_RELEASE_PACKAGES transient value could still have the non-normalized package and therefore this array_find call could run on an object.
The normalization may need to run against $release->artifacts here before the array_find() call, and it may be worth abstracting the artifact normalization to a new static method on the ReleaseDocument class to reduce code duplication.
|
|
||
| $selected = pick_artifact_by_lang( [ $first_artifact, $second_artifact ], 'de-DE' ); | ||
|
|
||
| $this->assertContains( $selected, [ $first_artifact, $second_artifact ], 'Artifacts without lang should still return a valid artifact.' ); |
There was a problem hiding this comment.
Double-checking the reasoning why this checks that $selected is in an array rather than being a specific artifact. Is it because of inconsistent sorting across PHP versions or?
4 participants
Fixes #496.
This changeset fixes package handling when a FAIR repository returns
releases[].artifacts.packageas a single object instead of an array.Per the FAIR core spec, the
artifactsmap allows each artifact type value to be either an object or a list of objects. The plugin was normalizing neither shape internally and some consumer paths assumed array access, which caused failures such asCannot use object of type stdClass as arrayduring install/update flows.This patch normalizes release artifact values to arrays at parse time in
ReleaseDocument::from_data(), so downstream code can consume a consistent internal shape regardless of how the repository serialized the artifact. Consumer paths that read package download URLs or inspect package artifacts were updated accordingly, and regression tests were added to cover both valid spec shapes.Spec references
artifactsis a JSON object whose values “MUST be objects or lists of objects”artifacts.packageTesting
Added parser regression tests for: