Skip to content

Commit

Permalink
H-3389: Return better status codes for data type and property type API (
Browse files Browse the repository at this point in the history 
#5299)
  • Loading branch information
@TimDiekmann
TimDiekmann authored Oct 2, 2024
1 parent 47bbebc commit 39238ce
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 161 deletions.
98 changes: 32 additions & 66 deletions apps/hash-graph/libs/api/src/rest/data_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use graph::{
patch_id_and_parse,
},
store::{
BaseUrlAlreadyExists, DataTypeStore, OntologyVersionDoesNotExist, StorePool,
DataTypeStore, OntologyVersionDoesNotExist, StorePool,
error::VersionedUrlAlreadyExists,
ontology::{
ArchiveDataTypeParams, CreateDataTypeParams, GetDataTypeSubgraphParams,
Expand Down Expand Up @@ -204,24 +204,21 @@ async fn create_data_type<S, A>(
temporal_client: Extension<Option<Arc<TemporalClient>>>,
domain_validator: Extension<DomainValidator>,
body: Json<CreateDataTypeRequest>,
) -> Result<Json<ListOrValue<DataTypeMetadata>>, StatusCode>
) -> Result<Json<ListOrValue<DataTypeMetadata>>, Response>
where
S: StorePool + Send + Sync,
for<'pool> S::Store<'pool, A::Api<'pool>>: RestApiStore,
A: AuthorizationApiPool + Send + Sync,
{
let authorization_api = authorization_api_pool.acquire().await.map_err(|error| {
tracing::error!(?error, "Could not acquire access to the authorization API");
StatusCode::INTERNAL_SERVER_ERROR
})?;
let authorization_api = authorization_api_pool
.acquire()
.await
.map_err(report_to_response)?;

let mut store = store_pool
.acquire(authorization_api, temporal_client.0)
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not acquire store");
StatusCode::INTERNAL_SERVER_ERROR
})?;
.map_err(report_to_response)?;

let Json(CreateDataTypeRequest {
schema,
Expand All @@ -236,37 +233,26 @@ where
let mut metadata = store
.create_data_types(
actor_id,
schema.into_iter().map(|schema| {
domain_validator.validate(&schema).map_err(|report| {
tracing::error!(error=?report, id=%schema.id, "Data Type ID failed to validate");
StatusCode::UNPROCESSABLE_ENTITY
})?;

Ok(CreateDataTypeParams {
schema,
classification: OntologyTypeClassificationMetadata::Owned { owned_by_id },
relationships: relationships.clone(),
conflict_behavior: ConflictBehavior::Fail,
provenance: provenance.clone(),
conversions: conversions.clone(),
schema
.into_iter()
.map(|schema| {
domain_validator
.validate(&schema)
.map_err(report_to_response)?;

Ok(CreateDataTypeParams {
schema,
classification: OntologyTypeClassificationMetadata::Owned { owned_by_id },
relationships: relationships.clone(),
conflict_behavior: ConflictBehavior::Fail,
provenance: provenance.clone(),
conversions: conversions.clone(),
})
})
}).collect::<Result<Vec<_>, StatusCode>>()?
.collect::<Result<Vec<_>, Response>>()?,
)
.await
.map_err(|report| {
// TODO: consider adding the data type, or at least its URL in the trace
tracing::error!(error=?report, "Could not create data types");

if report.contains::<PermissionAssertion>() {
return StatusCode::FORBIDDEN;
}
if report.contains::<BaseUrlAlreadyExists>() {
return StatusCode::CONFLICT;
}

// Insertion/update errors are considered internal server errors.
StatusCode::INTERNAL_SERVER_ERROR
})?;
.map_err(report_to_response)?;

if is_list {
Ok(Json(ListOrValue::List(metadata)))
Expand Down Expand Up @@ -562,7 +548,7 @@ async fn update_data_type<S, A>(
authorization_api_pool: Extension<Arc<A>>,
temporal_client: Extension<Option<Arc<TemporalClient>>>,
body: Json<UpdateDataTypeRequest>,
) -> Result<Json<DataTypeMetadata>, StatusCode>
) -> Result<Json<DataTypeMetadata>, Response>
where
S: StorePool + Send + Sync,
A: AuthorizationApiPool + Send + Sync,
Expand All @@ -577,25 +563,17 @@ where

type_to_update.version = OntologyTypeVersion::new(type_to_update.version.inner() + 1);

let data_type = patch_id_and_parse(&type_to_update, schema).map_err(|report| {
tracing::error!(error=?report, "Couldn't patch schema and convert to Data Type");
StatusCode::UNPROCESSABLE_ENTITY
// TODO: We should probably return more information to the client
// see https://linear.app/hash/issue/H-3009
})?;
let data_type = patch_id_and_parse(&type_to_update, schema).map_err(report_to_response)?;

let authorization_api = authorization_api_pool.acquire().await.map_err(|error| {
tracing::error!(?error, "Could not acquire access to the authorization API");
StatusCode::INTERNAL_SERVER_ERROR
})?;
let authorization_api = authorization_api_pool
.acquire()
.await
.map_err(report_to_response)?;

let mut store = store_pool
.acquire(authorization_api, temporal_client.0)
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not acquire store");
StatusCode::INTERNAL_SERVER_ERROR
})?;
.map_err(report_to_response)?;

store
.update_data_type(actor_id, UpdateDataTypesParams {
Expand All @@ -605,19 +583,7 @@ where
conversions,
})
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not update data type");

if report.contains::<PermissionAssertion>() {
return StatusCode::FORBIDDEN;
}
if report.contains::<OntologyVersionDoesNotExist>() {
return StatusCode::NOT_FOUND;
}

// Insertion/update errors are considered internal server errors.
StatusCode::INTERNAL_SERVER_ERROR
})
.map_err(report_to_response)
.map(Json)
}

