Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions src/Metadata/Parameters.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ public function __construct(array $parameters = [])
$parameterName = $parameter->getKey();
}

$key = \sprintf('%s.%s', $parameter::class, $parameterName);
// `:property` is a template expanded per-property later; multiple templates with disjoint properties must coexist.
if (str_contains((string) $parameterName, ':property')) {
$key = \sprintf('%s.%s.%s', $parameter::class, $parameterName, self::propertyDiscriminator($parameter));
} else {
$key = \sprintf('%s.%s', $parameter::class, $parameterName);
}

$this->parameters[$key] = [$parameterName, $parameter];
}
Expand All @@ -61,19 +66,38 @@ public function getIterator(): \Traversable

public function add(string $key, Parameter $value): self
{
// `:property` is a template expanded per-property later; templates with disjoint properties coexist, identical ones override.
$isTemplate = str_contains($key, ':property');
$valueDiscriminator = $isTemplate ? self::propertyDiscriminator($value) : null;

foreach ($this->parameters as $i => [$parameterName, $parameter]) {
if ($parameterName === $key && $value::class === $parameter::class) {
$this->parameters[$i] = [$key, $value];
if ($parameterName !== $key || $value::class !== $parameter::class) {
continue;
}

return $this;
if ($isTemplate && self::propertyDiscriminator($parameter) !== $valueDiscriminator) {
continue;
}

$this->parameters[$i] = [$key, $value];

return $this;
}

$this->parameters[] = [$key, $value];

return $this;
}

private static function propertyDiscriminator(Parameter $parameter): string
{
if ($properties = $parameter->getProperties()) {
return '['.implode(',', $properties).']';
}

return $parameter->getProperty() ?? '';
}

/**
* @template T of Parameter
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ private function mergeOperationParameters(Metadata $resource, Parameters $global
$parameterName = $key;
}

if (!$parameters->has($parameterName, $parameter::class)) {
// `:property` is a template expanded per-property later; multiple templates must coexist.
if (str_contains((string) $parameterName, ':property') || !$parameters->has($parameterName, $parameter::class)) {
$parameters->add($parameterName, $parameter);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ private function getProperties(string $resourceClass, ?Parameter $parameter = nu
if ($parameter) {
$paramKey = $parameter->getProperties() ? ($parameter->getKey() ?? '') : ($parameter->getProperty() ?? $parameter->getKey() ?? '');
}
$k = $resourceClass.$paramKey.(\is_string($parameter->getFilter()) ? $parameter->getFilter() : '').$filterClass;
// Include properties list so repeated `:property` templates with disjoint properties get distinct cache entries.
$paramProperties = $parameter?->getProperties() ? '['.implode(',', $parameter->getProperties()).']' : '';
$k = $resourceClass.$paramKey.$paramProperties.(\is_string($parameter->getFilter()) ? $parameter->getFilter() : '').$filterClass;
if (isset($this->localPropertyCache[$k])) {
return $this->localPropertyCache[$k];
}
Expand Down
28 changes: 28 additions & 0 deletions src/Metadata/Tests/ParametersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,32 @@ public function testDuplicated(): void
$this->assertSame($r2, $parameters->get('a'));
$this->assertSame($r4, $parameters->get('a', HeaderParameter::class));
}

public function testPropertyPlaceholderKeysAreNotDeduplicated(): void
{
$r1 = new QueryParameter(key: ':property', properties: ['field1', 'field2']);
$r2 = new QueryParameter(key: ':property', properties: ['field3', 'field4']);
$parameters = new Parameters([$r1, $r2]);

$this->assertCount(2, $parameters);

$collected = [];
foreach ($parameters as $key => $parameter) {
$collected[] = [$key, $parameter];
}

$this->assertSame(':property', $collected[0][0]);
$this->assertSame(':property', $collected[1][0]);
$this->assertSame(['field1', 'field2'], $collected[0][1]->getProperties());
$this->assertSame(['field3', 'field4'], $collected[1][1]->getProperties());
}

public function testPropertyPlaceholderKeysAreNotDeduplicatedViaAdd(): void
{
$parameters = new Parameters();
$parameters->add(':property', new QueryParameter(key: ':property', properties: ['field1', 'field2']));
$parameters->add(':property', new QueryParameter(key: ':property', properties: ['field3', 'field4']));

$this->assertCount(2, $parameters);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,40 @@ public function testNestedPropertyWithNameConverter(): void
$this->assertSame('search[related.nested]', $searchNestedParam->getKey());
}

public function testRepeatedPropertyPlaceholderAttributesExpandPerPropertyFilter(): void
{
$nameCollection = $this->createStub(PropertyNameCollectionFactoryInterface::class);
$nameCollection->method('create')->willReturn(new PropertyNameCollection(['field1', 'field2', 'field3', 'field4']));

$propertyMetadata = $this->createStub(PropertyMetadataFactoryInterface::class);
$propertyMetadata->method('create')->willReturn(new ApiProperty(readable: true));

$filterLocator = $this->createStub(ContainerInterface::class);
$filterLocator->method('has')->willReturn(false);

$parameterFactory = new ParameterResourceMetadataCollectionFactory(
$nameCollection,
$propertyMetadata,
new AttributesResourceMetadataCollectionFactory(),
$filterLocator
);

$collection = $parameterFactory->create(HasRepeatedPropertyPlaceholderParameter::class);
$operation = $collection->getOperation(forceCollection: true);
$parameters = $operation->getParameters();

$this->assertInstanceOf(Parameters::class, $parameters);

foreach (['field1', 'field2', 'field3', 'field4'] as $field) {
$this->assertTrue($parameters->has($field), \sprintf('Parameter "%s" should exist after :property expansion', $field));
}

$this->assertInstanceOf(RepeatedPlaceholderExactFilter::class, $parameters->get('field1')->getFilter());
$this->assertInstanceOf(RepeatedPlaceholderExactFilter::class, $parameters->get('field2')->getFilter());
$this->assertInstanceOf(RepeatedPlaceholderBooleanFilter::class, $parameters->get('field3')->getFilter());
$this->assertInstanceOf(RepeatedPlaceholderBooleanFilter::class, $parameters->get('field4')->getFilter());
}

private function createNestedPropertyFactory(): ParameterResourceMetadataCollectionFactory
{
$nameCollection = $this->createStub(PropertyNameCollectionFactoryInterface::class);
Expand Down Expand Up @@ -428,6 +462,34 @@ public function testSimplePropertyHasNoNestedPropertyInfo(): void
}
}

class RepeatedPlaceholderExactFilter implements FilterInterface
{
public function getDescription(string $resourceClass): array
{
return [];
}
}

class RepeatedPlaceholderBooleanFilter implements FilterInterface
{
public function getDescription(string $resourceClass): array
{
return [];
}
}

#[ApiResource]
#[QueryParameter(key: ':property', filter: new RepeatedPlaceholderExactFilter(), properties: ['field1', 'field2'])]
#[QueryParameter(key: ':property', filter: new RepeatedPlaceholderBooleanFilter(), properties: ['field3', 'field4'])]
class HasRepeatedPropertyPlaceholderParameter
{
public $id;
public $field1;
public $field2;
public $field3;
public $field4;
}

#[ApiResource(
operations: [
new GetCollection(
Expand Down
Loading