Merge pull request #28 from Virtual-Repetitions/computed-type-support

Add @computed type support to Postgres subsystem

Author: scoop1120

Cargo.lock

@@ -1,5 +1,5 @@
 [workspace.package]
-version = "0.29.3"
+version = "0.29.4"
 edition = "2024"
 
 # See https://github.com/mozilla/application-services/blob/main/Cargo.toml for the reasons why we use this structure

Cargo.toml

@@ -156,6 +156,10 @@ where
             let mut command = tokio::process::Command::new(&server_binary);
 
             command.kill_on_drop(true);
+            command
+                .stdin(Stdio::inherit())
+                .stdout(Stdio::inherit())
+                .stderr(Stdio::inherit());
 
             if let Some(port) = server_port {
                 command.env(EXO_SERVER_PORT, port.to_string());

crates/cli/src/util/watcher.rs

@@ -84,12 +84,12 @@ fn relative_path(
     folder_path.join(path)
 }
 
-pub fn read_relative_file(test_path: &PathBuf, path: &str) -> Result<String, std::io::Error> {
+pub fn read_relative_file(test_path: &Path, path: &str) -> Result<String, std::io::Error> {
     std::fs::read_to_string(test_path.join(path))
 }
 
 pub fn assert_file_content(
-    test_path: &PathBuf,
+    test_path: &Path,
     path: &str,
     actual_content: &str,
     test_name: &str,
@@ -102,7 +102,7 @@ pub fn assert_file_content(
             path.replace(".expected.", ".actual.")
         } else {
             // Drop the extension and add ".actual" followed by the extension
-            let extension = path.split('.').last().unwrap();
+            let extension = path.split('.').next_back().unwrap();
             let file_name = path.split('.').next().unwrap();
             format!("{}.actual.{}", file_name, extension)
         }
@@ -111,13 +111,11 @@ pub fn assert_file_content(
 
     let actual_content = actual_content.trim();
 
-    if !compare_strings_ignoring_whitespace(actual_content, &expected_content) {
+    if !compare_strings_ignoring_whitespace(actual_content, expected_content) {
         std::fs::write(actual_file, actual_content).unwrap();
         return Err(format!("{}: {}", "File content mismatch".red(), test_name));
-    } else {
-        if actual_file.exists() {
-            std::fs::remove_file(actual_file).unwrap();
-        }
+    } else if actual_file.exists() {
+        std::fs::remove_file(actual_file).unwrap();
     }
 
     Ok(())

crates/common/src/test_support.rs

@@ -105,8 +105,8 @@ impl SubsystemBuilder for PostgresSubsystemBuilder {
             (
                 "computed",
                 AnnotationSpec {
-                    targets: &[AnnotationTarget::Field],
-                    no_params: false,
+                    targets: &[AnnotationTarget::Field, AnnotationTarget::Type],
+                    no_params: true,
                     single_params: false,
                     mapped_params: Some(&[
                         MappedAnnotationParamSpec {

crates/postgres-subsystem/postgres-builder/src/plugin.rs

@@ -13,7 +13,6 @@ use core_model_builder::error::ModelBuildingError;
 use postgres_core_model::aggregate::{
     AggregateField, AggregateFieldType, AggregateType, ScalarAggregateFieldKind,
 };
-use postgres_core_model::types::EntityRepresentation;
 
 use crate::resolved_type::ResolvedType;
 use crate::resolved_type::ResolvedTypeEnv;
@@ -28,7 +27,7 @@ pub fn aggregate_type_name(type_name: &str) -> String {
 
 fn needs_aggregate(resolved_type: &ResolvedType) -> bool {
     if let ResolvedType::Composite(c) = resolved_type
-        && c.representation == EntityRepresentation::Json
+        && c.representation.is_json_like()
     {
         return false;
     }

crates/postgres-subsystem/postgres-core-builder/src/aggregate_type_builder.rs

@@ -109,7 +109,7 @@ fn expand_database_info(
     resolved_env: &ResolvedTypeEnv,
     building: &mut DatabaseBuilding,
 ) -> Result<(), ModelBuildingError> {
-    if resolved_type.representation == EntityRepresentation::Json {
+    if resolved_type.representation.is_json_like() {
         return Ok(());
     }
 
@@ -200,7 +200,7 @@ fn expand_type_relations(
     resolved_env: &ResolvedTypeEnv,
     building: &mut DatabaseBuilding,
 ) -> Result<(), ModelBuildingError> {
-    if resolved_type.representation == EntityRepresentation::Json {
+    if resolved_type.representation.is_json_like() {
         return Ok(());
     }
 
@@ -227,14 +227,14 @@ fn expand_type_relations(
                 .ok_or_else(|| {
                     ModelBuildingError::Generic(format!(
                         "The field `{}.{}` references type `{}` which is not registered in the Postgres subsystem. \
-                         Ensure this type is defined, imported, or marked with `@json` if it represents computed data.",
+                         Ensure this type is defined, imported, or marked with `@json` or `@computed` if it represents computed data.",
                         resolved_type.name,
                         field.name,
                         field.typ.innermost().type_name
                     ))
                 })?;
             if let ResolvedType::Composite(ct) = field_type
-                && ct.representation == EntityRepresentation::Json
+                && ct.representation.is_json_like()
             {
                 return Ok(());
             }
@@ -492,9 +492,7 @@ fn create_columns(
                                     PhysicalColumn {
                                         table_id,
                                         name: column_name.to_string(),
-                                        typ: if composite.representation
-                                            == EntityRepresentation::Json
-                                        {
+                                        typ: if composite.representation.is_json_like() {
                                             Box::new(JsonColumnType)
                                         } else {
                                             // A placeholder value. Will be resolved in the next phase (see expand_type_relations)

crates/postgres-subsystem/postgres-core-builder/src/database_builder.rs

@@ -1000,8 +1000,42 @@ fn resolve_composite_type(
             (None, None) => true,
         };
 
-        let representation = if ct.annotations.contains("json") {
+        let is_json = ct.annotations.contains("json");
+        let is_computed = ct.annotations.contains("computed");
+
+        if is_json && is_computed {
+            errors.push(Diagnostic {
+                level: Level::Error,
+                message: format!("Type '{}' cannot use both @json and @computed", ct.name),
+                code: Some("C000".to_string()),
+                spans: vec![SpanLabel {
+                    span: ct.span,
+                    style: SpanStyle::Primary,
+                    label: None,
+                }],
+            });
+        }
+
+        if is_computed && table_annotation.is_some() {
+            errors.push(Diagnostic {
+                level: Level::Error,
+                message: format!(
+                    "Type '{}' cannot use @table when marked as @computed",
+                    ct.name
+                ),
+                code: Some("C000".to_string()),
+                spans: vec![SpanLabel {
+                    span: ct.span,
+                    style: SpanStyle::Primary,
+                    label: None,
+                }],
+            });
+        }
+
+        let representation = if is_json {
             EntityRepresentation::Json
+        } else if is_computed {
+            EntityRepresentation::Computed
         } else if table_managed {
             EntityRepresentation::Managed
         } else {
@@ -1010,13 +1044,13 @@ fn resolve_composite_type(
 
         let access_annotation = ct.annotations.get("access");
 
-        let is_json = representation == EntityRepresentation::Json;
+        let is_json_like = representation.is_json_like();
 
-        if is_json && access_annotation.is_some() {
+        if is_json_like && access_annotation.is_some() {
             errors.push(Diagnostic {
                 level: Level::Error,
                 message: format!(
-                    "Cannot use @access for type {}. Json types behave like a primitive (and thus have always-allowed access)",
+                    "Cannot use @access for type {}. Json and computed types behave like a primitive (and thus have always-allowed access)",
                     ct.name
                 ),
                 code: Some("C000".to_string()),
@@ -1040,7 +1074,7 @@ fn resolve_composite_type(
         let join_table_config = join_table_annotation
             .and_then(|annotation| parse_join_table_annotation(ct, annotation, errors));
 
-        let mut access = if is_json {
+        let mut access = if is_json_like {
             // As if the user has annotated with `access(true)`
             ResolvedAccess {
                 default: Some(AstExpr::BooleanLiteral(true, default_span())),
@@ -1054,7 +1088,7 @@ fn resolve_composite_type(
 
         let (resolved_fields, ownership_field_found) = resolve_composite_type_fields(
             ct,
-            is_json,
+            is_json_like,
             table_managed,
             module_base_path,
             typechecked_system,
@@ -1120,7 +1154,7 @@ fn resolve_composite_type(
 
 fn resolve_composite_type_fields(
     ct: &AstModel<Typed>,
-    is_json: bool,
+    is_json_like: bool,
     table_managed: bool,
     module_base_path: &Path,
     typechecked_system: &TypecheckedSystem,
@@ -1138,13 +1172,13 @@ fn resolve_composite_type_fields(
 
         let access_annotation = field.annotations.get("access");
 
-        if is_json && access_annotation.is_some() {
+        if is_json_like && access_annotation.is_some() {
             errors.push(Diagnostic {
-                level: Level::Error,
-                message: format!(
-                    "Cannot use @access for field '{}' in a type with a '@json' annotation",
-                    field.name
-                ),
+            level: Level::Error,
+            message: format!(
+                "Cannot use @access for field '{}' in a type with a '@json' or '@computed' annotation",
+                field.name
+            ),
                 code: Some("C000".to_string()),
                 spans: vec![SpanLabel {
                     span: field.span,
@@ -1944,6 +1978,8 @@ fn compute_column_info(
         _ => field_name.to_snake_case(),
     };
     let enclosing_is_json = enclosing_type.annotations.contains("json");
+    let enclosing_is_computed = enclosing_type.annotations.contains("computed");
+    let enclosing_is_json_like = enclosing_is_json || enclosing_is_computed;
 
     let id_column_names = |field: &AstField<Typed>| -> Result<Vec<String>, Diagnostic> {
         let user_supplied_column_mapping = column_annotation_mapping(field);
@@ -2013,7 +2049,9 @@ fn compute_column_info(
         AstFieldType::Plain(..) => {
             match field_base_type.to_typ(types).deref(types) {
                 Type::Composite(field_type) => {
-                    if field_type.annotations.contains("json") {
+                    if field_type.annotations.contains("json")
+                        || (enclosing_is_json_like && enclosing_is_computed)
+                    {
                         return Ok(ColumnInfo {
                             names: vec![compute_column_name(&field.name)],
                             self_column: true,
@@ -2132,9 +2170,9 @@ fn compute_column_info(
                     }
                 }
                 Type::Set(typ) => {
-                    if enclosing_is_json
-                        && let Type::Composite(field_type) = typ.deref(types)
-                        && field_type.annotations.contains("json")
+                    if let Type::Composite(field_type) = typ.deref(types)
+                        && (field_type.annotations.contains("json")
+                            || (enclosing_is_json_like && enclosing_is_computed))
                     {
                         return Ok(ColumnInfo {
                             names: vec![compute_column_name(&field.name)],
@@ -2227,17 +2265,29 @@ fn compute_column_info(
                             indices,
                             cardinality: None,
                         })
-                    } else if enclosing_is_json
-                        && let Type::Composite(field_type) = &underlying_resolved
-                        && field_type.annotations.contains("json")
-                    {
-                        Ok(ColumnInfo {
-                            names: vec![compute_column_name(&field.name)],
-                            self_column: true,
-                            unique_constraints,
-                            indices,
-                            cardinality: None,
-                        })
+                    } else if let Type::Composite(field_type) = &underlying_resolved {
+                        if field_type.annotations.contains("json")
+                            || (enclosing_is_json_like && enclosing_is_computed)
+                        {
+                            Ok(ColumnInfo {
+                                names: vec![compute_column_name(&field.name)],
+                                self_column: true,
+                                unique_constraints,
+                                indices,
+                                cardinality: None,
+                            })
+                        } else {
+                            Err(Diagnostic {
+                                level: Level::Error,
+                                message: "Arrays of non-primitives are not supported".to_string(),
+                                code: Some("C000".to_string()),
+                                spans: vec![SpanLabel {
+                                    span: field.span,
+                                    style: SpanStyle::Primary,
+                                    label: None,
+                                }],
+                            })
+                        }
                     } else {
                         Err(Diagnostic {
                             level: Level::Error,
@@ -2674,6 +2724,37 @@ mod tests {
         );
     }
 
+    #[multiplatform_test]
+    fn computed_collection_of_computed_types() {
+        let resolved = create_resolved_system_from_src(
+            r#"
+        @postgres
+        module ContentModule {
+            @computed
+            type TeamAvailablePlaybook {
+                tags: Array<TeamAvailablePlaybookTag>
+                lessons: Set<TeamAvailablePlaybookLesson>
+            }
+
+            @computed
+            type TeamAvailablePlaybookTag {
+                label: String
+            }
+
+            @computed
+            type TeamAvailablePlaybookLesson {
+                id: Int
+            }
+        }
+        "#,
+        );
+
+        assert!(
+            resolved.is_ok(),
+            "Expected @computed collection fields to resolve without errors"
+        );
+    }
+
     #[test]
     fn with_access() {
         File::create("logger.js").unwrap();