diff --git a/apps/hash-api/src/graph/ontology/primitive/data-type.ts b/apps/hash-api/src/graph/ontology/primitive/data-type.ts index d271bdfe2ca..b45db4428bf 100644 --- a/apps/hash-api/src/graph/ontology/primitive/data-type.ts +++ b/apps/hash-api/src/graph/ontology/primitive/data-type.ts @@ -59,7 +59,7 @@ export const createDataType: ImpureGraphFunction< const shortname = webShortname ?? (await getWebShortname(ctx, authentication, { - accountOrAccountGroupId: params.webId, + accountOrAccountGroupId: webId, })); const { graphApi } = ctx; @@ -81,7 +81,6 @@ export const createDataType: ImpureGraphFunction< authentication.actorId, { schema, - webId, provenance: { ...ctx.provenance, ...params.provenance, diff --git a/apps/hash-api/src/graph/ontology/primitive/entity-type.ts b/apps/hash-api/src/graph/ontology/primitive/entity-type.ts index 0e788ba5795..4270d41d1f9 100644 --- a/apps/hash-api/src/graph/ontology/primitive/entity-type.ts +++ b/apps/hash-api/src/graph/ontology/primitive/entity-type.ts @@ -115,7 +115,6 @@ export const createEntityType: ImpureGraphFunction< const { data: metadata } = await graphApi.createEntityType( authentication.actorId, { - webId, schema, provenance: { ...ctx.provenance, diff --git a/apps/hash-api/src/graph/ontology/primitive/property-type.ts b/apps/hash-api/src/graph/ontology/primitive/property-type.ts index c43367c8e41..498bd5b4caa 100644 --- a/apps/hash-api/src/graph/ontology/primitive/property-type.ts +++ b/apps/hash-api/src/graph/ontology/primitive/property-type.ts @@ -69,7 +69,6 @@ export const createPropertyType: ImpureGraphFunction< const { data: metadata } = await graphApi.createPropertyType( authentication.actorId, { - webId, schema, provenance: { ...ctx.provenance, diff --git a/apps/hash-api/src/graph/ontology/primitive/util.ts b/apps/hash-api/src/graph/ontology/primitive/util.ts index c215969e749..04682e4e52d 100644 --- a/apps/hash-api/src/graph/ontology/primitive/util.ts +++ b/apps/hash-api/src/graph/ontology/primitive/util.ts @@ -1,11 +1,9 @@ import type { VersionedUrl, WebId } from "@blockprotocol/type-system"; -import { entityIdFromComponents } from "@blockprotocol/type-system"; +import { getWebById } from "@local/hash-graph-sdk/principal/web"; import { frontendUrl } from "@local/hash-isomorphic-utils/environment"; import { isSelfHostedInstance } from "@local/hash-isomorphic-utils/instance"; import type { ImpureGraphFunction } from "../../context-types"; -import { getOrgById } from "../../knowledge/system-types/org"; -import { getUser } from "../../knowledge/system-types/user"; export const isExternalTypeId = (typeId: VersionedUrl) => !typeId.startsWith(frontendUrl) && @@ -19,27 +17,19 @@ export const getWebShortname: ImpureGraphFunction< accountOrAccountGroupId: WebId; }, Promise -> = async (ctx, authentication, params) => { - const namespace = ( - (await getUser(ctx, authentication, { - entityId: entityIdFromComponents( - params.accountOrAccountGroupId, - params.accountOrAccountGroupId, - ), - })) ?? - (await getOrgById(ctx, authentication, { - entityId: entityIdFromComponents( - params.accountOrAccountGroupId, - params.accountOrAccountGroupId, - ), - }).catch(() => undefined)) - )?.shortname; - - if (!namespace) { - throw new Error( - `failed to get namespace for owner: ${params.accountOrAccountGroupId}`, - ); - } - - return namespace; -}; +> = (ctx, authentication, params) => + getWebById(ctx.graphApi, authentication, params.accountOrAccountGroupId).then( + (web) => { + if (!web) { + throw new Error( + `failed to get web for id: ${params.accountOrAccountGroupId}`, + ); + } + if (!web.shortname) { + throw new Error( + `Shortname is not set for web: ${params.accountOrAccountGroupId}`, + ); + } + return web.shortname; + }, + ); diff --git a/libs/@blockprotocol/type-system/rust/src/ontology/json_schema/domain_validator.rs b/libs/@blockprotocol/type-system/rust/src/ontology/json_schema/domain_validator.rs index 259b0c77533..be4a00dc727 100644 --- a/libs/@blockprotocol/type-system/rust/src/ontology/json_schema/domain_validator.rs +++ b/libs/@blockprotocol/type-system/rust/src/ontology/json_schema/domain_validator.rs @@ -30,10 +30,9 @@ pub trait ValidateOntologyType { fn validate(&self, ontology_type: &T) -> Result<(), Report>; } -#[expect(dead_code, reason = "We currently don't validate the shortname")] struct ShortNameAndKind<'a> { - pub short_name: &'a str, - pub kind: &'a str, + short_name: &'a str, + kind: &'a str, } /// Responsible for validating Type Urls against a known valid pattern. @@ -103,6 +102,19 @@ impl DomainValidator { Ok(ShortNameAndKind { short_name, kind }) } + + /// Extracts the shortname from a type URL. + /// + /// # Errors + /// + /// - [`DomainValidationError`], if the URL doesn't match or is missing the shortname capture + pub fn extract_shortname<'a>( + &'a self, + url: &'a str, + ) -> Result<&'a str, Report> { + self.extract_shortname_and_kind(url) + .map(|result| result.short_name) + } } impl ValidateOntologyType for DomainValidator { @@ -124,10 +136,6 @@ impl ValidateOntologyType for DomainValidator { }); } - // TODO: check that the user has write access to the shortname, this will require us - // making the graph aware of shortnames. We can store them alongside accountIds. We - // should not have to make the graph aware of User entities being a thing however. - // see https://linear.app/hash/issue/H-3010 Ok(()) } } @@ -151,10 +159,6 @@ impl ValidateOntologyType for DomainValidator { }); } - // TODO: check that the user has write access to the shortname, this will require us - // making the graph aware of shortnames. We can store them alongside accountIds. We - // should not have to make the graph aware of User entities being a thing however. - // see https://linear.app/hash/issue/H-3010 Ok(()) } } @@ -178,10 +182,6 @@ impl ValidateOntologyType for DomainValidator { }); } - // TODO: check that the user has write access to the shortname, this will require us - // making the graph aware of shortnames. We can store them alongside accountIds. We - // should not have to make the graph aware of User entities being a thing however. - // see https://linear.app/hash/issue/H-3010 Ok(()) } } diff --git a/libs/@local/graph/api/openapi/openapi.json b/libs/@local/graph/api/openapi/openapi.json index 747b1889656..e27a5e543fe 100644 --- a/libs/@local/graph/api/openapi/openapi.json +++ b/libs/@local/graph/api/openapi/openapi.json @@ -3756,7 +3756,6 @@ "type": "object", "required": [ "schema", - "webId", "provenance", "conversions" ], @@ -3782,9 +3781,6 @@ } } ] - }, - "webId": { - "$ref": "#/components/schemas/WebId" } }, "additionalProperties": false @@ -3858,7 +3854,6 @@ "type": "object", "required": [ "schema", - "webId", "provenance" ], "properties": { @@ -3877,9 +3872,6 @@ } } ] - }, - "webId": { - "$ref": "#/components/schemas/WebId" } }, "additionalProperties": false @@ -3908,7 +3900,6 @@ "type": "object", "required": [ "schema", - "webId", "provenance" ], "properties": { @@ -3927,9 +3918,6 @@ } } ] - }, - "webId": { - "$ref": "#/components/schemas/WebId" } }, "additionalProperties": false diff --git a/libs/@local/graph/api/src/rest/data_type.rs b/libs/@local/graph/api/src/rest/data_type.rs index ab0f81ab6df..17ffedc52bf 100644 --- a/libs/@local/graph/api/src/rest/data_type.rs +++ b/libs/@local/graph/api/src/rest/data_type.rs @@ -1,7 +1,7 @@ //! Web routes for CRU operations on Data Types. use alloc::sync::Arc; -use std::collections::{HashMap, HashSet}; +use std::collections::{HashMap, HashSet, hash_map}; use axum::{ Extension, Router, @@ -14,6 +14,7 @@ use hash_graph_postgres_store::{ store::error::{OntologyVersionDoesNotExist, VersionedUrlAlreadyExists}, }; use hash_graph_store::{ + account::AccountStore as _, data_type::{ ArchiveDataTypeParams, CreateDataTypeParams, DataTypeConversionTargets, DataTypeQueryToken, DataTypeStore, FindDataTypeConversionTargetsParams, FindDataTypeConversionTargetsResponse, @@ -143,7 +144,6 @@ impl DataTypeResource { struct CreateDataTypeRequest { #[schema(inline)] schema: MaybeListOfDataType, - web_id: WebId, provenance: ProvidedOntologyEditionProvenance, conversions: HashMap, } @@ -182,33 +182,53 @@ where let Json(CreateDataTypeRequest { schema, - web_id, provenance, conversions, }) = body; let is_list = matches!(&schema, ListOrValue::List(_)); + let mut web_cache = HashMap::::new(); + let mut params = Vec::new(); + for schema in schema { + domain_validator + .validate(&schema) + .map_err(report_to_response)?; + + let shortname = domain_validator + .extract_shortname(schema.id.base_url.as_str()) + .map_err(report_to_response)?; + + let web_id = match web_cache.entry(shortname.to_owned()) { + hash_map::Entry::Occupied(entry) => *entry.get(), + hash_map::Entry::Vacant(entry) => { + let web_id = store + .get_web_by_shortname(actor_id, shortname) + .await + .map_err(report_to_response)? + .ok_or_else(|| { + status_to_response(Status::<()>::new( + hash_status::StatusCode::NotFound, + Some(format!("No web found for shortname `{shortname}`")), + vec![], + )) + })? + .id; + *entry.insert(web_id) + } + }; + + params.push(CreateDataTypeParams { + schema, + ownership: OntologyOwnership::Local { web_id }, + conflict_behavior: ConflictBehavior::Fail, + provenance: provenance.clone(), + conversions: conversions.clone(), + }); + } + let mut metadata = store - .create_data_types( - actor_id, - schema - .into_iter() - .map(|schema| { - domain_validator - .validate(&schema) - .map_err(report_to_response)?; - - Ok(CreateDataTypeParams { - schema, - ownership: OntologyOwnership::Local { web_id }, - conflict_behavior: ConflictBehavior::Fail, - provenance: provenance.clone(), - conversions: conversions.clone(), - }) - }) - .collect::, BoxedResponse>>()?, - ) + .create_data_types(actor_id, params) .await .map_err(report_to_response)?; diff --git a/libs/@local/graph/api/src/rest/entity_type.rs b/libs/@local/graph/api/src/rest/entity_type.rs index 1a6e26f6654..850eb98e820 100644 --- a/libs/@local/graph/api/src/rest/entity_type.rs +++ b/libs/@local/graph/api/src/rest/entity_type.rs @@ -14,6 +14,7 @@ use hash_graph_postgres_store::{ store::error::{BaseUrlAlreadyExists, OntologyVersionDoesNotExist, VersionedUrlAlreadyExists}, }; use hash_graph_store::{ + account::AccountStore as _, entity_type::{ ArchiveEntityTypeParams, CommonQueryEntityTypesParams, CreateEntityTypeParams, EntityTypeQueryToken, EntityTypeResolveDefinitions, EntityTypeStore, @@ -170,7 +171,6 @@ where struct CreateEntityTypeRequest { #[schema(inline)] schema: MaybeListOfEntityType, - web_id: WebId, provenance: ProvidedOntologyEditionProvenance, } @@ -230,47 +230,69 @@ where )) })?; - let Json(CreateEntityTypeRequest { - schema, - web_id, - provenance, - }) = body; + let Json(CreateEntityTypeRequest { schema, provenance }) = body; let is_list = matches!(&schema, ListOrValue::List(_)); + let mut web_cache = HashMap::::new(); + let mut params = Vec::new(); + for schema in schema { + domain_validator.validate(&schema).map_err(|report| { + tracing::error!(error=?report, id=%schema.id, "Entity Type ID failed to validate"); + status_to_response(Status::new( + hash_status::StatusCode::InvalidArgument, + Some( + "Entity Type ID failed to validate against the given domain regex. Are you \ + sure the service is able to host a type under the domain you supplied?" + .to_owned(), + ), + vec![StatusPayloadInfo::Error(ErrorInfo::new( + HashMap::from([( + "entityTypeId".to_owned(), + serde_json::to_value(&schema.id) + .expect("Could not serialize entity type id"), + )]), + // TODO: We should encapsulate these Reasons within the type system, perhaps + // requiring top level contexts to implement a trait + // `ErrorReason::to_reason` or perhaps as a big enum + "INVALID_TYPE_ID".to_owned(), + ))], + )) + })?; + + let shortname = domain_validator + .extract_shortname(schema.id.base_url.as_str()) + .map_err(report_to_response)?; + + let web_id = match web_cache.entry(shortname.to_owned()) { + hash_map::Entry::Occupied(entry) => *entry.get(), + hash_map::Entry::Vacant(entry) => { + let web_id = store + .get_web_by_shortname(actor_id, shortname) + .await + .map_err(report_to_response)? + .ok_or_else(|| { + status_to_response(Status::new( + hash_status::StatusCode::NotFound, + Some(format!("No web found for shortname `{shortname}`")), + vec![], + )) + })? + .id; + *entry.insert(web_id) + } + }; + + params.push(CreateEntityTypeParams { + schema, + ownership: OntologyOwnership::Local { web_id }, + conflict_behavior: ConflictBehavior::Fail, + provenance: provenance.clone(), + }); + } + let mut metadata = store - .create_entity_types( - actor_id, - schema.into_iter().map(|schema| { - domain_validator.validate(&schema).map_err(|report| { - tracing::error!(error=?report, id=%schema.id, "Entity Type ID failed to validate"); - status_to_response(Status::new( - hash_status::StatusCode::InvalidArgument, - Some("Entity Type ID failed to validate against the given domain regex. Are you sure the service is able to host a type under the domain you supplied?".to_owned()), - vec![StatusPayloadInfo::Error(ErrorInfo::new( - HashMap::from([ - ( - "entityTypeId".to_owned(), - serde_json::to_value(&schema.id) - .expect("Could not serialize entity type id"), - ), - ]), - // TODO: We should encapsulate these Reasons within the type system, perhaps - // requiring top level contexts to implement a trait `ErrorReason::to_reason` - // or perhaps as a big enum - "INVALID_TYPE_ID".to_owned() - ))], - )) - })?; - - Ok(CreateEntityTypeParams { - schema, - ownership: OntologyOwnership::Local { web_id }, - conflict_behavior: ConflictBehavior::Fail, - provenance: provenance.clone(), - }) - }).collect::, BoxedResponse>>()? - ) + .create_entity_types(actor_id, params) .await .map_err(|report| { tracing::error!(error=?report, "Could not create entity types"); @@ -300,7 +322,8 @@ where metadata, // TODO: We should encapsulate these Reasons within the type system, // perhaps requiring top level contexts to implement a trait - // `ErrorReason::to_reason` or perhaps as a big enum, or as an attachment + // `ErrorReason::to_reason` or perhaps as a big enum, or as an + // attachment "BASE_URI_ALREADY_EXISTS".to_owned(), ))], )); diff --git a/libs/@local/graph/api/src/rest/property_type.rs b/libs/@local/graph/api/src/rest/property_type.rs index 28ca67f3a63..712582fdc0f 100644 --- a/libs/@local/graph/api/src/rest/property_type.rs +++ b/libs/@local/graph/api/src/rest/property_type.rs @@ -1,7 +1,7 @@ //! Web routes for CRU operations on Property types. use alloc::sync::Arc; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet, hash_map}; use axum::{ Extension, Router, @@ -14,6 +14,7 @@ use hash_graph_postgres_store::{ store::error::{OntologyVersionDoesNotExist, VersionedUrlAlreadyExists}, }; use hash_graph_store::{ + account::AccountStore as _, pool::StorePool, property_type::{ ArchivePropertyTypeParams, CreatePropertyTypeParams, HasPermissionForPropertyTypesParams, @@ -127,7 +128,6 @@ impl PropertyTypeResource { struct CreatePropertyTypeRequest { #[schema(inline)] schema: MaybeListOfPropertyType, - web_id: WebId, provenance: ProvidedOntologyEditionProvenance, } @@ -163,33 +163,50 @@ where .await .map_err(report_to_response)?; - let Json(CreatePropertyTypeRequest { - schema, - web_id, - provenance, - }) = body; + let Json(CreatePropertyTypeRequest { schema, provenance }) = body; let is_list = matches!(&schema, ListOrValue::List(_)); + let mut web_cache = HashMap::::new(); + let mut params = Vec::new(); + for schema in schema { + domain_validator + .validate(&schema) + .map_err(report_to_response)?; + + let shortname = domain_validator + .extract_shortname(schema.id.base_url.as_str()) + .map_err(report_to_response)?; + + let web_id = match web_cache.entry(shortname.to_owned()) { + hash_map::Entry::Occupied(entry) => *entry.get(), + hash_map::Entry::Vacant(entry) => { + let web_id = store + .get_web_by_shortname(actor_id, shortname) + .await + .map_err(report_to_response)? + .ok_or_else(|| { + status_to_response(Status::<()>::new( + hash_status::StatusCode::NotFound, + Some(format!("No web found for shortname `{shortname}`")), + vec![], + )) + })? + .id; + *entry.insert(web_id) + } + }; + + params.push(CreatePropertyTypeParams { + schema, + ownership: OntologyOwnership::Local { web_id }, + conflict_behavior: ConflictBehavior::Fail, + provenance: provenance.clone(), + }); + } + let mut metadata = store - .create_property_types( - actor_id, - schema - .into_iter() - .map(|schema| { - domain_validator - .validate(&schema) - .map_err(report_to_response)?; - - Ok(CreatePropertyTypeParams { - schema, - ownership: OntologyOwnership::Local { web_id }, - conflict_behavior: ConflictBehavior::Fail, - provenance: provenance.clone(), - }) - }) - .collect::, BoxedResponse>>()?, - ) + .create_property_types(actor_id, params) .await .map_err(report_to_response)?; diff --git a/tests/graph/http/tests/ambiguous.http b/tests/graph/http/tests/ambiguous.http index 58bb0ee0a23..6ebcd4e09c1 100644 --- a/tests/graph/http/tests/ambiguous.http +++ b/tests/graph/http/tests/ambiguous.http @@ -28,7 +28,7 @@ Content-Type: application/json X-Authenticated-User-Actor-Id: {{system_machine_id}} { - "shortname": "test-user", + "shortname": "alice", "registrationComplete": true } @@ -46,7 +46,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": [ { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/data-type", @@ -95,7 +94,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/data-type", "kind": "dataType", @@ -134,7 +132,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/data-type", "kind": "dataType", @@ -174,7 +171,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/property-type", "kind": "propertyType", @@ -209,7 +205,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/entity-type", "kind": "entityType", diff --git a/tests/graph/http/tests/circular-links.http b/tests/graph/http/tests/circular-links.http index b3e82ad5ca7..154b3f05e08 100644 --- a/tests/graph/http/tests/circular-links.http +++ b/tests/graph/http/tests/circular-links.http @@ -28,7 +28,7 @@ Content-Type: application/json X-Authenticated-User-Actor-Id: {{system_machine_id}} { - "shortname": "test-user", + "shortname": "snapshot", "registrationComplete": true } @@ -76,7 +76,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/entity-type", "kind": "entityType", @@ -109,7 +108,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/entity-type", "kind": "entityType", diff --git a/tests/graph/http/tests/friendship.http b/tests/graph/http/tests/friendship.http index 74cbbc3b4ce..c234ad91975 100644 --- a/tests/graph/http/tests/friendship.http +++ b/tests/graph/http/tests/friendship.http @@ -28,7 +28,7 @@ Content-Type: application/json X-Authenticated-User-Actor-Id: {{system_machine_id}} { - "shortname": "test-user", + "shortname": "alice", "registrationComplete": true } @@ -117,7 +117,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": [ { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/data-type", @@ -335,7 +334,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/property-type", "kind": "propertyType", @@ -390,7 +388,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/property-type", "kind": "propertyType", @@ -628,7 +625,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/entity-type", "kind": "entityType", @@ -727,7 +723,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": { "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/entity-type", "kind": "entityType", diff --git a/tests/graph/http/tests/link-inheritance.http b/tests/graph/http/tests/link-inheritance.http index c6a19cb2400..c264dae63b6 100644 --- a/tests/graph/http/tests/link-inheritance.http +++ b/tests/graph/http/tests/link-inheritance.http @@ -28,7 +28,7 @@ Content-Type: application/json X-Authenticated-User-Actor-Id: {{system_machine_id}} { - "shortname": "test-user", + "shortname": "snapshot", "registrationComplete": true } @@ -46,7 +46,6 @@ Accept: application/json X-Authenticated-User-Actor-Id: {{user_id}} { - "webId": "{{user_id}}", "schema": [{ "$schema": "https://blockprotocol.org/types/modules/graph/0.3/schema/entity-type", "kind": "entityType",