Fix: LegacyProxy::get_instance_of throws when trying to get an instance of a non-Woo namespaced class

[Konamiman]

Changes proposed in this Pull Request:

The get_instance_of method of the LegacyProxy class isn't intended to retrieve instances of classes that live in the src directory (the dependency injection container should be used for these instead), and will throw an exception if that is attempted. However, in order to check if it should throw it just checks if the class name has a namespace; this will cause a false positive for namespaced classes defined outside of WooCommerce.

This pull request fixes this: the method will throw only if the full class name starts with Automattic\WooCommerce\.

How to test the changes in this Pull Request:

The bug was discovered after merging #44833: there are plugins that register namespaced REST API controllers and cause the exception. Thus for testing the fix we'll use the following code:

<?php

namespace Foo\Bar;

class Fizz {
	public function register_routes() {
		register_rest_route( 'fizz', '/buzz', [[
			'methods' => \WP_REST_Server::READABLE,
			'callback'  => fn($request) => 'Buzz!',
			'permission_callback' => fn($request) => true,
		]]);
	}
}

add_filter('woocommerce_rest_api_get_rest_namespaces', function($classes) {
	$classes['fizz'] = [\Foo\Bar\Fizz::class];
	return $classes;
});

To setup this code:

• If you are testing locally: put the code in a new fizz.php file in the root directory of the plugin, and add the following at the end of the woocommerce.php file: include __DIR__ . '/fizz.php';
• If you are testing using a remote server: install the Code Snippets plugin and add the code in a new snippet.

Once the code is prepared, run the following in the command line (or perform the equivalent request with Postman or a similar tool):

curl <site root>/wp-json/fizz/buzz

• Without the fix this will result in an error, and the server logs will show the following error: GET /wp-json/fizz/buzz - Uncaught Exception: The LegacyProxy class is not intended for getting instances of classes in the src directory, please use init method injection or the instance of Automattic\WooCommerce\Vendor\Psr\Container\ContainerInterface for that.
• With the fix the response will be the string "Buzz!".

Changelog entry

• Automatically create a changelog entry from the details below.

Significance

• Patch
• Minor
• Major

Type

• Fix - Fixes an existing bug
• Add - Adds functionality
• Update - Update existing functionality
• Dev - Development related task
• Tweak - A minor adjustment to the codebase
• Performance - Address performance issues
• Enhancement - Improvement to existing functionality

Message

Fix LegacyProxy::get_instance_of throwing an error when trying to get an instance of a non-Woo namespaced class.

Comment

[github-actions]

Hi @lsinger,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[github-actions]

Test Results Summary

Commit SHA: 9426278

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 37s
E2E Tests	275	0	0	72	0	347	6m 11s

---

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

[lsinger]

This looks good and tests as intended. The exception is now covered by unit tests in both positive and negative cases: a test fails if it's thrown when it shouldn't be, and a test fails if it's not thrown when it should be. LGTM! 🚢

[lsinger]

@Konamiman I edited your PR description to a) generate a changelog file and b) address an omission in the fizz.php test code that was unrelated to the problem that was solved but generated an error on the site frontend. The latter I did just to make sure that if we look at this again in the future, the test code doesn't throw any unexpected errors. Because I made those edits I'm going to let you check them and merge the PR yourself instead of merging it directly.