Cleanup code comments and update README

README.md

@@ -48,10 +48,28 @@ The orch site provider loads sites and clusters from a remote orchestration http
 use Garden\Sites\Clients\OrchHttpClient;
 use Garden\Sites\Orch\OrchSiteProvider;
 use Garden\Sites\Orch\OrchCluster;
+use Symfony\Component\Cache\Adapter\MemcachedAdapter;
+use Symfony\Component\Cache\Adapter\RedisAdapter;
+use Symfony\Contracts\Cache\CacheInterface;
 
 $orchHttpClient = new OrchHttpClient("https://orch.vanilla.localhost", "access-token-here");
 $siteProvider = new OrchSiteProvider($orchHttpClient, OrchCluster::REGION_AMS1, OrchCluster::NETWORK_PRODUCTION);
 
+// It is highly recommended to set a user-agent for network requests.
+$siteProvider->setUserAgent("my-service:1.0");
+
+/**
+ * Site providers do various caching of results. By default an in-memory cache is used, but especially with an orch-client
+ * it is recommended to configure a persistent cache like memcached or redis.
+ * Caches must implement {@link CacheInterface}
+ */
+
+$cache = new RedisAdapter(/** Configuration here. */);
+// or
+$cache = new MemcachedAdapter(/** Configuration here. */);
+
+$siteProvider->setCache($cache);
+
 # Region/network can be changed later
 $siteProvider->setRegionAndNetwork(OrchCluster::REGION_YUL1, OrchCluster::NETWORK_DEVELOPMENT);
 ```

src/Clients/OrchClusterClient.php

@@ -8,8 +8,10 @@
 
 use Garden\Http\HttpClient;
 use Garden\Sites\Orch\OrchCluster;
-use Illuminate\Support\Facades\Http;
 
+/**
+ * HTTP client for making requests to a cluster's API.
+ */
 class OrchClusterClient extends HttpClient
 {
     public function __construct(OrchCluster $cluster)

src/Clients/OrchHttpClient.php

@@ -13,6 +13,10 @@
  */
 class OrchHttpClient extends HttpClient
 {
+    /**
+     * @param string $orchBaseUrl
+     * @param string $accessToken
+     */
     public function __construct(string $orchBaseUrl, string $accessToken)
     {
         parent::__construct($orchBaseUrl);

src/Cluster.php

@@ -6,6 +6,8 @@
 
 namespace Garden\Sites;
 
+use Garden\Sites\Orch\OrchCluster;
+
 /**
  * Class representing a cluster of sites.
  */
@@ -41,9 +43,9 @@ public function getClusterID(): string
      * Get a string identifying the region that the cluster belongs to. This is normally an acronym for some geo-located datacenter.
      *
      * Examples:
-     * - mtl
-     * - sjc / sfo
-     * - ams
+     * - mtl - {@link OrchCluster::REGION_YUL1}
+     * - sjc / sfo - {@link OrchCluster::REGION_SJC}
+     * - ams - {@link OrchCluster::REGION_AMS1}
      *
      * @return string
      */
@@ -56,8 +58,8 @@ public function getRegion(): string
      * Get a string identifying the type of network the cluster runs on.
      *
      * Examples
-     * - production
-     * - development
+     * - production {@link OrchCluster::NETWORK_PRODUCTION}
+     * - development {@link OrchCluster::NETWORK_DEVELOPMENT}
      * - localhost
      *
      * @return string

src/Exceptions/BadApiCredentialsException.php

@@ -8,6 +8,9 @@
 
 use Garden\Utils\ContextException;
 
+/**
+ * Exception thrown when required API credentials are missing for a network request.
+ */
 class BadApiCredentialsException extends ContextException
 {
 }

src/Exceptions/ClusterNotFoundException.php

@@ -8,6 +8,9 @@
 
 use Garden\Utils\ContextException;
 
+/**
+ * Exception thrown when a cluster could not be loaded.
+ */
 class ClusterNotFoundException extends ContextException
 {
     public function __construct(string $clusterID)

src/Exceptions/ConfigLoadingException.php

@@ -8,6 +8,9 @@
 
 use Garden\Utils\ContextException;
 
+/**
+ * Exception thrown when a site's config fails to load.
+ */
 class ConfigLoadingException extends ContextException
 {
     public function __construct(string $details = "", ?\Throwable $prev = null)

src/Exceptions/SiteNotFoundException.php

@@ -8,6 +8,9 @@
 
 use Garden\Utils\ContextException;
 
+/**
+ * Exception thrown when a site could not be loaded.
+ */
 class SiteNotFoundException extends ContextException
 {
     public function __construct(int $siteID)

src/FileUtils.php

@@ -12,8 +12,12 @@
 final class FileUtils
 {
     /**
+     * Iterate through all file in a directory matching a specific pattern.
+     * This iterator yields full file paths.
+     *
      * @param string $baseDir
      * @param non-empty-string $pattern
+     *
      * @return \Generator
      */
     public static function iterateFiles(string $baseDir, string $pattern): \Generator

src/Orch/OrchCluster.php

@@ -9,6 +9,9 @@
 use Garden\Sites\Clients\OrchClusterClient;
 use Garden\Sites\Cluster;
 
+/**
+ * Implementation of a cluster loaded from orchestration.
+ */
 class OrchCluster extends Cluster
 {
     public const REGION_YUL1 = "mtl";
@@ -21,6 +24,14 @@ class OrchCluster extends Cluster
     /** @var string Secret used in communication with the cluster. */
     private string $secret;
 
+    /**
+     * Constructor.
+     *
+     * @param string $clusterID
+     * @param string $region
+     * @param string $network
+     * @param string $secret
+     */
     public function __construct(string $clusterID, string $region, string $network, string $secret)
     {
         $this->secret = $secret;

src/Orch/OrchSite.php

@@ -17,6 +17,13 @@
  */
 class OrchSite extends Site
 {
+    /**
+     * Constructor.
+     *
+     * @param SiteRecord $siteRecord
+     * @param OrchSiteProvider $siteProvider
+     * @param HttpHandlerInterface $httpHandler
+     */
     public function __construct(
         SiteRecord $siteRecord,
         OrchSiteProvider $siteProvider,

src/Orch/OrchSiteProvider.php

@@ -28,6 +28,16 @@ public function __construct(OrchHttpClient $orchHttpClient, string $region, stri
     {
         parent::__construct($region, $network);
         $this->orchHttpClient = $orchHttpClient;
+    }
+
+    /**
+     * Overridden to set the user agent.
+     *
+     * @param string $userAgent
+     */
+    public function setUserAgent(string $userAgent): void
+    {
+        parent::setUserAgent($userAgent);
         $this->orchHttpClient->setUserAgent($this->getUserAgent());
     }
 

src/SiteProvider.php

@@ -8,18 +8,18 @@
 
 use Garden\Sites\Exceptions\ClusterNotFoundException;
 use Garden\Sites\Exceptions\SiteNotFoundException;
-use Garden\Sites\Orch\OrchCluster;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
-use Symfony\Contracts\Cache\CacheInterface;
 use Symfony\Contracts\Cache\ItemInterface;
 
 /**
+ * Class for loading sites and clusters.
+ *
  * @template TSite of Site
  * @template TCluster of Cluster
  */
 abstract class SiteProvider
 {
-    protected CacheInterface $cache;
+    protected \Symfony\Contracts\Cache\CacheInterface $cache;
 
     protected string $userAgent = "vanilla-sites-package";
 
@@ -56,19 +56,29 @@ public function setRegionAndNetwork(string $region, string $network): void
     /**
      * Apply a symfony cache contract.
      *
-     * @param CacheInterface $cache
+     * @param \Symfony\Contracts\Cache\CacheInterface $cache
      * @return void
      */
-    public function setCache(CacheInterface $cache): void
+    public function setCache(\Symfony\Contracts\Cache\CacheInterface $cache): void
     {
         $this->cache = $cache;
     }
 
+    /**
+     * Get the user agent to use for all network requests originating from the provider.
+     *
+     * @return string
+     */
     public function getUserAgent(): string
     {
         return $this->userAgent;
     }
 
+    /**
+     * Set the user agent used in network requests originating from the provider.
+     *
+     * @param string $userAgent
+     */
     public function setUserAgent(string $userAgent): void
     {
         $this->userAgent = $userAgent;
@@ -152,6 +162,9 @@ public function getSites(): array
     abstract public function getSite(int $siteID): Site;
 
     /**
+     * Method to load and return all clusters from its original data source.
+     * Results will be cached.
+     *
      * @return array<string, TCluster>
      */
     abstract protected function loadAllClusters(): array;

tests/BaseSitesTestCase.php

@@ -8,21 +8,30 @@
 
 use Garden\Http\HttpResponse;
 use Garden\Http\Mocks\MockHttpHandler;
-use Garden\Sites\Clients\SiteHttpClient;
 use Garden\Sites\Exceptions\BadApiCredentialsException;
 use Garden\Sites\Exceptions\ClusterNotFoundException;
 use Garden\Sites\Exceptions\SiteNotFoundException;
 use Garden\Sites\SiteProvider;
 use Garden\Sites\Tests\Fixtures\ExpectedSite;
 use PHPUnit\Framework\TestCase;
 
+/**
+ * Base tests case with common tests between different providers.
+ */
 abstract class BaseSitesTestCase extends TestCase
 {
     const SID_INVALID = 142141;
 
+    /**
+     * Create a configured site provider instance.
+     *
+     * @return SiteProvider
+     */
     abstract public function siteProvider(): SiteProvider;
 
     /**
+     *  Provide expected sites to various tests.
+     *
      * @return iterable<array-key, array<ExpectedSite>>
      */
     abstract public function provideExpectedSites(): iterable;
@@ -94,6 +103,7 @@ public function testValidSites(ExpectedSite $expectedSite): void
         $provider->setRegionAndNetwork($expectedSite->expectedRegion, $expectedSite->expectedNetwork);
         $site = $provider->getSite($expectedSite->getSiteID());
         $expectedSite->assertMatchesSite($site);
+        $expectedSite->assertConfigsMatchSite($site);
 
         // Sites have a cluster
         $cluster = $site->getCluster();

tests/Fixtures/ExpectedSite.php

@@ -10,6 +10,9 @@
 use Garden\Sites\SiteRecord;
 use PHPUnit\Framework\TestCase;
 
+/**
+ * Class holding expecations about sites from a provider.
+ */
 class ExpectedSite extends SiteRecord
 {
     private array $expectedConfigs;
@@ -19,17 +22,19 @@ class ExpectedSite extends SiteRecord
     public string $expectedRegion = "localhost";
     public string $expectedNetwork = "localhost";
 
+    /**
+     * @param int $siteID
+     * @param int $accountID
+     * @param string $clusterID
+     * @param string $baseUrl
+     * @param array $expectedConfigs
+     */
     public function __construct(int $siteID, int $accountID, string $clusterID, string $baseUrl, array $expectedConfigs)
     {
         parent::__construct($siteID, $accountID, $clusterID, $baseUrl);
         $this->expectedConfigs = $expectedConfigs;
     }
 
-    public function getExpectedConfigs(): array
-    {
-        return $this->expectedConfigs;
-    }
-
     /**
      * @return $this
      */
@@ -39,13 +44,22 @@ public function expectNoSystemToken(): ExpectedSite
         return $this;
     }
 
+    /**
+     * @param string $region
+     * @param string $network
+     * @return $this
+     */
     public function expectNetworkAndRegion(string $region, string $network): self
     {
         $this->expectedRegion = $region;
         $this->expectedNetwork = $network;
         return $this;
     }
 
+    /**
+     * @param Site $site
+     * @return void
+     */
     public function assertMatchesSite(Site $site): void
     {
         $suffix = "to match expected site '{$this->getBaseUrl()}'";
@@ -55,6 +69,10 @@ public function assertMatchesSite(Site $site): void
         TestCase::assertEquals($this->getBaseUrl(), $site->getBaseUrl(), "Expected baseUrl {$suffix}.");
     }
 
+    /**
+     * @param Site $site
+     * @return void
+     */
     public function assertConfigsMatchSite(Site $site): void
     {
         foreach ($this->expectedConfigs as $key => $value) {