From 521940d9880a063520b968c45b79c0292fdbe027 Mon Sep 17 00:00:00 2001
From: Brett McBride <brett.a.mcbride@gmail.com>
Date: Tue, 16 Jan 2024 08:33:18 +1100
Subject: [PATCH] optimize resource merging (#1214)

- do not revalidate/normalize resource attributes on merge, since they have already been validated/normalized
- string concat is about 10x faster than sprintf, so use it in ClassConstantAccessor since it's called many times
---
 src/SDK/Common/Attribute/AttributesBuilder.php    | 15 +++++++++++++++
 .../Attribute/AttributesBuilderInterface.php      |  1 +
 .../Attribute/FilteredAttributesBuilder.php       |  5 +++++
 src/SDK/Common/Util/ClassConstantAccessor.php     |  2 +-
 src/SDK/Resource/ResourceInfo.php                 |  4 ++--
 .../Unit/SDK/Common/Attribute/AttributesTest.php  | 13 +++++++++++++
 6 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/src/SDK/Common/Attribute/AttributesBuilder.php b/src/SDK/Common/Attribute/AttributesBuilder.php
index 5c1150638..ee855b34a 100644
--- a/src/SDK/Common/Attribute/AttributesBuilder.php
+++ b/src/SDK/Common/Attribute/AttributesBuilder.php
@@ -43,6 +43,21 @@ public function build(): AttributesInterface
         return new Attributes($this->attributes, $this->droppedAttributesCount);
     }
 
+    public function merge(AttributesInterface $old, AttributesInterface $updating): AttributesInterface
+    {
+        $new = $old->toArray();
+        $dropped = $old->getDroppedAttributesCount() + $updating->getDroppedAttributesCount();
+        foreach ($updating->toArray() as $key => $value) {
+            if (count($new) === $this->attributeCountLimit && !array_key_exists($key, $new)) {
+                $dropped++;
+            } else {
+                $new[$key] = $value;
+            }
+        }
+
+        return new Attributes($new, $dropped);
+    }
+
     public function offsetExists($offset): bool
     {
         return array_key_exists($offset, $this->attributes);
diff --git a/src/SDK/Common/Attribute/AttributesBuilderInterface.php b/src/SDK/Common/Attribute/AttributesBuilderInterface.php
index 7e3d64062..14f41190f 100644
--- a/src/SDK/Common/Attribute/AttributesBuilderInterface.php
+++ b/src/SDK/Common/Attribute/AttributesBuilderInterface.php
@@ -9,4 +9,5 @@
 interface AttributesBuilderInterface extends ArrayAccess
 {
     public function build(): AttributesInterface;
+    public function merge(AttributesInterface $old, AttributesInterface $updating): AttributesInterface;
 }
diff --git a/src/SDK/Common/Attribute/FilteredAttributesBuilder.php b/src/SDK/Common/Attribute/FilteredAttributesBuilder.php
index d79cff96a..28bb00905 100644
--- a/src/SDK/Common/Attribute/FilteredAttributesBuilder.php
+++ b/src/SDK/Common/Attribute/FilteredAttributesBuilder.php
@@ -37,6 +37,11 @@ public function build(): AttributesInterface
         return new Attributes($attributes->toArray(), $dropped);
     }
 
+    public function merge(AttributesInterface $old, AttributesInterface $updating): AttributesInterface
+    {
+        return $this->builder->merge($old, $updating);
+    }
+
     public function offsetExists($offset): bool
     {
         return $this->builder->offsetExists($offset);
diff --git a/src/SDK/Common/Util/ClassConstantAccessor.php b/src/SDK/Common/Util/ClassConstantAccessor.php
index 237e70ba5..b4ab1198e 100644
--- a/src/SDK/Common/Util/ClassConstantAccessor.php
+++ b/src/SDK/Common/Util/ClassConstantAccessor.php
@@ -30,6 +30,6 @@ public static function getValue(string $className, string $constantName)
 
     private static function getFullName(string $className, string $constantName): string
     {
-        return sprintf('%s::%s', $className, $constantName);
+        return $className . '::' . $constantName;
     }
 }
diff --git a/src/SDK/Resource/ResourceInfo.php b/src/SDK/Resource/ResourceInfo.php
index a5dc1eb18..da43e820e 100644
--- a/src/SDK/Resource/ResourceInfo.php
+++ b/src/SDK/Resource/ResourceInfo.php
@@ -67,9 +67,9 @@ public function serialize(): string
     public function merge(ResourceInfo $updating): ResourceInfo
     {
         $schemaUrl = self::mergeSchemaUrl($this->getSchemaUrl(), $updating->getSchemaUrl());
-        $attributes = $updating->getAttributes()->toArray() + $this->getAttributes()->toArray();
+        $attributes = Attributes::factory()->builder()->merge($this->getAttributes(), $updating->getAttributes());
 
-        return ResourceInfo::create(Attributes::create($attributes), $schemaUrl);
+        return ResourceInfo::create($attributes, $schemaUrl);
     }
 
     /**
diff --git a/tests/Unit/SDK/Common/Attribute/AttributesTest.php b/tests/Unit/SDK/Common/Attribute/AttributesTest.php
index 23bb72a97..727920749 100644
--- a/tests/Unit/SDK/Common/Attribute/AttributesTest.php
+++ b/tests/Unit/SDK/Common/Attribute/AttributesTest.php
@@ -169,4 +169,17 @@ public function test_replace_attribute_does_not_increase_dropped_attributes_coun
         $attributesBuilder['foo'] = 'bar';
         $this->assertEquals(0, $attributesBuilder->build()->getDroppedAttributesCount());
     }
+
+    public function test_merge_attributes(): void
+    {
+        $one = Attributes::factory(1)->builder(['foo' => 'foo', 'foobar' => 'foobar'])->build();
+        $two = Attributes::factory(2)->builder(['bar' => 'bar', 'baz' => 'baz'])->build();
+        $merged = Attributes::factory(2)->builder()->merge($one, $two);
+
+        $this->assertEquals([
+            'foo' => 'foo',
+            'bar' => 'bar',
+        ], $merged->toArray());
+        $this->assertSame(2, $merged->getDroppedAttributesCount(), 'foobar and baz dropped');
+    }
 }