From 1dd0ebb6b3535ec5246b13ca357a1eb2b8b3e5e2 Mon Sep 17 00:00:00 2001
From: Damodar Lohani <dlohani48@gmail.com>
Date: Mon, 20 Apr 2026 05:58:27 +0000
Subject: [PATCH] refactor: expose shape() as an iterative instance method

Per review: make the canonical-shape helper a public instance method on Query (`shape()`) rather than a private static inside `fingerprint()`, and replace the recursive walk with an iterative stack-based post-order traversal.

Added test covering leaf, logical, elemMatch, and 4-level-deep nested shapes to verify iterative equivalence with the previous recursive version.
---
 src/Query/Query.php       | 58 +++++++++++++++++++++++++++++++--------
 tests/Query/QueryTest.php | 31 +++++++++++++++++++++
 2 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/src/Query/Query.php b/src/Query/Query.php
index 624717c..49b3c7d 100644
--- a/src/Query/Query.php
+++ b/src/Query/Query.php
@@ -457,9 +457,9 @@ public static function parseQueries(array $queries): array
      * different parameter values produce the same fingerprint, which is
      * useful for pattern-based counting and slow-query grouping.
      *
-     * Logical queries (`and`, `or`, `elemMatch`) are recursively fingerprinted
-     * so their inner structure contributes to the hash — two `and(...)`
-     * queries with different child shapes produce different fingerprints.
+     * Logical queries (`and`, `or`, `elemMatch`) contribute their inner
+     * structure to the hash via `Query::shape()` — two `and(...)` queries
+     * with different child shapes produce different fingerprints.
      *
      * Accepts either raw query strings or parsed Query objects.
      *
@@ -481,7 +481,7 @@ public static function fingerprint(array $queries): string
                 throw new QueryException('Invalid query element for fingerprint: expected string or Query instance');
             }
 
-            $shapes[] = self::queryShape($query);
+            $shapes[] = $query->shape();
         }
 
         \sort($shapes);
@@ -490,26 +490,60 @@ public static function fingerprint(array $queries): string
     }
 
     /**
-     * Canonical shape string for a single Query — recursive for logical types.
+     * Canonical shape string for this Query — values excluded.
+     *
+     * Non-logical queries produce `method:attribute`. Logical queries
+     * (`and`, `or`, `elemMatch`) produce `method:attribute(child1|child2|…)`
+     * with children sorted so child order does not affect the shape.
+     *
+     * Implemented iteratively: walks the tree into a preorder list via a
+     * stack, then processes the reversed list so each node's children are
+     * always resolved before the node itself.
      */
-    private static function queryShape(self $query): string
+    public function shape(): string
     {
-        $method = $query->getMethod();
+        // 1. Preorder flatten the tree.
+        $nodes = [];
+        $stack = [$this];
+        while ($stack) {
+            /** @var self $node */
+            $node = \array_pop($stack);
+            $nodes[] = $node;
+
+            if (! \in_array($node->method, self::LOGICAL_TYPES, true)) {
+                continue;
+            }
+            foreach ($node->values as $child) {
+                if ($child instanceof self) {
+                    $stack[] = $child;
+                }
+            }
+        }
+
+        // 2. Process reversed so children are always shaped before parents.
+        $shapes = [];
+        foreach (\array_reverse($nodes) as $node) {
+            $id = \spl_object_id($node);
+
+            if (! \in_array($node->method, self::LOGICAL_TYPES, true)) {
+                $shapes[$id] = $node->method.':'.$node->attribute;
+
+                continue;
+            }
 
-        if (\in_array($method, self::LOGICAL_TYPES, true)) {
             $childShapes = [];
-            foreach ($query->getValues() as $child) {
+            foreach ($node->values as $child) {
                 if ($child instanceof self) {
-                    $childShapes[] = self::queryShape($child);
+                    $childShapes[] = $shapes[\spl_object_id($child)];
                 }
             }
             \sort($childShapes);
 
             // Attribute is empty for and/or; meaningful for elemMatch (the field being matched).
-            return $method.':'.$query->getAttribute().'('.\implode('|', $childShapes).')';
+            $shapes[$id] = $node->method.':'.$node->attribute.'('.\implode('|', $childShapes).')';
         }
 
-        return $method.':'.$query->getAttribute();
+        return $shapes[\spl_object_id($this)];
     }
 
     /**
diff --git a/tests/Query/QueryTest.php b/tests/Query/QueryTest.php
index a3820df..6ecfc38 100644
--- a/tests/Query/QueryTest.php
+++ b/tests/Query/QueryTest.php
@@ -214,4 +214,35 @@ public function testFingerprintRejectsInvalidElements(): void
         $this->expectException(\Utopia\Query\Exception::class);
         Query::fingerprint([42]);
     }
+
+    public function testShape(): void
+    {
+        // Leaf queries
+        $this->assertSame('equal:name', Query::equal('name', ['Alice'])->shape());
+        $this->assertSame('greaterThan:age', Query::greaterThan('age', 18)->shape());
+
+        // Logical with empty attribute
+        $and = new Query(Query::TYPE_AND, '', [Query::equal('name', ['Alice']), Query::greaterThan('age', 18)]);
+        $this->assertSame('and:(equal:name|greaterThan:age)', $and->shape());
+
+        // elemMatch preserves the attribute (the field being matched)
+        $elem = new Query(Query::TYPE_ELEM_MATCH, 'tags', [Query::equal('name', ['php'])]);
+        $this->assertSame('elemMatch:tags(equal:name)', $elem->shape());
+
+        // Deeply nested — iterative traversal must match recursive result
+        $deep = new Query(Query::TYPE_AND, '', [
+            new Query(Query::TYPE_OR, '', [
+                Query::equal('a', ['x']),
+                new Query(Query::TYPE_AND, '', [
+                    Query::equal('b', ['y']),
+                    Query::lessThan('c', 5),
+                ]),
+            ]),
+            Query::greaterThan('d', 10),
+        ]);
+        $this->assertSame(
+            'and:(greaterThan:d|or:(and:(equal:b|lessThan:c)|equal:a))',
+            $deep->shape(),
+        );
+    }
 }