diff --git a/CHANGELOG.md b/CHANGELOG.md index 069553f..1aadeec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,13 @@ - + # Changelog +## Unreleased + +### Fixed + +- `InvalidDefaultSchemaDecorator` drops a property `default` from the JSON/OpenAPI schemas when the property schema itself rejects it (`pattern`, `minLength`, `maxLength`, `enum`). API Platform derives `default` from the PHP initializer, so `private string $slug = '';` next to a slug `#[Assert\Regex]` produced `default: ""` that its own `pattern` rejects, and form clients showed a validation error before anyone typed. Valid defaults are untouched; a pattern PHP cannot compile keeps its default. On by default, switch off with `schema_default_cleanup.enabled: false`. + ## 0.5.0 (2026-10-09) ### Added diff --git a/config/services_schema_default_cleanup.yaml b/config/services_schema_default_cleanup.yaml new file mode 100644 index 0000000..5c94e15 --- /dev/null +++ b/config/services_schema_default_cleanup.yaml @@ -0,0 +1,17 @@ +# file generated with AI assistance: Claude Code - 2026-10-10 00:15:00 UTC + +services: + _defaults: + autowire: false + autoconfigure: false + public: false + + # Drops property defaults that violate their own pattern/minLength/ + # maxLength/enum (e.g. `default: ""` next to a slug pattern). Priority 5 + # wraps below RelationFieldSchemaDecorator (10); the order does not + # matter for the result, the two touch different keys. + Dmstr\ApiPlatformUtils\OpenApi\InvalidDefaultSchemaDecorator: + decorates: 'api_platform.json_schema.schema_factory' + decoration_priority: 5 + arguments: + $decorated: '@.inner' diff --git a/src/DependencyInjection/ApiPlatformUtilsExtension.php b/src/DependencyInjection/ApiPlatformUtilsExtension.php index 2ea6441..52673de 100644 --- a/src/DependencyInjection/ApiPlatformUtilsExtension.php +++ b/src/DependencyInjection/ApiPlatformUtilsExtension.php @@ -50,6 +50,8 @@ public function load(array $configs, ContainerBuilder $container): void $container->setParameter('dmstr_api_platform_utils.stable_order.enabled', $config['stable_order']['enabled']); + $container->setParameter('dmstr_api_platform_utils.schema_default_cleanup.enabled', $config['schema_default_cleanup']['enabled']); + // Load service definitions $loader = new YamlFileLoader($container, new FileLocator(__DIR__ . '/../../config')); $loader->load('services.yaml'); @@ -75,6 +77,11 @@ public function load(array $configs, ContainerBuilder $container): void if ($config['stable_order']['enabled']) { $loader->load('services_stable_order.yaml'); } + + // On by default: it only removes defaults the schema itself rejects. + if ($config['schema_default_cleanup']['enabled']) { + $loader->load('services_schema_default_cleanup.yaml'); + } } public function getAlias(): string diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 6ba26ae..a30001f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -60,6 +60,17 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() + // Invalid property defaults in JSON schemas + ->arrayNode('schema_default_cleanup') + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('enabled') + ->defaultTrue() + ->info('Drop a property `default` from the JSON/OpenAPI schemas when the property schema itself rejects it (pattern, minLength, maxLength, enum), e.g. `default: ""` derived from `private string $slug = \'\'` next to a slug pattern') + ->end() + ->end() + ->end() + // Hydra Operations Subscriber ->arrayNode('hydra_operations') ->addDefaultsIfNotSet() diff --git a/src/OpenApi/InvalidDefaultSchemaDecorator.php b/src/OpenApi/InvalidDefaultSchemaDecorator.php new file mode 100644 index 0000000..19e4b4e --- /dev/null +++ b/src/OpenApi/InvalidDefaultSchemaDecorator.php @@ -0,0 +1,137 @@ +decorated->buildSchema( + $className, + $format, + $type, + $operation, + $schema, + $serializerContext, + $forceCollection, + ); + + $definitions = $schema->getDefinitions(); + foreach ($definitions as $key => $definition) { + $definitions[$key] = $this->cleanNode($definition); + } + + return $schema; + } + + /** + * Walks `properties` (nested objects included) and `allOf`/`anyOf`/`oneOf` + * branches, the places API Platform puts property schemas. + */ + private function cleanNode(mixed $node): mixed + { + if (!\is_array($node) && !$node instanceof \ArrayObject) { + return $node; + } + + if (isset($node['properties']) && (\is_array($node['properties']) || $node['properties'] instanceof \ArrayObject)) { + $properties = $node['properties']; + foreach ($properties as $name => $property) { + $properties[$name] = $this->cleanNode($this->dropInvalidDefault($property)); + } + $node['properties'] = $properties; + } + + foreach (['allOf', 'anyOf', 'oneOf'] as $keyword) { + if (isset($node[$keyword]) && \is_array($node[$keyword])) { + $branches = $node[$keyword]; + foreach ($branches as $index => $branch) { + $branches[$index] = $this->cleanNode($branch); + } + $node[$keyword] = $branches; + } + } + + return $node; + } + + private function dropInvalidDefault(mixed $property): mixed + { + if ((!\is_array($property) && !$property instanceof \ArrayObject) || !isset($property['default'])) { + return $property; + } + + if (!$this->violates($property, $property['default'])) { + return $property; + } + + unset($property['default']); + + return $property; + } + + private function violates(array|\ArrayObject $property, mixed $default): bool + { + if (isset($property['enum']) && \is_array($property['enum']) && !\in_array($default, $property['enum'], true)) { + return true; + } + + if (!\is_string($default)) { + return false; + } + + $length = mb_strlen($default); + if (isset($property['minLength']) && $length < (int) $property['minLength']) { + return true; + } + if (isset($property['maxLength']) && $length > (int) $property['maxLength']) { + return true; + } + + if (isset($property['pattern']) && \is_string($property['pattern'])) { + // JSON Schema patterns are ECMA-262 and unanchored; the delimiter + // is escaped, everything else is passed through as written. + $regex = '/' . str_replace('/', '\/', $property['pattern']) . '/u'; + $match = @preg_match($regex, $default); + if ($match === 0) { + return true; + } + // false: PHP cannot compile it — keep the default rather than guess + } + + return false; + } +} diff --git a/tests/DependencyInjection/ApiPlatformUtilsExtensionTest.php b/tests/DependencyInjection/ApiPlatformUtilsExtensionTest.php index 10b2484..e99a7d1 100644 --- a/tests/DependencyInjection/ApiPlatformUtilsExtensionTest.php +++ b/tests/DependencyInjection/ApiPlatformUtilsExtensionTest.php @@ -8,6 +8,7 @@ use Dmstr\ApiPlatformUtils\DependencyInjection\ApiPlatformUtilsExtension; use Dmstr\ApiPlatformUtils\Doctrine\Orm\Extension\StableOrderExtension; use Dmstr\ApiPlatformUtils\Metadata\AutoOrderResourceMetadataCollectionFactory; +use Dmstr\ApiPlatformUtils\OpenApi\InvalidDefaultSchemaDecorator; use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -67,6 +68,19 @@ public function testStableOrderServiceTag(): void ); } + public function testSchemaDefaultCleanupIsOnByDefaultAndCanBeSwitchedOff(): void + { + $on = $this->load([self::BASE]); + self::assertTrue($on->hasDefinition(InvalidDefaultSchemaDecorator::class)); + self::assertSame( + 'api_platform.json_schema.schema_factory', + $on->getDefinition(InvalidDefaultSchemaDecorator::class)->getDecoratedService()[0], + ); + + $off = $this->load([self::BASE + ['schema_default_cleanup' => ['enabled' => false]]]); + self::assertFalse($off->hasDefinition(InvalidDefaultSchemaDecorator::class)); + } + public function testLabelCandidatesOfSeveralConfigsAreDeduplicated(): void { $container = $this->load([ diff --git a/tests/OpenApi/InvalidDefaultSchemaDecoratorTest.php b/tests/OpenApi/InvalidDefaultSchemaDecoratorTest.php new file mode 100644 index 0000000..b18b92f --- /dev/null +++ b/tests/OpenApi/InvalidDefaultSchemaDecoratorTest.php @@ -0,0 +1,108 @@ + 'string', 'pattern' => '^([a-z0-9]+(?:-[a-z0-9]+)*)$']; + + public function testDropsDefaultThatViolatesPatternInsideAllOf(): void + { + // the shape of a `.jsonld` read schema: properties inside allOf[1] + $properties = $this->build(['Node.jsonld' => ['allOf' => [ + ['$ref' => '#/definitions/HydraItemBaseSchema'], + ['type' => 'object', 'properties' => ['slug' => self::SLUG + ['default' => '']]], + ]]])['Node.jsonld']['allOf'][1]['properties']; + + self::assertSame(self::SLUG, $properties['slug']); + } + + public function testKeepsDefaultsTheSchemaAccepts(): void + { + $properties = $this->build(['Node' => ['type' => 'object', 'properties' => [ + 'slug' => self::SLUG + ['default' => 'home'], + 'title' => ['type' => 'string', 'default' => ''], + 'status' => ['type' => 'string', 'enum' => ['draft', 'published'], 'default' => 'draft'], + 'position' => ['type' => 'integer', 'minimum' => 0, 'default' => 0], + ]]])['Node']['properties']; + + self::assertSame('home', $properties['slug']['default']); + self::assertSame('', $properties['title']['default']); + self::assertSame('draft', $properties['status']['default']); + self::assertSame(0, $properties['position']['default']); + } + + public function testDropsDefaultsViolatingLengthAndEnum(): void + { + $properties = $this->build(['Node' => ['type' => 'object', 'properties' => [ + 'name' => ['type' => 'string', 'minLength' => 1, 'default' => ''], + 'code' => ['type' => 'string', 'maxLength' => 2, 'default' => 'abc'], + 'status' => ['type' => 'string', 'enum' => ['draft'], 'default' => 'gone'], + ]]])['Node']['properties']; + + self::assertArrayNotHasKey('default', $properties['name']); + self::assertArrayNotHasKey('default', $properties['code']); + self::assertArrayNotHasKey('default', $properties['status']); + } + + public function testWalksNestedObjectProperties(): void + { + $nested = $this->build(['Node' => ['type' => 'object', 'properties' => [ + 'meta' => ['type' => 'object', 'properties' => ['key' => self::SLUG + ['default' => '']]], + ]]])['Node']['properties']['meta']['properties']; + + self::assertArrayNotHasKey('default', $nested['key']); + } + + public function testKeepsDefaultWhenPhpCannotCompileThePattern(): void + { + // ECMA-only syntax PHP's PCRE rejects: keep rather than guess + $properties = $this->build(['Node' => ['type' => 'object', 'properties' => [ + 'odd' => ['type' => 'string', 'pattern' => '(?<=a', 'default' => ''], + ]]])['Node']['properties']; + + self::assertSame('', $properties['odd']['default']); + } + + public function testPatternWithSlashIsEscaped(): void + { + $properties = $this->build(['Node' => ['type' => 'object', 'properties' => [ + 'path' => ['type' => 'string', 'pattern' => '^/[a-z]+$', 'default' => '/home'], + 'bad' => ['type' => 'string', 'pattern' => '^/[a-z]+$', 'default' => ''], + ]]])['Node']['properties']; + + self::assertSame('/home', $properties['path']['default']); + self::assertArrayNotHasKey('default', $properties['bad']); + } + + /** + * @param array $definitions + */ + private function build(array $definitions): \ArrayObject + { + $inner = new class(new \ArrayObject($definitions)) implements SchemaFactoryInterface { + public function __construct(private readonly \ArrayObject $definitions) + { + } + + public function buildSchema(string $className, string $format = 'json', string $type = Schema::TYPE_OUTPUT, ?Operation $operation = null, ?Schema $schema = null, ?array $serializerContext = null, bool $forceCollection = false): Schema + { + $schema = new Schema(); + $schema->setDefinitions($this->definitions); + + return $schema; + } + }; + + return (new InvalidDefaultSchemaDecorator($inner))->buildSchema('Node')->getDefinitions(); + } +}