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

Added UUID property describer #2098

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

stollr
Copy link
Contributor

@stollr stollr commented Apr 28, 2023

This describer adds support for UUIDs from the following libraries: symfony/uid and ramsey/uuid

This makes types of these classes configured as

"type":  "string",
"format": "uuid",
"pattern": "^[0-9a-f]{8}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{12}$"

This is based on the spec.

DependencyInjection/NelmioApiDocExtension.php Outdated Show resolved Hide resolved
PropertyDescriber/UuidPropertyDescriber.php Outdated Show resolved Hide resolved
PropertyDescriber/UuidPropertyDescriber.php Outdated Show resolved Hide resolved
PropertyDescriber/UuidPropertyDescriber.php Outdated Show resolved Hide resolved
@DjordyKoert
Copy link
Collaborator

DjordyKoert commented Jan 9, 2024

I would also like to see some tests for this

@DjordyKoert DjordyKoert mentioned this pull request Jan 9, 2024
11 tasks
@stollr
Copy link
Contributor Author

stollr commented Jan 10, 2024

I'll try to add some.

Copy link
Contributor

@DominicLuidold DominicLuidold left a comment

Choose a reason for hiding this comment

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

@stollr Any help needed with implementing test cases for this? 😃

DependencyInjection/NelmioApiDocExtension.php Outdated Show resolved Hide resolved
PropertyDescriber/UuidPropertyDescriber.php Outdated Show resolved Hide resolved
@stollr
Copy link
Contributor Author

stollr commented Apr 2, 2024

@DominicLuidold I have rebased the PR to current master. But running the tests, I get

  1. Nelmio\ApiDocBundle\Tests\Command\DumpCommandTest::testJson with data set "pretty print" (array(), 192)
    Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: The service "sensio_framework_extra.controller.listener" has a dependency on a non-existent service "annotation_reader".

Any idea how to fix this?

Update:
Nevermind. After looking at the continuous-integration.yml I understood that I have to composer remove friendsofsymfony/rest-bundle sensio/framework-extra-bundle --no-update --dev and composer update to fix the issue. ;-)

@DominicLuidold
Copy link
Contributor

@stollr Let me know if I can help in any other way 😄

@stollr stollr force-pushed the uuid_describer branch 2 times, most recently from e5f22fb to 3cd85c0 Compare April 2, 2024 22:04
@stollr
Copy link
Contributor Author

stollr commented Apr 2, 2024

@DominicLuidold I have added a functional and some unit tests and hope it is correct the way I did it.
Please let me know, if anything is missing or wrong.

composer.json Outdated Show resolved Hide resolved
src/DependencyInjection/NelmioApiDocExtension.php Outdated Show resolved Hide resolved
tests/Functional/Controller/ApiController81.php Outdated Show resolved Hide resolved
tests/Functional/Controller/ApiController80.php Outdated Show resolved Hide resolved
tests/Functional/FunctionalTest.php Outdated Show resolved Hide resolved
tests/PropertyDescriber/UuidPropertyDescriberTest.php Outdated Show resolved Hide resolved
@stollr
Copy link
Contributor Author

stollr commented Apr 28, 2024

@DjordyKoert I have addressed all comments and rebased the PR on master. I hope it is ready for merge, now :-)

@DjordyKoert
Copy link
Collaborator

@DjordyKoert I have addressed all comments and rebased the PR on master. I hope it is ready for merge, now :-)

Thanks I will try and have a look next week :)

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.

None yet

3 participants