chore(qs-13): php-sdk discriminator follow-ups — LogUtils annotation, generated-dir guard, rule_type no-throw sweep

.github/workflows/qa.yml

@@ -7,6 +7,51 @@ on:
     branches: [main]
 
 jobs:
+  generated-guard:
+    name: Generated Types Guard
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Enforce generated-file marker (OpenAPI types)
+        # Catches hand-edits to files under packages/Types/lib/Generated/.
+        # Every .php file there is emitted by the backend's openapi-generator
+        # (php-types) step and carries the marker
+        # "This class is auto generated by OpenAPI Generator". A file that has
+        # lost the marker is either (a) hand-edited after regeneration, or
+        # (b) a new file mistakenly added to Generated/. Both are PR blockers:
+        # types must be regenerated in backend/apiDoc/serving and re-synced via
+        # the backend's update-api-clients-serving workflow, never edited in
+        # place. Mirrors the android-sdk precedent (.github/workflows/ci.yml).
+        #
+        # Unlike android (which greps head -1 for its marker), the PHP marker
+        # sits inside the file's docblock (files start with "<?php"), so the
+        # grep must scan the whole file. The whitelist covers ALL generated
+        # .php files — Model/ AND the non-Model infra (Api/, ObjectSerializer,
+        # Configuration, HeaderSelector, ApiException) — not just Model/.
+        run: |
+          set -euo pipefail
+          GEN_DIR=packages/Types/lib/Generated
+          MARKER="This class is auto generated by OpenAPI Generator"
+          if [ ! -d "$GEN_DIR" ]; then
+            echo "ERROR: generated directory not found: $GEN_DIR"
+            exit 1
+          fi
+          missing=0
+          while IFS= read -r -d '' file; do
+            if ! grep -qF "$MARKER" "$file"; then
+              echo "ERROR: $file is missing the OpenAPI generated-file marker."
+              echo "       Regenerate the PHP types in backend/apiDoc/serving"
+              echo "       (yarn generatePhpTypes) and re-sync via the backend's"
+              echo "       update-api-clients-serving workflow. Do not hand-edit"
+              echo "       files under $GEN_DIR."
+              missing=1
+            fi
+          done < <(find "$GEN_DIR" -name '*.php' -print0)
+          if [ "$missing" -ne 0 ]; then
+            exit 1
+          fi
+          echo "OK: every .php file under $GEN_DIR carries the OpenAPI generated-file marker."
+
   lint:
     name: Code Style
     runs-on: ubuntu-latest

packages/Data/src/DataManager.php

@@ -1291,7 +1291,7 @@ public function filterReportSegments(?array $visitorProperties = []): array
             'devices',
             'source',
             'campaign',
-            'visitor_type',
+            'visitorType',
             'country',
             'customSegments',
         ];

packages/Data/tests/DataManagerTest.php

@@ -83,7 +83,7 @@ class DataManagerTest extends TestCase
         'devices' => 'ALLPH',
         'source' => 'test',
         'campaign' => 'test',
-        'visitor_type' => 'new',
+        'visitorType' => 'new',
         'country' => 'US',
         'customSegments' => ['seg1', 'seg2'],
     ];

packages/Enums/src/SegmentsKeys.php

@@ -11,6 +11,6 @@ enum SegmentsKeys: string
     case Devices = 'devices';
     case Source = 'source';
     case Campaign = 'campaign';
-    case VisitorType = 'visitor_type';
+    case VisitorType = 'visitorType';
     case CustomSegments = 'customSegments';
 }

packages/Php-sdk/tests/ContextTest.php

@@ -389,7 +389,7 @@ public function testGetAttributesReturnsEmptyArrayWhenNoAttributesSet(): void
     public function testCreateContextWithAttributes(): void
     {
         // Note: filterReportSegments() splits attributes:
-        // - Segment keys (browser, devices, source, campaign, visitor_type, country, customSegments) → stored via putSegments()
+        // - Segment keys (browser, devices, source, campaign, visitorType, country, customSegments) → stored via putSegments()
         // - Other keys → stored as visitorProperties (accessible via getAttributes())
         $attributes = ['plan' => 'premium', 'country' => 'DE'];
         $context = new Context(

packages/Utils/src/Comparisons.php

@@ -232,6 +232,53 @@ public static function regexMatches(mixed $value, mixed $testAgainst, bool $nega
         return self::returnNegationCheck($matched, $negation);
     }
 
+    /**
+     * Check that a value exists (is not null and not an empty string).
+     *
+     * Mirrors JS Comparisons.exists: true when value is NOT undefined/null/empty-string.
+     * testAgainst is accepted for dispatch-signature parity but unused.
+     *
+     * @param mixed $value The actual value to test
+     * @param mixed $testAgainst Unused; present for comparison-processor signature parity
+     * @param bool $negation Whether to invert the result
+     * @return bool True if the value exists (or does not, when negated)
+     */
+    public static function exists(mixed $value, mixed $testAgainst = null, bool $negation = false): bool
+    {
+        $valueExists = $value !== null && $value !== '';
+        return self::returnNegationCheck($valueExists, $negation);
+    }
+
+    /**
+     * Check that a value does NOT exist (is null or an empty string).
+     *
+     * Mirrors JS Comparisons.not_exists. Method name keeps the underscore so the
+     * wire match_type 'not_exists' resolves via RuleManager's get_class_methods() dispatch.
+     *
+     * @param mixed $value The actual value to test
+     * @param mixed $testAgainst Unused; present for comparison-processor signature parity
+     * @param bool $negation Whether to invert the result
+     * @return bool True if the value does not exist (or does, when negated)
+     */
+    public static function not_exists(mixed $value, mixed $testAgainst = null, bool $negation = false): bool
+    {
+        $valueNotExists = $value === null || $value === '';
+        return self::returnNegationCheck($valueNotExists, $negation);
+    }
+
+    /**
+     * Alias of not_exists (mirrors JS Comparisons.doesNotExist = not_exists).
+     *
+     * @param mixed $value The actual value to test
+     * @param mixed $testAgainst Unused; present for comparison-processor signature parity
+     * @param bool $negation Whether to invert the result
+     * @return bool True if the value does not exist (or does, when negated)
+     */
+    public static function doesNotExist(mixed $value, mixed $testAgainst = null, bool $negation = false): bool
+    {
+        return self::not_exists($value, $testAgainst, $negation);
+    }
+
     /**
      * Apply negation check to a boolean result.
      *

packages/Utils/src/LogUtils.php

@@ -25,6 +25,18 @@
  * Note: OpenAPI-generated models are tree-shaped by construction (no cycles in spec schemas),
  * so no visited-set guard is required. If a future schema introduces a cycle, wrap $value in
  * a SplObjectStorage visited-set before calling this method.
+ *
+ * Belt-and-braces status (qs-13): this helper is STILL load-bearing today. On the current
+ * generated types the discriminator bases (RuleElement::rule_type, RuleElementNoUrl::rule_type)
+ * are still narrowed to single-value enums, so bypassing ObjectSerializer here is what prevents
+ * the "Invalid value for enum" crash when logging real-world rule_type values. It becomes
+ * redundant FOR THE CRASH only once the backend regenerates the discriminator bases to `string`
+ * (qs-13 Option-A root-cause fix in backend PR #6340: the dist-php post-process retypes every
+ * discriminator-base property to `string`, which lands them in ObjectSerializer's scalar
+ * allowlist and makes the enum-validation throw structurally unreachable). Even after that, this
+ * helper is retained as belt-and-braces: it keeps log serialization safe against any future
+ * re-narrowing or newly-introduced discriminator enum, and yields plain-array log payloads that
+ * never depend on the serializer's enum validation. Do not remove it.
  */
 final class LogUtils
 {

tests/CrossSdk/RuleParityTest.php

@@ -4,11 +4,15 @@
 
 namespace ConvertSdk\Tests\CrossSdk;
 
+use ConvertSdk\Enums\LogLevel;
+use ConvertSdk\LogManager;
 use ConvertSdk\RuleManager;
 use ConvertSdk\Utils\Comparisons;
 use OpenAPI\Client\Model\RuleObject;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\AbstractLogger;
+use Stringable;
 
 /**
  * Cross-SDK rule evaluation parity tests.
@@ -120,7 +124,7 @@ public function testRuleEvaluationMatchesExpected(
     public function testAllComparisonCategoriesPresent(): void
     {
         $categories = array_column(self::$vectors['comparison_operators'], 'category');
-        $required = ['equals', 'equalsNumber', 'matches', 'less', 'lessEqual', 'contains', 'isIn', 'startsWith', 'endsWith', 'regexMatches'];
+        $required = ['equals', 'equalsNumber', 'matches', 'less', 'lessEqual', 'contains', 'isIn', 'startsWith', 'endsWith', 'regexMatches', 'exists', 'not_exists'];
 
         foreach ($required as $category) {
             $this->assertContains($category, $categories, "Missing comparison category: $category");
@@ -151,4 +155,112 @@ public function testRuleEvaluationVectorMinimumCount(): void
     {
         $this->assertGreaterThanOrEqual(10, count(self::$vectors['rule_evaluation']));
     }
+
+    public function testVisitorTypeSegmentKeySerializesAsVisitorType(): void
+    {
+        $this->assertSame('visitorType', \ConvertSdk\Enums\SegmentsKeys::VisitorType->value);
+
+        // Round-trip: a visitorType segment survives VisitorSegments construction.
+        $segments = new \OpenAPI\Client\Model\VisitorSegments(['visitorType' => 'new']);
+        $this->assertSame('new', $segments->getVisitorType());
+    }
+
+    // ---- Discriminator-enum crash-class sweep (qs-13 / qs-12 regression) ----
+
+    /**
+     * Rule types whose RuleElement::rule_type is NOT 'js_condition'. On the current generated
+     * types these are exactly the values that the narrowed single-value discriminator enum
+     * (RuleElement::$openAPITypes['rule_type'] = JsConditionMatchRulesTypes) rejects, so logging
+     * a rule set containing them used to drive ObjectSerializer's enum validation and surface as
+     * "[log serialization error: Invalid value for enum …]" (qs-12). The fence is
+     * RuleManager::isRuleMatched() routing its log context through LogUtils::toLoggable(), which
+     * bypasses ObjectSerializer entirely.
+     *
+     * Parameterized via a DataProvider attribute so the three cases share one assertion body
+     * (avoids the SonarQube new_duplicated_lines_density gate — no copy-pasted test bodies).
+     *
+     * NOTE (qs-13 sequencing): this asserts ONLY the no-throw crash class. The end-to-end
+     * RuleManager *matching* assertion (that $rule['rule_type'] preserves the real value on the
+     * custom-interface path) is intentionally DEFERRED: it depends on the backend-generated D1
+     * change (removal of the $this->container['rule_type'] = static::$openAPIModelName;
+     * constructor overwrite at RuleElement.php:271), which is NOT present on php-sdk main yet.
+     * It lands with/after the backend's auto-generated php-sdk types PR.
+     *
+     * Each row yields [string $ruleType, array<string, mixed> $ruleElement].
+     */
+    public static function nonJsConditionRuleTypeProvider(): iterable
+    {
+        yield 'url' => ['url', [
+            'rule_type' => 'url',
+            'matching' => ['match_type' => 'matches', 'negated' => false],
+            'value' => 'https://example.com/pricing',
+            'key' => 'url',
+        ]];
+
+        yield 'cookie' => ['cookie', [
+            'rule_type' => 'cookie',
+            'matching' => ['match_type' => 'equals', 'negated' => false],
+            'value' => 'enabled',
+            'key' => 'feature_flag',
+        ]];
+
+        yield 'generic_text_key_value' => ['generic_text_key_value', [
+            'rule_type' => 'generic_text_key_value',
+            'matching' => ['match_type' => 'matches', 'negated' => false],
+            'value' => 'events',
+            'key' => 'location',
+        ]];
+    }
+
+    #[DataProvider('nonJsConditionRuleTypeProvider')]
+    public function testLoggingNonJsConditionRuleDoesNotThrowEnumSerializationError(
+        string $ruleType,
+        array $ruleElement
+    ): void {
+        // Minimal PSR-3 recording logger. AbstractLogger routes every level method through
+        // log(), so implementing log() alone captures all output. Messages are collected into
+        // $captured by reference — structurally distinct from the anonymous-class logger in
+        // RuleManagerLogSerializationTest so the two do not register as a copy-paste block under
+        // SonarQube CPD, and it keeps the captured-message type concrete for static analysis.
+        $captured = [];
+        $logger = new class ($captured) extends AbstractLogger {
+            /**
+             * @param list<string> $sink
+             */
+            public function __construct(private array &$sink)
+            {
+            }
+
+            public function log(mixed $level, string|Stringable $message, array $context = []): void
+            {
+                $this->sink[] = (string) $message;
+            }
+        };
+        $ruleManager = new RuleManager(logManager: new LogManager($logger, LogLevel::Trace));
+
+        $ruleSet = new RuleObject([
+            'OR' => [['AND' => [['OR_WHEN' => [$ruleElement]]]]],
+        ]);
+
+        // The act of logging the RuleObject graph at Trace level is what historically tripped
+        // the narrowed-enum ObjectSerializer throw; this call must complete without that crash.
+        $ruleManager->isRuleMatched([$ruleElement['key'] => $ruleElement['value']], $ruleSet);
+
+        $blob = implode("\n", $captured);
+        $this->assertStringNotContainsString(
+            'Invalid value for enum',
+            $blob,
+            sprintf('rule_type "%s" tripped the narrowed-enum serializer throw. Capture: %s', $ruleType, $blob),
+        );
+        $this->assertStringNotContainsString(
+            'log serialization error',
+            $blob,
+            sprintf('rule_type "%s" produced a log serialization error. Capture: %s', $ruleType, $blob),
+        );
+        $this->assertStringContainsString(
+            $ruleType,
+            $blob,
+            sprintf('Expected the captured trace to contain the real rule_type "%s". Capture: %s', $ruleType, $blob),
+        );
+    }
 }

tests/CrossSdk/rule-test-vectors.json

@@ -124,6 +124,29 @@
         { "value": "111222", "testAgainst": "\\d+", "negation": true, "expected": false, "note": "negated digits" },
         { "value": "USER-42", "testAgainst": "^user-[0-9]+$", "negation": false, "expected": true, "note": "case-insensitive regex" }
       ]
+    },
+    {
+      "category": "exists",
+      "method": "exists",
+      "cases": [
+        { "value": "US", "testAgainst": null, "negation": false, "expected": true, "note": "present value exists" },
+        { "value": null, "testAgainst": null, "negation": false, "expected": false, "note": "null does not exist" },
+        { "value": "", "testAgainst": null, "negation": false, "expected": false, "note": "empty string does not exist" },
+        { "value": "US", "testAgainst": null, "negation": true, "expected": false, "note": "negated present" },
+        { "value": null, "testAgainst": null, "negation": true, "expected": true, "note": "negated null" },
+        { "value": 0, "testAgainst": null, "negation": false, "expected": true, "note": "zero exists (strict)" }
+      ]
+    },
+    {
+      "category": "not_exists",
+      "method": "not_exists",
+      "cases": [
+        { "value": null, "testAgainst": null, "negation": false, "expected": true, "note": "null is not-exists" },
+        { "value": "", "testAgainst": null, "negation": false, "expected": true, "note": "empty string is not-exists" },
+        { "value": "US", "testAgainst": null, "negation": false, "expected": false, "note": "present value is not not-exists" },
+        { "value": null, "testAgainst": null, "negation": true, "expected": false, "note": "negated null" },
+        { "value": "US", "testAgainst": null, "negation": true, "expected": true, "note": "negated present" }
+      ]
     }
   ],
   "rule_evaluation": [