From 8d13c10c802bd6816e81bf4128e50ceef0dc9393 Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 27 May 2024 10:28:29 +0300 Subject: [PATCH 01/13] added validate_all support --- .../commands/validate/validate_manager.py | 1 + .../validate/validators/base_validator.py | 21 +++++++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/demisto_sdk/commands/validate/validate_manager.py b/demisto_sdk/commands/validate/validate_manager.py index d1cbcca0e87..e3b9f500c89 100644 --- a/demisto_sdk/commands/validate/validate_manager.py +++ b/demisto_sdk/commands/validate/validate_manager.py @@ -70,6 +70,7 @@ def run_validations(self) -> int: content_object, self.configured_validations.ignorable_errors, self.configured_validations.support_level_dict, + self.validate_all, ), self.objects_to_run, ) diff --git a/demisto_sdk/commands/validate/validators/base_validator.py b/demisto_sdk/commands/validate/validators/base_validator.py index 42e9bfd8226..86396ce0c40 100644 --- a/demisto_sdk/commands/validate/validators/base_validator.py +++ b/demisto_sdk/commands/validate/validators/base_validator.py @@ -95,6 +95,7 @@ class BaseValidator(ABC, BaseModel, Generic[ContentTypes]): is_auto_fixable: ClassVar[bool] = False graph_interface: ClassVar[ContentGraphInterface] = None related_file_type: ClassVar[Optional[List[RelatedFileType]]] = None + run_on_all_files: ClassVar[bool] = None def get_content_types(self): args = (get_args(self.__orig_bases__[0]) or get_args(self.__orig_bases__[1]))[0] # type: ignore @@ -103,10 +104,11 @@ def get_content_types(self): return get_args(args) def should_run( - self, - content_item: ContentTypes, - ignorable_errors: list, - support_level_dict: dict, + self, + content_item: ContentTypes, + ignorable_errors: list, + support_level_dict: dict, + validate_all: bool ) -> bool: """check whether to run validation on the given content item or not. @@ -114,14 +116,15 @@ def should_run( content_item (BaseContent): The content item to run the validation on. ignorable_errors (list): The list of the errors that can be ignored. support_level_dict (dict): A dict with the lists of validation to run / not run according to the support level. - + validate_all: (bool) Returns: bool: True if the validation should run. Otherwise, return False. """ return all( [ - isinstance(content_item, self.get_content_types()), + # isinstance(content_item, self.get_content_types()), should_run_on_deprecated(self.run_on_deprecated, content_item), + should_run_on_all_files(self.run_on_all_files, validate_all), should_run_according_to_status( content_item.git_status, self.expected_git_statuses ), @@ -325,3 +328,9 @@ def should_run_on_deprecated(run_on_deprecated, content_item): if content_item.deprecated and not run_on_deprecated: return False return True + + +def should_run_on_all_files(run_on_all_files, validate_all): + if run_on_all_files is None or run_on_all_files == validate_all: + return True + return False \ No newline at end of file From 08d42a5e67aac3204bc89db8982371d56dbea4da Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 27 May 2024 15:54:25 +0300 Subject: [PATCH 02/13] revert --- .../commands/validate/validators/base_validator.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/demisto_sdk/commands/validate/validators/base_validator.py b/demisto_sdk/commands/validate/validators/base_validator.py index 86396ce0c40..3993a0e7db9 100644 --- a/demisto_sdk/commands/validate/validators/base_validator.py +++ b/demisto_sdk/commands/validate/validators/base_validator.py @@ -104,11 +104,11 @@ def get_content_types(self): return get_args(args) def should_run( - self, - content_item: ContentTypes, - ignorable_errors: list, - support_level_dict: dict, - validate_all: bool + self, + content_item: ContentTypes, + ignorable_errors: list, + support_level_dict: dict, + validate_all: bool ) -> bool: """check whether to run validation on the given content item or not. @@ -117,12 +117,13 @@ def should_run( ignorable_errors (list): The list of the errors that can be ignored. support_level_dict (dict): A dict with the lists of validation to run / not run according to the support level. validate_all: (bool) + Returns: bool: True if the validation should run. Otherwise, return False. """ return all( [ - # isinstance(content_item, self.get_content_types()), + isinstance(content_item, self.get_content_types()), should_run_on_deprecated(self.run_on_deprecated, content_item), should_run_on_all_files(self.run_on_all_files, validate_all), should_run_according_to_status( From b9367cad760716fe2c9d90272d6b0218e59fa44e Mon Sep 17 00:00:00 2001 From: Moshe Date: Sun, 2 Jun 2024 04:44:50 +0300 Subject: [PATCH 03/13] added execution_mode --- demisto_sdk/__main__.py | 11 ++++- demisto_sdk/commands/common/constants.py | 6 +++ demisto_sdk/commands/validate/initializer.py | 2 + .../commands/validate/validate_manager.py | 2 +- ...109_script_name_is_not_unique_validator.py | 29 ++---------- ..._name_is_not_unique_validator_all_files.py | 38 +++++++++++++++ ..._name_is_not_unique_validator_file_list.py | 47 +++++++++++++++++++ .../validate/validators/base_validator.py | 14 +++--- 8 files changed, 113 insertions(+), 36 deletions(-) create mode 100644 demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py create mode 100644 demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py diff --git a/demisto_sdk/__main__.py b/demisto_sdk/__main__.py index 477135402c8..cc1b8dfe43a 100644 --- a/demisto_sdk/__main__.py +++ b/demisto_sdk/__main__.py @@ -30,6 +30,7 @@ ENV_DEMISTO_SDK_MARKETPLACE, FileType, MarketplaceVersions, + ExecutionMode ) from demisto_sdk.commands.common.content_constant_paths import ( ALL_PACKS_DEPENDENCIES_DEFAULT_PATH, @@ -820,8 +821,13 @@ def validate(ctx, config, file_paths: str, **kwargs): sys.exit(1) try: is_external_repo = is_external_repository() - # default validate to -g --post-commit - if not kwargs.get("validate_all") and not kwargs["use_git"] and not file_path: + if kwargs.get("validate_all"): + execution_mode = ExecutionMode.ALL_FILES + elif file_path: + execution_mode = ExecutionMode.SPECIFIC_FILES + else: + execution_mode = ExecutionMode.USE_GIT + # default validate to -g --post-commit kwargs["use_git"] = True kwargs["post_commit"] = True exit_code = 0 @@ -919,6 +925,7 @@ def validate(ctx, config, file_paths: str, **kwargs): prev_ver=kwargs["prev_ver"], file_path=file_path, all_files=kwargs.get("validate_all"), + execution_mode=execution_mode, ) validator_v2 = ValidateManager( file_path=file_path, diff --git a/demisto_sdk/commands/common/constants.py b/demisto_sdk/commands/common/constants.py index 11623f245ae..824bba44712 100644 --- a/demisto_sdk/commands/common/constants.py +++ b/demisto_sdk/commands/common/constants.py @@ -1275,6 +1275,12 @@ class GitStatuses(StrEnum): DELETED = "D" +class ExecutionMode(StrEnum): + ALL_FILES = "-a" + USE_GIT = "-g" + SPECIFIC_FILES = "-i" + + FILE_TYPES_FOR_TESTING = [".py", ".js", ".yml", ".ps1"] # python subtypes diff --git a/demisto_sdk/commands/validate/initializer.py b/demisto_sdk/commands/validate/initializer.py index 3b52e44ec2d..504cdbed362 100644 --- a/demisto_sdk/commands/validate/initializer.py +++ b/demisto_sdk/commands/validate/initializer.py @@ -56,6 +56,7 @@ def __init__( prev_ver=None, file_path=None, all_files=False, + execution_mode=None, ): self.staged = staged self.use_git = use_git @@ -63,6 +64,7 @@ def __init__( self.all_files = all_files self.committed_only = committed_only self.prev_ver = prev_ver + self.execution_mode = execution_mode def validate_git_installed(self): """Initialize git util.""" diff --git a/demisto_sdk/commands/validate/validate_manager.py b/demisto_sdk/commands/validate/validate_manager.py index e3b9f500c89..8e162bba299 100644 --- a/demisto_sdk/commands/validate/validate_manager.py +++ b/demisto_sdk/commands/validate/validate_manager.py @@ -70,7 +70,7 @@ def run_validations(self) -> int: content_object, self.configured_validations.ignorable_errors, self.configured_validations.support_level_dict, - self.validate_all, + self.initializer.execution_mode, ), self.objects_to_run, ) diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py index 7385a5000bc..64bb736d90f 100644 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py @@ -1,5 +1,7 @@ from __future__ import annotations +from abc import ABC + from typing import Iterable, List from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH @@ -13,7 +15,7 @@ ContentTypes = Script -class DuplicatedScriptNameValidator(BaseValidator[ContentTypes]): +class DuplicatedScriptNameValidator(BaseValidator, ABC): error_code = "SC109" description = ( "Validate that there are no scripts with the same type and the same name." @@ -28,28 +30,3 @@ class DuplicatedScriptNameValidator(BaseValidator[ContentTypes]): ) related_field = "name" is_auto_fixable = False - - def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: - """ - Validate that there are no duplicate names of scripts - when the script name included `alert`. - """ - file_paths_to_objects = { - str(content_item.path.relative_to(CONTENT_PATH)): content_item - for content_item in content_items - } - query_results = self.graph.get_duplicate_script_name_included_incident( - list(file_paths_to_objects) - ) - - return [ - ValidationResult( - validator=self, - message=self.error_message.format( - replace_incident_to_alert(script_name), script_name - ), - content_object=file_paths_to_objects[file_path], - ) - for script_name, file_path in query_results.items() - if file_path in file_paths_to_objects - ] diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py new file mode 100644 index 00000000000..2cac590a640 --- /dev/null +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py @@ -0,0 +1,38 @@ +from __future__ import annotations + +from typing import Iterable, List + +from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.common.tools import replace_incident_to_alert +from demisto_sdk.commands.content_graph.objects.script import Script +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) +from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator import ( + DuplicatedScriptNameValidator +) +ContentTypes = Script + + +class DuplicatedScriptNameValidatorAllFiles(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): + expected_execution_mode = [ExecutionMode.ALL_FILES] + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + """ + Validate that there are no duplicate names of scripts + when the script name included `alert`. + """ + print('all') + query_results = self.graph.get_duplicate_script_name_included_incident([]) + + return [ + ValidationResult( + validator=self, + message=self.error_message.format( + replace_incident_to_alert(script_name), script_name + ), + content_object=file_path, + ) + for script_name, file_path in query_results.items() + ] diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py new file mode 100644 index 00000000000..8a6462f630e --- /dev/null +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py @@ -0,0 +1,47 @@ +from __future__ import annotations + +from typing import Iterable, List + +from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH +from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.common.tools import replace_incident_to_alert +from demisto_sdk.commands.content_graph.objects.script import Script +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) +from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator import ( + DuplicatedScriptNameValidator +) + +ContentTypes = Script + + +class DuplicatedScriptNameValidatorFileList(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): + expected_execution_mode = [ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT] + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + """ + Validate that there are no duplicate names of scripts + when the script name included `alert`. + """ + print('list') + file_paths_to_objects = { + str(content_item.path.relative_to(CONTENT_PATH)): content_item + for content_item in content_items + } + query_results = self.graph.get_duplicate_script_name_included_incident( + list(file_paths_to_objects) + ) + + return [ + ValidationResult( + validator=self, + message=self.error_message.format( + replace_incident_to_alert(script_name), script_name + ), + content_object=file_paths_to_objects[file_path], + ) + for script_name, file_path in query_results.items() + if file_path in file_paths_to_objects + ] diff --git a/demisto_sdk/commands/validate/validators/base_validator.py b/demisto_sdk/commands/validate/validators/base_validator.py index 3993a0e7db9..b9c3f799bbc 100644 --- a/demisto_sdk/commands/validate/validators/base_validator.py +++ b/demisto_sdk/commands/validate/validators/base_validator.py @@ -14,7 +14,7 @@ from pydantic import BaseModel -from demisto_sdk.commands.common.constants import GitStatuses +from demisto_sdk.commands.common.constants import GitStatuses, ExecutionMode from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.tools import is_abstract_class @@ -95,7 +95,7 @@ class BaseValidator(ABC, BaseModel, Generic[ContentTypes]): is_auto_fixable: ClassVar[bool] = False graph_interface: ClassVar[ContentGraphInterface] = None related_file_type: ClassVar[Optional[List[RelatedFileType]]] = None - run_on_all_files: ClassVar[bool] = None + expected_execution_mode: ClassVar[Optional[List[ExecutionMode]]] = None def get_content_types(self): args = (get_args(self.__orig_bases__[0]) or get_args(self.__orig_bases__[1]))[0] # type: ignore @@ -108,7 +108,7 @@ def should_run( content_item: ContentTypes, ignorable_errors: list, support_level_dict: dict, - validate_all: bool + execution_mode: ExecutionMode ) -> bool: """check whether to run validation on the given content item or not. @@ -116,7 +116,7 @@ def should_run( content_item (BaseContent): The content item to run the validation on. ignorable_errors (list): The list of the errors that can be ignored. support_level_dict (dict): A dict with the lists of validation to run / not run according to the support level. - validate_all: (bool) + execution_mode: (ExecutionMode) Returns: bool: True if the validation should run. Otherwise, return False. @@ -125,7 +125,7 @@ def should_run( [ isinstance(content_item, self.get_content_types()), should_run_on_deprecated(self.run_on_deprecated, content_item), - should_run_on_all_files(self.run_on_all_files, validate_all), + should_run_on_execution_mode(self.expected_execution_mode, execution_mode), should_run_according_to_status( content_item.git_status, self.expected_git_statuses ), @@ -331,7 +331,7 @@ def should_run_on_deprecated(run_on_deprecated, content_item): return True -def should_run_on_all_files(run_on_all_files, validate_all): - if run_on_all_files is None or run_on_all_files == validate_all: +def should_run_on_execution_mode(expected_execution_mode, execution_mode): + if not expected_execution_mode or execution_mode in expected_execution_mode: return True return False \ No newline at end of file From 719fcf7eef767e2e2105495e0cd08b7b993af947 Mon Sep 17 00:00:00 2001 From: Moshe Date: Sun, 2 Jun 2024 04:46:36 +0300 Subject: [PATCH 04/13] added new --- ...110_script_name_is_not_unique_validator.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py b/demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py new file mode 100644 index 00000000000..dc42f088567 --- /dev/null +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py @@ -0,0 +1,86 @@ +from __future__ import annotations + +from abc import ABC + +from typing import Iterable, List + +from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH +from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.common.tools import replace_incident_to_alert +from demisto_sdk.commands.content_graph.objects.script import Script +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +ContentTypes = Script + + +class DuplicatedScriptNameValidator(BaseValidator, ABC): + error_code = "SC110" + description = ( + "Validate that there are no scripts with the same type and the same name." + ) + rationale = "Duplicate names cause confusion and unpredictable behaviors." + error_message = ( + "Cannot create a script with the name {0}, because a script with the name {1} already exists.\n" + "(it will not be possible to create a new script whose name includes the word Alert/Alerts " + "if there is already a script with a similar name and only the word Alert/Alerts " + "is replaced by the word Incident/Incidents\nfor example: if there is a script `getIncident'" + "it will not be possible to create a script with the name `getAlert`)" + ) + related_field = "name" + is_auto_fixable = False + + +class DuplicatedScriptNameValidatorAllFiles(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): + expected_execution_mode = [ExecutionMode.ALL_FILES] + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + """ + Validate that there are no duplicate names of scripts + when the script name included `alert`. + """ + print('all 110') + query_results = self.graph.get_duplicate_script_name_included_incident([]) + + return [ + ValidationResult( + validator=self, + message=self.error_message.format( + replace_incident_to_alert(script_name), script_name + ), + content_object=file_path, + ) + for script_name, file_path in query_results.items() + ] + + +class DuplicatedScriptNameValidatorFileList(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): + expected_execution_mode = [ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT] + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + """ + Validate that there are no duplicate names of scripts + when the script name included `alert`. + """ + print('list 110') + file_paths_to_objects = { + str(content_item.path.relative_to(CONTENT_PATH)): content_item + for content_item in content_items + } + query_results = self.graph.get_duplicate_script_name_included_incident( + list(file_paths_to_objects) + ) + + return [ + ValidationResult( + validator=self, + message=self.error_message.format( + replace_incident_to_alert(script_name), script_name + ), + content_object=file_paths_to_objects[file_path], + ) + for script_name, file_path in query_results.items() + if file_path in file_paths_to_objects + ] \ No newline at end of file From 212b5c02571f64b971df8bdf0b741547ec820f18 Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 3 Jun 2024 10:12:10 +0300 Subject: [PATCH 05/13] changed to execution_mode --- demisto_sdk/__main__.py | 5 ++--- demisto_sdk/commands/validate/config_reader.py | 7 ++++--- demisto_sdk/commands/validate/initializer.py | 17 +++++++---------- .../commands/validate/validate_manager.py | 5 +---- .../validate/validators/base_validator.py | 12 ++++++++++-- 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/demisto_sdk/__main__.py b/demisto_sdk/__main__.py index cc1b8dfe43a..794ec88c671 100644 --- a/demisto_sdk/__main__.py +++ b/demisto_sdk/__main__.py @@ -823,6 +823,8 @@ def validate(ctx, config, file_paths: str, **kwargs): is_external_repo = is_external_repository() if kwargs.get("validate_all"): execution_mode = ExecutionMode.ALL_FILES + elif kwargs.get("use_git"): + execution_mode = ExecutionMode.USE_GIT elif file_path: execution_mode = ExecutionMode.SPECIFIC_FILES else: @@ -919,17 +921,14 @@ def validate(ctx, config, file_paths: str, **kwargs): category_to_run=kwargs.get("category_to_run"), ) initializer = Initializer( - use_git=kwargs["use_git"], staged=kwargs["staged"], committed_only=kwargs["post_commit"], prev_ver=kwargs["prev_ver"], file_path=file_path, - all_files=kwargs.get("validate_all"), execution_mode=execution_mode, ) validator_v2 = ValidateManager( file_path=file_path, - validate_all=kwargs.get("validate_all"), initializer=initializer, validation_results=validation_results, config_reader=config_reader, diff --git a/demisto_sdk/commands/validate/config_reader.py b/demisto_sdk/commands/validate/config_reader.py index f81d5c1bf9e..10c396d17b8 100644 --- a/demisto_sdk/commands/validate/config_reader.py +++ b/demisto_sdk/commands/validate/config_reader.py @@ -4,6 +4,7 @@ import toml from demisto_sdk.commands.common.logger import logger +from demisto_sdk.commands.common.constants import ExecutionMode USE_GIT = "use_git" PATH_BASED_VALIDATIONS = "path_based_validations" @@ -42,18 +43,18 @@ def __init__(self, config_file_path=None, category_to_run=None): exit(1) def gather_validations_to_run( - self, use_git: bool, ignore_support_level: Optional[bool] = False + self, execution_mode: bool, ignore_support_level: Optional[bool] = False ) -> ConfiguredValidations: """Extract the relevant information from the relevant category in the config file. Args: - use_git (bool): The use_git flag. + execution_mode (executionMode): The execution mode. Returns: Tuple[List, List, List, dict]: the select, warning, and ignorable errors sections from the given category, and the support_level dict with errors to ignore. """ - flag = self.category_to_run or (USE_GIT if use_git else PATH_BASED_VALIDATIONS) + flag = self.category_to_run or (USE_GIT if execution_mode == ExecutionMode.USE_GIT else PATH_BASED_VALIDATIONS) section = self.config_file_content.get(flag, {}) return ConfiguredValidations( section.get("select", []), diff --git a/demisto_sdk/commands/validate/initializer.py b/demisto_sdk/commands/validate/initializer.py index 504cdbed362..e521ab01231 100644 --- a/demisto_sdk/commands/validate/initializer.py +++ b/demisto_sdk/commands/validate/initializer.py @@ -22,6 +22,7 @@ SCRIPTS_DIR, GitStatuses, PathLevel, + ExecutionMode, ) from demisto_sdk.commands.common.content import Content from demisto_sdk.commands.common.logger import logger @@ -50,18 +51,14 @@ class Initializer: def __init__( self, - use_git=False, staged=None, committed_only=None, prev_ver=None, file_path=None, - all_files=False, execution_mode=None, ): self.staged = staged - self.use_git = use_git self.file_path = file_path - self.all_files = all_files self.committed_only = committed_only self.prev_ver = prev_ver self.execution_mode = execution_mode @@ -73,7 +70,7 @@ def validate_git_installed(self): self.branch_name = self.git_util.get_current_git_branch_or_hash() except (InvalidGitRepositoryError, TypeError): # if we are using git - fail the validation by raising the exception. - if self.use_git: + if self.execution_mode == ExecutionMode.USE_GIT: raise # if we are not using git - simply move on. else: @@ -313,13 +310,13 @@ def gather_objects_to_run_on( content_objects_to_run: Set[BaseContent] = set() invalid_content_items: Set[Path] = set() non_content_items: Set[Path] = set() - if self.use_git: + if self.execution_mode == ExecutionMode.USE_GIT: ( content_objects_to_run, invalid_content_items, non_content_items, ) = self.get_files_using_git() - elif self.file_path: + elif self.execution_mode == ExecutionMode.SPECIFIC_FILES: ( content_objects_to_run, invalid_content_items, @@ -327,21 +324,21 @@ def gather_objects_to_run_on( ) = self.paths_to_basecontent_set( set(self.load_files(self.file_path.split(","))) ) - elif self.all_files: + elif self.execution_mode == ExecutionMode.ALL_FILES: logger.info("Running validation on all files.") content_dto = ContentDTO.from_path() if not isinstance(content_dto, ContentDTO): raise Exception("no content found") content_objects_to_run = set(content_dto.packs) else: - self.use_git = True + self.execution_mode = ExecutionMode.USE_GIT self.committed_only = True ( content_objects_to_run, invalid_content_items, non_content_items, ) = self.get_files_using_git() - if not self.use_git: + if self.execution_mode != ExecutionMode.USE_GIT: content_objects_to_run_with_packs: Set[ BaseContent ] = self.get_items_from_packs(content_objects_to_run) diff --git a/demisto_sdk/commands/validate/validate_manager.py b/demisto_sdk/commands/validate/validate_manager.py index 8e162bba299..7627065c644 100644 --- a/demisto_sdk/commands/validate/validate_manager.py +++ b/demisto_sdk/commands/validate/validate_manager.py @@ -26,13 +26,11 @@ def __init__( validation_results: ResultWriter, config_reader: ConfigReader, initializer: Initializer, - validate_all=False, file_path=None, allow_autofix=False, ignore_support_level=False, ): self.ignore_support_level = ignore_support_level - self.validate_all = validate_all self.file_path = file_path self.allow_autofix = allow_autofix self.validation_results = validation_results @@ -44,11 +42,10 @@ def __init__( self.objects_to_run, self.invalid_items, ) = self.initializer.gather_objects_to_run_on() - self.use_git = self.initializer.use_git self.committed_only = self.initializer.committed_only self.configured_validations: ConfiguredValidations = ( self.config_reader.gather_validations_to_run( - use_git=self.use_git, ignore_support_level=self.ignore_support_level + self.initializer.execution_mode, ignore_support_level=self.ignore_support_level ) ) self.validators = self.filter_validators() diff --git a/demisto_sdk/commands/validate/validators/base_validator.py b/demisto_sdk/commands/validate/validators/base_validator.py index b9c3f799bbc..cb917ffdfd7 100644 --- a/demisto_sdk/commands/validate/validators/base_validator.py +++ b/demisto_sdk/commands/validate/validators/base_validator.py @@ -95,7 +95,7 @@ class BaseValidator(ABC, BaseModel, Generic[ContentTypes]): is_auto_fixable: ClassVar[bool] = False graph_interface: ClassVar[ContentGraphInterface] = None related_file_type: ClassVar[Optional[List[RelatedFileType]]] = None - expected_execution_mode: ClassVar[Optional[List[ExecutionMode]]] = None + expected_execution_mode: Optional[List[ExecutionMode]] = None def get_content_types(self): args = (get_args(self.__orig_bases__[0]) or get_args(self.__orig_bases__[1]))[0] # type: ignore @@ -332,6 +332,14 @@ def should_run_on_deprecated(run_on_deprecated, content_item): def should_run_on_execution_mode(expected_execution_mode, execution_mode): + """ + Check if the execution_mode is in the expected_execution_mode of validation. + Args: + execution_mode (ExecutionMode): The running execution_mode. + expected_execution_mode (Optional[list[ExecutionMode]]): The validation's expected execution_mode, if None then validation should run on all execution modes. + Returns: + bool: True if the given validation should run on the execution_mode. Otherwise, return False. + """ if not expected_execution_mode or execution_mode in expected_execution_mode: return True - return False \ No newline at end of file + return False From ceae8981d67b647ec9b6d3af0de64816208d2dbb Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 3 Jun 2024 11:25:33 +0300 Subject: [PATCH 06/13] revert --- ..._name_is_not_unique_validator_all_files.py | 1 - ..._name_is_not_unique_validator_file_list.py | 1 - ...110_script_name_is_not_unique_validator.py | 86 ------------------- .../validate/validators/base_validator.py | 2 +- 4 files changed, 1 insertion(+), 89 deletions(-) delete mode 100644 demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py index 2cac590a640..112579fbb06 100644 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py @@ -23,7 +23,6 @@ def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResu Validate that there are no duplicate names of scripts when the script name included `alert`. """ - print('all') query_results = self.graph.get_duplicate_script_name_included_incident([]) return [ diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py index 8a6462f630e..16e2d1235b7 100644 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py @@ -25,7 +25,6 @@ def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResu Validate that there are no duplicate names of scripts when the script name included `alert`. """ - print('list') file_paths_to_objects = { str(content_item.path.relative_to(CONTENT_PATH)): content_item for content_item in content_items diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py b/demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py deleted file mode 100644 index dc42f088567..00000000000 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC110_script_name_is_not_unique_validator.py +++ /dev/null @@ -1,86 +0,0 @@ -from __future__ import annotations - -from abc import ABC - -from typing import Iterable, List - -from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH -from demisto_sdk.commands.common.constants import ExecutionMode -from demisto_sdk.commands.common.tools import replace_incident_to_alert -from demisto_sdk.commands.content_graph.objects.script import Script -from demisto_sdk.commands.validate.validators.base_validator import ( - BaseValidator, - ValidationResult, -) - -ContentTypes = Script - - -class DuplicatedScriptNameValidator(BaseValidator, ABC): - error_code = "SC110" - description = ( - "Validate that there are no scripts with the same type and the same name." - ) - rationale = "Duplicate names cause confusion and unpredictable behaviors." - error_message = ( - "Cannot create a script with the name {0}, because a script with the name {1} already exists.\n" - "(it will not be possible to create a new script whose name includes the word Alert/Alerts " - "if there is already a script with a similar name and only the word Alert/Alerts " - "is replaced by the word Incident/Incidents\nfor example: if there is a script `getIncident'" - "it will not be possible to create a script with the name `getAlert`)" - ) - related_field = "name" - is_auto_fixable = False - - -class DuplicatedScriptNameValidatorAllFiles(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): - expected_execution_mode = [ExecutionMode.ALL_FILES] - - def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: - """ - Validate that there are no duplicate names of scripts - when the script name included `alert`. - """ - print('all 110') - query_results = self.graph.get_duplicate_script_name_included_incident([]) - - return [ - ValidationResult( - validator=self, - message=self.error_message.format( - replace_incident_to_alert(script_name), script_name - ), - content_object=file_path, - ) - for script_name, file_path in query_results.items() - ] - - -class DuplicatedScriptNameValidatorFileList(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): - expected_execution_mode = [ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT] - - def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: - """ - Validate that there are no duplicate names of scripts - when the script name included `alert`. - """ - print('list 110') - file_paths_to_objects = { - str(content_item.path.relative_to(CONTENT_PATH)): content_item - for content_item in content_items - } - query_results = self.graph.get_duplicate_script_name_included_incident( - list(file_paths_to_objects) - ) - - return [ - ValidationResult( - validator=self, - message=self.error_message.format( - replace_incident_to_alert(script_name), script_name - ), - content_object=file_paths_to_objects[file_path], - ) - for script_name, file_path in query_results.items() - if file_path in file_paths_to_objects - ] \ No newline at end of file diff --git a/demisto_sdk/commands/validate/validators/base_validator.py b/demisto_sdk/commands/validate/validators/base_validator.py index cb917ffdfd7..37d0fc5f371 100644 --- a/demisto_sdk/commands/validate/validators/base_validator.py +++ b/demisto_sdk/commands/validate/validators/base_validator.py @@ -95,7 +95,7 @@ class BaseValidator(ABC, BaseModel, Generic[ContentTypes]): is_auto_fixable: ClassVar[bool] = False graph_interface: ClassVar[ContentGraphInterface] = None related_file_type: ClassVar[Optional[List[RelatedFileType]]] = None - expected_execution_mode: Optional[List[ExecutionMode]] = None + expected_execution_mode: ClassVar[Optional[List[ExecutionMode]]] = None def get_content_types(self): args = (get_args(self.__orig_bases__[0]) or get_args(self.__orig_bases__[1]))[0] # type: ignore From fee66b30bfea006818382c81329e7263690ffbe9 Mon Sep 17 00:00:00 2001 From: Moshe Date: Tue, 4 Jun 2024 11:21:39 +0300 Subject: [PATCH 07/13] added validator gr104 --- ...104_is_pack_display_name_already_exists.py | 41 +++++++++++++++++++ ...k_display_name_already_exists_all_files.py | 23 +++++++++++ ..._display_name_already_exists_list_files.py | 24 +++++++++++ 3 files changed, 88 insertions(+) create mode 100644 demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py create mode 100644 demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py create mode 100644 demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py new file mode 100644 index 00000000000..0d6c3a7965c --- /dev/null +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py @@ -0,0 +1,41 @@ + +from __future__ import annotations + +from abc import ABC + +from typing import Iterable, List + +from demisto_sdk.commands.content_graph.parsers.related_files import RelatedFileType +from demisto_sdk.commands.content_graph.objects.pack import Pack +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +ContentTypes = Pack + + +class IsPackDisplayNameAlreadyExistsValidator(BaseValidator, ABC): + error_code = "GR104" + description = "Validate that there are no duplicate display names in the repo" + rationale = " Validate the existance of duplicate display names" + error_message = "Pack '{content_name}' has a duplicate display_name as: {pack_display_names} " + related_field = "" + is_auto_fixable = False + related_file_type = [RelatedFileType.JSON] + + def is_valid_use_graph(self, file_paths_to_objects: Iterable[str]) -> List[ValidationResult]: + query_results = self.graph.get_duplicate_pack_display_name( + list(file_paths_to_objects) + ) + return [ + ValidationResult( + validator=self, + message=self.error_message.format( + content_id=content_id, pack_display_names=(', '.join(duplicate_names_id)) + ), + content_object=content_id, + ) + for content_id, duplicate_names_id in query_results + ] + diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py new file mode 100644 index 00000000000..860d1a31b3d --- /dev/null +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py @@ -0,0 +1,23 @@ + +from __future__ import annotations + +from typing import Iterable, List + +from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.content_graph.objects.pack import Pack +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists import IsPackDisplayNameAlreadyExistsValidator + + +ContentTypes = Pack + + +class IsPackDisplayNameAlreadyExistsValidatorAllFiles(IsPackDisplayNameAlreadyExistsValidator, BaseValidator[ContentTypes]): + expected_execution_mode = [ExecutionMode.ALL_FILES] + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + return self.is_valid_use_graph([]) diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py new file mode 100644 index 00000000000..e0f8d2ccec8 --- /dev/null +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py @@ -0,0 +1,24 @@ + +from __future__ import annotations + +from typing import Iterable, List + +from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH +from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.content_graph.objects.pack import Pack +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists import IsPackDisplayNameAlreadyExistsValidator + +ContentTypes = Pack + + +class IsPackDisplayNameAlreadyExistsValidatorListFiles(IsPackDisplayNameAlreadyExistsValidator, BaseValidator[ContentTypes]): + expected_execution_mode = [ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT] + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + file_paths_to_objects = [item.pack_id for item in content_items] + return self.is_valid_use_graph(file_paths_to_objects) From c91e167efb7b11f5ea48a56b9dde8c69f2574487 Mon Sep 17 00:00:00 2001 From: Moshe Date: Thu, 6 Jun 2024 14:33:42 +0300 Subject: [PATCH 08/13] fixed --- .../tests/SC_graph_validation_test.py | 62 ++++++++++++++++--- ...104_is_pack_display_name_already_exists.py | 26 +++++--- ...k_display_name_already_exists_all_files.py | 2 +- ..._display_name_already_exists_list_files.py | 3 +- ...109_script_name_is_not_unique_validator.py | 5 -- ..._name_is_not_unique_validator_all_files.py | 8 ++- 6 files changed, 80 insertions(+), 26 deletions(-) diff --git a/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py b/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py index a4614238bd9..4cbc9821d4f 100644 --- a/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py +++ b/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py @@ -4,10 +4,13 @@ ) from demisto_sdk.commands.validate.validators.base_validator import BaseValidator from demisto_sdk.commands.validate.validators.SC_validators import ( - SC109_script_name_is_not_unique_validator, + SC109_script_name_is_not_unique_validator_file_list, SC109_script_name_is_not_unique_validator_all_files ) -from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator import ( - DuplicatedScriptNameValidator, +from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator_file_list import ( + DuplicatedScriptNameValidatorFileList, +) +from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator_all_files import ( + DuplicatedScriptNameValidatorAllFiles ) from TestSuite.repo import Repo @@ -19,7 +22,7 @@ ] -def test_DuplicatedScriptNameValidator_is_valid(mocker, graph_repo: Repo): +def test_DuplicatedScriptNameValidatorFileList_is_valid(mocker, graph_repo: Repo): """ Given - A content repo with 8 scripts: @@ -28,14 +31,13 @@ def test_DuplicatedScriptNameValidator_is_valid(mocker, graph_repo: Repo): - 2 scripts (test_alert3, test_incident3) supported by MP V2 with SKIP_PREPARE_SCRIPT_NAME = "script-name-incident-to-alert". - 2 scripts (test_alert4, test_incident4) where only one is supported by MP V2 without SKIP_PREPARE_SCRIPT_NAME = "script-name-incident-to-alert". When - - running DuplicatedScriptNameValidator is_valid function. + - running DuplicatedScriptNameValidatorFileList is_valid function. Then - Validate that only the first pair of scripts appear in the results, and the rest of the scripts is valid. """ mocker.patch.object( - SC109_script_name_is_not_unique_validator, "CONTENT_PATH", new=graph_repo.path + SC109_script_name_is_not_unique_validator_file_list, "CONTENT_PATH", new=graph_repo.path ) - pack = graph_repo.create_pack() pack.create_script("test_incident_1").set_data(marketplaces=MP_XSOAR_AND_V2) @@ -56,9 +58,53 @@ def test_DuplicatedScriptNameValidator_is_valid(mocker, graph_repo: Repo): BaseValidator.graph_interface = graph_repo.create_graph() - results = DuplicatedScriptNameValidator().is_valid( + results = DuplicatedScriptNameValidatorFileList().is_valid( [script.object for script in pack.scripts] ) assert len(results) == 1 assert "test_alert_1.yml" == results[0].content_object.path.name + + +def test_DuplicatedScriptNameValidatorAllFiles_is_valid(mocker, graph_repo: Repo): + """ + Given + - A content repo with 8 scripts: + - 2 scripts (test_alert1, test_incident1) supported by MP V2 without SKIP_PREPARE_SCRIPT_NAME = "script-name-incident-to-alert". + - 2 scripts (test_alert2, test_incident2) not supported by MP V2 without SKIP_PREPARE_SCRIPT_NAME = "script-name-incident-to-alert". + - 2 scripts (test_alert3, test_incident3) supported by MP V2 with SKIP_PREPARE_SCRIPT_NAME = "script-name-incident-to-alert". + - 2 scripts (test_alert4, test_incident4) where only one is supported by MP V2 without SKIP_PREPARE_SCRIPT_NAME = "script-name-incident-to-alert". + When + - running DuplicatedScriptNameValidatorAllFiles is_valid function. + Then + - Validate that only the first pair of scripts appear in the results, and the rest of the scripts is valid. + """ + mocker.patch.object( + SC109_script_name_is_not_unique_validator_all_files, "CONTENT_PATH", new=graph_repo.path + ) + pack = graph_repo.create_pack() + + pack.create_script("test_incident_1").set_data(marketplaces=MP_XSOAR_AND_V2) + pack.create_script("test_alert_1").set_data(marketplaces=MP_XSOAR_AND_V2) + + pack.create_script("test_incident_2").set_data(marketplaces=MP_XSOAR) + pack.create_script("test_alert_2").set_data(marketplaces=MP_XSOAR) + + pack.create_script("test_incident_3").set_data( + skipprepare=[SKIP_PREPARE_SCRIPT_NAME], marketplaces=MP_V2 + ) + pack.create_script("test_alert_3").set_data( + skipprepare=[SKIP_PREPARE_SCRIPT_NAME], marketplaces=MP_V2 + ) + + pack.create_script("test_incident_4").set_data(marketplaces=MP_XSOAR_AND_V2) + pack.create_script("test_alert_4").set_data(marketplaces=MP_XSOAR) + + BaseValidator.graph_interface = graph_repo.create_graph() + + results = DuplicatedScriptNameValidatorAllFiles().is_valid( + [script.object for script in pack.scripts] + ) + + assert len(results) == 1 + assert "test_alert_1.yml" == results[0].content_object.path.name \ No newline at end of file diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py index 0d6c3a7965c..f9d48cb90e1 100644 --- a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py @@ -19,23 +19,31 @@ class IsPackDisplayNameAlreadyExistsValidator(BaseValidator, ABC): error_code = "GR104" description = "Validate that there are no duplicate display names in the repo" rationale = " Validate the existance of duplicate display names" - error_message = "Pack '{content_name}' has a duplicate display_name as: {pack_display_names} " + error_message = "Pack '{content_id}' has a duplicate display_name as: {pack_display_id} " related_field = "" is_auto_fixable = False related_file_type = [RelatedFileType.JSON] - def is_valid_use_graph(self, file_paths_to_objects: Iterable[str]) -> List[ValidationResult]: - query_results = self.graph.get_duplicate_pack_display_name( - list(file_paths_to_objects) - ) + def is_valid_display_name(self, content_items: Iterable[ContentTypes], all_files=False) -> List[ValidationResult]: + + file_paths_to_objects = { + str(content_item.path).replace(f'{content_item.repo_path}/', '') + for content_item in content_items} + + content_id_to_objects = {item.object.object_id: item for item in content_items} + + query_list = list(file_paths_to_objects) if not all_files else [] + + query_results = self.graph.get_duplicate_pack_display_name(query_list) + return [ ValidationResult( validator=self, message=self.error_message.format( - content_id=content_id, pack_display_names=(', '.join(duplicate_names_id)) + content_id=content_id, pack_display_id=(', '.join(duplicate_names_id)) ), - content_object=content_id, + content_object=content_id_to_objects[content_id], ) for content_id, duplicate_names_id in query_results - ] - + if content_id in content_id_to_objects + ] \ No newline at end of file diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py index 860d1a31b3d..d52b87b7f48 100644 --- a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py @@ -20,4 +20,4 @@ class IsPackDisplayNameAlreadyExistsValidatorAllFiles(IsPackDisplayNameAlreadyEx expected_execution_mode = [ExecutionMode.ALL_FILES] def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: - return self.is_valid_use_graph([]) + return self.is_valid_display_name(content_items, True) diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py index e0f8d2ccec8..7773aca459c 100644 --- a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py @@ -20,5 +20,4 @@ class IsPackDisplayNameAlreadyExistsValidatorListFiles(IsPackDisplayNameAlreadyE expected_execution_mode = [ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT] def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: - file_paths_to_objects = [item.pack_id for item in content_items] - return self.is_valid_use_graph(file_paths_to_objects) + return self.is_valid_display_name(content_items, False) diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py index 64bb736d90f..bee9fec119e 100644 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator.py @@ -2,14 +2,9 @@ from abc import ABC -from typing import Iterable, List - -from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH -from demisto_sdk.commands.common.tools import replace_incident_to_alert from demisto_sdk.commands.content_graph.objects.script import Script from demisto_sdk.commands.validate.validators.base_validator import ( BaseValidator, - ValidationResult, ) ContentTypes = Script diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py index 112579fbb06..67d06aff99f 100644 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py @@ -2,6 +2,7 @@ from typing import Iterable, List +from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.constants import ExecutionMode from demisto_sdk.commands.common.tools import replace_incident_to_alert from demisto_sdk.commands.content_graph.objects.script import Script @@ -23,6 +24,10 @@ def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResu Validate that there are no duplicate names of scripts when the script name included `alert`. """ + file_paths_to_objects = { + str(content_item.path.relative_to(CONTENT_PATH)): content_item + for content_item in content_items + } query_results = self.graph.get_duplicate_script_name_included_incident([]) return [ @@ -31,7 +36,8 @@ def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResu message=self.error_message.format( replace_incident_to_alert(script_name), script_name ), - content_object=file_path, + content_object=file_paths_to_objects[file_path], ) for script_name, file_path in query_results.items() + if file_path in file_paths_to_objects ] From e9e987ec324df0661449d4a122d71cef4f4f0c1c Mon Sep 17 00:00:00 2001 From: Moshe Date: Sun, 9 Jun 2024 11:39:00 +0300 Subject: [PATCH 09/13] added unit test --- TestSuite/pack.py | 2 + .../validate/tests/GR_validators_test.py | 61 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 demisto_sdk/commands/validate/tests/GR_validators_test.py diff --git a/TestSuite/pack.py b/TestSuite/pack.py index a9f85848890..942a1c56e0d 100644 --- a/TestSuite/pack.py +++ b/TestSuite/pack.py @@ -70,6 +70,8 @@ class Pack(TestSuiteBase): def __init__(self, packs_dir: Path, name: str, repo): # Initiate lists: self.name = name + self.id = name + self.node_id = name self._repo = repo self.repo_path = repo.path self.integrations: List[Integration] = list() diff --git a/demisto_sdk/commands/validate/tests/GR_validators_test.py b/demisto_sdk/commands/validate/tests/GR_validators_test.py new file mode 100644 index 00000000000..fd4e9023518 --- /dev/null +++ b/demisto_sdk/commands/validate/tests/GR_validators_test.py @@ -0,0 +1,61 @@ + +from demisto_sdk.commands.validate.validators.base_validator import BaseValidator +from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists_all_files import ( + IsPackDisplayNameAlreadyExistsValidatorAllFiles +) +from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists_list_files import ( + IsPackDisplayNameAlreadyExistsValidatorListFiles, +) +from TestSuite.repo import Repo + + +def test_IsPackDisplayNameAlreadyExistsValidatorListFiles_is_valid(graph_repo: Repo): + """ + Given + - 3 packs, and 2 of them are with the same name + When + - running IsPackDisplayNameAlreadyExistsValidatorListFiles is_valid function. + Then + - Validate that we got the error messages for the duplicate name. + """ + + graph_repo.create_pack(name='pack1') + + graph_repo.create_pack(name='pack2') + graph_repo.packs[1].pack_metadata.update({"name": 'pack1',}) + + graph_repo.create_pack(name='pack3') + + BaseValidator.graph_interface = graph_repo.create_graph() + + results = IsPackDisplayNameAlreadyExistsValidatorListFiles().is_valid( + [pack for pack in graph_repo.packs] + ) + + assert len(results) == 2 + + +def test_IsPackDisplayNameAlreadyExistsValidatorAllFiles_is_valid(graph_repo: Repo): + """ + Given + - 3 packs, and 2 of them are with the same name + When + - running IsPackDisplayNameAlreadyExistsValidatorAllFiles is_valid function. + Then + - Validate that we got the error messages for the duplicate name. + """ + + graph_repo.create_pack(name='pack1') + + graph_repo.create_pack(name='pack2') + graph_repo.packs[1].pack_metadata.update({"name": 'pack1', }) + + graph_repo.create_pack(name='pack3') + + BaseValidator.graph_interface = graph_repo.create_graph() + + results = IsPackDisplayNameAlreadyExistsValidatorAllFiles().is_valid( + [pack for pack in graph_repo.packs] + ) + + assert len(results) == 2 From bf06167a604d748821690760a095c168c12241dc Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 10 Jun 2024 02:48:26 +0300 Subject: [PATCH 10/13] added support of using graph in the init_validation_script.py --- demisto_sdk/scripts/init_validation_script.py | 99 +++++++++++++++++-- 1 file changed, 89 insertions(+), 10 deletions(-) diff --git a/demisto_sdk/scripts/init_validation_script.py b/demisto_sdk/scripts/init_validation_script.py index 5513d311268..7baed2fcbe2 100644 --- a/demisto_sdk/scripts/init_validation_script.py +++ b/demisto_sdk/scripts/init_validation_script.py @@ -190,6 +190,7 @@ def __init__(self): self.run_on_deprecated = "" self.min_content_type_val = 1 self.max_content_type_val = int(list(CONTENT_TYPES_DICT.keys())[-1]) + self.using_graph = False def run_initializer(self): """ @@ -207,6 +208,7 @@ def run_info_request_functions(self): """ self.initialize_error_details() self.initialize_validation_details() + self.initialize_using_graph() self.initialize_file_name() def initialize_error_details(self): @@ -304,9 +306,11 @@ def initialize_validator_class_name(self): ) if not validator_class_name.endswith("Validator"): validator_class_name = f"{validator_class_name}Validator" - self.class_declaration = ( - f"class {validator_class_name}(BaseValidator[ContentTypes]):" - ) + self.class_name = validator_class_name + if not self.using_graph: + self.class_declaration = (f"class {validator_class_name}(BaseValidator[ContentTypes]):") + else: + self.class_declaration = (f"class {validator_class_name}(BaseValidator, ABC):") def initialize_git_statuses(self): """ @@ -433,6 +437,18 @@ def initialize_deprecation_info(self): if run_on_deprecated in ["Y", "y"]: self.run_on_deprecated = "\n run_on_deprecated = True" + def initialize_using_graph(self): + """ + Request the info wether the validation is using graph or not and ensure the input is valid. + """ + using_graph = str(input("does the validation using graph? (Y/N): ")) + while not using_graph or using_graph not in ["Y", "N", "y", "n"]: + using_graph = str( + input("Please enter wether the validation using graph or not? (Y/N): ") + ) + if using_graph in ["Y", "y"]: + self.using_graph = True + def initialize_file_name(self): """ Request the file name, ensure the given name is valid. @@ -462,6 +478,9 @@ def run_generators_function(self): self.generate_is_valid_function() self.generate_fix_function() self.generate_file_info() + if self.using_graph: + self.using_graph_all_files_class = self.Generate_files_according_execution_mode('AllFiles') + self.using_graph_list_files_class = self.Generate_files_according_execution_mode('ListFiles') def generate_related_file_section(self): """ @@ -492,20 +511,25 @@ def generate_imports(self): Generate the imports section string. """ self.imports = "from __future__ import annotations\n\n" + if self.using_graph: + self.imports += "from abc import ABC\n\n" if len(self.content_types) == 1: self.imports += "from typing import Iterable, List\n\n" else: self.imports += "from typing import Iterable, List, Union\n\n" + if self.git_statuses: self.imports += ( "from demisto_sdk.commands.common.constants import GitStatuses\n" ) + self.related_files_imports = '' if self.related_files: - self.imports += "from demisto_sdk.commands.content_graph.parsers.related_files import RelatedFileType\n" + self.related_files_imports += "from demisto_sdk.commands.content_graph.parsers.related_files import RelatedFileType\n" for content_type in self.content_types: - self.imports += ( + self.related_files_imports += ( f"{CONTENT_TYPES_DICT.get(content_type, {}).get('import', '')}\n" ) + self.imports += self.related_files_imports fix_result_import = "FixResult,\n " if self.support_fix else "" self.imports += f"""from demisto_sdk.commands.validate.validators.base_validator import ( BaseValidator, @@ -531,7 +555,8 @@ def generate_is_valid_function(self): """ Generate the is_valid function. """ - self.is_valid_method = """ + if not self.using_graph: + self.is_valid_method = """ def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: return [ ValidationResult( @@ -544,7 +569,23 @@ def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResu # Add your validation right here ) ] - """ + """ + + else: + self.is_valid_method = """ + def is_valid_using_graph(self, content_items: Iterable[ContentTypes], validate_all_files) -> List[ValidationResult]: + return [ + ValidationResult( + validator=self, + message=self.error_message, + content_object=content_item, + ) + for content_item in content_items + if ( + # Add your validation right here + ) + ] + """ def generate_fix_function(self): """ @@ -568,8 +609,8 @@ def generate_file_info(self): self.file_name = pascal_to_snake(self.class_declaration[6:-39]) if not self.file_name.startswith(self.error_code): self.file_name = f"{self.error_code}_{self.file_name}" - if not self.file_name.endswith(".py"): - self.file_name = f"{self.file_name}.py" + if self.file_name.endswith(".py"): + self.file_name = f"{self.file_name[0:-3]}" dir_path = ( f"demisto_sdk/commands/validate/validators/{self.error_code[:2]}_validators" ) @@ -577,6 +618,38 @@ def generate_file_info(self): os.makedirs(dir_path) self.file_path = f"{dir_path}/{self.file_name}" + def Generate_files_according_execution_mode(self, execution_mode): + """Generate files according to the execution_mode if using graph""" + if execution_mode == "AllFiles": + expected_execution_mode = '[ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT]' + all_files = True + else: + expected_execution_mode = '[ExecutionMode.ALL_FILES]' + all_files = False + return f""" +from __future__ import annotations + +from typing import Iterable, List + +from demisto_sdk.commands.common.constants import ExecutionMode +{self.related_files_imports} +from demisto_sdk.commands.validate.validators.base_validator import ( + BaseValidator, + ValidationResult, +) + +from demisto_sdk.commands.validate.validators.{self.error_code[:2]}_validators.{self.file_name} import {self.class_name} + +{self.supported_content_types} + + +class {self.class_name}{execution_mode}({self.class_name}, BaseValidator[ContentTypes]): + expected_execution_mode = {expected_execution_mode} + + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: + return self.is_valid_using_graph(content_items, {all_files}) + """ + """ Create new py file """ def create_new_py_file(self): @@ -584,7 +657,7 @@ def create_new_py_file(self): insert all the information into the validation template and write the validation into a new py file with the given name under demisto_sdk/commands/validate/validators/_validators. """ - with open(self.file_path, "w") as file: + with open(f'{self.file_path}.py', "w") as file: # Write the content into VALIDATION_TEMPLATE new_file_content = Template(VALIDATION_TEMPLATE).safe_substitute( imports=self.imports, @@ -605,6 +678,12 @@ def create_new_py_file(self): ) file.write(new_file_content) + if self.using_graph: + with open(f'{self.file_path}_all_files.py', "w") as file: + file.write(self.using_graph_all_files_class) + with open(f'{self.file_path}_list_files.py', "w") as file: + file.write(self.using_graph_list_files_class) + def main(): try: From 91f46c35aca0e0107261c281000d8e8b76b19796 Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 10 Jun 2024 02:48:57 +0300 Subject: [PATCH 11/13] fixed --- .../validate/tests/validators_test.py | 28 +++++++++---------- ..._display_name_already_exists_list_files.py | 1 - 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/demisto_sdk/commands/validate/tests/validators_test.py b/demisto_sdk/commands/validate/tests/validators_test.py index dc10248a42d..6ebb1ee7f12 100644 --- a/demisto_sdk/commands/validate/tests/validators_test.py +++ b/demisto_sdk/commands/validate/tests/validators_test.py @@ -8,7 +8,7 @@ import toml from more_itertools import map_reduce -from demisto_sdk.commands.common.constants import INTEGRATIONS_DIR, GitStatuses +from demisto_sdk.commands.common.constants import INTEGRATIONS_DIR, GitStatuses, ExecutionMode from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json from demisto_sdk.commands.content_graph.common import ContentType @@ -125,18 +125,18 @@ def test_filter_validators(mocker, validations_to_run, sub_classes, expected_res @pytest.mark.parametrize( - "category_to_run, use_git, config_file_content, expected_results, ignore_support_level", + "category_to_run, execution_mode, config_file_content, expected_results, ignore_support_level", [ ( None, - True, + ExecutionMode.USE_GIT, {"use_git": {"select": ["BA101", "BC100", "PA108"]}}, ConfiguredValidations(["BA101", "BC100", "PA108"], [], [], {}), False, ), ( "custom_category", - True, + ExecutionMode.USE_GIT, { "ignorable_errors": ["BA101"], "custom_category": { @@ -149,14 +149,14 @@ def test_filter_validators(mocker, validations_to_run, sub_classes, expected_res ), ( None, - False, + ExecutionMode.SPECIFIC_FILES, {"path_based_validations": {"select": ["BA101", "BC100", "PA108"]}}, ConfiguredValidations(["BA101", "BC100", "PA108"], [], [], {}), False, ), ( None, - True, + ExecutionMode.USE_GIT, { "support_level": {"community": {"ignore": ["BA101", "BC100", "PA108"]}}, "use_git": {"select": ["TE105", "TE106", "TE107"]}, @@ -171,7 +171,7 @@ def test_filter_validators(mocker, validations_to_run, sub_classes, expected_res ), ( None, - True, + ExecutionMode.USE_GIT, { "support_level": {"community": {"ignore": ["BA101", "BC100", "PA108"]}}, "use_git": {"select": ["TE105", "TE106", "TE107"]}, @@ -184,7 +184,7 @@ def test_filter_validators(mocker, validations_to_run, sub_classes, expected_res def test_gather_validations_to_run( mocker, category_to_run, - use_git, + execution_mode, config_file_content, expected_results, ignore_support_level, @@ -192,11 +192,11 @@ def test_gather_validations_to_run( """ Given a category_to_run, a use_git flag, a config file content, and a ignore_support_level flag. - - Case 1: No category to run, use_git flag set to True, config file content with only use_git.select section, and ignore_support_level set to False. - - Case 2: A custom category to run, use_git flag set to True, config file content with use_git.select, and custom_category with both ignorable_errors and select sections, and ignore_support_level set to False. - - Case 3: No category to run, use_git flag set to False, config file content with path_based_validations.select section, and ignore_support_level set to False. - - Case 4: No category to run, use_git flag set to True, config file content with use_git.select, and support_level.community.ignore section, and ignore_support_level set to False. - - Case 5: No category to run, use_git flag set to True, config file content with use_git.select, and support_level.community.ignore section, and ignore_support_level set to True. + - Case 1: No category to run, execution_mode set to use_git, config file content with only use_git.select section, and ignore_support_level set to False. + - Case 2: A custom category to run, execution_mode set to use_git, config file content with use_git.select, and custom_category with both ignorable_errors and select sections, and ignore_support_level set to False. + - Case 3: No category to run, execution_mode not set to use_git, config file content with path_based_validations.select section, and ignore_support_level set to False. + - Case 4: No category to run, execution_mode set to use_git, config file content with use_git.select, and support_level.community.ignore section, and ignore_support_level set to False. + - Case 5: No category to run, execution_mode set to use_git, config file content with use_git.select, and support_level.community.ignore section, and ignore_support_level set to True. When - Calling the gather_validations_to_run function. Then @@ -209,7 +209,7 @@ def test_gather_validations_to_run( mocker.patch.object(toml, "load", return_value=config_file_content) config_reader = ConfigReader(category_to_run=category_to_run) results: ConfiguredValidations = config_reader.gather_validations_to_run( - use_git=use_git, ignore_support_level=ignore_support_level + execution_mode=execution_mode, ignore_support_level=ignore_support_level ) assert results.validations_to_run == expected_results.validations_to_run assert results.ignorable_errors == expected_results.ignorable_errors diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py index 7773aca459c..e9d56520f34 100644 --- a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py @@ -3,7 +3,6 @@ from typing import Iterable, List -from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.constants import ExecutionMode from demisto_sdk.commands.content_graph.objects.pack import Pack from demisto_sdk.commands.validate.validators.base_validator import ( From 237c17c03e4f0560d81ae048e9355dbb50ee5ae5 Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 10 Jun 2024 03:21:05 +0300 Subject: [PATCH 12/13] pre commit fixes --- demisto_sdk/__main__.py | 2 +- .../tests/SC_graph_validation_test.py | 19 +++++++---- .../commands/validate/config_reader.py | 8 +++-- .../commands/validate/default_config.toml | 2 ++ demisto_sdk/commands/validate/initializer.py | 2 +- .../validate/tests/GR_validators_test.py | 27 ++++++++++------ .../validate/tests/validators_test.py | 14 ++++++-- .../commands/validate/validate_manager.py | 3 +- ...104_is_pack_display_name_already_exists.py | 28 +++++++++------- ...k_display_name_already_exists_all_files.py | 17 +++++----- ..._display_name_already_exists_list_files.py | 16 ++++++---- ..._name_is_not_unique_validator_all_files.py | 9 ++++-- ..._name_is_not_unique_validator_file_list.py | 8 +++-- .../validate/validators/base_validator.py | 8 +++-- demisto_sdk/scripts/init_validation_script.py | 32 ++++++++++++------- 15 files changed, 123 insertions(+), 72 deletions(-) diff --git a/demisto_sdk/__main__.py b/demisto_sdk/__main__.py index 0cb9748222d..e417b128895 100644 --- a/demisto_sdk/__main__.py +++ b/demisto_sdk/__main__.py @@ -28,9 +28,9 @@ from demisto_sdk.commands.common.constants import ( DEMISTO_SDK_MARKETPLACE_XSOAR_DIST_DEV, ENV_DEMISTO_SDK_MARKETPLACE, + ExecutionMode, FileType, MarketplaceVersions, - ExecutionMode ) from demisto_sdk.commands.common.content_constant_paths import ( ALL_PACKS_DEPENDENCIES_DEFAULT_PATH, diff --git a/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py b/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py index 4cbc9821d4f..d1ab1ec9139 100644 --- a/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py +++ b/demisto_sdk/commands/content_graph/tests/SC_graph_validation_test.py @@ -4,14 +4,15 @@ ) from demisto_sdk.commands.validate.validators.base_validator import BaseValidator from demisto_sdk.commands.validate.validators.SC_validators import ( - SC109_script_name_is_not_unique_validator_file_list, SC109_script_name_is_not_unique_validator_all_files + SC109_script_name_is_not_unique_validator_all_files, + SC109_script_name_is_not_unique_validator_file_list, +) +from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator_all_files import ( + DuplicatedScriptNameValidatorAllFiles, ) from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator_file_list import ( DuplicatedScriptNameValidatorFileList, ) -from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator_all_files import ( - DuplicatedScriptNameValidatorAllFiles -) from TestSuite.repo import Repo MP_XSOAR = [MarketplaceVersions.XSOAR.value] @@ -36,7 +37,9 @@ def test_DuplicatedScriptNameValidatorFileList_is_valid(mocker, graph_repo: Repo - Validate that only the first pair of scripts appear in the results, and the rest of the scripts is valid. """ mocker.patch.object( - SC109_script_name_is_not_unique_validator_file_list, "CONTENT_PATH", new=graph_repo.path + SC109_script_name_is_not_unique_validator_file_list, + "CONTENT_PATH", + new=graph_repo.path, ) pack = graph_repo.create_pack() @@ -80,7 +83,9 @@ def test_DuplicatedScriptNameValidatorAllFiles_is_valid(mocker, graph_repo: Repo - Validate that only the first pair of scripts appear in the results, and the rest of the scripts is valid. """ mocker.patch.object( - SC109_script_name_is_not_unique_validator_all_files, "CONTENT_PATH", new=graph_repo.path + SC109_script_name_is_not_unique_validator_all_files, + "CONTENT_PATH", + new=graph_repo.path, ) pack = graph_repo.create_pack() @@ -107,4 +112,4 @@ def test_DuplicatedScriptNameValidatorAllFiles_is_valid(mocker, graph_repo: Repo ) assert len(results) == 1 - assert "test_alert_1.yml" == results[0].content_object.path.name \ No newline at end of file + assert "test_alert_1.yml" == results[0].content_object.path.name diff --git a/demisto_sdk/commands/validate/config_reader.py b/demisto_sdk/commands/validate/config_reader.py index 10c396d17b8..4474aecb8d2 100644 --- a/demisto_sdk/commands/validate/config_reader.py +++ b/demisto_sdk/commands/validate/config_reader.py @@ -3,8 +3,8 @@ import toml -from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.common.logger import logger USE_GIT = "use_git" PATH_BASED_VALIDATIONS = "path_based_validations" @@ -54,7 +54,11 @@ def gather_validations_to_run( Tuple[List, List, List, dict]: the select, warning, and ignorable errors sections from the given category, and the support_level dict with errors to ignore. """ - flag = self.category_to_run or (USE_GIT if execution_mode == ExecutionMode.USE_GIT else PATH_BASED_VALIDATIONS) + flag = self.category_to_run or ( + USE_GIT + if execution_mode == ExecutionMode.USE_GIT + else PATH_BASED_VALIDATIONS + ) section = self.config_file_content.get(flag, {}) return ConfiguredValidations( section.get("select", []), diff --git a/demisto_sdk/commands/validate/default_config.toml b/demisto_sdk/commands/validate/default_config.toml index 6b4ef9fc9ae..fad968fdaf6 100644 --- a/demisto_sdk/commands/validate/default_config.toml +++ b/demisto_sdk/commands/validate/default_config.toml @@ -45,6 +45,7 @@ select = [ "RM104", "RM105", "RM113", "RM114", "CL100", "GF100", "GF101", + "GR104", "IF100", "IF101", "IF102", "IF103", "IF104", "IF105", "IF106", "IM100", "IM101", "IM108", "IM109", "RP101" @@ -66,6 +67,7 @@ select = [ "RM104", "RM105", "RM113", "RM114", "CL100", "GF100", "GF101", + "GR104", "IF100", "IF101", "IF102", "IF103", "IF104", "IF105", "IF106", "IM101", "IM108", "RP101" diff --git a/demisto_sdk/commands/validate/initializer.py b/demisto_sdk/commands/validate/initializer.py index 6a32a4cd921..e5b984fc7da 100644 --- a/demisto_sdk/commands/validate/initializer.py +++ b/demisto_sdk/commands/validate/initializer.py @@ -20,9 +20,9 @@ PLAYBOOKS_DIR, RELEASE_NOTES_DIR, SCRIPTS_DIR, + ExecutionMode, GitStatuses, PathLevel, - ExecutionMode, ) from demisto_sdk.commands.common.content import Content from demisto_sdk.commands.common.logger import logger diff --git a/demisto_sdk/commands/validate/tests/GR_validators_test.py b/demisto_sdk/commands/validate/tests/GR_validators_test.py index fd4e9023518..55ef83f237e 100644 --- a/demisto_sdk/commands/validate/tests/GR_validators_test.py +++ b/demisto_sdk/commands/validate/tests/GR_validators_test.py @@ -1,7 +1,6 @@ - from demisto_sdk.commands.validate.validators.base_validator import BaseValidator from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists_all_files import ( - IsPackDisplayNameAlreadyExistsValidatorAllFiles + IsPackDisplayNameAlreadyExistsValidatorAllFiles, ) from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists_list_files import ( IsPackDisplayNameAlreadyExistsValidatorListFiles, @@ -19,12 +18,16 @@ def test_IsPackDisplayNameAlreadyExistsValidatorListFiles_is_valid(graph_repo: R - Validate that we got the error messages for the duplicate name. """ - graph_repo.create_pack(name='pack1') + graph_repo.create_pack(name="pack1") - graph_repo.create_pack(name='pack2') - graph_repo.packs[1].pack_metadata.update({"name": 'pack1',}) + graph_repo.create_pack(name="pack2") + graph_repo.packs[1].pack_metadata.update( + { + "name": "pack1", + } + ) - graph_repo.create_pack(name='pack3') + graph_repo.create_pack(name="pack3") BaseValidator.graph_interface = graph_repo.create_graph() @@ -45,12 +48,16 @@ def test_IsPackDisplayNameAlreadyExistsValidatorAllFiles_is_valid(graph_repo: Re - Validate that we got the error messages for the duplicate name. """ - graph_repo.create_pack(name='pack1') + graph_repo.create_pack(name="pack1") - graph_repo.create_pack(name='pack2') - graph_repo.packs[1].pack_metadata.update({"name": 'pack1', }) + graph_repo.create_pack(name="pack2") + graph_repo.packs[1].pack_metadata.update( + { + "name": "pack1", + } + ) - graph_repo.create_pack(name='pack3') + graph_repo.create_pack(name="pack3") BaseValidator.graph_interface = graph_repo.create_graph() diff --git a/demisto_sdk/commands/validate/tests/validators_test.py b/demisto_sdk/commands/validate/tests/validators_test.py index 6ebb1ee7f12..d900724ac1f 100644 --- a/demisto_sdk/commands/validate/tests/validators_test.py +++ b/demisto_sdk/commands/validate/tests/validators_test.py @@ -8,7 +8,11 @@ import toml from more_itertools import map_reduce -from demisto_sdk.commands.common.constants import INTEGRATIONS_DIR, GitStatuses, ExecutionMode +from demisto_sdk.commands.common.constants import ( + INTEGRATIONS_DIR, + ExecutionMode, + GitStatuses, +) from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json from demisto_sdk.commands.content_graph.common import ContentType @@ -435,7 +439,9 @@ def test_should_run(validator, expected_results): - Case 2: Should return False. - Case 3: Should return False. """ - assert expected_results == validator.should_run(INTEGRATION, [], {}) + assert expected_results == validator.should_run( + INTEGRATION, [], {}, execution_mode=ExecutionMode.USE_GIT + ) def test_object_collection_with_readme_path(repo): @@ -476,7 +482,9 @@ def test_object_collection_with_pack_path(repo): integration = pack.create_integration(yml=yml_content) integration.code.write("from MicrosoftApiModule import *") integration.readme.write("test") - initializer = Initializer(file_path=pack.path) + initializer = Initializer( + file_path=pack.path, execution_mode=ExecutionMode.SPECIFIC_FILES + ) obj_set, _ = initializer.gather_objects_to_run_on() obj_types = {obj.content_type for obj in obj_set} assert obj_types == {ContentType.INTEGRATION, ContentType.PACK} diff --git a/demisto_sdk/commands/validate/validate_manager.py b/demisto_sdk/commands/validate/validate_manager.py index 7627065c644..9ebe00d849e 100644 --- a/demisto_sdk/commands/validate/validate_manager.py +++ b/demisto_sdk/commands/validate/validate_manager.py @@ -45,7 +45,8 @@ def __init__( self.committed_only = self.initializer.committed_only self.configured_validations: ConfiguredValidations = ( self.config_reader.gather_validations_to_run( - self.initializer.execution_mode, ignore_support_level=self.ignore_support_level + self.initializer.execution_mode, + ignore_support_level=self.ignore_support_level, ) ) self.validators = self.filter_validators() diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py index f9d48cb90e1..6c483292d5b 100644 --- a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists.py @@ -1,15 +1,13 @@ - from __future__ import annotations from abc import ABC - from typing import Iterable, List -from demisto_sdk.commands.content_graph.parsers.related_files import RelatedFileType from demisto_sdk.commands.content_graph.objects.pack import Pack +from demisto_sdk.commands.content_graph.parsers.related_files import RelatedFileType from demisto_sdk.commands.validate.validators.base_validator import ( - BaseValidator, - ValidationResult, + BaseValidator, + ValidationResult, ) ContentTypes = Pack @@ -19,18 +17,23 @@ class IsPackDisplayNameAlreadyExistsValidator(BaseValidator, ABC): error_code = "GR104" description = "Validate that there are no duplicate display names in the repo" rationale = " Validate the existance of duplicate display names" - error_message = "Pack '{content_id}' has a duplicate display_name as: {pack_display_id} " + error_message = ( + "Pack '{content_id}' has a duplicate display_name as: {pack_display_id} " + ) related_field = "" is_auto_fixable = False related_file_type = [RelatedFileType.JSON] - def is_valid_display_name(self, content_items: Iterable[ContentTypes], all_files=False) -> List[ValidationResult]: + def is_valid_display_name( + self, content_items: Iterable[ContentTypes], all_files=False + ) -> List[ValidationResult]: file_paths_to_objects = { - str(content_item.path).replace(f'{content_item.repo_path}/', '') - for content_item in content_items} + str(content_item.path).replace(f"{content_item.repo_path}/", "") # type: ignore[attr-defined] + for content_item in content_items + } - content_id_to_objects = {item.object.object_id: item for item in content_items} + content_id_to_objects = {item.object.object_id: item for item in content_items} # type: ignore[attr-defined] query_list = list(file_paths_to_objects) if not all_files else [] @@ -40,10 +43,11 @@ def is_valid_display_name(self, content_items: Iterable[ContentTypes], all_files ValidationResult( validator=self, message=self.error_message.format( - content_id=content_id, pack_display_id=(', '.join(duplicate_names_id)) + content_id=content_id, + pack_display_id=(", ".join(duplicate_names_id)), ), content_object=content_id_to_objects[content_id], ) for content_id, duplicate_names_id in query_results if content_id in content_id_to_objects - ] \ No newline at end of file + ] diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py index d52b87b7f48..5632fa43a94 100644 --- a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_all_files.py @@ -1,4 +1,3 @@ - from __future__ import annotations from typing import Iterable, List @@ -6,18 +5,20 @@ from demisto_sdk.commands.common.constants import ExecutionMode from demisto_sdk.commands.content_graph.objects.pack import Pack from demisto_sdk.commands.validate.validators.base_validator import ( - BaseValidator, - ValidationResult, + BaseValidator, + ValidationResult, +) +from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists import ( + IsPackDisplayNameAlreadyExistsValidator, ) - -from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists import IsPackDisplayNameAlreadyExistsValidator - ContentTypes = Pack -class IsPackDisplayNameAlreadyExistsValidatorAllFiles(IsPackDisplayNameAlreadyExistsValidator, BaseValidator[ContentTypes]): +class IsPackDisplayNameAlreadyExistsValidatorAllFiles( + IsPackDisplayNameAlreadyExistsValidator, BaseValidator[ContentTypes] +): expected_execution_mode = [ExecutionMode.ALL_FILES] - + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: return self.is_valid_display_name(content_items, True) diff --git a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py index e9d56520f34..f8d765ff62d 100644 --- a/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py +++ b/demisto_sdk/commands/validate/validators/GR_validators/GR104_is_pack_display_name_already_exists_list_files.py @@ -1,4 +1,3 @@ - from __future__ import annotations from typing import Iterable, List @@ -6,17 +5,20 @@ from demisto_sdk.commands.common.constants import ExecutionMode from demisto_sdk.commands.content_graph.objects.pack import Pack from demisto_sdk.commands.validate.validators.base_validator import ( - BaseValidator, - ValidationResult, + BaseValidator, + ValidationResult, +) +from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists import ( + IsPackDisplayNameAlreadyExistsValidator, ) - -from demisto_sdk.commands.validate.validators.GR_validators.GR104_is_pack_display_name_already_exists import IsPackDisplayNameAlreadyExistsValidator ContentTypes = Pack -class IsPackDisplayNameAlreadyExistsValidatorListFiles(IsPackDisplayNameAlreadyExistsValidator, BaseValidator[ContentTypes]): +class IsPackDisplayNameAlreadyExistsValidatorListFiles( + IsPackDisplayNameAlreadyExistsValidator, BaseValidator[ContentTypes] +): expected_execution_mode = [ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT] - + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: return self.is_valid_display_name(content_items, False) diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py index 67d06aff99f..9c7bccbb04f 100644 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_all_files.py @@ -2,8 +2,8 @@ from typing import Iterable, List -from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.tools import replace_incident_to_alert from demisto_sdk.commands.content_graph.objects.script import Script from demisto_sdk.commands.validate.validators.base_validator import ( @@ -11,12 +11,15 @@ ValidationResult, ) from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator import ( - DuplicatedScriptNameValidator + DuplicatedScriptNameValidator, ) + ContentTypes = Script -class DuplicatedScriptNameValidatorAllFiles(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): +class DuplicatedScriptNameValidatorAllFiles( + DuplicatedScriptNameValidator, BaseValidator[ContentTypes] +): expected_execution_mode = [ExecutionMode.ALL_FILES] def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: diff --git a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py index 16e2d1235b7..3da1dd6e02a 100644 --- a/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py +++ b/demisto_sdk/commands/validate/validators/SC_validators/SC109_script_name_is_not_unique_validator_file_list.py @@ -2,8 +2,8 @@ from typing import Iterable, List -from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.constants import ExecutionMode +from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.tools import replace_incident_to_alert from demisto_sdk.commands.content_graph.objects.script import Script from demisto_sdk.commands.validate.validators.base_validator import ( @@ -11,13 +11,15 @@ ValidationResult, ) from demisto_sdk.commands.validate.validators.SC_validators.SC109_script_name_is_not_unique_validator import ( - DuplicatedScriptNameValidator + DuplicatedScriptNameValidator, ) ContentTypes = Script -class DuplicatedScriptNameValidatorFileList(DuplicatedScriptNameValidator, BaseValidator[ContentTypes]): +class DuplicatedScriptNameValidatorFileList( + DuplicatedScriptNameValidator, BaseValidator[ContentTypes] +): expected_execution_mode = [ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT] def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: diff --git a/demisto_sdk/commands/validate/validators/base_validator.py b/demisto_sdk/commands/validate/validators/base_validator.py index 37d0fc5f371..d4f48831242 100644 --- a/demisto_sdk/commands/validate/validators/base_validator.py +++ b/demisto_sdk/commands/validate/validators/base_validator.py @@ -14,7 +14,7 @@ from pydantic import BaseModel -from demisto_sdk.commands.common.constants import GitStatuses, ExecutionMode +from demisto_sdk.commands.common.constants import ExecutionMode, GitStatuses from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.tools import is_abstract_class @@ -108,7 +108,7 @@ def should_run( content_item: ContentTypes, ignorable_errors: list, support_level_dict: dict, - execution_mode: ExecutionMode + execution_mode: ExecutionMode, ) -> bool: """check whether to run validation on the given content item or not. @@ -125,7 +125,9 @@ def should_run( [ isinstance(content_item, self.get_content_types()), should_run_on_deprecated(self.run_on_deprecated, content_item), - should_run_on_execution_mode(self.expected_execution_mode, execution_mode), + should_run_on_execution_mode( + self.expected_execution_mode, execution_mode + ), should_run_according_to_status( content_item.git_status, self.expected_git_statuses ), diff --git a/demisto_sdk/scripts/init_validation_script.py b/demisto_sdk/scripts/init_validation_script.py index 7baed2fcbe2..8cdffcc3262 100644 --- a/demisto_sdk/scripts/init_validation_script.py +++ b/demisto_sdk/scripts/init_validation_script.py @@ -308,9 +308,13 @@ def initialize_validator_class_name(self): validator_class_name = f"{validator_class_name}Validator" self.class_name = validator_class_name if not self.using_graph: - self.class_declaration = (f"class {validator_class_name}(BaseValidator[ContentTypes]):") + self.class_declaration = ( + f"class {validator_class_name}(BaseValidator[ContentTypes]):" + ) else: - self.class_declaration = (f"class {validator_class_name}(BaseValidator, ABC):") + self.class_declaration = ( + f"class {validator_class_name}(BaseValidator, ABC):" + ) def initialize_git_statuses(self): """ @@ -479,8 +483,12 @@ def run_generators_function(self): self.generate_fix_function() self.generate_file_info() if self.using_graph: - self.using_graph_all_files_class = self.Generate_files_according_execution_mode('AllFiles') - self.using_graph_list_files_class = self.Generate_files_according_execution_mode('ListFiles') + self.using_graph_all_files_class = ( + self.Generate_files_according_execution_mode("AllFiles") + ) + self.using_graph_list_files_class = ( + self.Generate_files_according_execution_mode("ListFiles") + ) def generate_related_file_section(self): """ @@ -522,7 +530,7 @@ def generate_imports(self): self.imports += ( "from demisto_sdk.commands.common.constants import GitStatuses\n" ) - self.related_files_imports = '' + self.related_files_imports = "" if self.related_files: self.related_files_imports += "from demisto_sdk.commands.content_graph.parsers.related_files import RelatedFileType\n" for content_type in self.content_types: @@ -621,10 +629,12 @@ def generate_file_info(self): def Generate_files_according_execution_mode(self, execution_mode): """Generate files according to the execution_mode if using graph""" if execution_mode == "AllFiles": - expected_execution_mode = '[ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT]' + expected_execution_mode = ( + "[ExecutionMode.SPECIFIC_FILES, ExecutionMode.USE_GIT]" + ) all_files = True else: - expected_execution_mode = '[ExecutionMode.ALL_FILES]' + expected_execution_mode = "[ExecutionMode.ALL_FILES]" all_files = False return f""" from __future__ import annotations @@ -645,7 +655,7 @@ def Generate_files_according_execution_mode(self, execution_mode): class {self.class_name}{execution_mode}({self.class_name}, BaseValidator[ContentTypes]): expected_execution_mode = {expected_execution_mode} - + def is_valid(self, content_items: Iterable[ContentTypes]) -> List[ValidationResult]: return self.is_valid_using_graph(content_items, {all_files}) """ @@ -657,7 +667,7 @@ def create_new_py_file(self): insert all the information into the validation template and write the validation into a new py file with the given name under demisto_sdk/commands/validate/validators/_validators. """ - with open(f'{self.file_path}.py', "w") as file: + with open(f"{self.file_path}.py", "w") as file: # Write the content into VALIDATION_TEMPLATE new_file_content = Template(VALIDATION_TEMPLATE).safe_substitute( imports=self.imports, @@ -679,9 +689,9 @@ def create_new_py_file(self): file.write(new_file_content) if self.using_graph: - with open(f'{self.file_path}_all_files.py', "w") as file: + with open(f"{self.file_path}_all_files.py", "w") as file: file.write(self.using_graph_all_files_class) - with open(f'{self.file_path}_list_files.py', "w") as file: + with open(f"{self.file_path}_list_files.py", "w") as file: file.write(self.using_graph_list_files_class) From 8a3e8c8fe2a0e9d98ecb6786f266ce8d2fd5ba8a Mon Sep 17 00:00:00 2001 From: Moshe Date: Mon, 10 Jun 2024 11:11:51 +0300 Subject: [PATCH 13/13] added changelog --- .changelog/4301.yml | 4 ++++ demisto_sdk/commands/validate/sdk_validation_config.toml | 2 ++ 2 files changed, 6 insertions(+) create mode 100644 .changelog/4301.yml diff --git a/.changelog/4301.yml b/.changelog/4301.yml new file mode 100644 index 00000000000..0e50b730155 --- /dev/null +++ b/.changelog/4301.yml @@ -0,0 +1,4 @@ +changes: +- description: added support in the new validation. For validators that run on all files and use the graph. + type: internal +pr_number: 4301 diff --git a/demisto_sdk/commands/validate/sdk_validation_config.toml b/demisto_sdk/commands/validate/sdk_validation_config.toml index 37f075147bc..6e273782378 100644 --- a/demisto_sdk/commands/validate/sdk_validation_config.toml +++ b/demisto_sdk/commands/validate/sdk_validation_config.toml @@ -36,6 +36,7 @@ select = [ "RM101", "RN103", "RM104", "RM105", "RM106", "RM109", "RM113", "RM114", "CL100", "GF100", "GF101", "GF102", + "GR104", "IF100", "IF101", "IF102", "IF103", "IF104", "IF105", "IF106", "IF116", "IM100", "IM101", "IM108", "IM109", "IM106", "IM111", "RP101", "BC106", "BC107", @@ -62,6 +63,7 @@ select = [ "RM104", "RM105", "RM113", "RM114", "CL100", "GF100", "GF101", + "GR104", "IF100", "IF101", "IF102", "IF103", "IF104", "IF105", "IF106", "IF116", "IM101", "IM108", "IM111", "RP101",