Skip to content
This repository was archived by the owner on Feb 5, 2026. It is now read-only.

Fix REST API initialization for composer package usage #55

Merged
gziolo merged 4 commits into from
Sep 5, 2025
Merged

Fix REST API initialization for composer package usage #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd5748b
Fix REST API initialization for composer package usage
budzanowski Sep 3, 2025
d497b0b
Remove trailing whitespace
budzanowski Sep 3, 2025
867af46
Use tabs for indent.
budzanowski Sep 3, 2025
9c305db
Merge branch 'trunk' into fix/rest-api-composer-initialization
gziolo Sep 5, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions abilities-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,3 @@


require_once WP_ABILITIES_API_DIR . 'includes/bootstrap.php';

if ( function_exists( 'add_action' ) ) {
add_action( 'rest_api_init', array( 'WP_REST_Abilities_Init', 'register_routes' ) );
}
9 changes: 9 additions & 0 deletions includes/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

declare( strict_types = 1 );

if ( ! defined( 'ABSPATH' ) ) {
return; // Not in WordPress context
}

// Version of the plugin.
if ( ! defined( 'WP_ABILITIES_API_VERSION' ) ) {
define( 'WP_ABILITIES_API_VERSION', '0.1.0' );
Expand All @@ -34,4 +38,9 @@
// Load REST API init class for plugin bootstrap.
if ( ! class_exists( 'WP_REST_Abilities_Init' ) ) {
require_once __DIR__ . '/rest-api/class-wp-rest-abilities-init.php';

// Initialize REST API routes when WordPress is available.
if ( function_exists( 'add_action' ) ) {

@justlevine justlevine Sep 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the function check if we've already proven ABSPATH, or do we need ABSPATH if we're anyway checking for add_action() before running any WP code?

@gziolo gziolo Sep 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

@budzanowski budzanowski Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ABSPATH prevents processing of the file outside WP env - this is generally useful and in our context stops processing of the file during PHPUnit start procedure, when WP is not yet ready. We want to control the moment this gets loaded.

add_action tests could be considered redundant if ABSPATH is present. Though in WP startup procedure ABSPATH is defined earlier than add_action becomes available. Potentially we can have ABSPATH but no add_action. This just prevents a very "wrong" usage patter from crashing the site.

@gziolo gziolo Sep 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also fine to be extra careful. Let's land this as is, as it won't be included in WP core in the same fashion.
add_action( 'rest_api_init', array( 'WP_REST_Abilities_Init', 'register_routes' ) );
}
}
3 changes: 2 additions & 1 deletion tests/bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
tests_add_filter(
'muplugins_loaded',
static function (): void {
require_once dirname( __DIR__ ) . '/abilities-api.php';
// Require ( to bypass require_once ).
require dirname( __DIR__ ) . '/includes/bootstrap.php';

@justlevine justlevine Sep 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'd feel better about skipping abilities-api.php if understood the purpose of all this, but since it's being left out of core, it doesn't actually matter)

@budzanowski budzanowski Sep 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a test bootstrap. The purpose of this is to circumvent the quirkiness of PHPUnit which automatically loads autoloader, at the very beginning, and thus is triggering the loading of the library. Since it is doing this before WordPress environment has been loaded nothing gets actually loaded ( ABSPATH test prevents it ). Though PHP remembers the require_once calls. If we would load again abilities-api.php nothing would happen, because require_once for bootstrap.php file has been already called. Thus I have made a decision to call it directly for tests once again ( using require to force the load ) - after the WP environment has been loaded. This allows proper loading of all library elements and registering of the filter.

For normal operations from inside WP or a plugin this alterations would not be necessary - this is just for the unit tests and avoids tampering with require_once directives inside the production code which I wanted to avoid.

@gziolo gziolo Sep 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is tricky as autoloader requires the same file, but it doesn't run any logic because of ABSPATH check, so it looks that all is correct.

@justlevine justlevine Sep 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@budzanowski yes I understood the what and the how, it's the premise that we should be "triggering the loading of the library" - or more precisely calling that autoloading is now conflated with instantiating the REST endpoint - but still have a separate bootstrap file that confuses me. The architectural implication is that there would be other functionality in ./abilities-api.php now or in the future that we'd no longer be able to test... but again since it's not true IRL it doesn't matter)

@budzanowski budzanowski Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justlevine I agree this could makes things tricky down the road, but I did not wanted to spend too much time anticipating what may be required in the future without actually knowing how this will be used. I bet we will be able to test ./abilities-api.php separately if needed so hopefully this is not limiting.
}
);

Expand Down