From bcedd9d567c7934ff6228613ef6309e3aa3f5351 Mon Sep 17 00:00:00 2001 From: Sebastiaan Stok Date: Fri, 13 Mar 2026 19:49:36 +0100 Subject: [PATCH] [Core] Fix GenericFieldSetBuilder::set() unresolve Setting an explicit field for the builder didn't remove the unresolved field if one existed. Because unresolved fields are checked first this would have used the unresolved field instead of the explicit one --- lib/Core/GenericFieldSetBuilder.php | 5 +- lib/Core/Tests/GenericFieldSetBuilderTest.php | 54 ++++++++++++++++--- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/lib/Core/GenericFieldSetBuilder.php b/lib/Core/GenericFieldSetBuilder.php index d9e0fc3f..8b762483 100644 --- a/lib/Core/GenericFieldSetBuilder.php +++ b/lib/Core/GenericFieldSetBuilder.php @@ -36,7 +36,10 @@ public function __construct( public function set(FieldConfig $field): self { - $this->fields[$field->getName()] = $field; + $name = $field->getName(); + + unset($this->unresolvedFields[$name]); + $this->fields[$name] = $field; return $this; } diff --git a/lib/Core/Tests/GenericFieldSetBuilderTest.php b/lib/Core/Tests/GenericFieldSetBuilderTest.php index e03285a3..a7314034 100644 --- a/lib/Core/Tests/GenericFieldSetBuilderTest.php +++ b/lib/Core/Tests/GenericFieldSetBuilderTest.php @@ -22,6 +22,7 @@ use Rollerworks\Component\Search\GenericFieldSetBuilder; use Rollerworks\Component\Search\SearchFactory; use Rollerworks\Component\Search\Tests\Fixtures\BarType; +use Rollerworks\Component\Search\Tests\Fixtures\FooSubType; use Rollerworks\Component\Search\Tests\Fixtures\FooType; /** @@ -56,11 +57,14 @@ static function ($args) use ($test) { */ public function add_fields(): void { - $this->builder->add('id', FooType::class); + $this->builder->add('id', FooType::class, ['foo' => 'bar']); $this->builder->add('name', BarType::class); self::assertTrue($this->builder->has('id')); self::assertTrue($this->builder->has('name')); + + $this->assertBuilderFieldConfigurationEquals('id', FooType::class, ['foo' => 'bar']); + $this->assertBuilderFieldConfigurationEquals('name', BarType::class); } /** @@ -71,6 +75,7 @@ public function always_gives_a_resolved_field(): void $this->builder->add('id', FooType::class, ['foo' => 'bar']); $this->assertBuilderFieldConfigurationEquals('id', FooType::class, ['foo' => 'bar']); + self::assertFalse($this->builder->has('name')); } /** @@ -78,14 +83,11 @@ public function always_gives_a_resolved_field(): void */ public function set_pre_configured_field(): void { - $field = $this->prophesize(FieldConfig::class); - $field->getName()->willReturn('id'); - - $field = $field->reveal(); - + $field = $this->createFieldConfigMock('id'); $this->builder->set($field); self::assertTrue($this->builder->has('id')); + self::assertFalse($this->builder->has('name')); self::assertSame($field, $this->builder->get('id')); } @@ -118,6 +120,29 @@ public function get_build_field_set(): void self::assertFieldConfigurationEquals($fieldSet->get('gid'), 'gid', FooType::class); } + /** + * @test + */ + public function remove_existing_unresolved_when_setting_explicit(): void + { + $this->builder->add('id', FooType::class, ['max' => 5000]); + + $this->builder->set($field = $this->createFieldConfigMock('id')); + $this->builder->set($field2 = $this->createFieldConfigMock('name')); + + self::assertSame($field, $this->builder->get('id')); + self::assertSame($field2, $this->builder->get('name')); + + $fieldSet = $this->builder->getFieldSet('test'); + + self::assertEquals('test', $fieldSet->getSetName()); + self::assertFieldConfigurationEquals($fieldSet->get('id'), 'id', FooSubType::class); + self::assertFieldConfigurationEquals($fieldSet->get('name'), 'name', FooSubType::class); + } + + /** + * @param array $options + */ private function assertBuilderFieldConfigurationEquals(string $name, string $type, array $options = []): void { $field = $this->builder->get($name); @@ -126,10 +151,25 @@ private function assertBuilderFieldConfigurationEquals(string $name, string $typ self::assertEquals($options, $field->getOptions()); } + /** + * @param array $options + */ private static function assertFieldConfigurationEquals(FieldConfig $field, string $name, string $type, array $options = []): void { self::assertEquals($name, $field->getName()); - self::assertInstanceOf($type, $field->getType()->getInnerType()); + self::assertInstanceOf($type, $field->getType()->getInnerType(), 'Expected type to be ' . $type . ', got ' . $field->getType()->getInnerType()::class . ' instead.'); self::assertEquals($options, $field->getOptions()); } + + private function createFieldConfigMock(string $name, string $typeName = FooSubType::class): FieldConfig + { + $type = $this->createMock(ResolvedFieldType::class); + $type->method('getInnerType')->willReturn(new $typeName()); + + $field = $this->createMock(FieldConfig::class); + $field->method('getName')->willReturn($name); + $field->method('getType')->willReturn($type); + + return $field; + } }