Account for package metadata that doesn't include filename when deriving hashed filename

[kasparsd]

Fixes #492.

Updated get_hashed_filename() method to build a stable, hashed install path for a FAIR package from its metadata.

It works in this order:

1. It derives a short DID hash from the package ID.
2. It tries to split filename into two parts: a slug-like directory name and a plugin file name.
3. If the filename does not provide a slug, it falls back to the metadata slug.
4. If the filename does not provide a file portion, it assumes the main plugin file should be slug.php.
5. It appends the DID hash to the slug unless it is already present.
6. If the package type is wp-plugin, it returns a WordPress-style plugin path in the form slug-didhash/file.php.
7. For non-plugin types, it returns only the hashed slug.

The intent of this version is:

• prefer the slug embedded in filename when available
• gracefully recover when filename is missing or incomplete
• avoid double-appending the DID hash
• return plugin paths as directory/file.php
• return non-plugin paths as just the hashed directory/slug name

Examples for this implementation:

• plugin with filename = example/example.php returns example-<hash>/example.php
• plugin with missing filename falls back to example-<hash>/example.php
• theme with slug example returns example-<hash>

[github-actions]

🧪 Test this PR on Playground

[kasparsd]

When $metadata->filename isn't specified (such as https://github.com/fairpm/fair-plugin/releases/latest/download/fair-metadata.json), the explode() failed with:

Deprecated: explode(): Passing null to parameter #2 ($string) of type string is deprecated in /inc/packages/namespace.php on line 694

and the $file was undefined which caused:

Warning: Undefined array key 1 in /inc/packages/namespace.php on line 694

[afragen]

I think the problem is that $metadata->filename is supposed to exist and that the fair-metadata.json was incorrectly created.

[kasparsd]

Yeah, the general package spec doesn’t require it so I think it makes sense that we support that scenario as a fallback.

https://github.com/fairpm/fair-protocol/blob/83be5fa21fc1f72d5bcbf2bdd654f9e72274bc39/specification.md?plain=1#L197

Arguably, WP determines the bootstrap file dynamically based on header presence (and themes don’t even need it) so I think should probably refactor this eventually.

[afragen]

We did update the spec and it now requires it.

[kasparsd]

@afragen I think the spec shouldn't require it. None of the WP packages have that concept (at least from the existing WP-org API perspective).

I created #500 to illustrate how WP core resolves the plugin_file dynamically during install and never gets it from the update API. Asking package publishers to include the filename is simply unnecessary.

In #500 I demonstrate how we can resolve all the necessary filename data from the canonical places in WP without having to assume or deal with mismatches on the filesystem (what active_plugins thinks) and what the metadata reports.

[kasparsd]

Ensure the output type is always correct and use the constant value from the global namespace since that is what defined( 'FAIR_DEFAULT_REPO_DOMAIN' ) checks for.

[costdev]

Thanks for the changes @kasparsd! I won't get to test this or do a full review, but hopefully someone has availability and can help determine if any further tests would be good here.

[kasparsd]

All the code review feedback has been addressed. Added a few additional phpunit test cases to account for the edge cases resolved.

[costdev]

@cdils I won't get to do a full review of this PR. If someone else if available to do a full review, that would be great!

Edit: It looks like @afragen started a review

Changes made

[afragen]

Also, not sure I understand why a function named get_hashed_filename() no longer returns $filename.

[kasparsd]

It returns the filename only for wp-plugin type and returns the slug in all other cases. That is what this updated approach makes clear with a conditional return for that one edge case for WP plugins.

Feel free to push an update or I can update it if you can advise on the updated logic here.

[cdils]

@costdev sorry–I didn't mean to trigger a review from you as I saw your earlier comment. I was attempting to get rid of resolved conversations pertaining to the previous review. Blame it on my github skillz. :)

[costdev]

No worries haha!

[afragen]

Is there a test for the current state of the FAIR Connect where fair-plugin/plugin.php doesn’t list a filename in the metadata?

[kasparsd]

@afragen Yes, this pull request adds those test cases, including this one with the missing filename:

https://github.com/kasparsd/fair-plugin/blob/a29caf53ec45d65666c92c8b78edaf41860228f6/tests/phpunit/tests/Packages/GetHashedFilenameTest.php#L34-L47

My proposal is to remove the filename requirement eventually since they're not really required as WP resolves the plugin file automatically by scanning all root-level PHP files for the Plugin Name header during install/activation. Created #500 to demonstrate that. But that's for a separate discussion.