Expand Down
37 changes: 8 additions & 29 deletions apps/hash-graph/libs/api/src/rest/entity_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ async fn update_entity_type<S, A>(
authorization_api_pool: Extension<Arc<A>>,
temporal_client: Extension<Option<Arc<TemporalClient>>>,
body: Json<UpdateEntityTypeRequest>,
) -> Result<Json<EntityTypeMetadata>, StatusCode>
) -> Result<Json<EntityTypeMetadata>, Response>
where
S: StorePool + Send + Sync,
A: AuthorizationApiPool + Send + Sync,
Expand All @@ -871,26 +871,17 @@ where

type_to_update.version = OntologyTypeVersion::new(type_to_update.version.inner() + 1);

let entity_type = patch_id_and_parse(&type_to_update, schema).map_err(|report| {
tracing::error!(error=?report, "Couldn't convert schema to Entity Type");
// TODO: Consider an UNPROCESSABLE_ENTITY_TYPE code?
StatusCode::UNPROCESSABLE_ENTITY
// TODO: We should probably return more information to the client
// see https://linear.app/hash/issue/H-3009
})?;
let entity_type = patch_id_and_parse(&type_to_update, schema).map_err(report_to_response)?;

let authorization_api = authorization_api_pool.acquire().await.map_err(|error| {
tracing::error!(?error, "Could not acquire access to the authorization API");
StatusCode::INTERNAL_SERVER_ERROR
})?;
let authorization_api = authorization_api_pool
.acquire()
.await
.map_err(report_to_response)?;

let mut store = store_pool
.acquire(authorization_api, temporal_client.0)
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not acquire store");
StatusCode::INTERNAL_SERVER_ERROR
})?;
.map_err(report_to_response)?;

store
.update_entity_type(actor_id, UpdateEntityTypesParams {
Expand All @@ -901,19 +892,7 @@ where
provenance,
})
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not update entity type");

if report.contains::<PermissionAssertion>() {
return StatusCode::FORBIDDEN;
}
if report.contains::<OntologyVersionDoesNotExist>() {
return StatusCode::NOT_FOUND;
}

// Insertion/update errors are considered internal server errors.
StatusCode::INTERNAL_SERVER_ERROR
})
.map_err(report_to_response)
.map(Json)
}

Expand Down
96 changes: 31 additions & 65 deletions apps/hash-graph/libs/api/src/rest/property_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use graph::{
patch_id_and_parse,
},
store::{
BaseUrlAlreadyExists, OntologyVersionDoesNotExist, PropertyTypeStore, StorePool,
OntologyVersionDoesNotExist, PropertyTypeStore, StorePool,
error::VersionedUrlAlreadyExists,
ontology::{
ArchivePropertyTypeParams, CreatePropertyTypeParams, GetPropertyTypeSubgraphParams,
Expand Down Expand Up @@ -198,24 +198,21 @@ async fn create_property_type<S, A>(
temporal_client: Extension<Option<Arc<TemporalClient>>>,
domain_validator: Extension<DomainValidator>,
body: Json<CreatePropertyTypeRequest>,
) -> Result<Json<ListOrValue<PropertyTypeMetadata>>, StatusCode>
) -> Result<Json<ListOrValue<PropertyTypeMetadata>>, Response>
where
S: StorePool + Send + Sync,
for<'pool> S::Store<'pool, A::Api<'pool>>: RestApiStore,
A: AuthorizationApiPool + Send + Sync,
{
let authorization_api = authorization_api_pool.acquire().await.map_err(|error| {
tracing::error!(?error, "Could not acquire access to the authorization API");
StatusCode::INTERNAL_SERVER_ERROR
})?;
let authorization_api = authorization_api_pool
.acquire()
.await
.map_err(report_to_response)?;

let mut store = store_pool
.acquire(authorization_api, temporal_client.0)
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not acquire store");
StatusCode::INTERNAL_SERVER_ERROR
})?;
.map_err(report_to_response)?;

