[BC] Refactor modTransportProvider to PSR-7 requests, remove modRestClient

[Mark-H]

What does it do?

Refactors the internals of modTransportProvider to use the PSR-7 requests implemented in #15779.

Tested against MODX.com, modmore.com, and modstore.pro package providers.

While standard packages and provider interaction works as expected without server-side changes, this is a Breaking Change specifically for paid extras that currently use $provider->request to validate licenses/retrieve decryption keys as that now returns a different (incompatible) response object. There's really no way around updating those extras with a new version of the encrypted vehicle and license check logic.

Other uses that directly send requests on the provider would also be affected, but are not currently known to me.

Why is it needed?

modRestClient was deprecated in 2.3, but its removal was held off by modTransportProvider using it heavily. Now that we have a better system for HTTP requests, it needed an update.

How to test

Use the package manager like a normal user would: browse, search, download, update etc.

Related issue(s)/PR(s)

Fixes #15469

[Mark-H]

Sorry; spotted some issues I thought I'd already fixed last night. Amazing what some sleep can do to find bugs in your own code 😬

[opengeek]

@Mark-H — My only concern on this is the methods returning error strings. Would it be better for these methods to throw exceptions rather than returning string error messages?

[Mark-H]

That would certainly be a cleaner implementation, at the expense of additional breaking changes because this string return for an error is how it works currently. I'd be okay with that.

[opengeek]

That would certainly be a cleaner implementation, at the expense of additional breaking changes because this string return for an error is how it works currently. I'd be okay with that.

Oh. I was under the impression you were adding these string error messages as a breaking change already. Perhaps I misunderstood some of the method signature changes and what looked like additions of string error messages being returned. I'll review this again in more detail.

[Mark-H]

I mean, you're not wrong... I think that was already the behavior of a few of the methods, and what the callers (i.e. the workspace/rest processors) were checking for, so I tried to make it consistent which means a string is now a new return type for some of the methods that didn't have the error handling before.

In hindsight exceptions make a lot more sense so I'm happy to go back in and replace that. Just don't know when, could be a in week or two... or after dinner tonight. :P