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

Fix: Inconsistency in Context Concatenation with fetch_data #6050

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

anoziere
Copy link
Contributor

Q A
Branch? current stable version branch for bug fixes
Tickets Closes #2594
License MIT
Doc PR N/A

I have noticed an inconsistency in the ItemNormalizer.php file regarding the handling of the $context array in the updateObjectToPopulate method.

In the try block, the context is concatenated with ['fetch_data' => true] when calling getResourceFromIri. However, in the corresponding catch block for LegacyInvalidArgumentException|InvalidArgumentException, the same concatenation with fetch_data is not performed when setting the OBJECT_TO_POPULATE in the context:

// In try block
$context[self::OBJECT_TO_POPULATE] = $this->iriConverter->getResourceFromIri(
    (string) $data['id'], 
    $context + ['fetch_data' => true]
);

// In catch block
$context[self::OBJECT_TO_POPULATE] = $this->iriConverter->getResourceFromIri(
    $iri, 
    ['fetch_data' => true]
);

so I just added the context in the case of catch.

@soyuka soyuka changed the base branch from main to 3.2 December 18, 2023 08:56
@soyuka soyuka force-pushed the fix_context_inconsistency branch 2 times, most recently from 325d4d1 to af379ae Compare December 18, 2023 09:08
@soyuka soyuka changed the base branch from 3.2 to 3.1 December 18, 2023 10:00
@soyuka soyuka force-pushed the fix_context_inconsistency branch from af379ae to def5a9c Compare December 18, 2023 10:01
@soyuka
Copy link
Member

soyuka commented Dec 18, 2023

I'm really unsure about that, also you're using json right?

@anoziere
Copy link
Contributor Author

I'm really unsure about that, also you're using json right?

Yes, we encountered this issue while using JSON in a project where we integrated API Platform as an additional layer. We had to redo quite a bit (the project in question is based on Propel), but we faced this issue when using a PUT request and in the context of returning the modified object.

I don't understand why the context would be different in this case. Is there a specific constraint that leads to only having fetch_data in this case?

@soyuka soyuka force-pushed the fix_context_inconsistency branch from 7b4b3cc to 88976ff Compare December 19, 2023 15:31
@soyuka soyuka merged commit 9660a19 into api-platform:3.1 Dec 19, 2023
1 check passed
@soyuka
Copy link
Member

soyuka commented Dec 19, 2023

had to add a test and reword your commit please look at our commit rules inside CONTRIBUTING next time, thanks for the patch!

Romaixn pushed a commit to Romaixn/core that referenced this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistency in Context Concatenation with fetch_data in ItemNormalizer.php
2 participants