dev/core#3833 Update CRM_Extension_Downloader to not use dynamic properties

[braders]

Overview

Update CRM_Extension_Downloader to not use dynamic properties.

First of many required changes for dev/core#3833

Before

The CRM_Extension_Downloader class used dynamic properties which are going to be deprecated in PHP 8.2, and which harm readability.

After

$manager and containerDir are no longer dynamic properties.

I've re-arranged some of the properties and methods so that the constructor comes first, and so that the property order better matches the order in which they are defined.

Whilst in this file I've also removed the line $destDir = $this->containerDir . DIRECTORY_SEPARATOR . $key; as $destDir was not being used.

Technical Details

I've decided to make manager and containerDir private, as I could not see a need for them to be public. That said it required some thinking about, and happy to hear arguments to the contrary. This highlights both why resolving #3833 will be a good excercise, but also why it may be difficult and time-consuming.

I also think guzzleClient could be private rather than protected, but didn't change it incase there is a reason I am not seeing.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

Change looks sensible. 👍

I've decided to make manager and containerDir private...

Yeah, There shouldn't be anyone else accessing these two properties. The canonical way for other classes to lookup extension data is through CRM_Extension_System. (And in a quick grep for >manager and >containerDir, I don't see anyone else reading these properties from Downloader.)

In terms of QA - I don't think there's much unit-test coverage, but (once the test-run finishes) we can probably cover most risks with a light r-run (Administer => System =>Extensions => Add new => Civirules...) on the autobuild site.

[totten]

Test failure is not related. Basic r-run on autobuild site worked fine. Merging.