Skip to content

Fix BC-break when using short-syntax notation for access_control #3113

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

Merged
merged 3 commits into from
Sep 27, 2019

Conversation

antograssiot
Copy link
Contributor

@antograssiot antograssiot commented Sep 27, 2019

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

This BC-break as introduced in #2992

When using the accesControl attribute directly in the ApiRessource annotation, the AttributeHydratorTrait would throw the following:

Unknown property "accessControl" on annotation "ApiPlatform\Core\Annotation  \ApiResource". 

if (!property_exists($this, $key)) {
throw new InvalidArgumentException(sprintf('Unknown property "%s" on annotation "%s".', $key, self::class));
}

@teohhanhui
Copy link
Contributor

This must target 2.5 instead of master.

@antograssiot
Copy link
Contributor Author

another option would be to update directly the AttributeExtractorTrait with something like, it would allow to trigger proper deprecation, WDYT ?

        if (array_key_exists('accessControl', $values)) {
            $values['security'] = $values['accessControl'];
            @trigger_error('Attribute "accessControl" is deprecated in annotation since API Platform 2.5, prefer using "security" attribute instead', E_USER_DEPRECATED);
            unset($values['accessControl']);
        }
        if (array_key_exists('accessControlMessage', $values)) {
            $values['securityMessage'] = $values['accessControlMessage'];
            @trigger_error('Attribute "accessControlMessage" is deprecated in annotation since API Platform 2.5, prefer using "securityMessage" attribute instead', E_USER_DEPRECATED);
            unset($values['accessControlMessage']);
        }

@antograssiot antograssiot changed the base branch from master to 2.5 September 27, 2019 09:06
@teohhanhui
Copy link
Contributor

You should cherry pick your commit. We don't want the commits from master to go into 2.5.

@antograssiot
Copy link
Contributor Author

You should cherry pick your commit. We don't want the commits from master to go into 2.5.

I know it was ongoing ;)

@antograssiot
Copy link
Contributor Author

I've updated the test to add @group legacy to avoid the folllowing in PHPUNIT, but it is not directly related and I don't like it very much... is there any other way to silence them ?

Remaining self deprecation notices (4)

  2x: Attribute "accessControl" is deprecated in annotation since API Platform 2.5, prefer using "security" attribute instead
    1x in BooleanFilterTest::setUp from ApiPlatform\Core\Tests\Bridge\Doctrine\Orm\Filter
    1x in SwaggerCommandTest::testExecuteWithAliasVersion3 from ApiPlatform\Core\Tests\Bridge\Symfony\Bundle\Command

  2x: Attribute "accessControlMessage" is deprecated in annotation since API Platform 2.5, prefer using "securityMessage" attribute instead
    1x in BooleanFilterTest::setUp from ApiPlatform\Core\Tests\Bridge\Doctrine\Orm\Filter
    1x in SwaggerCommandTest::testExecuteWithAliasVersion3 from ApiPlatform\Core\Tests\Bridge\Symfony\Bundle\Command

@soyuka soyuka merged commit 9a6853f into api-platform:2.5 Sep 27, 2019
@soyuka
Copy link
Member

soyuka commented Sep 27, 2019

thanks @antograssiot !

@antograssiot antograssiot deleted the security-bc-break branch September 29, 2019 17:16
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.

3 participants