feat: Extend the project creation api in v4#766
Conversation
gtema
force-pushed
the
feature/v4-project-create
branch
from
82abb91 to
1fe84dc
Compare
| #[cfg_attr(feature = "builder", builder(default))] | ||
| #[cfg_attr(feature = "openapi", schema(inline, additional_properties))] | ||
| #[serde(flatten)] | ||
| //pub extra: ExtraFields, |
There was a problem hiding this comment.
please remove this commented line. I know it is the copy, just no sense to copy dead code over
| @@ -0,0 +1,270 @@ | |||
| use axum::{ | |||
There was a problem hiding this comment.
please add a license header as in other files
|
|
||
| // The Validator that check format of the project id if it given by user. | ||
| #[cfg(feature = "validate")] | ||
| fn validate_uuid(id: &str) -> Result<(), ValidationError> { |
There was a problem hiding this comment.
we are currently discussing this in https://review.opendev.org/c/openstack/keystone-specs/+/983440 One big problem is that we need to ensure it is a dash-less uuid. Otherwise we must reject it as bad request
| /// | ||
| /// Creates a project, where the project may act as a domain. | ||
| #[utoipa::path( | ||
| post, |
There was a problem hiding this comment.
we definitely need to add operation_id param otherwise the openapi spec clashes
| setter(strip_option, into) | ||
| ) | ||
| )] | ||
| #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] |
There was a problem hiding this comment.
additional attribute ..., derive(utoipa::ToSchema), schema(as ProjectCreateV4) is necessary to separate them in the openapi
|
|
||
| /// New project creation request. | ||
| #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] | ||
| #[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] |
There was a problem hiding this comment.
here you also need to "rename" the openapi schema with schema(as ProjectCreateRequestV4)
|
|
||
| pub(super) fn openapi_router() -> OpenApiRouter<ServiceState> { | ||
| OpenApiRouter::new() | ||
| .routes(routes!(create::create)) // v4-specific with optional id |
There was a problem hiding this comment.
problem with that merge is that it technically overwrites the new v3 handler. The unittest is not able to catch this you just mock the response. You would be only capable capturing this in the api tests. I would like to ask you to add explicit new tests into tests/api/src/resource/<maybe_project_v4> testing specifying the id
konac-hamza
force-pushed
the
feature/v4-project-create
branch
2 times, most recently
from
7b70946 to
bc1e3ea
Compare
| #[cfg(feature = "validate")] | ||
| fn validate_uuid(id: &str) -> Result<(), ValidationError> { | ||
| // Dashless UUIDs must be exactly 32 hex characters, no hyphens | ||
| let is_dashless = id.len() == 32 && !id.contains('-'); |
There was a problem hiding this comment.
this is not a sufficient validation: uuid regulates also which characters can be present. Most reasonable approach is to try to parse it as uuid (try_parse), get the 'simple' formatter and compare it matches the input
There was a problem hiding this comment.
Thanks for your review :). I will fix it as soon as possible.
konac-hamza
force-pushed
the
feature/v4-project-create
branch
from
9c17078 to
339e4f7
Compare
Signed-off-by: Hamza Konac <hamza.konac@tubitak.gov.tr>
Add dash-less uuid validator Signed-off-by: Hamza Konac <hamza.konac@tubitak.gov.tr>
Signed-off-by: Hamza Konac <hamza.konac@tubitak.gov.tr>
Signed-off-by: Hamza Konac <hamza.konac@tubitak.gov.tr>
Signed-off-by: Hamza Konac <hamza.konac@tubitak.gov.tr>
konac-hamza
force-pushed
the
feature/v4-project-create
branch
from
339e4f7 to
e9d51b7
Compare
2 participants
ProjectCreatetype with an optionalProjectIDfield and remaining v3 project types in theapi-typescratecreatenow delegate v3 routing to a public method defined in thekeystonecrate