fix: parameter aggregation fix (oscal-compass#1443)

* fix: parameter aggregation fix Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: arranging tests Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: triggering build Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: increase time out for cache response Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: profile-values are shown in markdown even when there are values already Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: adding alt identifier validation Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: correct profile values validation Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: remove parameter aggregation from assembly and remove label being shown in assembled profile Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: correcting test failures and various formatting issues Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: change order for parameters in markdown Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> * fix: not setting empty values Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]> --------- Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
jpower432 · Sep 19, 2023 · 56a3409 · 56a3409
1 parent 6936859
commit 56a3409
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 56 deletions.
diff --git a/tests/trestle/core/commands/author/profile_test.py b/tests/trestle/core/commands/author/profile_test.py
@@ -933,6 +933,7 @@ def test_profile_force_overwrite(tmp_trestle_dir: pathlib.Path, monkeypatch: Mon
     header, tree = md_api.processor.process_markdown(md_path)
 
     assert header
+    header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = []
     old_value = header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES]
     header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = 'New value'
 
@@ -948,6 +949,7 @@ def test_profile_force_overwrite(tmp_trestle_dir: pathlib.Path, monkeypatch: Mon
     test_utils.execute_command_and_assert(prof_generate, 0, monkeypatch)
 
     header, _ = md_api.processor.process_markdown(md_path)
+    header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = []
     assert header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] == old_value
 
     # test that file is unchanged
