CM-46137 - Add visual separators of row groups; reorder columns (#289)

Author: MarshalX

cycode/cli/apps/scan/code_scanner.py

@@ -49,7 +49,7 @@
 
 def scan_sca_pre_commit(ctx: typer.Context) -> None:
     scan_type = ctx.obj['scan_type']
-    scan_parameters = get_default_scan_parameters(ctx)
+    scan_parameters = get_scan_parameters(ctx)
     git_head_documents, pre_committed_documents = get_pre_commit_modified_documents(
         ctx.obj['progress_bar'], ScanProgressBarSection.PREPARE_LOCAL_FILES
     )
@@ -83,15 +83,14 @@ def scan_sca_commit_range(ctx: typer.Context, path: str, commit_range: str) -> N
     scan_commit_range_documents(ctx, from_commit_documents, to_commit_documents, scan_parameters=scan_parameters)
 
 
-def scan_disk_files(ctx: typer.Context, paths: Tuple[str, ...]) -> None:
-    scan_parameters = get_scan_parameters(ctx, paths)
+def scan_disk_files(ctx: click.Context, paths: Tuple[str]) -> None:
     scan_type = ctx.obj['scan_type']
     progress_bar = ctx.obj['progress_bar']
 
     try:
         documents = get_relevant_documents(progress_bar, ScanProgressBarSection.PREPARE_LOCAL_FILES, scan_type, paths)
         perform_pre_scan_documents_actions(ctx, scan_type, documents)
-        scan_documents(ctx, documents, scan_parameters=scan_parameters)
+        scan_documents(ctx, documents, get_scan_parameters(ctx, paths))
     except Exception as e:
         handle_scan_exception(ctx, e)
 
@@ -155,14 +154,12 @@ def _enrich_scan_result_with_data_from_detection_rules(
 
 def _get_scan_documents_thread_func(
     ctx: typer.Context, is_git_diff: bool, is_commit_range: bool, scan_parameters: dict
-) -> Tuple[Callable[[List[Document]], Tuple[str, CliError, LocalScanResult]], str]:
+) -> Callable[[List[Document]], Tuple[str, CliError, LocalScanResult]]:
     cycode_client = ctx.obj['client']
     scan_type = ctx.obj['scan_type']
     severity_threshold = ctx.obj['severity_threshold']
     sync_option = ctx.obj['sync']
     command_scan_type = ctx.info_name
-    aggregation_id = str(_generate_unique_id())
-    scan_parameters['aggregation_id'] = aggregation_id
 
     def _scan_batch_thread_func(batch: List[Document]) -> Tuple[str, CliError, LocalScanResult]:
         local_scan_result = error = error_message = None
@@ -231,7 +228,7 @@ def _scan_batch_thread_func(batch: List[Document]) -> Tuple[str, CliError, Local
 
         return scan_id, error, local_scan_result
 
-    return _scan_batch_thread_func, aggregation_id
+    return _scan_batch_thread_func
 
 
 def scan_commit_range(
@@ -291,20 +288,17 @@ def scan_commit_range(
     logger.debug('List of commit ids to scan, %s', {'commit_ids': commit_ids_to_scan})
     logger.debug('Starting to scan commit range (it may take a few minutes)')
 
-    scan_documents(ctx, documents_to_scan, is_git_diff=True, is_commit_range=True)
+    scan_documents(ctx, documents_to_scan, get_scan_parameters(ctx, (path,)), is_git_diff=True, is_commit_range=True)
     return None
 
 
 def scan_documents(
     ctx: typer.Context,
     documents_to_scan: List[Document],
+    scan_parameters: dict,
     is_git_diff: bool = False,
     is_commit_range: bool = False,
-    scan_parameters: Optional[dict] = None,
 ) -> None:
-    if not scan_parameters:
-        scan_parameters = get_default_scan_parameters(ctx)
-
     scan_type = ctx.obj['scan_type']
     progress_bar = ctx.obj['progress_bar']
 
@@ -319,19 +313,13 @@ def scan_documents(
         )
         return
 
-    scan_batch_thread_func, aggregation_id = _get_scan_documents_thread_func(
-        ctx, is_git_diff, is_commit_range, scan_parameters
-    )
+    scan_batch_thread_func = _get_scan_documents_thread_func(ctx, is_git_diff, is_commit_range, scan_parameters)
     errors, local_scan_results = run_parallel_batched_scan(
         scan_batch_thread_func, scan_type, documents_to_scan, progress_bar=progress_bar
     )
 
-    if len(local_scan_results) > 1:
-        # if we used more than one batch, we need to fetch aggregate report url
-        aggregation_report_url = _try_get_aggregation_report_url_if_needed(
-            scan_parameters, ctx.obj['client'], scan_type
-        )
-        set_aggregation_report_url(ctx, aggregation_report_url)
+    aggregation_report_url = _try_get_aggregation_report_url_if_needed(scan_parameters, ctx.obj['client'], scan_type)
+    _set_aggregation_report_url(ctx, aggregation_report_url)
 
     progress_bar.set_section_length(ScanProgressBarSection.GENERATE_REPORT, 1)
     progress_bar.update(ScanProgressBarSection.GENERATE_REPORT)
@@ -341,25 +329,6 @@ def scan_documents(
     print_results(ctx, local_scan_results, errors)
 
 
-def set_aggregation_report_url(ctx: typer.Context, aggregation_report_url: Optional[str] = None) -> None:
-    ctx.obj['aggregation_report_url'] = aggregation_report_url
-
-
-def _try_get_aggregation_report_url_if_needed(
-    scan_parameters: dict, cycode_client: 'ScanClient', scan_type: str
-) -> Optional[str]:
-    aggregation_id = scan_parameters.get('aggregation_id')
-    if not scan_parameters.get('report'):
-        return None
-    if aggregation_id is None:
-        return None
-    try:
-        report_url_response = cycode_client.get_scan_aggregation_report_url(aggregation_id, scan_type)
-        return report_url_response.report_url
-    except Exception as e:
-        logger.debug('Failed to get aggregation report url: %s', str(e))
-
-
 def scan_commit_range_documents(
     ctx: typer.Context,
     from_documents_to_scan: List[Document],
@@ -384,7 +353,7 @@ def scan_commit_range_documents(
     try:
         progress_bar.set_section_length(ScanProgressBarSection.SCAN, 1)
 
-        scan_result = init_default_scan_result(cycode_client, scan_id, scan_type)
+        scan_result = init_default_scan_result(scan_id)
         if should_scan_documents(from_documents_to_scan, to_documents_to_scan):
             logger.debug('Preparing from-commit zip')
             from_commit_zipped_documents = zip_documents(scan_type, from_documents_to_scan)
@@ -522,7 +491,7 @@ def perform_scan_async(
         cycode_client,
         scan_async_result.scan_id,
         scan_type,
-        scan_parameters.get('report'),
+        scan_parameters,
     )
 
 
@@ -557,16 +526,14 @@ def perform_commit_range_scan_async(
     logger.debug(
         'Async commit range scan request has been triggered successfully, %s', {'scan_id': scan_async_result.scan_id}
     )
-    return poll_scan_results(
-        cycode_client, scan_async_result.scan_id, scan_type, scan_parameters.get('report'), timeout
-    )
+    return poll_scan_results(cycode_client, scan_async_result.scan_id, scan_type, scan_parameters, timeout)
 
 
 def poll_scan_results(
     cycode_client: 'ScanClient',
     scan_id: str,
     scan_type: str,
-    should_get_report: bool = False,
+    scan_parameters: dict,
     polling_timeout: Optional[int] = None,
 ) -> ZippedFileScanResult:
     if polling_timeout is None:
@@ -583,7 +550,7 @@ def poll_scan_results(
             print_debug_scan_details(scan_details)
 
         if scan_details.scan_status == consts.SCAN_STATUS_COMPLETED:
-            return _get_scan_result(cycode_client, scan_type, scan_id, scan_details, should_get_report)
+            return _get_scan_result(cycode_client, scan_type, scan_id, scan_details, scan_parameters)
 
         if scan_details.scan_status == consts.SCAN_STATUS_ERROR:
             raise custom_exceptions.ScanAsyncError(
@@ -675,18 +642,19 @@ def parse_pre_receive_input() -> str:
     return pre_receive_input.splitlines()[0]
 
 
-def get_default_scan_parameters(ctx: typer.Context) -> dict:
+def _get_default_scan_parameters(ctx: click.Context) -> dict:
     return {
         'monitor': ctx.obj.get('monitor'),
         'report': ctx.obj.get('report'),
         'package_vulnerabilities': ctx.obj.get('package-vulnerabilities'),
         'license_compliance': ctx.obj.get('license-compliance'),
         'command_type': ctx.info_name,
+        'aggregation_id': str(_generate_unique_id()),
     }
 
 
-def get_scan_parameters(ctx: typer.Context, paths: Tuple[str, ...]) -> dict:
-    scan_parameters = get_default_scan_parameters(ctx)
+def get_scan_parameters(ctx: typer.Context, paths: Optional[Tuple[str]] = None) -> dict:
+    scan_parameters = _get_default_scan_parameters(ctx)
 
     if not paths:
         return scan_parameters
@@ -894,36 +862,51 @@ def _get_scan_result(
     scan_type: str,
     scan_id: str,
     scan_details: 'ScanDetailsResponse',
-    should_get_report: bool = False,
+    scan_parameters: dict,
 ) -> ZippedFileScanResult:
     if not scan_details.detections_count:
-        return init_default_scan_result(cycode_client, scan_id, scan_type, should_get_report)
+        return init_default_scan_result(scan_id)
 
     scan_raw_detections = cycode_client.get_scan_raw_detections(scan_type, scan_id)
 
     return ZippedFileScanResult(
         did_detect=True,
         detections_per_file=_map_detections_per_file_and_commit_id(scan_type, scan_raw_detections),
         scan_id=scan_id,
-        report_url=_try_get_report_url_if_needed(cycode_client, should_get_report, scan_id, scan_type),
+        report_url=_try_get_any_report_url_if_needed(cycode_client, scan_id, scan_type, scan_parameters),
     )
 
 
-def init_default_scan_result(
-    cycode_client: 'ScanClient', scan_id: str, scan_type: str, should_get_report: bool = False
-) -> ZippedFileScanResult:
+def init_default_scan_result(scan_id: str) -> ZippedFileScanResult:
     return ZippedFileScanResult(
         did_detect=False,
         detections_per_file=[],
         scan_id=scan_id,
-        report_url=_try_get_report_url_if_needed(cycode_client, should_get_report, scan_id, scan_type),
     )
 
 
+def _try_get_any_report_url_if_needed(
+    cycode_client: 'ScanClient',
+    scan_id: str,
+    scan_type: str,
+    scan_parameters: dict,
+) -> Optional[str]:
+    """Tries to get aggregation report URL if needed, otherwise tries to get report URL."""
+    aggregation_report_url = None
+    if scan_parameters:
+        _try_get_report_url_if_needed(cycode_client, scan_id, scan_type, scan_parameters)
+        aggregation_report_url = _try_get_aggregation_report_url_if_needed(scan_parameters, cycode_client, scan_type)
+
+    if aggregation_report_url:
+        return aggregation_report_url
+
+    return _try_get_report_url_if_needed(cycode_client, scan_id, scan_type, scan_parameters)
+
+
 def _try_get_report_url_if_needed(
-    cycode_client: 'ScanClient', should_get_report: bool, scan_id: str, scan_type: str
+    cycode_client: 'ScanClient', scan_id: str, scan_type: str, scan_parameters: dict
 ) -> Optional[str]:
-    if not should_get_report:
+    if not scan_parameters.get('report', False):
         return None
 
     try:
@@ -933,6 +916,27 @@ def _try_get_report_url_if_needed(
         logger.debug('Failed to get report URL', exc_info=e)
 
 
+def _set_aggregation_report_url(ctx: click.Context, aggregation_report_url: Optional[str] = None) -> None:
+    ctx.obj['aggregation_report_url'] = aggregation_report_url
+
+
+def _try_get_aggregation_report_url_if_needed(
+    scan_parameters: dict, cycode_client: 'ScanClient', scan_type: str
+) -> Optional[str]:
+    if not scan_parameters.get('report', False):
+        return None
+
+    aggregation_id = scan_parameters.get('aggregation_id')
+    if aggregation_id is None:
+        return None
+
+    try:
+        report_url_response = cycode_client.get_scan_aggregation_report_url(aggregation_id, scan_type)
+        return report_url_response.report_url
+    except Exception as e:
+        logger.debug('Failed to get aggregation report url: %s', str(e))
+
+
 def _map_detections_per_file_and_commit_id(scan_type: str, raw_detections: List[dict]) -> List[DetectionsPerFile]:
     """Converts list of detections (async flow) to list of DetectionsPerFile objects (sync flow).
 

cycode/cli/apps/scan/pre_commit/pre_commit_command.py

@@ -4,7 +4,7 @@
 import typer
 
 from cycode.cli import consts
-from cycode.cli.apps.scan.code_scanner import scan_documents, scan_sca_pre_commit
+from cycode.cli.apps.scan.code_scanner import get_scan_parameters, scan_documents, scan_sca_pre_commit
 from cycode.cli.files_collector.excluder import exclude_irrelevant_documents_to_scan
 from cycode.cli.files_collector.repository_documents import (
     get_diff_file_content,
@@ -44,4 +44,4 @@ def pre_commit_command(
         documents_to_scan.append(Document(get_path_by_os(get_diff_file_path(file)), get_diff_file_content(file)))
 
     documents_to_scan = exclude_irrelevant_documents_to_scan(scan_type, documents_to_scan)
-    scan_documents(ctx, documents_to_scan, is_git_diff=True)
+    scan_documents(ctx, documents_to_scan, get_scan_parameters(ctx), is_git_diff=True)

cycode/cli/apps/scan/repository/repository_command.py

@@ -63,7 +63,6 @@ def repository_command(
         perform_pre_scan_documents_actions(ctx, scan_type, documents_to_scan)
 
         logger.debug('Found all relevant files for scanning %s', {'path': path, 'branch': branch})
-        scan_parameters = get_scan_parameters(ctx, (str(path),))
-        scan_documents(ctx, documents_to_scan, scan_parameters=scan_parameters)
+        scan_documents(ctx, documents_to_scan, get_scan_parameters(ctx, (path,)))
     except Exception as e:
         handle_scan_exception(ctx, e)

cycode/cli/printers/tables/sca_table_printer.py

@@ -1,5 +1,5 @@
 from collections import defaultdict
-from typing import TYPE_CHECKING, Dict, List
+from typing import TYPE_CHECKING, Dict, List, Set, Tuple
 
 import typer
 
@@ -37,9 +37,12 @@ def _print_results(self, local_scan_results: List['LocalScanResult']) -> None:
         for policy_id, detections in detections_per_policy_id.items():
             table = self._get_table(policy_id)
 
-            for detection in self._sort_and_group_detections(detections):
+            resulting_detections, group_separator_indexes = self._sort_and_group_detections(detections)
+            for detection in resulting_detections:
                 self._enrich_table_with_values(policy_id, table, detection)
 
+            table.set_group_separator_indexes(group_separator_indexes)
+
             self._print_summary_issues(len(detections), self._get_title(policy_id))
             self._print_table(table)
 
@@ -76,7 +79,7 @@ def __package_sort_key(detection: Detection) -> int:
     def _sort_detections_by_package(self, detections: List[Detection]) -> List[Detection]:
         return sorted(detections, key=self.__package_sort_key)
 
-    def _sort_and_group_detections(self, detections: List[Detection]) -> List[Detection]:
+    def _sort_and_group_detections(self, detections: List[Detection]) -> Tuple[List[Detection], Set[int]]:
         """Sort detections by severity and group by repository, code project and package name.
 
         Note:
@@ -85,7 +88,8 @@ def _sort_and_group_detections(self, detections: List[Detection]) -> List[Detect
             Grouping by code projects also groups by ecosystem.
             Because manifest files are unique per ecosystem.
         """
-        result = []
+        resulting_detections = []
+        group_separator_indexes = set()
 
         # we sort detections by package name to make persist output order
         sorted_detections = self._sort_detections_by_package(detections)
@@ -96,9 +100,10 @@ def _sort_and_group_detections(self, detections: List[Detection]) -> List[Detect
             for code_project_group in grouped_by_code_project.values():
                 grouped_by_package = self.__group_by(code_project_group, 'package_name')
                 for package_group in grouped_by_package.values():
-                    result.extend(self._sort_detections_by_severity(package_group))
+                    group_separator_indexes.add(len(resulting_detections) - 1)  # indexing starts from 0
+                    resulting_detections.extend(self._sort_detections_by_severity(package_group))
 
-        return result
+        return resulting_detections, group_separator_indexes
 
     def _get_table(self, policy_id: str) -> Table:
         table = Table()

cycode/cli/printers/tables/table.py

@@ -1,5 +1,5 @@
 import urllib.parse
-from typing import TYPE_CHECKING, Dict, List, Optional
+from typing import TYPE_CHECKING, Dict, List, Optional, Set
 
 from rich.markup import escape
 from rich.table import Table as RichTable
@@ -12,6 +12,8 @@ class Table:
     """Helper class to manage columns and their values in the right order and only if the column should be presented."""
 
     def __init__(self, column_infos: Optional[List['ColumnInfo']] = None) -> None:
+        self._group_separator_indexes: Set[int] = set()
+
         self._columns: Dict['ColumnInfo', List[str]] = {}
         if column_infos:
             self._columns = {columns: [] for columns in column_infos}
@@ -35,6 +37,9 @@ def add_file_path_cell(self, column: 'ColumnInfo', path: str) -> None:
         escaped_path = escape(encoded_path)
         self._add_cell_no_error(column, f'[link file://{escaped_path}]{path}')
 
+    def set_group_separator_indexes(self, group_separator_indexes: Set[int]) -> None:
+        self._group_separator_indexes = group_separator_indexes
+
     def _get_ordered_columns(self) -> List['ColumnInfo']:
         # we are sorting columns by index to make sure that columns will be printed in the right order
         return sorted(self._columns, key=lambda column_info: column_info.index)
@@ -53,7 +58,7 @@ def get_table(self) -> 'RichTable':
             extra_args = column.column_opts if column.column_opts else {}
             table.add_column(header=column.name, overflow='fold', **extra_args)
 
-        for raw in self.get_rows():
-            table.add_row(*raw)
+        for index, raw in enumerate(self.get_rows()):
+            table.add_row(*raw, end_section=index in self._group_separator_indexes)
 
         return table