Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Validations #4753

Merged
merged 56 commits into from
Jan 12, 2025
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
454ca0f
first
merit-maita Jan 2, 2025
384449e
added rn
merit-maita Jan 2, 2025
eb81dfe
added IF117
merit-maita Jan 2, 2025
e773e26
added IF117
merit-maita Jan 2, 2025
463e924
added IF117 to toml
merit-maita Jan 2, 2025
46126ba
fixes in IF117
merit-maita Jan 2, 2025
1a1bd97
fixes in IF117
merit-maita Jan 2, 2025
b43e102
fix
merit-maita Jan 2, 2025
5e50a55
edit
merit-maita Jan 2, 2025
2d2542c
revert
merit-maita Jan 2, 2025
523b8cf
edited object
merit-maita Jan 2, 2025
1fa38d3
edit
merit-maita Jan 2, 2025
e39ed5a
fixes in IF117
merit-maita Jan 2, 2025
2c8caf9
fixes in IF117
merit-maita Jan 2, 2025
3c085b7
fixes in IF117
merit-maita Jan 5, 2025
883833c
eit IF109
merit-maita Jan 5, 2025
5a51a96
eit IF109
merit-maita Jan 5, 2025
12a4fc4
fixes in IF117
merit-maita Jan 5, 2025
9304de5
Merge branch 'refs/heads/master' into refactoring
merit-maita Jan 5, 2025
7df0019
Merge branch 'refs/heads/master' into refactoring
merit-maita Jan 6, 2025
dd0df1b
edits
merit-maita Jan 7, 2025
1f6ac4e
Merge branch 'master' into refactoring
merit-maita Jan 7, 2025
b3a31a4
edit
merit-maita Jan 7, 2025
1b91119
edits for pre-commit
merit-maita Jan 7, 2025
0a63420
added nl
merit-maita Jan 7, 2025
e36e650
added unittests
merit-maita Jan 7, 2025
c124dae
added unittests for if109
merit-maita Jan 7, 2025
08c93c2
edit
merit-maita Jan 7, 2025
8fb8c15
edit after results
merit-maita Jan 7, 2025
2d8dc4d
minor ruff edit
merit-maita Jan 7, 2025
3c2426f
edit comment
merit-maita Jan 7, 2025
f0a1fc9
edited the rn
merit-maita Jan 8, 2025
c076dfb
edits after cr
merit-maita Jan 8, 2025
4ab4b68
pre-commit edits
merit-maita Jan 8, 2025
bdd53e2
edited ut
merit-maita Jan 8, 2025
86c02c7
edits after cr
merit-maita Jan 8, 2025
4329fba
edits after pre-commit
merit-maita Jan 8, 2025
715287c
minor edit
merit-maita Jan 8, 2025
75533f6
pre-commit edits
merit-maita Jan 8, 2025
f1fa212
minor fix
merit-maita Jan 9, 2025
474dc60
Merge branch 'master' into refactoring
merit-maita Jan 12, 2025
1373c9b
fixes after cr
merit-maita Jan 12, 2025
f708b2f
fixes after cr
merit-maita Jan 12, 2025
a060263
added aliases field
merit-maita Jan 12, 2025
1efa63a
fix
merit-maita Jan 12, 2025
24cfd14
added to if118
merit-maita Jan 12, 2025
3a19ea4
update
merit-maita Jan 12, 2025
63f303f
edit
merit-maita Jan 12, 2025
1c5f04e
revert 118
merit-maita Jan 12, 2025
f7f514c
revert
merit-maita Jan 12, 2025
61e5f42
returned field
merit-maita Jan 12, 2025
d540ae1
added usage
merit-maita Jan 12, 2025
dff2670
change field
merit-maita Jan 12, 2025
b4618a4
change parsers
merit-maita Jan 12, 2025
7fa9a96
revert
merit-maita Jan 12, 2025
e38cb83
revert
merit-maita Jan 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changelog/4753.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
changes:
YuvHayun marked this conversation as resolved.
Show resolved Hide resolved
- description: "Added IF109 to the new validation format. The validation Checks whether an incident or indicator field
has a valid required field value."
type: feature
- description: "Added IF117 to the new validation format. The validation Checks whether an incident or indicator field
aliases have a valid marketplaces field value."
type: feature
pr_number: 4753
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class IndicatorIncidentField(ContentItem):
group: int = Field(None, exclude=True)
unsearchable: Optional[bool] = Field(None, exclude=True)
version: Optional[int] = 0
required: bool = Field(None, alias="required")
associated_types: list = Field(None, alias="associatedTypes")

def _upload(
self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ def __init__(
self.content = self.json_data.get("content")
self.system = self.json_data.get("system")
self.group = self.json_data.get("group")
self.required = self.json_data.get("required")
self.associated_types = self.json_data.get("associatedTypes")

self.connect_to_dependencies()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ def __init__(
super().__init__(path, pack_marketplaces, git_sha=git_sha)
self.associated_to_all = self.json_data.get("associatedToAll")
self.select_values = self.json_data.get("selectValues")
self.required = self.json_data.get("required")
self.associated_types = self.json_data.get("associatedTypes")

self.connect_to_dependencies()

Expand Down
3 changes: 3 additions & 0 deletions demisto_sdk/commands/validate/sdk_validation_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ select = [
"IF105",
"IF106",
"IF116",
"IF117",
"IF118",
"IF119",
"IM101",
Expand Down Expand Up @@ -470,10 +471,12 @@ select = [
"IF104",
"IF105",
"IF106",
"IF109",
"IF111",
"IF113",
"IF115",
"IF116",
"IF117",
"IF118",
"IF119",
"IM100",
Expand Down
123 changes: 123 additions & 0 deletions demisto_sdk/commands/validate/tests/IF_validators_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

import pytest

from demisto_sdk.commands.common.constants import GitStatuses, MarketplaceVersions
from demisto_sdk.commands.content_graph.objects.incident_field import IncidentField
from demisto_sdk.commands.validate.tests.test_tools import (
REPO,
create_incident_field_object,
create_incident_type_object,
create_old_file_pointers,
)
from demisto_sdk.commands.validate.validators.IF_validators.IF100_is_valid_name_and_cli_name import (
Expand All @@ -32,6 +34,9 @@
INCIDENT_PROHIBITED_CLI_NAMES,
IsCliNameReservedWordValidator,
)
from demisto_sdk.commands.validate.validators.IF_validators.IF109_invalid_required_field import (
IsValidRequiredFieldValidator,
)
from demisto_sdk.commands.validate.validators.IF_validators.IF111_is_field_type_changed import (
IsFieldTypeChangedValidator,
)
Expand All @@ -45,6 +50,9 @@
from demisto_sdk.commands.validate.validators.IF_validators.IF116_select_values_cannot_contain_empty_values_in_multi_select_types import (
SelectValuesCannotContainEmptyValuesInMultiSelectTypesValidator,
)
from demisto_sdk.commands.validate.validators.IF_validators.IF117_invalid_marketplaces_in_alias import (
IsValidAliasMarketplaceValidator,
)
from demisto_sdk.commands.validate.validators.IF_validators.IF118_is_alias_inner_alias_valid import (
IsAliasInnerAliasValidator,
)
Expand Down Expand Up @@ -745,3 +753,118 @@ def test_IsAliasInnerAliasValidator():
result[0].message
== "The following aliases have inner aliases: alias_1, alias_2"
)


def test_IsValidAliasMarketplaceValidator(mocker):
"""
Given:
- An incident field with aliases, one of them with invalid marketplaces field.
When:
- Running validate on an incident field.
Then:
- Validate that the incorrect alias is caught.
"""
aliases_names = [
{
"cliName": "alias_1",
"type": "shortText",
"marketplaces": [MarketplaceVersions.XSOAR],
},
{
"cliName": "alias_2",
"type": "shortText",
"marketplaces": [MarketplaceVersions.MarketplaceV2],
},
]
inc_field = create_incident_field_object(
["Aliases"],
[aliases_names],
)
aliases = []
for item in aliases_names:
alias = create_incident_field_object(
paths=["cliName"], values=[item.get("cliName")]
)
alias.marketplaces = item.get("marketplaces")
aliases.append(alias)

mocker.patch.object(IsValidAliasMarketplaceValidator, "graph", return_value="graph")
mocker.patch.object(
IsValidAliasMarketplaceValidator,
"_get_incident_fields_by_aliases",
return_value=aliases,
)
result = IsValidAliasMarketplaceValidator().obtain_invalid_content_items(
[inc_field]
)

assert (
result[0].message
== "The following fields exist as aliases and have invalid 'marketplaces' key value: \nalias_2\n "
"the value of the 'marketplaces' key in these fields should be ['xsoar']."
)


@pytest.mark.parametrize(
"items, expected_number_of_failures, expected_msgs",
[
(
{
create_incident_field_object(
paths=["required", "associatedToAll"], values=["false", "true"]
): GitStatuses.MODIFIED
},
0,
[],
), # inc field not required -> okay
(
{
create_incident_field_object(
paths=["required", "associatedToAll"], values=["true", "true"]
): GitStatuses.ADDED
},
1,
["A required IncidentField should not be associated with all types."],
),
(
{
create_incident_field_object(
paths=["required", "associatedToAll", "associatedTypes"],
values=["true", "false", ["New Type", "Old Type"]],
): GitStatuses.ADDED,
create_incident_type_object(
paths=["id"], values=["New Type"]
): GitStatuses.ADDED,
},
1,
[
"An already existing Types like Old Type cannot be added to an IncidentField with required value equals true."
],
),
],
)
def test_IsValidRequiredFieldValidator(
merit-maita marked this conversation as resolved.
Show resolved Hide resolved
items, expected_number_of_failures, expected_msgs
):
"""
Given:
- Incident fields.
When:
- Running validate on an incident fields.
Then:
- Validate that the field has a valid required value.
"""
merit-maita marked this conversation as resolved.
Show resolved Hide resolved
content_items = []
for item, status in items.items():
item.git_status = status
item.old_base_content_object = item.copy(deep=True)
content_items.append(item)

result = IsValidRequiredFieldValidator().obtain_invalid_content_items(content_items)
assert len(result) == len(expected_msgs)
assert all(
[
result.message == expected_msg
for result, expected_msg in zip(result, expected_msgs)
]
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
from __future__ import annotations

from typing import Iterable, List, Union, cast

from demisto_sdk.commands.common.constants import GitStatuses
from demisto_sdk.commands.content_graph.objects.incident_field import IncidentField
from demisto_sdk.commands.content_graph.objects.incident_type import IncidentType
from demisto_sdk.commands.content_graph.objects.indicator_field import IndicatorField
from demisto_sdk.commands.content_graph.objects.indicator_type import IndicatorType
from demisto_sdk.commands.validate.validators.base_validator import (
BaseValidator,
ValidationResult,
)

ContentTypes = Union[IncidentField, IndicatorField, IncidentType, IndicatorType]


class IsValidRequiredFieldValidator(BaseValidator[ContentTypes]):
error_code = "IF109"
description = "Checks if the incident field required field is valid."
rationale = "In case an incident field is required, newly added associated incident types should be new."
error_message = "{0}"
related_field = "required"
expected_git_statuses = [GitStatuses.MODIFIED, GitStatuses.ADDED]
merit-maita marked this conversation as resolved.
Show resolved Hide resolved

def obtain_invalid_content_items(
self, content_items: Iterable[ContentTypes]
) -> List[ValidationResult]:
fields_items, types_items = self.sort_content_items(content_items)
return [
ValidationResult(
validator=self,
message=self.error_message.format(error_res),
content_object=content_item,
)
for content_item in fields_items
if (error_res := self.is_invalid_required_field(content_item, types_items))
]

@staticmethod
def sort_content_items(content_items: Iterable[ContentTypes]):
"""
Sort Content Items into two lists of Incident/Indicator fields and Incident/Indicator types.

Args:
content_items (Iterable[ContentTypes]): The content items list

Returns:
fields_items: items of type Incident/Indicator fields.
types_items: items of type Incident/Indicator types.
"""
types_items: list[str] = []
fields_items: list[Union[IncidentField, IndicatorField]] = []
for item in content_items:
if (
isinstance(item, IncidentType) or isinstance(item, IndicatorType)
) and item.git_status == GitStatuses.ADDED:
types_items.append(item.object_id)
elif isinstance(item, IncidentField) or isinstance(item, IndicatorField):
fields_items.append(item)

return fields_items, types_items

@staticmethod
def is_invalid_required_field(
YuvHayun marked this conversation as resolved.
Show resolved Hide resolved
content_item: Union[IncidentField, IndicatorField], added_types: list[str]
):
"""
Get from the graph the actual fields for the given aliases

Args:
content_item (Union[IncidentField, IndicatorField]): The incident field or indicator field to check its required field.
added_types (list): incident or indicator types that were created/added in the same pull request.

Returns:
str or None: return none in case it's valid, the error message in case it's not.
"""
# Required fields should not be associated to all
if content_item.required and content_item.associated_to_all:
return (
f"A required {content_item.content_type.value}"
f" should not be associated with all types."
)

if content_item.git_status == GitStatuses.MODIFIED:
old_file = cast(
Union[IncidentField, IndicatorField],
content_item.old_base_content_object,
)

# Required value for an already existing field cannot be changed
if content_item.required != old_file.required:
YuvHayun marked this conversation as resolved.
Show resolved Hide resolved
return (
f"Required value should not be changed for an already existing"
f" {content_item.content_type.value}."
)

# An already existing Incident/Indicator Type cannot be added to Incident/Indicator Field with required value true
if content_item.required and len(content_item.associated_types) > len(
old_file.associated_types
):
new_types = list(
filter(
lambda x: x not in old_file.associated_types,
content_item.associated_types,
)
)
invalid_new_types = []
for new_type in new_types:
if new_type not in added_types:
invalid_new_types.append(new_type)
if invalid_new_types:
return (
f"An already existing Type like {', '.join(invalid_new_types)} cannot be added to an "
f"{content_item.content_type.value} "
f"with required value equals true."
)

# new field
elif content_item.required:
associated_types = content_item.associated_types
invalid_associated_types = []
# An already existing Incident/Indicator Type cannot be added to Incident/Indicator Field with required value true
for associated_type in associated_types:
if associated_type not in added_types:
invalid_associated_types.append(associated_type)
if invalid_associated_types:
return (
f"An already existing Types like {', '.join(invalid_associated_types)} cannot be added to an "
f"{content_item.content_type.value} "
f"with required value equals true."
)
return
Loading
Loading