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

add api-platform for a more complete API #1491

Open
tacman opened this issue Nov 20, 2024 · 5 comments
Open

add api-platform for a more complete API #1491

tacman opened this issue Nov 20, 2024 · 5 comments

Comments

@tacman
Copy link
Contributor

tacman commented Nov 20, 2024

I submitted a PR to fix #1488 , but it feels clunky. We can get pagination, filtering and sorting with a consistent and well-loved API by configuring API platform.

Since /api is already used, we could use /api2 or /apip for the route. Then with very little work we can search by multiple vendors or types, paginate, etc.

image

I'm happy to make a PR with this, maybe mark it as experimental during development?

I just submitted a PR to fix an api platform issue (symfony/symfony#58928 (comment)) but with that fix, I still can't get it to work because of a nelmio_security.yaml setting, without commenting this out.

#            script-src:
#                - 'unsafe-eval' # TODO get rid of this, but it requires getting rid of hogan (part of instantsearch, maybe upgrade to v4 will fix this)
#                - 'https://www.gstatic.com/recaptcha/' # TODO could be replaced by simply 'https:' for simplicity's sake once strict-dynamic support is more broadly available 75% in early 2022 per https://caniuse.com/?search=csp%20strict-dynamic
#                - 'strict-dynamic'

I also had to tweak the stateless setting. Both of these are small.

Is the packagist data available as a dump for testing?

@stof
Copy link
Contributor

stof commented Nov 20, 2024

How would this scale for the amount of packages present in packagist.org ?

Currently, the recommendation for tools needing to access the metadata of packages is to use the composer repository endpoints instead of the API, as this scales much better (the composer repository is served from static files by the webserver). The API endpoint for package currently has a 12h cache to avoid scalability issues (see https://packagist.org/apidoc#get-package-data).
That's also one of the reasons why the listing endpoint is only listing package names, not full packages.

I just submitted a PR to fix an api platform issue (symfony/symfony#58928 (comment)) but with that fix, I still can't get it to work because of a nelmio_security.yaml setting, without commenting this out.

You should ask the API Platform team how to make their Swagger UI compatible with CSP instead of disabling CSP on packagist.org.

@tacman
Copy link
Contributor Author

tacman commented Nov 20, 2024

How would this scale for the amount of packages present in packagist.org ?

I don't know. One of the reasons I get involved with projects like this is to learn about how big "real world" projects handle scaling. For example, I use redis in a pretty superficial way, but in going through this code I learned a bit more about how it can be used.

@stof
Copy link
Contributor

stof commented Nov 20, 2024

Also, I'm not sure we need API Platform here, given that the Packagist API is mostly read-only, and the few writable endpoints (creating a package, editing a package and updating a package) have special needs and are likely easier to implement outside API Platform

@stof
Copy link
Contributor

stof commented Nov 20, 2024

@tacman given that the usage of fields switches the list.json endpoint to a different response structure (which might have been better as a separate URL instead, but this is too late for that), the knplabs/packagist-api package should be made aware of that (and maybe exposing the usage of fields through a different method instead, to allow having clean return types in the PHP API).

@Seldaek
Copy link
Member

Seldaek commented Nov 27, 2024

Yeah I'm not really too keen on adding this just for the few benefits, given the little API surface we have, I'd rather keep the dependencies to a minimum as it all incurs maintenance cost, as well as security risk.

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

No branches or pull requests

3 participants