Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(state): add headers to comply with LDP specification #6917

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
style: apply php-cs-fixer
LaurentHuzard committed Jan 15, 2025
commit 2f5e432504fcba39af0e28742ac89ec64b50022c
2 changes: 1 addition & 1 deletion src/State/Processor/RespondProcessor.php
Original file line number Diff line number Diff line change
@@ -168,7 +168,7 @@ private function getAllowedMethods(?string $resourceClass, ?string $currentUriTe
{
$allowedMethods = self::DEFAULT_ALLOWED_METHOD;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should filter here the operations with a different URI Template than the one of the current operation to fix the issue described in my 1st comment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point—I’ll sort this out.

if (null !== $currentUriTemplate && null !== $resourceClass && $this->resourceClassResolver->isResourceClass($resourceClass)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (null !== $currentUriTemplate && null !== $resourceClass && $this->resourceClassResolver->isResourceClass($resourceClass)) {
if (null !== $currentUriTemplate && null !== $resourceClass && $this->resourceClassResolver?->isResourceClass($resourceClass)) {

$resourceMetadataCollection =$this->resourceCollectionMetadataFactory ? $this->resourceCollectionMetadataFactory->create($resourceClass) : new ResourceMetadataCollection($resourceClass);
$resourceMetadataCollection = $this->resourceCollectionMetadataFactory ? $this->resourceCollectionMetadataFactory->create($resourceClass) : new ResourceMetadataCollection($resourceClass);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$resourceMetadataCollection = $this->resourceCollectionMetadataFactory ? $this->resourceCollectionMetadataFactory->create($resourceClass) : new ResourceMetadataCollection($resourceClass);
$resourceMetadataCollection = $this->resourceCollectionMetadataFactory ? $this->resourceCollectionMetadataFactory->create($resourceClass) : new ResourceMetadataCollection($resourceClass);

Better not set the extra headers at all if the factory is missing.

It's an optional feature after all.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it's an optional feature, should I add a configuration variable to enable it?

foreach ($resourceMetadataCollection as $resource) {
foreach ($resource->getOperations() as $operation) {
if ($operation->getUriTemplate() === $currentUriTemplate) {