Rework RemoteConfig template building and tests

kreait · Oct 19, 2020 · 88bdc51 · 88bdc51
1 parent 1d38eb8
commit 88bdc51
Show file tree

Hide file tree

Showing 10 changed files with 221 additions and 93 deletions.
diff --git a/.php_cs.dist b/.php_cs.dist
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 $finder = PhpCsFixer\Finder::create()
     ->in(['src', 'tests']);
 
@@ -35,6 +37,7 @@ return PhpCsFixer\Config::create()
         'ordered_imports' => true,
         'ordered_interfaces' => true,
         'phpdoc_line_span' => [
+            'const' => 'single',
             'property' => 'single',
         ],
         'phpdoc_no_empty_return' => true,
@@ -46,7 +49,7 @@ return PhpCsFixer\Config::create()
         'php_unit_no_expectation_annotation' => true,
         'php_unit_set_up_tear_down_visibility' => true,
         'php_unit_test_case_static_method_calls' => [
-            'call_type' => 'this'
+            'call_type' => 'this',
         ],
         'phpdoc_align' => false,
         'phpdoc_order' => true,

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,9 +19,11 @@
   * `Kreait\Firebase\Messaging::unsubscribeFromAllTopics($registrationTokenOrTokens)`
 * The RemoteConfig component now support Parameter Groups.
   ([Documentation](https://firebase-php.readthedocs.io/en/latest/remote-config.html#parameter-groups))
-
 ### Changed
   * Replaced usage of deprecated Guzzle helpers
+### Deprecated
+  * `Kreait\Firebase\RemoteConfig\Parameter::fromArray()`
+  * `Kreait\Firebase\RemoteConfig\Template::fromResponse()`
 
 ## [5.9.0] - 2020-10-04
 ### Added

diff --git a/src/Firebase/Messaging/MessageTarget.php b/src/Firebase/Messaging/MessageTarget.php
@@ -12,9 +12,7 @@ final class MessageTarget
     public const TOKEN = 'token';
     public const TOPIC = 'topic';
 
-    /**
-     * @internal
-     */
+    /** @internal */
     public const UNKNOWN = 'unknown';
 
     public const TYPES = [

diff --git a/src/Firebase/RemoteConfig.php b/src/Firebase/RemoteConfig.php
@@ -13,6 +13,7 @@
 use Kreait\Firebase\RemoteConfig\Version;
 use Kreait\Firebase\RemoteConfig\VersionNumber;
 use Kreait\Firebase\Util\JSON;
+use Psr\Http\Message\ResponseInterface;
 use Traversable;
 
 /**
@@ -39,7 +40,7 @@ public function __construct(ApiClient $client)
      */
     public function get(): Template
     {
-        return Template::fromResponse($this->client->getTemplate());
+        return $this->buildTemplateFromResponse($this->client->getTemplate());
     }
 
     /**
@@ -52,9 +53,7 @@ public function get(): Template
      */
     public function validate($template): void
     {
-        $template = $template instanceof Template ? $template : Template::fromArray($template);
-
-        $this->client->validateTemplate($template);
+        $this->client->validateTemplate($this->ensureTemplate($template));
     }
 
     /**
@@ -66,26 +65,24 @@ public function validate($template): void
      */
     public function publish($template): string
     {
-        $template = $template instanceof Template ? $template : Template::fromArray($template);
-
-        $etag = $this->client->publishTemplate($template)->getHeader('ETag');
+        $etag = $this->client
+            ->publishTemplate($this->ensureTemplate($template))
+            ->getHeader('ETag');
 
         return \array_shift($etag) ?: '';
     }
 
     /**
      * Returns a version with the given number.
      *
-     * @param VersionNumber|mixed $versionNumber
+     * @param VersionNumber|int|string $versionNumber
      *
      * @throws VersionNotFound
      * @throws RemoteConfigException if something went wrong
      */
     public function getVersion($versionNumber): Version
     {
-        $versionNumber = $versionNumber instanceof VersionNumber
-            ? $versionNumber
-            : VersionNumber::fromValue($versionNumber);
+        $versionNumber = $this->ensureVersionNumber($versionNumber);
 
         foreach ($this->listVersions() as $version) {
             if ($version->versionNumber()->equalsTo($versionNumber)) {
@@ -99,20 +96,16 @@ public function getVersion($versionNumber): Version
     /**
      * Returns a version with the given number.
      *
-     * @param VersionNumber|mixed $versionNumber
+     * @param VersionNumber|int|string $versionNumber
      *
      * @throws VersionNotFound
      * @throws RemoteConfigException if something went wrong
      */
     public function rollbackToVersion($versionNumber): Template
     {
-        $versionNumber = $versionNumber instanceof VersionNumber
-            ? $versionNumber
-            : VersionNumber::fromValue($versionNumber);
-
-        $response = $this->client->rollbackToVersion($versionNumber);
+        $versionNumber = $this->ensureVersionNumber($versionNumber);
 
-        return Template::fromResponse($response);
+        return $this->buildTemplateFromResponse($this->client->rollbackToVersion($versionNumber));
     }
 
     /**
@@ -145,4 +138,30 @@ public function listVersions($query = null): Traversable
             $pageToken = $result['nextPageToken'] ?? null;
         } while ($pageToken);
     }
+
+    /**
+     * @param Template|array<string, mixed> $value
+     */
+    private function ensureTemplate($value): Template
+    {
+        return $value instanceof Template ? $value : Template::fromArray($value);
+    }
+
+    /**
+     * @param VersionNumber|int|string $value
+     */
+    private function ensureVersionNumber($value): VersionNumber
+    {
+        return $value instanceof VersionNumber ? $value : VersionNumber::fromValue($value);
+    }
+
+    private function buildTemplateFromResponse(ResponseInterface $response): Template
+    {
+        $etagHeader = $response->getHeader('ETag');
+        $etag = \array_shift($etagHeader) ?: '*';
+
+        $data = JSON::decode((string) $response->getBody(), true);
+
+        return Template::fromArray($data, $etag);
+    }
 }
diff --git a/src/Firebase/RemoteConfig/Condition.php b/src/Firebase/RemoteConfig/Condition.php
@@ -44,6 +44,11 @@ public function name(): string
         return $this->name;
     }
 
+    public function expression(): string
+    {
+        return $this->expression;
+    }
+
     public function withExpression(string $expression): self
     {
         $condition = clone $this;

diff --git a/src/Firebase/RemoteConfig/Parameter.php b/src/Firebase/RemoteConfig/Parameter.php
@@ -92,6 +92,9 @@ public function conditionalValues(): array
     }
 
     /**
+     * @deprecated 5.10.0
+     * @codeCoverageIgnore
+     *
      * @param array<string, mixed> $data
      */
     public static function fromArray(array $data): self

diff --git a/src/Firebase/RemoteConfig/Template.php b/src/Firebase/RemoteConfig/Template.php
@@ -7,7 +7,6 @@
 use Kreait\Firebase\Exception\InvalidArgumentException;
 use Kreait\Firebase\Util\JSON;
 use Psr\Http\Message\ResponseInterface;
-use Throwable;
 
 class Template implements \JsonSerializable
 {
@@ -37,6 +36,9 @@ public static function new(): self
 
     /**
      * @internal
+     *
+     * @deprecated 5.10.0
+     * @codeCoverageIgnore
      */
     public static function fromResponse(ResponseInterface $response): self
     {
@@ -56,41 +58,67 @@ public static function fromArray(array $data, ?string $etag = null): self
         $template->etag = $etag ?? '*';
 
         foreach ((array) ($data['conditions'] ?? []) as $conditionData) {
-            $template->conditions[(string) $conditionData['name']] = Condition::fromArray($conditionData);
+            $template = $template->withCondition(self::buildCondition($conditionData['name'], $conditionData));
         }
 
         foreach ((array) ($data['parameters'] ?? []) as $name => $parameterData) {
-            $template->parameters[(string) $name] = Parameter::fromArray([(string) $name => $parameterData]);
+            $template = $template->withParameter(self::buildParameter($name, $parameterData));
         }
 
         foreach ((array) ($data['parameterGroups'] ?? []) as $name => $parameterGroupData) {
-            $group = ParameterGroup::named((string) $name)
-                ->withDescription((string) ($parameterGroupData['description'] ?? ''));
+            $template = $template->withParameterGroup(self::buildParameterGroup($name, $parameterGroupData));
+        }
 
-            foreach ($parameterGroupData['parameters'] ?? [] as $parameterName => $parameterData) {
-                $parameter = Parameter::named($parameterName)
-                    ->withDescription((string) ($parameterData['description'] ?? ''))
-                    ->withDefaultValue(DefaultValue::fromArray($parameterData['defaultValue'] ?? []));
+        if (\is_array($data['version'] ?? null)) {
+            $template->version = Version::fromArray($data['version']);
+        }
 
-                foreach ((array) ($parameterData['conditionalValues'] ?? []) as $key => $conditionalValueData) {
-                    $parameter = $parameter->withConditionalValue(new ConditionalValue($key, $conditionalValueData['value']));
-                }
+        return $template;
+    }
 
-                $group = $group->withParameter($parameter);
-            }
+    /**
+     * @param array<string, string> $data
+     */
+    private static function buildCondition(string $name, array $data): Condition
+    {
+        $condition = Condition::named($name)->withExpression($data['expression']);
 
-            $template->parameterGroups[$group->name()] = $group;
+        if ($tagColor = $data['tagColor'] ?? null) {
+            $condition = $condition->withTagColor(new TagColor($tagColor));
         }
 
-        if (\is_array($data['version'] ?? null)) {
-            try {
-                $template->version = Version::fromArray($data['version']);
-            } catch (Throwable $e) {
-                $template->version = null;
-            }
+        return $condition;
+    }
+
+    /**
+     * @param array<string, mixed> $data
+     */
+    private static function buildParameter(string $name, array $data): Parameter
+    {
+        $parameter = Parameter::named($name)
+            ->withDescription((string) ($data['description'] ?? ''))
+            ->withDefaultValue(DefaultValue::fromArray($data['defaultValue'] ?? []));
+
+        foreach ((array) ($data['conditionalValues'] ?? []) as $key => $conditionalValueData) {
+            $parameter = $parameter->withConditionalValue(new ConditionalValue($key, $conditionalValueData['value']));
         }
 
-        return $template;
+        return $parameter;
+    }
+
+    /**
+     * @param array<string, mixed> $parameterGroupData
+     */
+    private static function buildParameterGroup(string $name, array $parameterGroupData): ParameterGroup
+    {
+        $group = ParameterGroup::named($name)
+            ->withDescription((string) ($parameterGroupData['description'] ?? ''));
+
+        foreach ($parameterGroupData['parameters'] ?? [] as $parameterName => $parameterData) {
+            $group = $group->withParameter(self::buildParameter($parameterName, $parameterData));
+        }
+
+        return $group;
     }
 
     /**
@@ -101,6 +129,14 @@ public function etag(): string
         return $this->etag;
     }
 
+    /**
+     * @return Condition[]
+     */
+    public function conditions(): array
+    {
+        return $this->conditions;
+    }
+
     /**
      * @return Parameter[]
      */
@@ -165,7 +201,7 @@ private function assertThatAllConditionalValuesAreValid(Parameter $parameter): v
     public function jsonSerialize(): array
     {
         return [
-            'conditions' => \array_values($this->conditions),
+            'conditions' => !empty($this->conditions) ? \array_values($this->conditions) : null,
             'parameters' => !empty($this->parameters) ? $this->parameters : null,
             'parameterGroups' => !empty($this->parameterGroups) ? $this->parameterGroups : null,
         ];