@@ -1129,20 +1131,37 @@ def test_profile_generate_assemble_parameter_aggregation(
     nist_cat, _ = ModelUtils.load_model_for_class(tmp_trestle_dir, 'nist_cat', cat.Catalog, FileContentType.JSON)
 
     appended_prop = {'name': 'aggregates', 'value': 'at-02_odp.01'}
+    second_appended_prop = {'name': 'aggregates', 'value': 'at-02_odp.02'}
+    third_appended_prop = {'name': 'alt-identifier', 'value': 'this_is_an_identifier'}
     ac_1 = nist_cat.groups[0].controls[0]
-    ac_1.params[2].props = []
-    ac_1.params[2].props.append(appended_prop)
+    ac_1.params[6].props = []
+    ac_1.params[6].props.append(appended_prop)
+    ac_1.params[6].props.append(second_appended_prop)
+    ac_1.params[6].props.append(third_appended_prop)
     appended_extra_param = {
         'id': 'at-02_odp.01',
         'props': [{
             'name': 'label', 'value': 'AT-02_ODP[01]', 'class': 'sp800-53a'
         }],
         'label': 'frequency',
+        'values': ['value-1', 'value-2'],
+        'guidelines': [{
+            'prose': 'blah'
+        }]
+    }
+    second_appended_extra_param = {
+        'id': 'at-02_odp.02',
+        'props': [{
+            'name': 'label', 'value': 'AT-02_ODP[02]', 'class': 'sp800-53a'
+        }],
+        'label': 'frequency',
+        'values': ['value-3', 'value-4'],
         'guidelines': [{
             'prose': 'blah'
         }]
     }
     ac_1.params.append(appended_extra_param)
+    ac_1.params.append(second_appended_extra_param)
 
     ModelUtils.save_top_level_model(nist_cat, tmp_trestle_dir, 'nist_cat', FileContentType.JSON)
 

diff --git a/tests/trestle/core/commands/author/ssp_test.py b/tests/trestle/core/commands/author/ssp_test.py
@@ -215,7 +215,6 @@ def test_ssp_generate_header_edit(tmp_trestle_dir: pathlib.Path) -> None:
     # confirm new items were added from yaml but not when the same key was alread present (values not updated)
     header, tree = md_api.processor.process_markdown(ac_1)
     assert 'control-origination' in header
-    assert header['x-trestle-set-params']['ac-1_prm_5']['values'] is None
     assert header['x-trestle-set-params']['ac-1_prm_5']['label'] == 'meetings cancelled from cli yaml'
 
     # generate again with header and DO overwrite header values

diff --git a/trestle/common/const.py b/trestle/common/const.py
@@ -583,6 +583,10 @@
 
 CONTROL_IMPLEMENTATION = 'control-implementation'
 
+AGGREGATES = 'aggregates'
+
+ALT_IDENTIFIER = 'alt-identifier'
+
 IMPLEMENTED_REQUIREMENT = 'implemented-requirement'
 
 # Following 5 are allowed control origination values for

diff --git a/trestle/common/model_utils.py b/trestle/common/model_utils.py
@@ -628,6 +628,12 @@ def dict_to_parameter(param_dict: Dict[str, Any]) -> common.Parameter:
         if const.DISPLAY_NAME in param_dict:
             display_name = param_dict.pop(const.DISPLAY_NAME)
             props.append(common.Property(name=const.DISPLAY_NAME, value=display_name, ns=const.TRESTLE_GENERIC_NS))
+        if const.AGGREGATES in param_dict:
+            # removing aggregates as this is prop just informative in markdown
+            param_dict.pop(const.AGGREGATES)
+        if const.ALT_IDENTIFIER in param_dict:
+            # removing alt-identifier as this is prop just informative in markdown
+            param_dict.pop(const.ALT_IDENTIFIER)
 
         if 'ns' in param_dict:
             param_dict.pop('ns')

diff --git a/trestle/core/catalog/catalog_reader.py b/trestle/core/catalog/catalog_reader.py
@@ -71,9 +71,16 @@ def read_additional_content(
                 )
                 alters_map[sort_id] = control_alters
                 for param_id, param_dict in control_param_dict.items():
+                    param_dict[const.VALUES] = param_dict[const.VALUES] if const.VALUES in param_dict else []
                     # if profile_values are present, overwrite values with them
                     if const.PROFILE_VALUES in param_dict:
-                        param_dict[const.VALUES] = param_dict.pop(const.PROFILE_VALUES)
+                        if param_dict[const.PROFILE_VALUES] != [] and param_dict[const.PROFILE_VALUES] is not None:
+                            if not write_mode and '<REPLACE_ME>' in param_dict[const.PROFILE_VALUES]:
+                                param_dict[const.PROFILE_VALUES].remove('<REPLACE_ME>')
+                            if param_dict[const.PROFILE_VALUES] != [] and param_dict[const.PROFILE_VALUES] is not None:
+                                param_dict[const.VALUES] = param_dict[const.PROFILE_VALUES]
+                        if not write_mode:
+                            param_dict.pop(const.PROFILE_VALUES)
                     final_param_dict[param_id] = param_dict
                     param_sort_map[param_id] = sort_id
         new_alters: List[prof.Alter] = []

diff --git a/trestle/core/catalog/catalog_writer.py b/trestle/core/catalog/catalog_writer.py
@@ -19,7 +19,7 @@
 
 import trestle.common.const as const
 import trestle.oscal.catalog as cat
-from trestle.common.list_utils import as_list, deep_get, delete_list_from_list, none_if_empty
+from trestle.common.list_utils import as_list, deep_get, none_if_empty
 from trestle.common.model_utils import ModelUtils
 from trestle.core.catalog.catalog_interface import CatalogInterface
 from trestle.core.catalog.catalog_merger import CatalogMerger
@@ -63,18 +63,6 @@ def write_catalog_as_profile_markdown(
 
             # get all params and vals for this control from the resolved profile catalog with block adds in effect
             control_param_dict = ControlInterface.get_control_param_dict(control, False)
-            to_delete = []
-            # removes aggregate parameters to be non-editable in markdowns
-            props_by_param_ids = {}
-            for param_id, values_dict in control_param_dict.items():
-                props_by_param_ids[param_id] = [
-                    prop for prop in as_list(values_dict.props) if prop.name == 'aggregates'
-                ]
-            to_delete = list({k: v for k, v in props_by_param_ids.items() if v != []}.keys())
-            unique_params_to_del = list(set(to_delete))
-            if unique_params_to_del:
-                delete_list_from_list(control_param_dict, unique_params_to_del)
-
             set_param_dict = self._construct_set_parameters_dict(profile_set_param_dict, control_param_dict, context)
 
             if set_param_dict:
@@ -174,15 +162,43 @@ def _construct_set_parameters_dict(
                     # all the other elements are from the profile set_param
                     new_dict[const.VALUES] = orig_dict.get(const.VALUES, None)
                     new_dict[const.GUIDELINES] = orig_dict.get(const.GUIDELINES, None)
+                    if new_dict[const.VALUES] is None:
+                        new_dict.pop(const.VALUES)
+                    if new_dict[const.GUIDELINES] is None:
+                        new_dict.pop(const.GUIDELINES)
             else:
                 # if the profile doesnt change this param at all, show it in the header with values
                 tmp_dict = ModelUtils.parameter_to_dict(param_dict, True)
                 values = tmp_dict.get('values', None)
-                new_dict = {'id': param_id, 'values': values}
+                # if values are None then don´t display them in the markdown
+                if values is not None:
+                    new_dict = {'id': param_id, 'values': values, const.PROFILE_VALUES: ['<REPLACE_ME>']}
+                else:
+                    new_dict = {'id': param_id, const.PROFILE_VALUES: ['<REPLACE_ME>']}
             new_dict.pop('id', None)
-            if display_name:
+            # validates if there are aggregated parameter values to the current parameter
+            aggregated_props = [prop for prop in as_list(param_dict.props) if prop.name == const.AGGREGATES]
+            if aggregated_props != []:
+                props_to_add = []
+                for prop in aggregated_props:
+                    props_to_add.append(prop.value)
+                new_dict[const.AGGREGATES] = props_to_add
+                new_dict.pop(const.PROFILE_VALUES, None)
+            alt_identifier = [prop for prop in as_list(param_dict.props) if prop.name == const.ALT_IDENTIFIER]
+            if alt_identifier != []:
+                new_dict[const.ALT_IDENTIFIER] = alt_identifier[0].value
+            # adds display name, if no display name then do not add to dict
+            if display_name != '' and display_name is not None:
                 new_dict[const.DISPLAY_NAME] = display_name
-            key_order = (const.LABEL, const.GUIDELINES, const.PROFILE_VALUES, const.VALUES, const.DISPLAY_NAME)
+            key_order = (
+                const.LABEL,
+                const.GUIDELINES,
+                const.VALUES,
+                const.AGGREGATES,
+                const.ALT_IDENTIFIER,
+                const.DISPLAY_NAME,
+                const.PROFILE_VALUES
+            )
             ordered_dict = {k: new_dict[k] for k in key_order if k in new_dict.keys()}
             set_param_dict[param_id] = ordered_dict
 

diff --git a/trestle/core/commands/author/profile.py b/trestle/core/commands/author/profile.py
@@ -285,33 +285,6 @@ def _replace_modify_set_params(
             profile.modify.set_parameters = none_if_empty(profile.modify.set_parameters)
         return changed
 
-    @staticmethod
-    def _add_aggregated_parameter(
-        param: Any, param_dict: Dict[str, Any], control_id: str, controls: Any, param_map: Dict[str, str]
-    ) -> None:
-        """
-        Add aggregated parameter value to original parameter.
-
-        Notes:
-            None
-        """
-        # verifies aggregated param is not on grabbed param dict
-        if param.id not in list(param_dict.keys()):
-            param.values = []
-            agg_props = [prop for prop in param.props if prop.name == 'aggregates']
-            for prop in as_list(agg_props):
-                if param_dict[prop.value].get('values'):
-                    agg_param_values = param_dict[prop.value].get('values')
-                    for value in as_list(agg_param_values):
-                        param.values.append(value)
-                else:
-                    agg_param_values = [p for p in controls[control_id] if p.id == prop.value][0]
-                    param.values.append(agg_param_values.props[0].value)
-                dict_param = ModelUtils.parameter_to_dict(param, False)
-                param_dict[param.id] = dict_param
-                param_map[param.id] = control_id
-        return None
-
     @staticmethod
     def assemble_profile(
         trestle_root: pathlib.Path,
@@ -381,14 +354,6 @@ def assemble_profile(
         catalog_api = CatalogAPI(catalog=catalog, context=context)
         found_alters, param_dict, param_map = catalog_api.read_additional_content_from_md(label_as_key=True)
 
-        controls = {}
-        for group in as_list(catalog.groups):
-            for control in as_list(group.controls):
-                controls[control.id] = control.params
-        for control_id, params in controls.items():
-            for param in as_list(params):
-                ProfileAssemble._add_aggregated_parameter(param, param_dict, control_id, controls, param_map)
-        # technically if allowed sections is [] it means no sections are allowed
         if allowed_sections is not None:
             for bad_part in [
                     part for alter in found_alters for add in as_list(alter.adds)

diff --git a/trestle/core/remote/cache.py b/trestle/core/remote/cache.py
@@ -286,7 +286,7 @@ def _do_fetch(self) -> None:
             auth = HTTPBasicAuth(self._username, self._password)
 
         try:
-            response = requests.get(self._url, auth=auth, verify=verify, timeout=10)
+            response = requests.get(self._url, auth=auth, verify=verify, timeout=30)
         except Exception as e:
             raise TrestleError(f'Cache update failure to connect via HTTPS: {self._url} ({e})')