Skip to content

Commit aba2e81

Browse files
stereomonstereomon
authored
CC-38744 Fix Service Points acceptance tests: optional-field validation + stale fixture cache (#599)
CC-38744 [CI] Acceptance test fail for Service Points
1 parent 145b647 commit aba2e81

4 files changed

Lines changed: 217 additions & 40 deletions

File tree

‎src/Spryker/ApiPlatform/DependencyInjection/Compiler/ApiPlatformDecoratorPass.php‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Spryker\ApiPlatform\Metadata\CodeBucketResourceClassResolver;
1313
use Spryker\ApiPlatform\Metadata\CodeBucketResourceNameCollectionFactory;
1414
use Spryker\ApiPlatform\OpenApi\Decorator\OpenApiDecorator;
15+
use Spryker\ApiPlatform\State\OptionalFieldFilteringValidateProvider;
1516
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
1617
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1718
use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -31,29 +32,47 @@
3132
*/
3233
class ApiPlatformDecoratorPass implements CompilerPassInterface
3334
{
35+
protected const string SERVICE_ID_RESOURCE_CLASS_RESOLVER = 'api_platform.resource_class_resolver';
36+
37+
protected const string SERVICE_ID_NAME_COLLECTION_FACTORY_CACHED = 'api_platform.metadata.resource.name_collection_factory.cached';
38+
39+
protected const string SERVICE_ID_OPENAPI_FACTORY = 'api_platform.openapi.factory';
40+
41+
protected const string SERVICE_ID_VALIDATE_STATE_PROVIDER = 'api_platform.state_provider.validate';
42+
43+
protected const string TAG_FORMAT_TRANSFORMER = 'spryker_api_platform.format_transformer';
44+
45+
protected const string REFERENCE_INNER = '.inner';
46+
3447
public function process(ContainerBuilder $container): void
3548
{
36-
if (!$container->has('api_platform.resource_class_resolver')) {
49+
if (!$container->has(static::SERVICE_ID_RESOURCE_CLASS_RESOLVER)) {
3750
return;
3851
}
3952

4053
$container->register(CodeBucketResourceClassResolver::class, CodeBucketResourceClassResolver::class)
41-
->setDecoratedService('api_platform.resource_class_resolver')
42-
->setArguments([new Reference('.inner')]);
54+
->setDecoratedService(static::SERVICE_ID_RESOURCE_CLASS_RESOLVER)
55+
->setArguments([new Reference(static::REFERENCE_INNER)]);
4356

44-
if ($container->has('api_platform.metadata.resource.name_collection_factory.cached')) {
57+
if ($container->has(static::SERVICE_ID_NAME_COLLECTION_FACTORY_CACHED)) {
4558
$container->register(CodeBucketResourceNameCollectionFactory::class, CodeBucketResourceNameCollectionFactory::class)
46-
->setDecoratedService('api_platform.metadata.resource.name_collection_factory.cached')
47-
->setArguments([new Reference('.inner')]);
59+
->setDecoratedService(static::SERVICE_ID_NAME_COLLECTION_FACTORY_CACHED)
60+
->setArguments([new Reference(static::REFERENCE_INNER)]);
4861
}
4962

50-
if ($container->has('api_platform.openapi.factory')) {
63+
if ($container->has(static::SERVICE_ID_OPENAPI_FACTORY)) {
5164
$container->register(OpenApiDecorator::class, OpenApiDecorator::class)
52-
->setDecoratedService('api_platform.openapi.factory')
65+
->setDecoratedService(static::SERVICE_ID_OPENAPI_FACTORY)
5366
->setArguments([
54-
new Reference('.inner'),
55-
new TaggedIteratorArgument('spryker_api_platform.format_transformer'),
67+
new Reference(static::REFERENCE_INNER),
68+
new TaggedIteratorArgument(static::TAG_FORMAT_TRANSFORMER),
5669
]);
5770
}
71+
72+
if ($container->has(static::SERVICE_ID_VALIDATE_STATE_PROVIDER)) {
73+
$container->register(OptionalFieldFilteringValidateProvider::class, OptionalFieldFilteringValidateProvider::class)
74+
->setDecoratedService(static::SERVICE_ID_VALIDATE_STATE_PROVIDER)
75+
->setArguments([new Reference(static::REFERENCE_INNER)]);
76+
}
5877
}
5978
}

‎src/Spryker/ApiPlatform/Generator/ConstraintFormatter.php‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,28 @@ public function setFqcnConstraintMap(array $fqcnConstraintMap): void
6969
$this->fqcnConstraintMap = $fqcnConstraintMap;
7070
}
7171

72+
protected const string OPTIONAL_PAYLOAD_SUFFIX = ", payload: ['source' => 'optional']";
73+
7274
/**
7375
* @param array<string> $groups
7476
*/
7577
public function generateConstraintAttribute(mixed $constraint, array $groups): string
78+
{
79+
return $this->formatConstraintAttribute($constraint, $groups, '');
80+
}
81+
82+
/**
83+
* @param array<string> $groups
84+
*/
85+
public function generateConstraintAttributeWithOptionalPayload(mixed $constraint, array $groups): string
86+
{
87+
return $this->formatConstraintAttribute($constraint, $groups, static::OPTIONAL_PAYLOAD_SUFFIX);
88+
}
89+
90+
/**
91+
* @param array<string> $groups
92+
*/
93+
protected function formatConstraintAttribute(mixed $constraint, array $groups, string $payloadSuffix): string
7694
{
7795
$groupsString = implode("', '", $groups);
7896

@@ -81,10 +99,10 @@ public function generateConstraintAttribute(mixed $constraint, array $groups): s
8199
$normalized = $this->normalizeFqcn($constraint);
82100
$alias = $this->fqcnConstraintMap[$normalized]['alias'] ?? $this->parseConstraintFqcn($constraint)['shortName'];
83101

84-
return sprintf("#[%s(groups: ['%s'])]", $alias, $groupsString);
102+
return sprintf("#[%s(groups: ['%s']%s)]", $alias, $groupsString, $payloadSuffix);
85103
}
86104

87-
return sprintf("#[Assert\\%s(groups: ['%s'])]", $constraint, $groupsString);
105+
return sprintf("#[Assert\\%s(groups: ['%s']%s)]", $constraint, $groupsString, $payloadSuffix);
88106
}
89107

90108
if (!is_array($constraint)) {
@@ -99,29 +117,29 @@ public function generateConstraintAttribute(mixed $constraint, array $groups): s
99117
$alias = $this->fqcnConstraintMap[$normalized]['alias'] ?? $this->parseConstraintFqcn($constraintName)['shortName'];
100118

101119
if (!is_array($options)) {
102-
return sprintf("#[%s(groups: ['%s'])]", $alias, $groupsString);
120+
return sprintf("#[%s(groups: ['%s']%s)]", $alias, $groupsString, $payloadSuffix);
103121
}
104122

105123
$formattedOptions = $this->formatConstraintOptions($options, $constraintName, $groups);
106124

107125
if ($formattedOptions === '') {
108-
return sprintf("#[%s(groups: ['%s'])]", $alias, $groupsString);
126+
return sprintf("#[%s(groups: ['%s']%s)]", $alias, $groupsString, $payloadSuffix);
109127
}
110128

111-
return sprintf("#[%s(%s, groups: ['%s'])]", $alias, $formattedOptions, $groupsString);
129+
return sprintf("#[%s(%s, groups: ['%s']%s)]", $alias, $formattedOptions, $groupsString, $payloadSuffix);
112130
}
113131

114132
if (!is_array($options)) {
115-
return sprintf("#[Assert\\%s(groups: ['%s'])]", $constraintName, $groupsString);
133+
return sprintf("#[Assert\\%s(groups: ['%s']%s)]", $constraintName, $groupsString, $payloadSuffix);
116134
}
117135

118136
$formattedOptions = $this->formatConstraintOptions($options, $constraintName, $groups);
119137

120138
if ($formattedOptions === '') {
121-
return sprintf("#[Assert\\%s(groups: ['%s'])]", $constraintName, $groupsString);
139+
return sprintf("#[Assert\\%s(groups: ['%s']%s)]", $constraintName, $groupsString, $payloadSuffix);
122140
}
123141

124-
return sprintf("#[Assert\\%s(%s, groups: ['%s'])]", $constraintName, $formattedOptions, $groupsString);
142+
return sprintf("#[Assert\\%s(%s, groups: ['%s']%s)]", $constraintName, $formattedOptions, $groupsString, $payloadSuffix);
125143
}
126144

127145
/**

‎src/Spryker/ApiPlatform/Generator/ValidationAttributeGenerator.php‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@
4545
* Key behaviors:
4646
* - Maps operations to validation groups (Post → 'customers:create', Patch → 'customers:update')
4747
* - Deduplicates same constraints across groups (merges groups when constraint is identical)
48-
* - Unwraps Optional constraints and skips NotBlank inside Optional wrappers
48+
* - Unwraps Optional constraints and emits their inner constraints as property attributes;
49+
* together with the ClassGenerator omitting `= null` defaults for `required: false` properties,
50+
* this preserves Optional semantics — Symfony Validator skips uninitialized properties,
51+
* so an absent field bypasses validation while an explicit-null field is validated.
4952
* - Handles fully qualified class name constraints with alias resolution
5053
*/
5154
class ValidationAttributeGenerator
@@ -82,13 +85,10 @@ public function generate(array $validationSchema, array $operations, string $pro
8285
$nestedConstraints = $this->extractNestedConstraintsFromOptional($constraint);
8386

8487
foreach ($nestedConstraints as $nestedConstraint) {
85-
if ($this->shouldSkipConstraintForOptionalField($nestedConstraint)) {
86-
continue;
87-
}
88-
8988
$constraintsWithGroups[] = [
9089
'constraint' => $nestedConstraint,
9190
'group' => $group,
91+
'fromOptional' => true,
9292
];
9393
}
9494

@@ -98,6 +98,7 @@ public function generate(array $validationSchema, array $operations, string $pro
9898
$constraintsWithGroups[] = [
9999
'constraint' => $constraint,
100100
'group' => $group,
101+
'fromOptional' => false,
101102
];
102103
}
103104
}
@@ -107,16 +108,28 @@ public function generate(array $validationSchema, array $operations, string $pro
107108
$attributes = [];
108109

109110
foreach ($deduplicatedConstraints as $constraintData) {
110-
$attributes[] = $this->constraintFormatter->generateConstraintAttribute($constraintData['constraint'], $constraintData['groups']);
111+
if ($constraintData['fromOptional']) {
112+
$attributes[] = $this->constraintFormatter->generateConstraintAttributeWithOptionalPayload(
113+
$constraintData['constraint'],
114+
$constraintData['groups'],
115+
);
116+
117+
continue;
118+
}
119+
120+
$attributes[] = $this->constraintFormatter->generateConstraintAttribute(
121+
$constraintData['constraint'],
122+
$constraintData['groups'],
123+
);
111124
}
112125

113126
return $attributes;
114127
}
115128

116129
/**
117-
* @param array<array{constraint: mixed, group: string}> $constraintsWithGroups
130+
* @param array<array{constraint: mixed, group: string, fromOptional: bool}> $constraintsWithGroups
118131
*
119-
* @return array<array{constraint: mixed, groups: array<string>}>
132+
* @return array<array{constraint: mixed, groups: array<string>, fromOptional: bool}>
120133
*/
121134
protected function deduplicateConstraintsByGroups(array $constraintsWithGroups): array
122135
{
@@ -125,13 +138,15 @@ protected function deduplicateConstraintsByGroups(array $constraintsWithGroups):
125138
foreach ($constraintsWithGroups as $item) {
126139
$constraint = $item['constraint'];
127140
$group = $item['group'];
141+
$fromOptional = $item['fromOptional'];
128142

129-
$key = $this->generateConstraintKey($constraint);
143+
$key = $this->generateConstraintKey($constraint) . '|' . ($fromOptional ? 'optional' : 'required');
130144

131145
if (!isset($groupedByConstraint[$key])) {
132146
$groupedByConstraint[$key] = [
133147
'constraint' => $constraint,
134148
'groups' => [],
149+
'fromOptional' => $fromOptional,
135150
];
136151
}
137152

@@ -194,19 +209,6 @@ protected function extractNestedConstraintsFromOptional(array $constraint): arra
194209
return $constraint['Optional']['constraints'];
195210
}
196211

197-
protected function shouldSkipConstraintForOptionalField(mixed $constraint): bool
198-
{
199-
if (is_string($constraint) && $constraint === 'NotBlank') {
200-
return true;
201-
}
202-
203-
if (is_array($constraint) && isset($constraint['NotBlank'])) {
204-
return true;
205-
}
206-
207-
return false;
208-
}
209-
210212
protected function normalizeFqcn(string $fqcn): string
211213
{
212214
return ltrim($fqcn, '\\');

‎src/Spryker/ApiPlatform/State/OptionalFieldFilteringValidateProvider.php‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
<?php
2+
3+
/**
4+
* Copyright © 2016-present Spryker Systems GmbH. All rights reserved.
5+
* Use of this software requires acceptance of the Evaluation License Agreement. See LICENSE file.
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace Spryker\ApiPlatform\State;
11+
12+
use ApiPlatform\Metadata\Operation;
13+
use ApiPlatform\State\ProviderInterface;
14+
use ApiPlatform\Validator\Exception\ValidationException;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\Validator\Constraint;
17+
use Symfony\Component\Validator\ConstraintViolationInterface;
18+
use Symfony\Component\Validator\ConstraintViolationList;
19+
20+
/**
21+
* Drops validation violations whose constraint was emitted from an `Optional` block in the
22+
* validation YAML AND whose property was not present in the submitted request body.
23+
*
24+
* Preserves legacy Glue REST semantics for fields wrapped in `Optional`:
25+
* - field absent from request body → no error (legacy behavior preserved)
26+
* - field present with null/empty value → validation fires normally (NotBlank → 422)
27+
* - bare-NotBlank required fields absent → "must not be blank" fires normally (no change)
28+
*
29+
* Optional-sourced constraints are tagged at generation time with `payload: ['source' => 'optional']`
30+
* by `ConstraintFormatter::generateConstraintAttributeWithOptionalPayload()`. This decorator inspects
31+
* that payload on each violation's constraint to decide whether the violation may be dropped.
32+
*
33+
* @implements \ApiPlatform\State\ProviderInterface<object>
34+
*/
35+
class OptionalFieldFilteringValidateProvider implements ProviderInterface
36+
{
37+
protected const string OPTIONAL_PAYLOAD_KEY = 'source';
38+
39+
protected const string OPTIONAL_PAYLOAD_VALUE = 'optional';
40+
41+
/**
42+
* @param \ApiPlatform\State\ProviderInterface<object> $decorated
43+
*/
44+
public function __construct(
45+
protected readonly ProviderInterface $decorated,
46+
) {
47+
}
48+
49+
/**
50+
* @param array<string, mixed> $uriVariables
51+
* @param array<string, mixed> $context
52+
*
53+
* @throws \ApiPlatform\Validator\Exception\ValidationException
54+
*/
55+
public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
56+
{
57+
try {
58+
return $this->decorated->provide($operation, $uriVariables, $context);
59+
} catch (ValidationException $exception) {
60+
$request = $context['request'] ?? null;
61+
62+
if (!$request instanceof Request) {
63+
throw $exception;
64+
}
65+
66+
$submittedFields = $this->resolveSubmittedFields($request);
67+
68+
if ($submittedFields === null) {
69+
throw $exception;
70+
}
71+
72+
$remainingViolations = [];
73+
74+
foreach ($exception->getConstraintViolationList() as $violation) {
75+
if ($this->shouldDropViolation($violation, $submittedFields)) {
76+
continue;
77+
}
78+
79+
$remainingViolations[] = $violation;
80+
}
81+
82+
if ($remainingViolations === []) {
83+
// All violations were Optional-sourced for fields not in the request body — re-run
84+
// the inner provider with validation disabled to get the deserialized body so the
85+
// processor can run normally.
86+
return $this->decorated->provide($operation->withValidate(false), $uriVariables, $context);
87+
}
88+
89+
throw new ValidationException(new ConstraintViolationList($remainingViolations));
90+
}
91+
}
92+
93+
/**
94+
* @param array<string> $submittedFields
95+
*/
96+
protected function shouldDropViolation(ConstraintViolationInterface $violation, array $submittedFields): bool
97+
{
98+
$propertyPath = $violation->getPropertyPath();
99+
100+
// Only top-level properties can be checked against the submitted body map.
101+
// Nested paths (e.g. `items[0].sku`) keep their original violation.
102+
if (str_contains($propertyPath, '.') || str_contains($propertyPath, '[')) {
103+
return false;
104+
}
105+
106+
if (in_array($propertyPath, $submittedFields, true)) {
107+
return false;
108+
}
109+
110+
$constraint = $violation->getConstraint();
111+
112+
if (!$constraint instanceof Constraint) {
113+
return false;
114+
}
115+
116+
$payload = $constraint->payload;
117+
118+
if (!is_array($payload)) {
119+
return false;
120+
}
121+
122+
return ($payload[static::OPTIONAL_PAYLOAD_KEY] ?? null) === static::OPTIONAL_PAYLOAD_VALUE;
123+
}
124+
125+
/**
126+
* @return array<string>|null
127+
*/
128+
protected function resolveSubmittedFields(Request $request): ?array
129+
{
130+
$body = json_decode((string)$request->getContent(), true);
131+
132+
if (!is_array($body) || !isset($body['data']['attributes']) || !is_array($body['data']['attributes'])) {
133+
return null;
134+
}
135+
136+
return array_map('strval', array_keys($body['data']['attributes']));
137+
}
138+
}

0 commit comments

Comments
 (0)