let Json(CreatePropertyTypeRequest {
schema,
Expand All @@ -229,36 +226,25 @@ where
let mut metadata = store
.create_property_types(
actor_id,
schema.into_iter().map(|schema| {
domain_validator.validate(&schema).map_err(|report| {
tracing::error!(error=?report, id=%schema.id, "Property Type ID failed to validate");
StatusCode::UNPROCESSABLE_ENTITY
})?;

Ok(CreatePropertyTypeParams {
schema,
classification: OntologyTypeClassificationMetadata::Owned { owned_by_id },
relationships: relationships.clone(),
conflict_behavior: ConflictBehavior::Fail,
provenance: provenance.clone()
schema
.into_iter()
.map(|schema| {
domain_validator
.validate(&schema)
.map_err(report_to_response)?;

Ok(CreatePropertyTypeParams {
schema,
classification: OntologyTypeClassificationMetadata::Owned { owned_by_id },
relationships: relationships.clone(),
conflict_behavior: ConflictBehavior::Fail,
provenance: provenance.clone(),
})
})
}).collect::<Result<Vec<_>, StatusCode>>()?
.collect::<Result<Vec<_>, Response>>()?,
)
.await
.map_err(|report| {
// TODO: consider adding the data type, or at least its URL in the trace
tracing::error!(error=?report, "Could not create data types");

if report.contains::<PermissionAssertion>() {
return StatusCode::FORBIDDEN;
}
if report.contains::<BaseUrlAlreadyExists>() {
return StatusCode::CONFLICT;
}

// Insertion/update errors are considered internal server errors.
StatusCode::INTERNAL_SERVER_ERROR
})?;
.map_err(report_to_response)?;

if is_list {
Ok(Json(ListOrValue::List(metadata)))
Expand Down Expand Up @@ -551,7 +537,7 @@ async fn update_property_type<S, A>(
authorization_api_pool: Extension<Arc<A>>,
temporal_client: Extension<Option<Arc<TemporalClient>>>,
body: Json<UpdatePropertyTypeRequest>,
) -> Result<Json<PropertyTypeMetadata>, StatusCode>
) -> Result<Json<PropertyTypeMetadata>, Response>
where
S: StorePool + Send + Sync,
A: AuthorizationApiPool + Send + Sync,
Expand All @@ -565,25 +551,17 @@ where

type_to_update.version = OntologyTypeVersion::new(type_to_update.version.inner() + 1);

let property_type = patch_id_and_parse(&type_to_update, schema).map_err(|report| {
tracing::error!(error=?report, "Couldn't patch schema and convert to Property Type");
StatusCode::UNPROCESSABLE_ENTITY
// TODO: We should probably return more information to the client
// see https://linear.app/hash/issue/H-3009
})?;
let property_type = patch_id_and_parse(&type_to_update, schema).map_err(report_to_response)?;

let authorization_api = authorization_api_pool.acquire().await.map_err(|error| {
tracing::error!(?error, "Could not acquire access to the authorization API");
StatusCode::INTERNAL_SERVER_ERROR
})?;
let authorization_api = authorization_api_pool
.acquire()
.await
.map_err(report_to_response)?;

let mut store = store_pool
.acquire(authorization_api, temporal_client.0)
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not acquire store");
StatusCode::INTERNAL_SERVER_ERROR
})?;
.map_err(report_to_response)?;

store
.update_property_type(actor_id, UpdatePropertyTypesParams {
Expand All @@ -592,19 +570,7 @@ where
provenance,
})
.await
.map_err(|report| {
tracing::error!(error=?report, "Could not update property type");

if report.contains::<PermissionAssertion>() {
return StatusCode::FORBIDDEN;
}
if report.contains::<OntologyVersionDoesNotExist>() {
return StatusCode::NOT_FOUND;
}

// Insertion/update errors are considered internal server errors.
StatusCode::INTERNAL_SERVER_ERROR
})
.map_err(report_to_response)
.map(Json)
}

Expand Down
Loading

0 comments on commit 39238ce

Please sign in to comment.