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

API docs and api initial v2 #615

Merged
merged 1 commit into from
Nov 13, 2023
Merged

API docs and api initial v2 #615

merged 1 commit into from
Nov 13, 2023

Conversation

datenangebot
Copy link
Collaborator

@datenangebot datenangebot commented Oct 12, 2023

  • add annotations for API routed methods
  • refactor to ensure type safety
  • add psalm types and usage
  • introduce new api v2 structure
  • refactor integration tests / add for api v2

@datenangebot datenangebot force-pushed the doc/api_docs_workshop branch from ef5366b to f525e06 Compare October 13, 2023 09:32
@datenangebot
Copy link
Collaborator Author

@datenangebot datenangebot marked this pull request as ready for review October 26, 2023 14:24
@datenangebot datenangebot changed the title API v2 API docs and api initial v2 Oct 26, 2023
@provokateurin
Copy link
Member

Here is a APIignore because of an psalm/api-docs error. It's known and in progress. https://github.com/nextcloud/tables/pull/615/files#diff-70e5309fcd3311d771b3db9c93490ea270fd5894769093765ec37edb68e5dd9bR24

I'm not sure what you mean? You shouldn't have @NoCSRFRequired on a method that serves the frontend files. The openapi-extractor detects this as reachable from the outside because it is in fact reachable from the outside.

@github-actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the reviewing process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR reviewing process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

@juliusknorr
Copy link
Member

@juliushaertl I don't need this when using the ocs routes, right? #615 (files)

Yes, this is only needed for CORS annotated controllers, not for OCS ones.

Here is a APIignore because of an psalm/api-docs error. It's known and in progress. #615 (files)

I'm not sure what you mean? You shouldn't have @NoCSRFRequired on a method that serves the frontend files. The openapi-extractor detects this as reachable from the outside because it is in fact reachable from the outside.

Why not? The page controller that serves the page doesn't get a CSRF token passed by the browser. This is one of the rare cases that should have the @NoCSRFRequired annotation. ;)

lib/Db/Column.php Outdated Show resolved Hide resolved
@datenangebot
Copy link
Collaborator Author

datenangebot commented Oct 27, 2023

@provokateurin for testing:

I guess this
https://github.com/nextcloud/tables/blob/doc/api_docs_workshop/lib/Controller/ApiColumnsController.php#L85
is not correct rendered with this result
https://github.com/nextcloud/tables/blob/doc/api_docs_workshop/openapi.json#L5957-L5965

  • add tests for the new column creation endpoints

@datenangebot
Copy link
Collaborator Author

datenangebot commented Oct 27, 2023

@github-actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the reviewing process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR reviewing process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

@datenangebot
Copy link
Collaborator Author

@juliushaertl @provokateurin Do you think this is a proper way to handle this kind of parameter right now? https://github.com/nextcloud/tables/blob/doc/api_docs_workshop/lib/Controller/ApiColumnsController.php#L212-L213

(The api extractor would not parse a deep array structure description directly.)

@juliusknorr
Copy link
Member

@juliushaertl @provokateurin Do you think this is a proper way to handle this kind of parameter right now? doc/api_docs_workshop/lib/Controller/ApiColumnsController.php#L212-L213

(The api extractor would not parse a deep array structure description directly.)

Wouldn't it be possible to define a custom type as it is actually an object json structure and not an array?

Btw. feel free to also leave such comments directly as as review comment, then such discussions are directly linked to the code and can be threaded/resolved a bit easier as a individual topic ;)

@provokateurin
Copy link
Member

Wouldn't it be possible to define a custom type as it is actually an object json structure and not an array?

This is possible, but there is no way to tell psalm that a string is actually a json encoded object. So the structure itself can be properly documented, just not that it is used here (except for a comment).

@datenangebot datenangebot force-pushed the doc/api_docs_workshop branch 2 times, most recently from 35e84f1 to 74414cb Compare November 6, 2023 07:14
- adjust setup to make use of the new API extractor
- add all needed types to extract API v1
- setup structure and first endpoints for API v2

add selection column description and test it

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

fix php 7.4 backwards compatibility

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

add tests for basic column creations

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

add a endpoint to request column objects

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

update openapi.json

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

make psalm return type more precise

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

fix psalm types recognition

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

fix route

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

cleanup debug info

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

Cleanup annotations

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

Hide sensitive data from showing to the users.

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

Refactor OCS-API-Controller to abstract class

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

Update lib/Db/Column.php

Co-authored-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Florian <florian.steffens@nextcloud.com>

Update lib/Controller/ApiGeneralController.php

Co-authored-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Florian <florian.steffens@nextcloud.com>

Update lib/Controller/ApiTablesController.php

Co-authored-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Florian <florian.steffens@nextcloud.com>

Update lib/Controller/ApiTablesController.php

Co-authored-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Florian <florian.steffens@nextcloud.com>

Update lib/Controller/MyOCSController.php

Co-authored-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Florian <florian.steffens@nextcloud.com>

add specific endpoints to create different columns by type
- fix error handling in ColumnService.php

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

add basic integration tests for the api v2 table endpoints

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

fix api routes & update API docs

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

cypress fix typo

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

initial new API setup
- introduce api v2
- organise API methods in own controllers
- Add virtual version tag to the names in docs
- use OCS routes and controllers

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

Setup skeleton to keep old api v1 and setup v2 with integration testing

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

try to use php8.1 for over all CI

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

add openapi.json

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

CI settings

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>

Make use of automated API doc generating
- add annotations for API routed methods
- refactor to ensure type safety
- add psalm types and usage
- correct API return codes ⚡️breaking changes ⚡️

Signed-off-by: Florian Steffens <florian.steffens@nextcloud.com>
@datenangebot datenangebot force-pushed the doc/api_docs_workshop branch from 74414cb to 046e7ec Compare November 9, 2023 10:07
@datenangebot datenangebot merged commit e53bc51 into main Nov 13, 2023
23 checks passed
@datenangebot datenangebot deleted the doc/api_docs_workshop branch November 13, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants