From 3f91272dd52442e280b2eee89e0bc65a56728b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 12:58:18 +0100 Subject: [PATCH 1/8] Fix double metdata when migrating --- composer.lock | 12 +++--- src/Database/Database.php | 15 ++++++- tests/e2e/Adapter/Scopes/AttributeTests.php | 46 +++++++++++++++++++++ 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/composer.lock b/composer.lock index c3b96bfde..b841ff464 100644 --- a/composer.lock +++ b/composer.lock @@ -539,16 +539,16 @@ }, { "name": "open-telemetry/exporter-otlp", - "version": "1.3.3", + "version": "1.3.4", "source": { "type": "git", "url": "https://github.com/opentelemetry-php/exporter-otlp.git", - "reference": "07b02bc71838463f6edcc78d3485c04b48fb263d" + "reference": "62e680d587beb42e5247aa6ecd89ad1ca406e8ca" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/opentelemetry-php/exporter-otlp/zipball/07b02bc71838463f6edcc78d3485c04b48fb263d", - "reference": "07b02bc71838463f6edcc78d3485c04b48fb263d", + "url": "https://api.github.com/repos/opentelemetry-php/exporter-otlp/zipball/62e680d587beb42e5247aa6ecd89ad1ca406e8ca", + "reference": "62e680d587beb42e5247aa6ecd89ad1ca406e8ca", "shasum": "" }, "require": { @@ -599,7 +599,7 @@ "issues": "https://github.com/open-telemetry/opentelemetry-php/issues", "source": "https://github.com/open-telemetry/opentelemetry-php" }, - "time": "2025-11-13T08:04:37+00:00" + "time": "2026-01-15T09:31:34+00:00" }, { "name": "open-telemetry/gen-otlp-protobuf", @@ -4520,7 +4520,7 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": {}, "prefer-stable": false, "prefer-lowest": false, "platform": { diff --git a/src/Database/Database.php b/src/Database/Database.php index a715fd56b..105a27bee 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2047,7 +2047,11 @@ public function createAttribute(string $collection, string $id, string $type, in } } - $collection->setAttribute('attributes', $attribute, Document::SET_TYPE_APPEND); + // Append attribute, removing any duplicates + $attributes = $collection->getAttribute('attributes', []); + $attributes = \array_filter($attributes, fn ($attr) => $attr['$id'] !== $id); + $attributes[] = $attribute; + $collection->setAttribute('attributes', \array_values($attributes)); $this->updateMetadata( collection: $collection, @@ -2169,10 +2173,17 @@ public function createAttributes(string $collection, array $attributes): bool } } + // Append attribute, removing any duplicates + $attributes = $collection->getAttribute('attributes', []); + $newAttributeIds = \array_map(fn ($attr) => $attr['$id'], $attributeDocuments); + $attributes = \array_filter($attributes, fn ($attr) => !\in_array($attr['$id'], $newAttributeIds)); + foreach ($attributeDocuments as $attributeDocument) { - $collection->setAttribute('attributes', $attributeDocument, Document::SET_TYPE_APPEND); + $attributes[] = $attributeDocument; } + $collection->setAttribute('attributes', \array_values($attributes)); + $this->updateMetadata( collection: $collection, rollbackOperation: fn () => $this->cleanupAttributes($collection->getId(), $attributeDocuments), diff --git a/tests/e2e/Adapter/Scopes/AttributeTests.php b/tests/e2e/Adapter/Scopes/AttributeTests.php index 9f1a4b31f..7420efb6d 100644 --- a/tests/e2e/Adapter/Scopes/AttributeTests.php +++ b/tests/e2e/Adapter/Scopes/AttributeTests.php @@ -2195,4 +2195,50 @@ public function testCreateAttributesDelete(): void $this->assertCount(1, $attrs); $this->assertEquals('b', $attrs[0]['$id']); } + + public function testCreateAttributeWhileMigrating(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + // Skip test if adapter doesn't support shared tables + if (!$database->getAdapter()->getSharedTables()) { + $this->expectNotToPerformAssertions(); + return; + } + + // Prepare collection + $database->createCollection('migration_test'); + $database->setMigrating(true); + + // First creation, as usual + $this->assertTrue($database->createAttribute('migration_test', 'status', Database::VAR_STRING, 128, true)); + + $collection = $database->getCollection('migration_test'); + $attributes = $collection->getAttribute('attributes'); + $this->assertCount(1, $attributes); + $this->assertEquals('status', $attributes[0]['$id']); + + // Second creation, no exceptions, no duplicates + $result = $database->createAttribute('migration_test', 'status', Database::VAR_STRING, 128, true); + $this->assertTrue($result); + + $collection = $database->getCollection('migration_test'); + $attributes = $collection->getAttribute('attributes'); + $this->assertCount(1, $attributes); + $this->assertEquals('status', $attributes[0]['$id']); + + // Third creation, same as second, once more, just in case + $result = $database->createAttribute('migration_test', 'status', Database::VAR_STRING, 128, true); + $this->assertTrue($result); + + $collection = $database->getCollection('migration_test'); + $attributes = $collection->getAttribute('attributes'); + $this->assertCount(1, $attributes); + $this->assertEquals('status', $attributes[0]['$id']); + + // Cleanup + $database->setMigrating(false); + $database->deleteCollection('migration_test'); + } } From 7ece8a838e220f975b08299f9ead7e29304b3281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 13:08:09 +0100 Subject: [PATCH 2/8] AI review fixes --- src/Database/Database.php | 16 +-- tests/e2e/Adapter/Scopes/AttributeTests.php | 110 ++++++++++++++++++-- 2 files changed, 112 insertions(+), 14 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 105a27bee..837c4d37a 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2048,10 +2048,10 @@ public function createAttribute(string $collection, string $id, string $type, in } // Append attribute, removing any duplicates - $attributes = $collection->getAttribute('attributes', []); - $attributes = \array_filter($attributes, fn ($attr) => $attr['$id'] !== $id); - $attributes[] = $attribute; - $collection->setAttribute('attributes', \array_values($attributes)); + $metadataAttributes = $collection->getAttribute('attributes', []); + $metadataAttributes = \array_filter($metadataAttributes, fn ($attribute) => $attribute['$id'] !== $id); + $metadataAttributes[] = $attribute; + $collection->setAttribute('attributes', \array_values($metadataAttributes)); $this->updateMetadata( collection: $collection, @@ -2174,15 +2174,15 @@ public function createAttributes(string $collection, array $attributes): bool } // Append attribute, removing any duplicates - $attributes = $collection->getAttribute('attributes', []); $newAttributeIds = \array_map(fn ($attr) => $attr['$id'], $attributeDocuments); - $attributes = \array_filter($attributes, fn ($attr) => !\in_array($attr['$id'], $newAttributeIds)); + $metadataAttributes = $collection->getAttribute('attributes', []); + $metadataAttributes = \array_filter($metadataAttributes, fn ($attr) => !\in_array($attr['$id'], $newAttributeIds)); foreach ($attributeDocuments as $attributeDocument) { - $attributes[] = $attributeDocument; + $metadataAttributes[] = $attributeDocument; } - $collection->setAttribute('attributes', \array_values($attributes)); + $collection->setAttribute('attributes', \array_values($metadataAttributes)); $this->updateMetadata( collection: $collection, diff --git a/tests/e2e/Adapter/Scopes/AttributeTests.php b/tests/e2e/Adapter/Scopes/AttributeTests.php index 7420efb6d..1e71fb435 100644 --- a/tests/e2e/Adapter/Scopes/AttributeTests.php +++ b/tests/e2e/Adapter/Scopes/AttributeTests.php @@ -2212,33 +2212,131 @@ public function testCreateAttributeWhileMigrating(): void $database->setMigrating(true); // First creation, as usual - $this->assertTrue($database->createAttribute('migration_test', 'status', Database::VAR_STRING, 128, true)); + $this->assertTrue($database->createAttribute('migration_test', 'statusColumn', Database::VAR_STRING, 128, true)); $collection = $database->getCollection('migration_test'); $attributes = $collection->getAttribute('attributes'); $this->assertCount(1, $attributes); - $this->assertEquals('status', $attributes[0]['$id']); + $this->assertSame('statusColumn', $attributes[0]['$id']); // Second creation, no exceptions, no duplicates - $result = $database->createAttribute('migration_test', 'status', Database::VAR_STRING, 128, true); + $result = $database->createAttribute('migration_test', 'statusColumn', Database::VAR_STRING, 128, true); $this->assertTrue($result); $collection = $database->getCollection('migration_test'); $attributes = $collection->getAttribute('attributes'); $this->assertCount(1, $attributes); - $this->assertEquals('status', $attributes[0]['$id']); + $this->assertSame('statusColumn', $attributes[0]['$id']); // Third creation, same as second, once more, just in case - $result = $database->createAttribute('migration_test', 'status', Database::VAR_STRING, 128, true); + $result = $database->createAttribute('migration_test', 'statusColumn', Database::VAR_STRING, 128, true); $this->assertTrue($result); $collection = $database->getCollection('migration_test'); $attributes = $collection->getAttribute('attributes'); $this->assertCount(1, $attributes); - $this->assertEquals('status', $attributes[0]['$id']); + $this->assertSame('statusColumn', $attributes[0]['$id']); // Cleanup $database->setMigrating(false); $database->deleteCollection('migration_test'); } + + public function testCreateAttributesWhileMigrating(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + // Skip test if adapter doesn't support shared tables + if (!$database->getAdapter()->getSharedTables()) { + $this->expectNotToPerformAssertions(); + return; + } + + // Skip test if adapter doesn't support batch create attributes + if (!$database->getAdapter()->getSupportForBatchCreateAttributes()) { + $this->expectNotToPerformAssertions(); + return; + } + + // Prepare collection + $database->createCollection('migration_test_batch'); + $database->setMigrating(true); + + // First creation, as usual + $attributes = [ + [ + '$id' => 'statusColumn', + 'type' => Database::VAR_STRING, + 'size' => 128, + 'required' => true + ], + [ + '$id' => 'count', + 'type' => Database::VAR_INTEGER, + 'size' => 0, + 'required' => false + ], + ]; + + $result = $database->createAttributes('migration_test_batch', $attributes); + $this->assertTrue($result); + + $collection = $database->getCollection('migration_test_batch'); + $attrs = $collection->getAttribute('attributes'); + $this->assertCount(2, $attrs); + $this->assertSame('statusColumn', $attrs[0]['$id']); + $this->assertSame('count', $attrs[1]['$id']); + + // Second creation, no exceptions, no duplicates + $result = $database->createAttributes('migration_test_batch', $attributes); + $this->assertTrue($result); + + $collection = $database->getCollection('migration_test_batch'); + $attrs = $collection->getAttribute('attributes'); + $this->assertCount(2, $attrs); + $this->assertSame('statusColumn', $attrs[0]['$id']); + $this->assertSame('count', $attrs[1]['$id']); + + // Third creation, same as second, once more, just in case + $result = $database->createAttributes('migration_test_batch', $attributes); + $this->assertTrue($result); + + $collection = $database->getCollection('migration_test_batch'); + $attrs = $collection->getAttribute('attributes'); + $this->assertCount(2, $attrs); + $this->assertSame('statusColumn', $attrs[0]['$id']); + $this->assertSame('count', $attrs[1]['$id']); + + // Test partial overlap - create one new attribute and one existing + $mixedAttributes = [ + [ + '$id' => 'status', // existing + 'type' => Database::VAR_STRING, + 'size' => 128, + 'required' => true + ], + [ + '$id' => 'active', // new + 'type' => Database::VAR_BOOLEAN, + 'size' => 0, + 'required' => false + ], + ]; + + $result = $database->createAttributes('migration_test_batch', $mixedAttributes); + $this->assertTrue($result); + + // Ensure all attributes exist + $collection = $database->getCollection('migration_test_batch'); + $attrs = $collection->getAttribute('attributes'); + $this->assertCount(3, $attrs); + $this->assertSame('statusColumn', $attrs[0]['$id']); + $this->assertSame('count', $attrs[1]['$id']); + $this->assertSame('active', $attrs[2]['$id']); + + // Cleanup + $database->setMigrating(false); + $database->deleteCollection('migration_test_batch'); + } } From 392336706503f939c8db9f029ef095f1973fcd19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 13:30:39 +0100 Subject: [PATCH 3/8] Fix bug --- src/Database/Database.php | 66 ++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 837c4d37a..fd071c3a4 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2018,19 +2018,26 @@ public function createAttribute(string $collection, string $id, string $type, in $filters = array_unique($filters); } - $attribute = $this->validateAttribute( - $collection, - $id, - $type, - $size, - $required, - $default, - $signed, - $array, - $format, - $formatOptions, - $filters - ); + try { + $attribute = $this->validateAttribute( + $collection, + $id, + $type, + $size, + $required, + $default, + $signed, + $array, + $format, + $formatOptions, + $filters + ); + } catch (DuplicateException $e) { + // HACK: Metadata should still be updated, can be removed when null tenant collections are supported. + if (!$this->adapter->getSharedTables() || !$this->isMigrating()) { + throw $e; + } + } $created = false; @@ -2140,19 +2147,26 @@ public function createAttributes(string $collection, array $attributes): bool $attribute['filters'] = []; } - $attributeDocument = $this->validateAttribute( - $collection, - $attribute['$id'], - $attribute['type'], - $attribute['size'], - $attribute['required'], - $attribute['default'], - $attribute['signed'], - $attribute['array'], - $attribute['format'], - $attribute['formatOptions'], - $attribute['filters'] - ); + try { + $attributeDocument = $this->validateAttribute( + $collection, + $attribute['$id'], + $attribute['type'], + $attribute['size'], + $attribute['required'], + $attribute['default'], + $attribute['signed'], + $attribute['array'], + $attribute['format'], + $attribute['formatOptions'], + $attribute['filters'] + ); + } catch (DuplicateException $e) { + // HACK: Metadata should still be updated, can be removed when null tenant collections are supported. + if (!$this->adapter->getSharedTables() || !$this->isMigrating()) { + throw $e; + } + } $attributeDocuments[] = $attributeDocument; } From 64c760cfec8485aee198ef2e38a16604e8e8c806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 13:37:12 +0100 Subject: [PATCH 4/8] Add looseness check --- src/Database/Database.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index fd071c3a4..846bdf55d 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2259,10 +2259,13 @@ private function validateAttribute( ): Document { // Attribute IDs are case-insensitive $attributes = $collection->getAttribute('attributes', []); + + // Loosen verification during migration + $isLoose = $this->adapter->getSharedTables() && $this->isMigrating(); /** @var array $attributes */ foreach ($attributes as $attribute) { - if (\strtolower($attribute->getId()) === \strtolower($id)) { + if (!$isLoose && \strtolower($attribute->getId()) === \strtolower($id)) { throw new DuplicateException('Attribute already exists in metadata'); } } @@ -2271,7 +2274,7 @@ private function validateAttribute( $schema = $this->getSchemaAttributes($collection->getId()); foreach ($schema as $attribute) { $newId = $this->adapter->filter($attribute->getId()); - if (\strtolower($newId) === \strtolower($id)) { + if (!$isLoose && \strtolower($newId) === \strtolower($id)) { throw new DuplicateException('Attribute already exists in schema'); } } From 72598ce4f4a9988b62e79eca7c18894e70d3de1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 13:37:17 +0100 Subject: [PATCH 5/8] Revert "Fix bug" This reverts commit 392336706503f939c8db9f029ef095f1973fcd19. --- src/Database/Database.php | 66 +++++++++++++++------------------------ 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 846bdf55d..ea392b9dd 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2018,26 +2018,19 @@ public function createAttribute(string $collection, string $id, string $type, in $filters = array_unique($filters); } - try { - $attribute = $this->validateAttribute( - $collection, - $id, - $type, - $size, - $required, - $default, - $signed, - $array, - $format, - $formatOptions, - $filters - ); - } catch (DuplicateException $e) { - // HACK: Metadata should still be updated, can be removed when null tenant collections are supported. - if (!$this->adapter->getSharedTables() || !$this->isMigrating()) { - throw $e; - } - } + $attribute = $this->validateAttribute( + $collection, + $id, + $type, + $size, + $required, + $default, + $signed, + $array, + $format, + $formatOptions, + $filters + ); $created = false; @@ -2147,26 +2140,19 @@ public function createAttributes(string $collection, array $attributes): bool $attribute['filters'] = []; } - try { - $attributeDocument = $this->validateAttribute( - $collection, - $attribute['$id'], - $attribute['type'], - $attribute['size'], - $attribute['required'], - $attribute['default'], - $attribute['signed'], - $attribute['array'], - $attribute['format'], - $attribute['formatOptions'], - $attribute['filters'] - ); - } catch (DuplicateException $e) { - // HACK: Metadata should still be updated, can be removed when null tenant collections are supported. - if (!$this->adapter->getSharedTables() || !$this->isMigrating()) { - throw $e; - } - } + $attributeDocument = $this->validateAttribute( + $collection, + $attribute['$id'], + $attribute['type'], + $attribute['size'], + $attribute['required'], + $attribute['default'], + $attribute['signed'], + $attribute['array'], + $attribute['format'], + $attribute['formatOptions'], + $attribute['filters'] + ); $attributeDocuments[] = $attributeDocument; } From af14b29b580e428fbe80910987a367e43f0d53d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 13:37:31 +0100 Subject: [PATCH 6/8] Update Database.php --- src/Database/Database.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index ea392b9dd..a10d20fcb 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -2245,9 +2245,9 @@ private function validateAttribute( ): Document { // Attribute IDs are case-insensitive $attributes = $collection->getAttribute('attributes', []); - + // Loosen verification during migration - $isLoose = $this->adapter->getSharedTables() && $this->isMigrating(); + $isLoose = $this->adapter->getSharedTables() && $this->isMigrating(); /** @var array $attributes */ foreach ($attributes as $attribute) { From b085f706b0840f0abd7e2723b7a230700b1d6e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 13:41:22 +0100 Subject: [PATCH 7/8] fix leftover --- tests/e2e/Adapter/Scopes/AttributeTests.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Adapter/Scopes/AttributeTests.php b/tests/e2e/Adapter/Scopes/AttributeTests.php index 1e71fb435..496c54ab9 100644 --- a/tests/e2e/Adapter/Scopes/AttributeTests.php +++ b/tests/e2e/Adapter/Scopes/AttributeTests.php @@ -2311,7 +2311,7 @@ public function testCreateAttributesWhileMigrating(): void // Test partial overlap - create one new attribute and one existing $mixedAttributes = [ [ - '$id' => 'status', // existing + '$id' => 'statusColumn', // existing 'type' => Database::VAR_STRING, 'size' => 128, 'required' => true From 64bd5f391ca8b79d4a78d4cb736543c907f80f50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 15 Jan 2026 14:23:35 +0100 Subject: [PATCH 8/8] Loosen assertions --- tests/e2e/Adapter/Scopes/AttributeTests.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/e2e/Adapter/Scopes/AttributeTests.php b/tests/e2e/Adapter/Scopes/AttributeTests.php index 496c54ab9..af091e383 100644 --- a/tests/e2e/Adapter/Scopes/AttributeTests.php +++ b/tests/e2e/Adapter/Scopes/AttributeTests.php @@ -2331,9 +2331,17 @@ public function testCreateAttributesWhileMigrating(): void $collection = $database->getCollection('migration_test_batch'); $attrs = $collection->getAttribute('attributes'); $this->assertCount(3, $attrs); - $this->assertSame('statusColumn', $attrs[0]['$id']); - $this->assertSame('count', $attrs[1]['$id']); - $this->assertSame('active', $attrs[2]['$id']); + + // TODO: Eventuelly rewrite to assertSame for each item; currently their order differs between providers + $attributes = [ + $attrs[0]['$id'], + $attrs[1]['$id'], + $attrs[2]['$id'] + ]; + + $this->assertContains('statusColumn', $attributes); + $this->assertContains('count', $attributes); + $this->assertContains('active', $attributes); // Cleanup $database->setMigrating(false);