Skip to content

Commit e14d514

Browse files
committed
fix: implement recursive extension context validation in FHIRValidationService
- Updated `validateExtensionContexts` to support recursive validation across nested resource trees. - Added cycle detection using `spl_object_id` to prevent infinite loops during validation. - Improved filtering logic to defer type-hierarchy resolution for non-matching or foreign-root contexts. - Updated invariant and extension validation to capture violations with detailed paths.
1 parent 6875211 commit e14d514

6 files changed

Lines changed: 287 additions & 48 deletions

src/Component/Validation/src/FHIRValidationService.php

Lines changed: 115 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ public function validate(
4747
}
4848
}
4949

50-
foreach ($this->validateExtensionContexts($resource) as $contextViolation) {
50+
$contextVisited = [];
51+
foreach ($this->validateExtensionContexts($resource, $this->getResourceFhirType($resource), '', $contextVisited) as $contextViolation) {
5152
$violations[] = $contextViolation;
5253
}
5354

@@ -315,76 +316,142 @@ private function validateModifierExtensions(object $resource, string $path, arra
315316
}
316317

317318
/**
318-
* Pass 2 (v1: top-level walk only): check extension context and contextInvariant
319-
* constraints for all extensions attached directly to $resource.
319+
* Pass 2: check extension context and contextInvariant constraints for all extensions
320+
* throughout the resource tree. Walks nested sub-elements recursively.
320321
*
321-
* Recursive sub-element walking and FHIR type-hierarchy resolution are deferred to v2.
322+
* At sub-element level (path contains a dot), only extensions whose context expression
323+
* is a dotted path sharing the current root resource type are checked. Bare type-name
324+
* contexts (e.g. "HumanName") and foreign-root paths (e.g. "ElementDefinition.binding"
325+
* inside StructureDefinition) require FHIR type-hierarchy resolution — deferred.
326+
*
327+
* @param array<int, true> $visited spl_object_id keys of already-visited objects (cycle guard)
322328
*
323329
* @return list<FHIRValidationViolation>
324330
*/
325-
private function validateExtensionContexts(object $resource): array
331+
private function validateExtensionContexts(object $resource, string $fhirPath, string $relPath, array &$visited): array
326332
{
327-
if (!method_exists($resource, 'getExtensions')) {
333+
$id = spl_object_id($resource);
334+
335+
if (isset($visited[$id])) {
328336
return [];
329337
}
330338

331-
$elementPath = $this->getResourceFhirType($resource);
332-
$violations = [];
333-
334-
/** @var list<object> $extensions */
335-
$extensions = $resource->getExtensions();
336-
337-
foreach ($extensions as $extension) {
338-
$ref = new \ReflectionClass($extension);
339-
$contextAttrs = array_map(
340-
static fn (\ReflectionAttribute $a): FHIRExtensionContext => $a->newInstance(),
341-
$ref->getAttributes(FHIRExtensionContext::class),
342-
);
339+
$visited[$id] = true;
340+
$violations = [];
343341

344-
if ($contextAttrs !== [] && !$this->contextPermitsPath($contextAttrs, $elementPath)) {
345-
$url = method_exists($extension, 'getExtensionUrl') ? ($extension->getExtensionUrl() ?? '') : '';
346-
$violations[] = new FHIRValidationViolation(
347-
severity: 'error',
348-
path: 'extension',
349-
message: sprintf(
350-
'Extension "%s" is not permitted on element "%s".',
351-
$url,
352-
$elementPath,
353-
),
354-
constraintClass: FHIRExtensionContext::class,
355-
profileGroup: null,
356-
invariantKey: null,
342+
if (method_exists($resource, 'getExtensions')) {
343+
$extViolationPath = $relPath !== '' ? $relPath . '.extension' : 'extension';
344+
345+
// At sub-element level, determine the root resource type so that only context
346+
// expressions explicitly targeting this resource's path hierarchy are evaluated.
347+
// Contexts without a dot or with a different root type require type-hierarchy
348+
// resolution and are deferred. At root level ($rootType === null) all
349+
// expressions are checked as before.
350+
$dotPos = strpos($fhirPath, '.');
351+
$rootType = $dotPos !== false ? substr($fhirPath, 0, $dotPos) : null;
352+
353+
/** @var list<object> $extensions */
354+
$extensions = $resource->getExtensions();
355+
356+
foreach ($extensions as $extension) {
357+
$ref = new \ReflectionClass($extension);
358+
$contextAttrs = array_map(
359+
static fn (\ReflectionAttribute $a): FHIRExtensionContext => $a->newInstance(),
360+
$ref->getAttributes(FHIRExtensionContext::class),
357361
);
358-
}
359-
360-
$invariantAttrs = array_map(
361-
static fn (\ReflectionAttribute $a): FHIRContextInvariant => $a->newInstance(),
362-
$ref->getAttributes(FHIRContextInvariant::class),
363-
);
364362

365-
foreach ($invariantAttrs as $invariant) {
366-
try {
367-
$result = $this->pathService->evaluate($invariant->expression, $resource);
368-
$passed = $result->count() === 1 && $result->first() === true;
369-
} catch (\Throwable) {
370-
$passed = false;
363+
// Sub-element filter: skip extensions whose context expressions cannot be
364+
// evaluated against a structural path without FHIR type-hierarchy resolution.
365+
if ($rootType !== null && $contextAttrs !== []) {
366+
$hasCheckable = false;
367+
foreach ($contextAttrs as $ctx) {
368+
if ($ctx->type === 'element'
369+
&& str_contains($ctx->expression, '.')
370+
&& str_starts_with($ctx->expression, $rootType . '.')
371+
) {
372+
$hasCheckable = true;
373+
break;
374+
}
375+
}
376+
if (!$hasCheckable) {
377+
continue;
378+
}
371379
}
372380

373-
if (!$passed) {
381+
if ($contextAttrs !== [] && !$this->contextPermitsPath($contextAttrs, $fhirPath)) {
374382
$url = method_exists($extension, 'getExtensionUrl') ? ($extension->getExtensionUrl() ?? '') : '';
375383
$violations[] = new FHIRValidationViolation(
376384
severity: 'error',
377-
path: 'extension',
385+
path: $extViolationPath,
378386
message: sprintf(
379-
'Extension "%s" contextInvariant failed: %s',
387+
'Extension "%s" is not permitted on element "%s".',
380388
$url,
381-
$invariant->expression,
389+
$fhirPath,
382390
),
383-
constraintClass: FHIRContextInvariant::class,
391+
constraintClass: FHIRExtensionContext::class,
384392
profileGroup: null,
385393
invariantKey: null,
386394
);
387395
}
396+
397+
$invariantAttrs = array_map(
398+
static fn (\ReflectionAttribute $a): FHIRContextInvariant => $a->newInstance(),
399+
$ref->getAttributes(FHIRContextInvariant::class),
400+
);
401+
402+
foreach ($invariantAttrs as $invariant) {
403+
try {
404+
$result = $this->pathService->evaluate($invariant->expression, $resource);
405+
$passed = $result->count() === 1 && $result->first() === true;
406+
} catch (\Throwable) {
407+
$passed = false;
408+
}
409+
410+
if (!$passed) {
411+
$url = method_exists($extension, 'getExtensionUrl') ? ($extension->getExtensionUrl() ?? '') : '';
412+
$violations[] = new FHIRValidationViolation(
413+
severity: 'error',
414+
path: $extViolationPath,
415+
message: sprintf(
416+
'Extension "%s" contextInvariant failed: %s',
417+
$url,
418+
$invariant->expression,
419+
),
420+
constraintClass: FHIRContextInvariant::class,
421+
profileGroup: null,
422+
invariantKey: null,
423+
);
424+
}
425+
}
426+
}
427+
}
428+
429+
$ref = new \ReflectionClass($resource);
430+
431+
foreach ($ref->getProperties(\ReflectionProperty::IS_PUBLIC) as $prop) {
432+
if (in_array($prop->getName(), ['extension', 'modifierExtension'], true)) {
433+
continue;
434+
}
435+
if ($prop->isInitialized($resource) === false) {
436+
continue;
437+
}
438+
439+
$value = $prop->getValue($resource);
440+
$subFhirPath = $fhirPath . '.' . $prop->getName();
441+
$subRelPath = $relPath !== '' ? $relPath . '.' . $prop->getName() : $prop->getName();
442+
443+
if (is_object($value)) {
444+
foreach ($this->validateExtensionContexts($value, $subFhirPath, $subRelPath, $visited) as $v) {
445+
$violations[] = $v;
446+
}
447+
} elseif (is_array($value)) {
448+
foreach ($value as $i => $item) {
449+
if (is_object($item)) {
450+
foreach ($this->validateExtensionContexts($item, $subFhirPath, $subRelPath . '[' . $i . ']', $visited) as $v) {
451+
$violations[] = $v;
452+
}
453+
}
454+
}
388455
}
389456
}
390457

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ardenexal\FHIRTools\Component\Validation\Tests\Unit;
6+
7+
use Ardenexal\FHIRTools\Component\FHIRPath\Service\FHIRPathService;
8+
use Ardenexal\FHIRTools\Component\Metadata\Attribute\Validation\FHIRExtensionContext;
9+
use Ardenexal\FHIRTools\Component\Validation\FHIRValidationService;
10+
use Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture\NestedContactWithExtensionsFixture;
11+
use Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture\PatientContactOnlyExtensionFixture;
12+
use Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture\PatientNameOnlyExtensionFixture;
13+
use Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture\PatientPermittedExtensionFixture;
14+
use Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture\PatientWithContactResourceFixture;
15+
use PHPUnit\Framework\TestCase;
16+
use Symfony\Component\Validator\ConstraintViolationList;
17+
use Symfony\Component\Validator\Validator\ValidatorInterface;
18+
19+
final class FHIRExtensionContextNestedValidationTest extends TestCase
20+
{
21+
private FHIRValidationService $service;
22+
23+
protected function setUp(): void
24+
{
25+
$validator = $this->createStub(ValidatorInterface::class);
26+
$validator->method('validate')->willReturn(new ConstraintViolationList());
27+
28+
$this->service = new FHIRValidationService($validator, new FHIRPathService());
29+
}
30+
31+
public function testNestedExtensionRestrictedToSiblingPathProducesError(): void
32+
{
33+
// PatientNameOnlyExtensionFixture context is "Patient.name".
34+
// Placed on Patient.contact — a sibling path — it must produce a violation.
35+
$contact = new NestedContactWithExtensionsFixture([new PatientNameOnlyExtensionFixture()]);
36+
$resource = new PatientWithContactResourceFixture(contact: [$contact]);
37+
38+
$report = $this->service->validate($resource);
39+
40+
self::assertCount(1, $report->errors(), 'Extension restricted to Patient.name must fail on Patient.contact');
41+
42+
$violation = $report->errors()[0];
43+
self::assertSame('error', $violation->severity);
44+
self::assertSame('contact[0].extension', $violation->path);
45+
self::assertSame(FHIRExtensionContext::class, $violation->constraintClass);
46+
self::assertStringContainsString('Patient.contact', $violation->message);
47+
self::assertStringContainsString('http://example.org/ext/patient-name-only', $violation->message);
48+
}
49+
50+
public function testNestedExtensionMatchingContactPathProducesNoViolation(): void
51+
{
52+
// PatientContactOnlyExtensionFixture context is "Patient.contact".
53+
// Placed on Patient.contact — it must be permitted.
54+
$contact = new NestedContactWithExtensionsFixture([new PatientContactOnlyExtensionFixture()]);
55+
$resource = new PatientWithContactResourceFixture(contact: [$contact]);
56+
57+
$report = $this->service->validate($resource);
58+
59+
self::assertCount(0, $report->errors(), 'Extension permitted on Patient.contact must produce no violation');
60+
}
61+
62+
public function testNestedExtensionWithBareResourceTypeContextIsDeferredProducesNoViolation(): void
63+
{
64+
// PatientPermittedExtensionFixture context is "Patient" (no dot).
65+
// Bare-type contexts at sub-element level are deferred (require type-hierarchy resolution).
66+
$contact = new NestedContactWithExtensionsFixture([new PatientPermittedExtensionFixture()]);
67+
$resource = new PatientWithContactResourceFixture(contact: [$contact]);
68+
69+
$report = $this->service->validate($resource);
70+
71+
self::assertCount(0, $report->errors(), 'Bare resource-type contexts at sub-element level must be deferred');
72+
}
73+
74+
public function testMultipleContactsOnlyForbiddenOneProducesOneErrorWithCorrectIndex(): void
75+
{
76+
$permitted = new NestedContactWithExtensionsFixture([new PatientContactOnlyExtensionFixture()]);
77+
$forbidden = new NestedContactWithExtensionsFixture([new PatientNameOnlyExtensionFixture()]);
78+
$resource = new PatientWithContactResourceFixture(contact: [$permitted, $forbidden]);
79+
80+
$report = $this->service->validate($resource);
81+
82+
self::assertCount(1, $report->errors());
83+
self::assertSame('contact[1].extension', $report->errors()[0]->path);
84+
}
85+
86+
public function testResourceWithEmptyContactArrayProducesNoViolations(): void
87+
{
88+
$resource = new PatientWithContactResourceFixture();
89+
90+
$report = $this->service->validate($resource);
91+
92+
self::assertCount(0, $report->errors());
93+
}
94+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture;
6+
7+
use Ardenexal\FHIRTools\Component\Metadata\Contract\FHIRExtensionInterface;
8+
use Ardenexal\FHIRTools\Component\Metadata\Traits\FHIRExtensionsTrait;
9+
10+
final class NestedContactWithExtensionsFixture
11+
{
12+
use FHIRExtensionsTrait;
13+
14+
/** @param list<FHIRExtensionInterface> $extension */
15+
public function __construct(
16+
private readonly array $extension = [],
17+
) {
18+
}
19+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture;
6+
7+
use Ardenexal\FHIRTools\Component\Metadata\Attribute\Validation\FHIRExtensionContext;
8+
use Ardenexal\FHIRTools\Component\Metadata\Contract\FHIRExtensionInterface;
9+
10+
#[FHIRExtensionContext(type: 'element', expression: 'Patient.contact')]
11+
final class PatientContactOnlyExtensionFixture implements FHIRExtensionInterface
12+
{
13+
public function getExtensionUrl(): ?string
14+
{
15+
return 'http://example.org/ext/patient-contact-only';
16+
}
17+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture;
6+
7+
use Ardenexal\FHIRTools\Component\Metadata\Attribute\Validation\FHIRExtensionContext;
8+
use Ardenexal\FHIRTools\Component\Metadata\Contract\FHIRExtensionInterface;
9+
10+
#[FHIRExtensionContext(type: 'element', expression: 'Patient.name')]
11+
final class PatientNameOnlyExtensionFixture implements FHIRExtensionInterface
12+
{
13+
public function getExtensionUrl(): ?string
14+
{
15+
return 'http://example.org/ext/patient-name-only';
16+
}
17+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Ardenexal\FHIRTools\Component\Validation\Tests\Unit\Fixture;
6+
7+
use Ardenexal\FHIRTools\Component\Metadata\Attribute\FhirResource;
8+
use Ardenexal\FHIRTools\Component\Metadata\Contract\FHIRExtensionInterface;
9+
use Ardenexal\FHIRTools\Component\Metadata\Traits\FHIRExtensionsTrait;
10+
11+
#[FhirResource(type: 'Patient', version: '4.0.1', url: 'http://hl7.org/fhir/StructureDefinition/Patient', fhirVersion: 'R4')]
12+
final class PatientWithContactResourceFixture
13+
{
14+
use FHIRExtensionsTrait;
15+
16+
/**
17+
* @param list<object> $contact
18+
* @param list<FHIRExtensionInterface> $extension
19+
*/
20+
public function __construct(
21+
public readonly array $contact = [],
22+
private readonly array $extension = [],
23+
) {
24+
}
25+
}

0 commit comments

Comments
 (0)