Merge pull request #183 from azriel91/feature/156/params-spec-mapping…

…-fns-separate

doc/src/SUMMARY.md

@@ -20,8 +20,9 @@
     - [Item](technical_concepts/item.md)
         - [Item Parameters](technical_concepts/item/item_parameters.md)
             - [Value Specification](technical_concepts/item/item_parameters/value_specification.md)
-            - [Params Definition](technical_concepts/item/item_parameters/params_definition.md)
+            - [Params Specification](technical_concepts/item/item_parameters/params_specification.md)
             - [Params Framework Support](technical_concepts/item/item_parameters/params_framework_support.md)
+            - [Mapping Functions](technical_concepts/item/item_parameters/mapping_functions.md)
     - [Item Graph](technical_concepts/item_graph.md)
         - [Initialization](technical_concepts/item_graph/initialization.md)
         - [State Inspection](technical_concepts/item_graph/state_inspection.md)

doc/src/technical_concepts/item/item_parameters/mapping_functions.md

@@ -0,0 +1,133 @@
+# Mapping Functions
+
+> Github Issue: [#156][#156]
+
+Mapping functions could be passed to a `Params`'s `FieldWiseSpec`. There are implications whether or not it is implemented.
+
+
+[#156]: https://github.com/azriel91/peace/issues/156
+
+
+### Summary
+
+| With it                                  | Without it                                     |
+|:-----------------------------------------|:-----------------------------------------------|
+| Runtime error on mismatched types        | Compilation error on mismatched types          |
+| Need to define mapping function key enum | Don't need to define mapping function key enum |
+| Don't need to specify params spec for "used what was set last time" for mapping functions -- always set in cmd ctx                      | May forget specifying "used what was set last time" for mapping functions, and hit runtime error |
+
+
+### With It
+
+### Developers
+
+1. Define an enum to name the function keys:
+
+    ```rust
+    enum MappingFunctions {
+        BucketNameFromBucketState,
+        IamPolicyArnFromIamPolicyState,
+    }
+    ```
+
+2. Define the mappings:
+
+    ```rust
+    let mapping_functions = {
+        let mut mapping_functions = MappingFunctions::new();
+        mapping_functions.insert(BucketNameFromBucketState, S3BucketState::bucket_name);
+        // ..
+
+        mapping_functions
+    };
+    ```
+
+    **Note:**
+
+    If we want to have a compilation error here, the `MappingFunctions::insert` function needs to have a type parameter that tells it the `Item > Params > Field` that the mapping function is for.
+
+    However, developers use `#[derive(Params)]` to derive the `<ItemParams>FieldWiseSpec`, and requiring them to specify something like the following is arduous:
+
+    ```rust
+    MappingFunction::insert<FromType, ToType>(BucketNameFromBucketState, S3BucketState::bucket_name)
+    ```
+
+
+3. Pass `MappingFunctions` to `CmdCtxBuilder`, for each code instantiation (may be just one):
+
+    ```rust
+    cmd_ctx_builder.with_mapping_functions(mapping_functions);
+    ```
+
+4. Not have to call `.with_item_params::<TheItem>(..)` in subsequent calls.
+
+
+#### Users
+
+1. Get runtime error if the mapping function type doesn't match, but it should be caught by tests.
+
+
+#### Framework Maintainers
+
+1. `MappingFunctions` map will have magic logic to store the function argument types and return type.
+2. Error reporting when types are mismatched.
+
+
+### Without It
+
+#### Developers
+
+1. Define the item params spec:
+
+    ```rust
+    // First execution
+    let s3_object_params_spec = S3ObjectParams::<WebApp>::field_wise_spec()
+        .with_file_path(web_app_path_local)
+        .with_object_key(object_key)
+        .with_bucket_name_from_map(S3BucketState::bucket_name)
+        .build();
+
+    // Subsequent executions
+    let s3_object_params_spec = S3ObjectParams::<WebApp>::field_wise_spec()
+        .with_bucket_name_from_map(S3BucketState::bucket_name)
+        .build();
+    ```
+
+2. Pass the item params spec to `CmdCtxBuilder`, for every separate code instantiation:
+
+    ```rust
+    cmd_ctx_builder
+        .with_item_params::<S3ObjectItem<WebApp>>(
+            item_id!("s3_object"),
+            s3_object_params_spec,
+        )
+    ```
+
+    This is somewhat of an inconvenience, because if this isn't done, the user / developer will have a runtime error, which looks like this:
+
+    ```md
+    peace_rt_model::params_specs_mismatch
+
+      × Item params specs do not match with the items in the flow.
+      help: The following items either have not had a params spec provided previously,
+            or had contained a mapping function, which cannot be loaded from disk.
+
+            So the params spec needs to be provided to the command context for:
+
+            * s3_object
+    ```
+
+When the closure passed to `with_*_from_map` doesn't have the argument type specified, or mismatches, the compilation error is still unclear. [rust#119888][rust#119888] will allow us to return a useful compilation error.
+
+
+[rust#119888]: https://github.com/rust-lang/rust/pull/119888
+
+
+#### Users
+
+No runtime error, because it will be caught at compile time.
+
+
+#### Framework Maintainers
+
+1. Error messages / diagnostics showing which `CmdCtx` is missing which item spec for which field, should be made clear.

doc/src/technical_concepts/item/item_parameters/params_definition.md → doc/src/technical_concepts/item/item_parameters/params_specification.md

@@ -1,4 +1,4 @@
-# Params Definition
+# Params Specification
 
 For an item to work with different values, the values must be passed in.
 
@@ -217,3 +217,21 @@ See:
 * [`optfield`](https://github.com/roignpar/optfield)
 * [`partial_derive`](https://github.com/rise0chen/partial_derive)
 * [`optional-field`](https://github.com/cvpartner/optional-field)
+
+## Implementation
+
+See the `params_derive` crate for code gen.
+
+The following command helps to see what's been generated.
+
+```bash
+cargo expand --package peace_item_blank blank_params \
+  | sd -f cm '^    #\[doc\(hidden\)\][\n](^[ ]{4}[a-z# ].+[\n])+^[ ]{4}\};\n' '' \
+  | sd -f cm '^    #\[automatically_derived\][\n](^[ ]{4}[# a-z{].*[\n])+^[ ]{4}\{?\}\n' '' \
+  | sd -f cm '^    #\[allow\(unused_qualifications\)\][\n](^[ ]{4}[# a-z{].*[\n])+^[ ]{4}\}\n' '' \
+  | sd -f cm '^    #\[serde\(bound = ""\)\]' '    #[derive(Serialize, Deserialize)]\n    #[serde(bound = "")]' \
+  | sd -f cm '^    extern crate test;[\n](^[ ]{4}.*[\n])+^\}' '}' \
+  | sd -f cm '^(    pub struct [A-Za-z0-9_]+Partial)' '    #[derive(PartialEq, Eq)]\n$1' \
+  | sd -f cm '^(    #\[derivative\()' '    #[derive(derivative::Derivative)]\n$1' \
+  | xclip -se c
+```

examples/envman/src/cmds/app_upload_cmd.rs

@@ -54,13 +54,7 @@ impl AppUploadCmd {
         let profile_key = WorkspaceParamsKey::Profile;
 
         let s3_object_params_spec = S3ObjectParams::<WebApp>::field_wise_spec()
-            .with_bucket_name_from_map(|s3_bucket_state: &S3BucketState| match s3_bucket_state {
-                S3BucketState::None => None,
-                S3BucketState::Some {
-                    name,
-                    creation_date: _,
-                } => Some(name.clone()),
-            })
+            .with_bucket_name_from_map(S3BucketState::bucket_name)
             .build();
 
         let mut cmd_ctx = {

examples/envman/src/cmds/env_cmd.rs

@@ -1,6 +1,6 @@
 use futures::future::LocalBoxFuture;
 use peace::{
-    cfg::{app_name, item_id, state::Generated, Profile},
+    cfg::{app_name, item_id, Profile},
     cmd::{
         ctx::CmdCtx,
         scopes::{
@@ -57,16 +57,7 @@ impl EnvCmd {
         let iam_role_params_spec = IamRoleParams::<WebApp>::field_wise_spec()
             .with_name_from_map(|profile: &Profile| Some(profile.to_string()))
             .with_path(iam_role_path)
-            .with_managed_policy_arn_from_map(|iam_policy_state: &IamPolicyState| {
-                let IamPolicyState::Some {
-                    policy_id_arn_version: Generated::Value(policy_id_arn_version),
-                    ..
-                } = iam_policy_state
-                else {
-                    return None;
-                };
-                Some(policy_id_arn_version.arn().to_string())
-            })
+            .with_managed_policy_arn_from_map(IamPolicyState::policy_id_arn_version)
             .build();
 
         let CmdOpts { profile_print } = cmd_opts;

examples/envman/src/flows/app_upload_flow.rs

@@ -98,13 +98,7 @@ impl AppUploadFlow {
         let s3_object_params_spec = S3ObjectParams::<WebApp>::field_wise_spec()
             .with_file_path(web_app_path_local)
             .with_object_key(object_key)
-            .with_bucket_name_from_map(|s3_bucket_state: &S3BucketState| match s3_bucket_state {
-                S3BucketState::None => None,
-                S3BucketState::Some {
-                    name,
-                    creation_date: _,
-                } => Some(name.clone()),
-            })
+            .with_bucket_name_from_map(S3BucketState::bucket_name)
             .build();
 
         Ok(AppUploadFlowParamsSpecs {

examples/envman/src/flows/env_deploy_flow.rs

@@ -1,7 +1,7 @@
 use std::path::PathBuf;
 
 use peace::{
-    cfg::{app_name, flow_id, item_id, state::Generated, Profile},
+    cfg::{app_name, flow_id, item_id, Profile},
     params::{Params, ParamsSpec},
     rt_model::{Flow, ItemGraphBuilder},
 };
@@ -17,7 +17,7 @@ use crate::{
         peace_aws_iam_policy::{IamPolicyItem, IamPolicyParams, IamPolicyState},
         peace_aws_iam_role::{IamRoleItem, IamRoleParams},
         peace_aws_instance_profile::{InstanceProfileItem, InstanceProfileParams},
-        peace_aws_s3_bucket::{S3BucketItem, S3BucketParams},
+        peace_aws_s3_bucket::{S3BucketItem, S3BucketParams, S3BucketState},
         peace_aws_s3_object::{S3ObjectItem, S3ObjectParams},
     },
     model::{EnvManError, RepoSlug, WebApp},
@@ -134,17 +134,7 @@ impl EnvDeployFlow {
         let iam_role_params_spec = IamRoleParams::<WebApp>::field_wise_spec()
             .with_name(iam_role_name)
             .with_path(path.clone())
-            .with_managed_policy_arn_from_map(|iam_policy_state: &IamPolicyState| {
-                if let IamPolicyState::Some {
-                    policy_id_arn_version: Generated::Value(policy_id_arn_version),
-                    ..
-                } = iam_policy_state
-                {
-                    Some(policy_id_arn_version.arn().to_string())
-                } else {
-                    None
-                }
-            })
+            .with_managed_policy_arn_from_map(IamPolicyState::policy_id_arn_version)
             .build();
         let instance_profile_params_spec = InstanceProfileParams::<WebApp>::field_wise_spec()
             .with_name(instance_profile_name)
@@ -161,7 +151,7 @@ impl EnvDeployFlow {
             .build();
         let s3_object_params_spec = S3ObjectParams::<WebApp>::field_wise_spec()
             .with_file_path(web_app_path_local)
-            .with_bucket_name(bucket_name)
+            .with_bucket_name_from_map(S3BucketState::bucket_name)
             .with_object_key(object_key)
             .build();
 

examples/envman/src/items/peace_aws_iam_policy/iam_policy_state.rs

@@ -32,6 +32,21 @@ pub enum IamPolicyState {
     },
 }
 
+impl IamPolicyState {
+    /// Returns the `policy_id_arn_version` if it exists.
+    pub fn policy_id_arn_version(&self) -> Option<String> {
+        if let IamPolicyState::Some {
+            policy_id_arn_version: Generated::Value(policy_id_arn_version),
+            ..
+        } = self
+        {
+            Some(policy_id_arn_version.arn().to_string())
+        } else {
+            None
+        }
+    }
+}
+
 fn path_default() -> String {
     String::from("/")
 }

examples/envman/src/items/peace_aws_s3_bucket/s3_bucket_state.rs

@@ -22,6 +22,19 @@ pub enum S3BucketState {
     },
 }
 
+impl S3BucketState {
+    /// Returns the bucket name if the bucket exists.
+    pub fn bucket_name(&self) -> Option<String> {
+        match self {
+            S3BucketState::None => None,
+            S3BucketState::Some {
+                name,
+                creation_date: _,
+            } => Some(name.clone()),
+        }
+    }
+}
+
 impl fmt::Display for S3BucketState {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {