Merge pull request #5310 from systeminit/si-fs/schema-attrs

feat: edit schema attributes, fix size after writes

lib/sdf-server/src/service/v2/fs.rs

@@ -1,7 +1,7 @@
 use std::collections::HashSet;
 
 use axum::{
-    extract::{Host, OriginalUri, Path},
+    extract::{Host, OriginalUri, Path, Query},
     response::{IntoResponse, Response},
     routing::{get, post},
     Json, Router,
@@ -10,14 +10,16 @@ use dal::{
     cached_module::CachedModule,
     func::authoring::{FuncAuthoringClient, FuncAuthoringError},
     pkg::{import_pkg_from_pkg, ImportOptions, PkgError},
+    schema::variant::authoring::{VariantAuthoringClient, VariantAuthoringError},
     workspace::WorkspaceId,
     ChangeSet, ChangeSetId, DalContext, FuncId, Schema, SchemaId, SchemaVariant, WsEvent,
     WsEventError,
 };
 use hyper::StatusCode;
 use si_events::audit_log::AuditLogKind;
 use si_frontend_types::fs::{
-    AssetFuncs, ChangeSet as FsChangeSet, Func as FsFunc, Schema as FsSchema, SetFuncCodeRequest,
+    AssetFuncs, ChangeSet as FsChangeSet, Func as FsFunc, Schema as FsSchema, SchemaAttributes,
+    SetFuncCodeRequest, VariantQuery,
 };
 use thiserror::Error;
 
@@ -58,6 +60,8 @@ pub enum FsError {
     SchemaVariant(#[from] dal::SchemaVariantError),
     #[error("transactions error: {0}")]
     Transactions(#[from] dal::TransactionsError),
+    #[error("variant authoring error: {0}")]
+    VariantAuthoring(#[from] VariantAuthoringError),
     #[error("workspace snapshot error: {0}")]
     WorkspaceSnapshot(#[from] dal::WorkspaceSnapshotError),
     #[error("ws event error: {0}")]
@@ -323,12 +327,17 @@ pub async fn get_asset_funcs(
     let mut result = AssetFuncs {
         locked: None,
         unlocked: None,
+        unlocked_attrs_size: 0,
+        locked_attrs_size: 0,
     };
 
     result.locked = match lookup_variant_for_schema(&ctx, schema_id, false).await? {
         Some(variant) => {
             let asset_func = variant.get_asset_func(&ctx).await?;
 
+            let attrs = make_schema_attrs(&variant);
+            result.locked_attrs_size = attrs.byte_size();
+
             Some(dal_func_to_fs_func(asset_func))
         }
         None => None,
@@ -338,6 +347,9 @@ pub async fn get_asset_funcs(
         Some(variant) => {
             let asset_func = variant.get_asset_func(&ctx).await?;
 
+            let attrs = make_schema_attrs(&variant);
+            result.unlocked_attrs_size = attrs.byte_size();
+
             Some(dal_func_to_fs_func(asset_func))
         }
         None => None,
@@ -462,6 +474,96 @@ async fn install_schema(
     Ok(())
 }
 
+fn make_schema_attrs(variant: &SchemaVariant) -> SchemaAttributes {
+    SchemaAttributes {
+        category: variant.category().to_owned(),
+        description: variant.description(),
+        display_name: variant.display_name().to_owned(),
+        link: variant.link(),
+        color: variant.color().to_owned(),
+        component_type: variant.component_type().into(),
+    }
+}
+
+async fn get_schema_attrs(
+    HandlerContext(builder): HandlerContext,
+    AccessBuilder(request_ctx): AccessBuilder,
+    PosthogClient(_posthog_client): PosthogClient,
+    OriginalUri(_original_uri): OriginalUri,
+    Host(_host_name): Host,
+    Path((_workspace_id, change_set_id, schema_id)): Path<(WorkspaceId, ChangeSetId, SchemaId)>,
+    Query(request): Query<VariantQuery>,
+) -> FsResult<Json<SchemaAttributes>> {
+    let ctx = builder
+        .build(request_ctx.build(change_set_id.into()))
+        .await?;
+
+    check_change_set(&ctx)?;
+
+    let variant = lookup_variant_for_schema(&ctx, schema_id, request.unlocked)
+        .await?
+        .ok_or(FsError::ResourceNotFound)?;
+
+    Ok(Json(make_schema_attrs(&variant)))
+}
+
+async fn set_schema_attrs(
+    HandlerContext(builder): HandlerContext,
+    AccessBuilder(request_ctx): AccessBuilder,
+    PosthogClient(_posthog_client): PosthogClient,
+    OriginalUri(_original_uri): OriginalUri,
+    Host(_host_name): Host,
+    Path((_workspace_id, change_set_id, schema_id)): Path<(WorkspaceId, ChangeSetId, SchemaId)>,
+    Json(attrs): Json<SchemaAttributes>,
+) -> FsResult<()> {
+    let ctx = builder
+        .build(request_ctx.build(change_set_id.into()))
+        .await?;
+
+    check_change_set_and_not_head(&ctx).await?;
+
+    let variant = lookup_variant_for_schema(&ctx, schema_id, true)
+        .await?
+        .ok_or(FsError::ResourceNotFound)?;
+
+    let schema = variant.schema(&ctx).await?;
+    let schema_name = schema.name();
+
+    VariantAuthoringClient::save_variant_content(
+        &ctx,
+        variant.id,
+        schema_name,
+        &attrs.display_name,
+        &attrs.category,
+        attrs.description.clone(),
+        attrs.link.clone(),
+        &attrs.color,
+        attrs.component_type.into(),
+        None::<String>,
+    )
+    .await?;
+
+    WsEvent::schema_variant_saved(
+        &ctx,
+        schema.id(),
+        variant.id(),
+        schema_name.to_string(),
+        attrs.category,
+        attrs.color,
+        attrs.component_type.into(),
+        attrs.link,
+        attrs.description,
+        attrs.display_name,
+    )
+    .await?
+    .publish_on_commit(&ctx)
+    .await?;
+
+    ctx.commit().await?;
+
+    Ok(())
+}
+
 pub fn fs_routes() -> Router<AppState> {
     Router::new()
         .route("/change-sets", get(list_change_sets))
@@ -474,6 +576,8 @@ pub fn fs_routes() -> Router<AppState> {
                 .route("/func-code/:func_id", post(set_func_code))
                 .route("/schemas", get(list_schemas))
                 .route("/schemas/:schema_id/asset_funcs", get(get_asset_funcs))
+                .route("/schemas/:schema_id/attrs", get(get_schema_attrs))
+                .route("/schemas/:schema_id/attrs", post(set_schema_attrs))
                 .route("/schemas/:schema_id/funcs/:kind", get(list_variant_funcs))
                 .route("/schemas/:schema_id/install", post(install_schema)),
         )

lib/si-filesystem/src/client.rs

@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use si_frontend_types::{
     fs::{
         AssetFuncs, ChangeSet, CreateChangeSetRequest, CreateChangeSetResponse, Func,
-        ListChangeSetsResponse, Schema, SetFuncCodeRequest,
+        ListChangeSetsResponse, Schema, SchemaAttributes, SetFuncCodeRequest, VariantQuery,
     },
     FuncKind,
 };
@@ -239,4 +239,39 @@ impl SiFsClient {
 
         Ok(())
     }
+
+    pub async fn get_schema_attrs(
+        &self,
+        change_set_id: ChangeSetId,
+        schema_id: SchemaId,
+        unlocked: bool,
+    ) -> SiFsClientResult<SchemaAttributes> {
+        Ok(self
+            .client
+            .get(self.fs_api_change_sets(&format!("schemas/{schema_id}/attrs"), change_set_id))
+            .bearer_auth(&self.token)
+            .query(&VariantQuery { unlocked })
+            .send()
+            .await?
+            .error_for_status()?
+            .json()
+            .await?)
+    }
+
+    pub async fn set_schema_attrs(
+        &self,
+        change_set_id: ChangeSetId,
+        schema_id: SchemaId,
+        attributes: SchemaAttributes,
+    ) -> SiFsClientResult<()> {
+        self.client
+            .post(self.fs_api_change_sets(&format!("schemas/{schema_id}/attrs"), change_set_id))
+            .bearer_auth(&self.token)
+            .json(&attributes)
+            .send()
+            .await?
+            .error_for_status()?;
+
+        Ok(())
+    }
 }

lib/si-filesystem/src/inode_table.rs

@@ -47,10 +47,12 @@ impl InodeEntry {
 #[allow(dead_code)]
 #[remain::sorted]
 pub enum InodeEntryData {
-    AssetFunc {
+    AssetDefinitionDir {
         func_id: FuncId,
         change_set_id: ChangeSetId,
+        schema_id: SchemaId,
         size: u64,
+        attrs_size: u64,
         unlocked: bool,
     },
     AssetFuncCode {
@@ -85,6 +87,11 @@ pub enum InodeEntryData {
         name: String,
         installed: bool,
     },
+    SchemaAttrsJson {
+        schema_id: SchemaId,
+        change_set_id: ChangeSetId,
+        unlocked: bool,
+    },
     SchemaDefinitions {
         schema_id: SchemaId,
         change_set_id: ChangeSetId,
@@ -128,6 +135,12 @@ pub struct InodeTable {
     gid: Gid,
 }
 
+pub enum Size {
+    Directory,
+    UseExisting(u64),
+    Force(u64),
+}
+
 impl InodeTable {
     pub fn new(root_entry: InodeEntryData, uid: Uid, gid: Gid) -> Self {
         let mut table = Self {
@@ -137,7 +150,13 @@ impl InodeTable {
             gid,
         };
 
-        table.upsert("/".into(), root_entry, FileType::Directory, true, None);
+        table.upsert(
+            "/".into(),
+            root_entry,
+            FileType::Directory,
+            true,
+            Size::Directory,
+        );
 
         table
     }
@@ -154,6 +173,15 @@ impl InodeTable {
             .cloned()
     }
 
+    // pub fn ino_for_path_with_parent(
+    //     &self,
+    //     parent_ino: Inode,
+    //     file_name: impl AsRef<Path>,
+    // ) -> InodeTableResult<Option<Inode>> {
+    //     let path = self.make_path(Some(parent_ino), file_name)?;
+    //     Ok(self.ino_for_path(&path))
+    // }
+
     pub fn ino_for_path(&self, path: &Path) -> Option<Inode> {
         self.entries_by_path.get(path).map(|entry| entry.ino)
     }
@@ -193,26 +221,28 @@ impl InodeTable {
         entry_data: InodeEntryData,
         kind: FileType,
         write: bool,
-        size: Option<u64>,
+        size: Size,
     ) -> InodeTableResult<Inode> {
         let path = self.make_path(Some(parent_ino), file_name)?;
 
         Ok(self.upsert(path, entry_data, kind, write, size))
     }
 
-    pub fn make_attrs(
-        &self,
-        ino: Inode,
-        kind: FileType,
-        write: bool,
-        size: Option<u64>,
-    ) -> FileAttr {
+    pub fn make_attrs(&self, ino: Inode, kind: FileType, write: bool, size: Size) -> FileAttr {
         let perm: u16 = match kind {
             FileType::Directory => if write { 0o755 } else { 0o555 }
             FileType::RegularFile => if write { 0o644 } else  { 0o444 }
             _ => unimplemented!("I don't know why this kind of file was upserted, Only directories and regular files supported"),
         };
-        let size = size.unwrap_or(512);
+
+        let size = match size {
+            Size::Directory => 512,
+            Size::UseExisting(fallback) => self
+                .entry_for_ino(ino)
+                .map(|entry| entry.attrs.size)
+                .unwrap_or(fallback),
+            Size::Force(forced_size) => forced_size,
+        };
 
         FileAttr {
             ino: ino.as_raw(),
@@ -250,7 +280,7 @@ impl InodeTable {
         let write = entry.write;
         let entry_data = entry.data;
 
-        self.upsert(ino_path, entry_data, kind, write, Some(size));
+        self.upsert(ino_path, entry_data, kind, write, Size::Force(size));
 
         Ok(())
     }
@@ -261,16 +291,20 @@ impl InodeTable {
         entry_data: InodeEntryData,
         kind: FileType,
         write: bool,
-        size: Option<u64>,
+        size: Size,
     ) -> Inode {
         let parent = path
             .parent()
             .and_then(|path| self.entries_by_path.get(&path.to_path_buf()))
             .map(|entry| entry.ino);
 
-        let next_ino = self.next_ino();
-
-        let attrs = self.make_attrs(next_ino, kind, write, size);
+        let (attrs, ino) = match self.entries_by_path.get(&path) {
+            Some(entry) => (self.make_attrs(entry.ino, kind, write, size), entry.ino),
+            None => {
+                let next_ino = self.next_ino();
+                (self.make_attrs(next_ino, kind, write, size), next_ino)
+            }
+        };
 
         let entry = self
             .entries_by_path
@@ -291,7 +325,7 @@ impl InodeTable {
             .or_insert_with(|| {
                 self.path_table.push(path);
                 InodeEntry {
-                    ino: next_ino,
+                    ino,
                     parent,
                     data: entry_data,
                     attrs,