From a51c4f7b2b04cbc92e9dc5c7a7730259418fe428 Mon Sep 17 00:00:00 2001 From: Nitin Awari <99824048+nitinawari@users.noreply.github.com> Date: Sat, 8 Feb 2025 00:02:01 +0530 Subject: [PATCH 1/5] Add local index names exclusion mechanism (#755) * algolia-index-limiting-in-dev * test cases updated * Update code --------- Co-authored-by: Arkadii Yakovets --- backend/.env.example | 3 +- backend/apps/common/index.py | 130 ++++++++- .../commands/algolia_update_suggestions.py | 21 +- backend/apps/github/index/issue.py | 25 +- backend/apps/github/index/release.py | 22 +- backend/apps/github/index/repository.py | 22 +- .../index/synonyms/{issue.txt => issues.txt} | 0 backend/apps/github/index/user.py | 18 +- backend/apps/owasp/index/chapter.py | 14 +- backend/apps/owasp/index/committee.py | 14 +- backend/apps/owasp/index/project.py | 54 ++-- .../synonyms/{project.txt => projects.txt} | 0 backend/settings/base.py | 3 + backend/tests/common/index_test.py | 274 ++++++++++-------- .../algolia_update_suggestions_test.py | 121 ++++---- 15 files changed, 394 insertions(+), 327 deletions(-) rename backend/apps/github/index/synonyms/{issue.txt => issues.txt} (100%) rename backend/apps/owasp/index/synonyms/{project.txt => projects.txt} (100%) diff --git a/backend/.env.example b/backend/.env.example index e60ec3501..49a82249e 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -1,5 +1,6 @@ DJANGO_ALGOLIA_APPLICATION_ID=None DJANGO_ALGOLIA_APPLICATION_REGION=None +DJANGO_ALGOLIA_EXCLUDED_LOCAL_INDEX_NAMES=None DJANGO_ALGOLIA_WRITE_API_KEY=None DJANGO_ALLOWED_HOSTS=* DJANGO_AWS_ACCESS_KEY_ID=None @@ -11,9 +12,9 @@ DJANGO_DB_PASSWORD=None DJANGO_DB_PORT=None DJANGO_DB_USER=None DJANGO_OPEN_AI_SECRET_KEY=None +DJANGO_RELEASE_VERSION=None DJANGO_SECRET_KEY=None DJANGO_SENTRY_DSN=None -DJANGO_RELEASE_VERSION=None DJANGO_SLACK_BOT_TOKEN=None DJANGO_SLACK_SIGNING_SECRET=None GITHUB_TOKEN=None diff --git a/backend/apps/common/index.py b/backend/apps/common/index.py index c44d3ff02..64671d9cc 100644 --- a/backend/apps/common/index.py +++ b/backend/apps/common/index.py @@ -1,32 +1,137 @@ -"""Algolia index synonyms support and record count.""" +"""Algolia index common classes and helpers.""" import logging from functools import lru_cache from pathlib import Path from algoliasearch.http.exceptions import AlgoliaException +from algoliasearch.query_suggestions.client import QuerySuggestionsClientSync from algoliasearch.search.client import SearchClientSync +from algoliasearch_django import AlgoliaIndex +from algoliasearch_django.decorators import register as algolia_register from django.conf import settings from apps.common.constants import NL logger = logging.getLogger(__name__) +EXCLUDED_LOCAL_INDEX_NAMES = ( + "chapters_suggestions", + "committees_suggestions", + "issues_suggestions", + "projects_contributors_count_asc", + "projects_contributors_count_desc", + "projects_forks_count_asc", + "projects_forks_count_desc", + "projects_name_asc", + "projects_name_desc", + "projects_query_suggestions", + "projects_stars_count_asc", + "projects_stars_count_desc", + "projects_suggestions", + "users_suggestions", +) IS_LOCAL_BUILD = settings.ENVIRONMENT == "Local" LOCAL_INDEX_LIMIT = 1000 -class IndexBase: - """Nest index synonyms mixin and record count.""" +class IndexRegistry: + """Registry to track and manage Algolia indices.""" + + _instance = None + + def __init__(self): + """Initialize index registry.""" + self.excluded_local_index_names = set() + self.load_excluded_local_index_names() + + @classmethod + def get_instance(cls): + """Get or create a singleton instance of IndexRegistry.""" + if cls._instance is None: + cls._instance = IndexRegistry() + return cls._instance + + def is_indexable(self, name: str): + """Check if index is on.""" + return name.lower() not in self.excluded_local_index_names if IS_LOCAL_BUILD else True + + def load_excluded_local_index_names(self): + """Load excluded local index names.""" + excluded_names = settings.ALGOLIA_EXCLUDED_LOCAL_INDEX_NAMES + self.excluded_local_index_names = set( + ( + excluded_name.strip().lower() + for excluded_name in excluded_names.strip().split(",") + if excluded_name.strip() + ) + if excluded_names and excluded_names != "None" + else EXCLUDED_LOCAL_INDEX_NAMES + ) + + return self + + +def is_indexable(index_name: str): + """Determine if an index should be created based on configuration.""" + return IndexRegistry.get_instance().is_indexable(index_name) + + +def register(model, **kwargs): + """Register index if configuration allows.""" + + def wrapper(index_cls): + return ( + algolia_register(model, **kwargs)(index_cls) + if is_indexable(f"{index_cls.index_name}") + else index_cls + ) + + return wrapper + + +class IndexBase(AlgoliaIndex): + """Base index class.""" @staticmethod def get_client(): - """Get the Algolia client.""" + """Return an instance of search client.""" return SearchClientSync( settings.ALGOLIA_APPLICATION_ID, settings.ALGOLIA_WRITE_API_KEY, ) + @staticmethod + def get_suggestions_client(): + """Get suggestions client.""" + return QuerySuggestionsClientSync( + settings.ALGOLIA_APPLICATION_ID, + settings.ALGOLIA_WRITE_API_KEY, + getattr(settings, "ALGOLIA_APPLICATION_REGION", None), + ) + + @staticmethod + def configure_replicas(index_name: str, replicas: dict): + """Configure replicas.""" + if not is_indexable(index_name): + return # Skip replicas configuration if base index is off. + + env = settings.ENVIRONMENT.lower() + + if indexable_replicas := { + f"{env}_{index_name}_{replica_name}": replica_ranking + for replica_name, replica_ranking in replicas.items() + if is_indexable(f"{index_name}_{replica_name}") + }: + client = IndexBase.get_client() + client.set_settings( + f"{env}_{index_name}", + {"replicas": sorted(indexable_replicas.keys())}, + ) + + for replica_name, replica_ranking in indexable_replicas.items(): + client.set_settings(replica_name, {"ranking": replica_ranking}) + @staticmethod def _parse_synonyms_file(file_path): """Parse synonyms file.""" @@ -89,21 +194,20 @@ def reindex_synonyms(app_name, index_name): @staticmethod @lru_cache(maxsize=1024) - def get_total_count(index_name, search_filters=None): + def get_total_count(index_name): """Get total count of records in index.""" client = IndexBase.get_client() try: - search_params = { - "query": "", - "hitsPerPage": 0, - "analytics": False, - } - if search_filters: - search_params["filters"] = search_filters return client.search_single_index( index_name=f"{settings.ENVIRONMENT.lower()}_{index_name}", - search_params=search_params, + search_params={"query": "", "hitsPerPage": 0, "analytics": False}, ).nb_hits except AlgoliaException: logger.exception("Error retrieving index count for '%s'", index_name) return 0 + + def get_queryset(self): + """Get queryset.""" + qs = self.get_entities() + + return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs diff --git a/backend/apps/common/management/commands/algolia_update_suggestions.py b/backend/apps/common/management/commands/algolia_update_suggestions.py index 4e59a07f1..2a8970fdd 100644 --- a/backend/apps/common/management/commands/algolia_update_suggestions.py +++ b/backend/apps/common/management/commands/algolia_update_suggestions.py @@ -1,22 +1,16 @@ """A command to update OWASP Nest suggestions index.""" -from algoliasearch.query_suggestions.client import QuerySuggestionsClientSync from django.conf import settings from django.core.management.base import BaseCommand +from apps.common.index import IndexBase, is_indexable + class Command(BaseCommand): help = "Create query suggestions for Algolia indices" def handle(self, *args, **kwargs): - if settings.ENVIRONMENT == "Local": - return - - client = QuerySuggestionsClientSync( - settings.ALGOLIA_APPLICATION_ID, - settings.ALGOLIA_WRITE_API_KEY, - settings.ALGOLIA_APPLICATION_REGION, - ) + client = IndexBase.get_suggestions_client() entity_configs = { "chapters": { @@ -90,20 +84,21 @@ def handle(self, *args, **kwargs): } print("\nThe following query suggestion index were updated:") + environment = settings.ENVIRONMENT.lower() for entity, suggestion_settings in entity_configs.items(): - source_index_name = f"{settings.ENVIRONMENT.lower()}_{entity}" - suggestions_index_name = f"{settings.ENVIRONMENT.lower()}_{entity}_suggestions" + if not is_indexable(entity) or not is_indexable(f"{entity}_suggestions"): + continue # Skip if the index name is excluded. configuration = { "sourceIndices": [ { - "indexName": source_index_name, + "indexName": f"{environment}_{entity}", **suggestion_settings, } ] } client.update_config( - index_name=suggestions_index_name, + index_name=f"{environment}_{entity}_suggestions", configuration=configuration, ) print(f"{7*' '} * {entity.capitalize()}") diff --git a/backend/apps/github/index/issue.py b/backend/apps/github/index/issue.py index 588aade1d..f76ff57bd 100644 --- a/backend/apps/github/index/issue.py +++ b/backend/apps/github/index/issue.py @@ -1,14 +1,11 @@ """GitHub issue index.""" -from algoliasearch_django import AlgoliaIndex -from algoliasearch_django.decorators import register - -from apps.common.index import IS_LOCAL_BUILD, LOCAL_INDEX_LIMIT, IndexBase +from apps.common.index import IndexBase, register from apps.github.models.issue import Issue @register(Issue) -class IssueIndex(AlgoliaIndex, IndexBase): +class IssueIndex(IndexBase): """Issue index.""" index_name = "issues" @@ -79,19 +76,17 @@ class IssueIndex(AlgoliaIndex, IndexBase): should_index = "is_indexable" - def get_queryset(self): - """Get queryset.""" - qs = Issue.open_issues.assignable.select_related( + @staticmethod + def update_synonyms(): + """Update synonyms.""" + return IndexBase.reindex_synonyms("github", "issues") + + def get_entities(self): + """Get entities for indexing.""" + return Issue.open_issues.assignable.select_related( "repository", ).prefetch_related( "assignees", "labels", "repository__project_set", ) - - return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs - - @staticmethod - def update_synonyms(): - """Update synonyms.""" - return IssueIndex.reindex_synonyms("github", "issue") diff --git a/backend/apps/github/index/release.py b/backend/apps/github/index/release.py index 00dad6fca..0469aab3e 100644 --- a/backend/apps/github/index/release.py +++ b/backend/apps/github/index/release.py @@ -1,14 +1,11 @@ """GitHub release Algolia index configuration.""" -from algoliasearch_django import AlgoliaIndex -from algoliasearch_django.decorators import register - -from apps.common.index import IS_LOCAL_BUILD, LOCAL_INDEX_LIMIT, IndexBase +from apps.common.index import IndexBase, register from apps.github.models.release import Release @register(Release) -class ReleaseIndex(AlgoliaIndex, IndexBase): +class ReleaseIndex(IndexBase): """Release index.""" index_name = "releases" @@ -53,15 +50,14 @@ class ReleaseIndex(AlgoliaIndex, IndexBase): should_index = "is_indexable" - def get_queryset(self): - """Get queryset for indexing.""" - qs = Release.objects.filter( - is_draft=False, - published_at__isnull=False, - ) - return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs - @staticmethod def update_synonyms(): """Update synonyms.""" ReleaseIndex.reindex_synonyms("github", "releases") + + def get_entities(self): + """Get entities for indexing.""" + return Release.objects.filter( + is_draft=False, + published_at__isnull=False, + ) diff --git a/backend/apps/github/index/repository.py b/backend/apps/github/index/repository.py index 57d613bb0..235164413 100644 --- a/backend/apps/github/index/repository.py +++ b/backend/apps/github/index/repository.py @@ -1,14 +1,11 @@ """GitHub repository Algolia index configuration.""" -from algoliasearch_django import AlgoliaIndex -from algoliasearch_django.decorators import register - -from apps.common.index import IS_LOCAL_BUILD, LOCAL_INDEX_LIMIT, IndexBase +from apps.common.index import IndexBase, register from apps.github.models.repository import Repository @register(Repository) -class RepositoryIndex(AlgoliaIndex, IndexBase): +class RepositoryIndex(IndexBase): """Repository index.""" index_name = "repositories" @@ -59,14 +56,15 @@ class RepositoryIndex(AlgoliaIndex, IndexBase): should_index = "is_indexable" - def get_queryset(self): - """Get queryset for indexing.""" - qs = Repository.objects.filter(is_template=False).prefetch_related( - "repositorycontributor_set" - ) - return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs - @staticmethod def update_synonyms(): """Update synonyms.""" RepositoryIndex.reindex_synonyms("github", "repositories") + + def get_entities(self): + """Get entities for indexing.""" + return Repository.objects.filter( + is_template=False, + ).prefetch_related( + "repositorycontributor_set", + ) diff --git a/backend/apps/github/index/synonyms/issue.txt b/backend/apps/github/index/synonyms/issues.txt similarity index 100% rename from backend/apps/github/index/synonyms/issue.txt rename to backend/apps/github/index/synonyms/issues.txt diff --git a/backend/apps/github/index/user.py b/backend/apps/github/index/user.py index e6773c8ec..c0bde5d7e 100644 --- a/backend/apps/github/index/user.py +++ b/backend/apps/github/index/user.py @@ -1,15 +1,12 @@ """GitHub user Algolia index configuration.""" -from algoliasearch_django import AlgoliaIndex -from algoliasearch_django.decorators import register - -from apps.common.index import IS_LOCAL_BUILD, LOCAL_INDEX_LIMIT, IndexBase +from apps.common.index import IndexBase, register from apps.github.models.organization import Organization from apps.github.models.user import User @register(User) -class UserIndex(AlgoliaIndex, IndexBase): +class UserIndex(IndexBase): """User index.""" index_name = "users" @@ -64,12 +61,13 @@ class UserIndex(AlgoliaIndex, IndexBase): should_index = "is_indexable" - def get_queryset(self): - """Get queryset for indexing.""" - qs = User.objects.exclude(login__in=Organization.get_logins()) - return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs - @staticmethod def update_synonyms(): """Update synonyms.""" UserIndex.reindex_synonyms("github", "users") + + def get_entities(self): + """Get entities for indexing.""" + return User.objects.exclude( + login__in=Organization.get_logins(), + ) diff --git a/backend/apps/owasp/index/chapter.py b/backend/apps/owasp/index/chapter.py index 612cd1b51..ecdb22565 100644 --- a/backend/apps/owasp/index/chapter.py +++ b/backend/apps/owasp/index/chapter.py @@ -1,14 +1,11 @@ """OWASP app chapter index.""" -from algoliasearch_django import AlgoliaIndex -from algoliasearch_django.decorators import register - -from apps.common.index import IS_LOCAL_BUILD, LOCAL_INDEX_LIMIT +from apps.common.index import IndexBase, register from apps.owasp.models.chapter import Chapter @register(Chapter) -class ChapterIndex(AlgoliaIndex): +class ChapterIndex(IndexBase): """Chapter index.""" index_name = "chapters" @@ -67,9 +64,8 @@ class ChapterIndex(AlgoliaIndex): should_index = "is_indexable" - def get_queryset(self): - """Get queryset.""" - qs = Chapter.active_chapters.select_related( + def get_entities(self): + """Get entities for indexing.""" + return Chapter.active_chapters.select_related( "owasp_repository", ) - return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs diff --git a/backend/apps/owasp/index/committee.py b/backend/apps/owasp/index/committee.py index 74951938f..a7ec1a02b 100644 --- a/backend/apps/owasp/index/committee.py +++ b/backend/apps/owasp/index/committee.py @@ -1,14 +1,11 @@ """OWASP app chapter index.""" -from algoliasearch_django import AlgoliaIndex -from algoliasearch_django.decorators import register - -from apps.common.index import IS_LOCAL_BUILD, LOCAL_INDEX_LIMIT +from apps.common.index import IndexBase, register from apps.owasp.models.committee import Committee @register(Committee) -class CommitteeIndex(AlgoliaIndex): +class CommitteeIndex(IndexBase): """Committee index.""" index_name = "committees" @@ -57,9 +54,8 @@ class CommitteeIndex(AlgoliaIndex): should_index = "is_indexable" - def get_queryset(self): - """Get queryset.""" - qs = Committee.active_committees.select_related( + def get_entities(self): + """Get entities for indexing.""" + return Committee.active_committees.select_related( "owasp_repository", ) - return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs diff --git a/backend/apps/owasp/index/project.py b/backend/apps/owasp/index/project.py index 987b9315c..7bfca0d97 100644 --- a/backend/apps/owasp/index/project.py +++ b/backend/apps/owasp/index/project.py @@ -1,15 +1,11 @@ """OWASP app project index.""" -from algoliasearch_django import AlgoliaIndex -from algoliasearch_django.decorators import register -from django.conf import settings - -from apps.common.index import IS_LOCAL_BUILD, LOCAL_INDEX_LIMIT, IndexBase +from apps.common.index import IndexBase, register from apps.owasp.models.project import Project @register(Project) -class ProjectIndex(AlgoliaIndex, IndexBase): +class ProjectIndex(IndexBase): """Project index.""" index_name = "projects" @@ -80,36 +76,30 @@ class ProjectIndex(AlgoliaIndex, IndexBase): should_index = "is_indexable" - def get_queryset(self): - """Get queryset.""" - qs = Project.objects.prefetch_related( - "organizations", - "repositories", - ) - return qs[:LOCAL_INDEX_LIMIT] if IS_LOCAL_BUILD else qs - - @staticmethod - def update_synonyms(): - """Update synonyms.""" - return ProjectIndex.reindex_synonyms("owasp", "project") - @staticmethod def configure_replicas(): """Configure the settings for project replicas.""" - env = settings.ENVIRONMENT.lower() - client = IndexBase.get_client() replicas = { - f"{env}_projects_name_asc": ["asc(idx_name)"], - f"{env}_projects_name_desc": ["desc(idx_name)"], - f"{env}_projects_stars_count_asc": ["asc(idx_stars_count)"], - f"{env}_projects_stars_count_desc": ["desc(idx_stars_count)"], - f"{env}_projects_contributors_count_asc": ["asc(idx_contributors_count)"], - f"{env}_projects_contributors_count_desc": ["desc(idx_contributors_count)"], - f"{env}_projects_forks_count_asc": ["asc(idx_forks_count)"], - f"{env}_projects_forks_count_desc": ["desc(idx_forks_count)"], + "contributors_count_asc": ["asc(idx_contributors_count)"], + "contributors_count_desc": ["desc(idx_contributors_count)"], + "forks_count_asc": ["asc(idx_forks_count)"], + "forks_count_desc": ["desc(idx_forks_count)"], + "name_asc": ["asc(idx_name)"], + "name_desc": ["desc(idx_name)"], + "stars_count_asc": ["asc(idx_stars_count)"], + "stars_count_desc": ["desc(idx_stars_count)"], } - client.set_settings(f"{env}_projects", {"replicas": list(replicas.keys())}) + IndexBase.configure_replicas("projects", replicas) + + @staticmethod + def update_synonyms(): + """Update synonyms.""" + return IndexBase.reindex_synonyms("owasp", "projects") - for replica_name, ranking in replicas.items(): - client.set_settings(replica_name, {"ranking": ranking}) + def get_entities(self): + """Get entities for indexing.""" + return Project.objects.prefetch_related( + "organizations", + "repositories", + ) diff --git a/backend/apps/owasp/index/synonyms/project.txt b/backend/apps/owasp/index/synonyms/projects.txt similarity index 100% rename from backend/apps/owasp/index/synonyms/project.txt rename to backend/apps/owasp/index/synonyms/projects.txt diff --git a/backend/settings/base.py b/backend/settings/base.py index c67147256..ec348b4e1 100644 --- a/backend/settings/base.py +++ b/backend/settings/base.py @@ -112,6 +112,9 @@ class Base(Configuration): ALGOLIA_APPLICATION_ID = values.SecretValue(environ_name="ALGOLIA_APPLICATION_ID") ALGOLIA_APPLICATION_REGION = values.SecretValue(environ_name="ALGOLIA_APPLICATION_REGION") + ALGOLIA_EXCLUDED_LOCAL_INDEX_NAMES = values.Value( + environ_name="ALGOLIA_EXCLUDED_LOCAL_INDEX_NAMES" + ) ALGOLIA_WRITE_API_KEY = values.SecretValue(environ_name="ALGOLIA_WRITE_API_KEY") ALGOLIA = { diff --git a/backend/tests/common/index_test.py b/backend/tests/common/index_test.py index 7c6e36707..26c7ca9d1 100644 --- a/backend/tests/common/index_test.py +++ b/backend/tests/common/index_test.py @@ -2,163 +2,169 @@ import pytest from algoliasearch.http.exceptions import AlgoliaException +from algoliasearch_django import AlgoliaIndex +from django.conf import settings +from django.test import override_settings -from apps.common.index import IndexBase +from apps.common.index import ( + EXCLUDED_LOCAL_INDEX_NAMES, + IndexBase, + IndexRegistry, + is_indexable, + register, +) +ENV = settings.ENVIRONMENT.lower() +TOTAL_COUNT = 42 -class MockSearchResponse: - def __init__(self, nb_hits): - self.nb_hits = nb_hits +class TestIndexRegistry: + def test_singleton_instance(self): + registry1 = IndexRegistry.get_instance() + registry2 = IndexRegistry.get_instance() + assert registry1 is registry2 -class TestIndexBase: @pytest.mark.parametrize( - ("file_content", "expected_synonyms"), + ("excluded_index_names", "expected"), [ - ( - """laptop: notebook, portable computer -desktop, workstation, pc -tablet: ipad, slate""", - [ - { - "objectID": "1", - "type": "oneWaySynonym", - "input": "laptop", - "synonyms": ["notebook", "portable computer"], - }, - { - "objectID": "2", - "type": "synonym", - "synonyms": ["desktop", "workstation", "pc"], - }, - { - "objectID": "3", - "type": "oneWaySynonym", - "input": "tablet", - "synonyms": ["ipad", "slate"], - }, - ], - ), - ( - """# Comment line -word1, word2 - -key: value1, value2""", - [ - { - "objectID": "2", - "type": "synonym", - "synonyms": ["word1", "word2"], - }, - { - "objectID": "4", - "type": "oneWaySynonym", - "input": "key", - "synonyms": ["value1", "value2"], - }, - ], - ), - ( - """main: synonym1, synonym2 -key: value1, value2""", - [ - { - "objectID": "1", - "type": "oneWaySynonym", - "input": "main", - "synonyms": ["synonym1", "synonym2"], - }, - { - "objectID": "2", - "type": "oneWaySynonym", - "input": "key", - "synonyms": ["value1", "value2"], - }, - ], - ), + ("test1,test2,test3", {"test1", "test2", "test3"}), + ("test1, test2, test3", {"test1", "test2", "test3"}), + ("", set(EXCLUDED_LOCAL_INDEX_NAMES)), + (None, set(EXCLUDED_LOCAL_INDEX_NAMES)), ], ) - def test_reindex_synonyms(self, file_content, expected_synonyms): - app_name = "test_app" - index_name = "test_index" + def test_load_excluded_index_names(self, excluded_index_names, expected): + with override_settings(ALGOLIA_EXCLUDED_LOCAL_INDEX_NAMES=excluded_index_names): + registry = IndexRegistry.get_instance().load_excluded_local_index_names() + + assert registry.excluded_local_index_names == expected + @pytest.mark.parametrize( + ("is_local", "index_name", "excluded_index_names", "expected"), + [ + (False, "excluded", "excluded", True), + (True, "other_name", "excluded_name", True), + (True, "excluded_index", "excluded_index", False), + ], + ) + def test_is_indexable(self, is_local, index_name, excluded_index_names, expected): with ( - patch("apps.common.index.settings") as mock_settings, - patch("apps.common.index.logger") as mock_logger, - patch("apps.common.index.SearchClientSync") as mock_search_client, - patch("pathlib.Path.open", mock_open(read_data=file_content)), + patch("apps.common.index.IS_LOCAL_BUILD", is_local), + override_settings(ALGOLIA_EXCLUDED_LOCAL_INDEX_NAMES=excluded_index_names), ): - mock_settings.BASE_DIR = "/base/dir" - mock_settings.ENVIRONMENT = "testenv" - mock_settings.ALGOLIA_APPLICATION_ID = "test_app_id" - mock_settings.ALGOLIA_WRITE_API_KEY = "test_api_key" + IndexRegistry.get_instance().load_excluded_local_index_names() - mock_client = MagicMock() - mock_search_client.return_value = mock_client + assert is_indexable(index_name) == expected - result = IndexBase.reindex_synonyms(app_name, index_name) - mock_client.clear_synonyms.assert_called_once_with(index_name="testenv_test_index") +class MockAlgoliaRegister: + def __init__(self, model, **kwargs): + self.model = model + self.kwargs = kwargs - mock_client.save_synonyms.assert_called_once() - call_args = mock_client.save_synonyms.call_args - assert call_args.kwargs["index_name"] == "testenv_test_index" - assert call_args.kwargs["synonym_hit"] == expected_synonyms - assert call_args.kwargs["replace_existing_synonyms"] is True + def __call__(self, cls): + return cls - assert result == len(expected_synonyms) - mock_logger.exception.assert_not_called() - def test_reindex_synonyms_save_error(self): - app_name = "test_app" - index_name = "test_index" - file_content = "word1, word2" +@patch("apps.common.index.algolia_register", MockAlgoliaRegister) +class TestConditionalRegister: + @pytest.fixture() + def mock_model(self): + model = MagicMock() + model._meta.app_label = "test_app" + model._meta.model_name = "test_model" + return model - with ( - patch("apps.common.index.settings") as mock_settings, - patch("apps.common.index.logger") as mock_logger, - patch("apps.common.index.SearchClientSync") as mock_search_client, - patch("pathlib.Path.open", mock_open(read_data=file_content)), - ): - mock_settings.BASE_DIR = "/base/dir" - mock_settings.ENVIRONMENT = "testenv" - mock_settings.ALGOLIA_APPLICATION_ID = "test_app_id" - mock_settings.ALGOLIA_WRITE_API_KEY = "test_api_key" + @patch("apps.common.index.settings") + @patch("apps.common.index.is_indexable") + def test_conditional_register_included(self, mock_is_indexable, mock_settings, mock_model): + mock_is_indexable.return_value = True + mock_settings.ENVIRONMENT = "test" - mock_client = MagicMock() - mock_client.save_synonyms.side_effect = AlgoliaException("API Error") - mock_search_client.return_value = mock_client + class TestIndex(AlgoliaIndex): + index_name = "test_index" - result = IndexBase.reindex_synonyms(app_name, index_name) + with patch("os.getenv") as mock_getenv: + mock_getenv.return_value = "" - assert result is None - mock_logger.exception.assert_called_once_with( - "Error saving synonyms for '%s'", "testenv_test_index" - ) + decorated = register(mock_model)(TestIndex) + assert isinstance(decorated, type) + assert issubclass(decorated, AlgoliaIndex) + + @patch("apps.common.index.settings") + @patch("apps.common.index.is_indexable") + def test_conditional_register_excluded(self, mock_is_indexable, mock_settings, mock_model): + mock_is_indexable.return_value = False + mock_settings.ENVIRONMENT = "test" + + class TestIndex(AlgoliaIndex): + index_name = "test_index" + + with patch("os.getenv") as mock_getenv: + mock_getenv.return_value = "test_app/test_model" + + decorated = register(mock_model)(TestIndex) + assert decorated is TestIndex - def test_get_total_count_success(self): - index_name = "test_index" - expected_hits = 42 +class TestIndexBase: + @pytest.fixture(autouse=True) + def _setup(self): with ( - patch("apps.common.index.settings") as mock_settings, - patch("apps.common.index.SearchClientSync") as mock_search_client, + patch("apps.common.index.settings") as self.mock_settings, + patch("apps.common.index.SearchClientSync") as self.mock_search_client, + patch("apps.common.index.QuerySuggestionsClientSync") as self.mock_suggestions_client, + patch("apps.common.index.logger") as self.mock_logger, ): - mock_settings.ENVIRONMENT = "testenv" - mock_settings.ALGOLIA_APPLICATION_ID = "test_app_id" - mock_settings.ALGOLIA_WRITE_API_KEY = "test_api_key" + self.mock_settings.ENVIRONMENT = "test" + self.mock_settings.ALGOLIA_APPLICATION_ID = "test_id" + self.mock_settings.ALGOLIA_WRITE_API_KEY = "test_key" + self.mock_settings.BASE_DIR = "/test/base/dir" + self.mock_client = MagicMock() + self.mock_search_client.return_value = self.mock_client + yield - mock_client = MagicMock() - mock_client.search_single_index.return_value = MockSearchResponse(expected_hits) - mock_search_client.return_value = mock_client + @pytest.mark.parametrize( + ("is_local", "is_indexable", "expected_replicas"), + [ + ( + False, + True, + ["test_index_name_index_name_attr_asc", "test_index_name_index_name_attr_desc"], + ), + (True, True, ["test_index_name_index_name_attr_asc"]), + (True, False, []), + ], + ) + def test_configure_replicas(self, is_local, is_indexable, expected_replicas): + replicas = {"index_name_attr_asc": ["asc"], "index_name_attr_desc": ["desc"]} + index_name = "index_name" - result = IndexBase.get_total_count(index_name) + with ( + patch("apps.common.index.IS_LOCAL_BUILD", is_local), + patch("apps.common.index.is_indexable") as mock_is_indexable, + ): - assert result == expected_hits - mock_client.search_single_index.assert_called_once_with( - index_name="testenv_test_index", - search_params={"query": "", "hitsPerPage": 0, "analytics": False}, - ) + def mock_is_indexable_func(name): + if name == index_name: + return is_indexable + if is_local: + return name in {"index_name_index_name_attr_asc"} + return True + + mock_is_indexable.side_effect = mock_is_indexable_func + + IndexBase.configure_replicas(index_name, replicas) + + if is_indexable: + self.mock_client.set_settings.assert_any_call( + f"{ENV}_{index_name}", {"replicas": expected_replicas} + ) + + def test_parse_synonyms_file_empty(self): + with patch("pathlib.Path.open", mock_open(read_data="\n \n# comment\n")): + result = IndexBase._parse_synonyms_file("test.txt") + assert result == [] def test_get_total_count_error(self): index_name = "test_index" @@ -188,3 +194,15 @@ def test_get_total_count_error(self): index_name="testenv_test_index", search_params={"query": "", "hitsPerPage": 0, "analytics": False}, ) + + def test_get_total_count_cache(self): + mock_response = MagicMock() + mock_response.nb_hits = TOTAL_COUNT + self.mock_client.search_single_index.return_value = mock_response + + IndexBase.get_total_count.cache_clear() + result1 = IndexBase.get_total_count("test_index") + result2 = IndexBase.get_total_count("test_index") + + assert result1 == result2 == TOTAL_COUNT + self.mock_client.search_single_index.assert_called_once() diff --git a/backend/tests/common/management/commands/algolia_update_suggestions_test.py b/backend/tests/common/management/commands/algolia_update_suggestions_test.py index c7d6fe0df..6540803e5 100644 --- a/backend/tests/common/management/commands/algolia_update_suggestions_test.py +++ b/backend/tests/common/management/commands/algolia_update_suggestions_test.py @@ -1,24 +1,21 @@ -"""Test cases for the algolia_update_suggestions command.""" - from io import StringIO from unittest.mock import patch import pytest from django.core.management import call_command +UPDATE_CONFIG_CALLS_COUNT = 5 -class TestUpdateSuggestionsCommand: - """Test cases for the update_suggestions command.""" +class TestUpdateSuggestionsCommand: @pytest.fixture(autouse=True) def _setup(self): - """Set up test environment.""" + """Set up test fixtures.""" self.stdout = StringIO() with patch( - "apps.common.management.commands.algolia_update_suggestions.QuerySuggestionsClientSync", + "apps.common.management.commands.algolia_update_suggestions.IndexBase.get_suggestions_client", autospec=True, ) as client_patch: - self.mock_client_class = client_patch self.mock_client = client_patch.return_value yield @@ -45,82 +42,62 @@ def _setup(self): ), ], ) - @patch("apps.common.management.commands.algolia_update_suggestions.settings") - def test_handle_updates(self, mock_settings, environment, expected_output): - """Test command output with different environments.""" - mock_settings.ENVIRONMENT = environment - mock_settings.ALGOLIA_APPLICATION_ID = "test_app_id" - mock_settings.ALGOLIA_WRITE_API_KEY = "test_api_key" - mock_settings.ALGOLIA_APPLICATION_REGION = "test_region" - - with patch("sys.stdout", new=StringIO()) as fake_out: + def test_handle_updates(self, environment, expected_output): + with ( + patch( + "apps.common.management.commands.algolia_update_suggestions.settings" + ) as mock_settings, + patch("apps.common.index.is_indexable", return_value=True), + patch("sys.stdout", new=StringIO()) as fake_out, + ): + mock_settings.ENVIRONMENT = environment call_command("algolia_update_suggestions") assert fake_out.getvalue() == expected_output - def test_client_configuration(self): - """Test if client is configured with correct settings.""" - with patch( - "apps.common.management.commands.algolia_update_suggestions.settings" - ) as mock_settings: + def test_skips_excluded_suggestions(self): + with ( + patch( + "apps.common.management.commands.algolia_update_suggestions.settings" + ) as mock_settings, + patch("apps.common.index.is_indexable") as mock_is_indexable, + patch("sys.stdout", new=StringIO()) as fake_out, + ): mock_settings.ENVIRONMENT = "development" - mock_settings.ALGOLIA_APPLICATION_ID = "test_app_id" - mock_settings.ALGOLIA_WRITE_API_KEY = "test_api_key" - mock_settings.ALGOLIA_APPLICATION_REGION = "test_region" - call_command("algolia_update_suggestions") + def mock_side_effect(name, index_type): + return name == "development_chapters_suggestions" and index_type == "suggestion" - self.mock_client_class.assert_called_once_with( - "test_app_id", - "test_api_key", - "test_region", - ) - - def test_update_config_calls(self): - """Test if update_config is called with correct parameters for each entity.""" - with patch( - "apps.common.management.commands.algolia_update_suggestions.settings" - ) as mock_settings: - mock_settings.ENVIRONMENT = "development" + mock_is_indexable.side_effect = mock_side_effect call_command("algolia_update_suggestions") + output = fake_out.getvalue() - expected_entities = ["chapters", "committees", "issues", "projects", "users"] - assert self.mock_client.update_config.call_count == len(expected_entities) - - for entity in expected_entities: - index_name = f"development_{entity}_suggestions" - source_index_name = f"development_{entity}" - - config_call = next( - call - for call in self.mock_client.update_config.call_args_list - if call[1]["index_name"] == index_name - ) - - config = config_call[1]["configuration"] - assert "sourceIndices" in config - assert len(config["sourceIndices"]) == 1 - assert config["sourceIndices"][0]["indexName"] == source_index_name + assert "\nThe following query suggestion index were updated:" in output + assert "* Chapters" in output - @patch("apps.common.management.commands.algolia_update_suggestions.settings") - def test_entity_configurations(self, mock_settings): - """Test if entity configurations are properly structured.""" - mock_settings.ENVIRONMENT = "development" + assert self.mock_client.update_config.call_count == UPDATE_CONFIG_CALLS_COUNT - call_command("algolia_update_suggestions") + update_call = self.mock_client.update_config.call_args_list[0] + assert update_call[1]["index_name"] == "development_chapters_suggestions" - projects_call = next( - call - for call in self.mock_client.update_config.call_args_list - if call[1]["index_name"] == "development_projects_suggestions" - ) - - config = projects_call[1]["configuration"]["sourceIndices"][0] - assert "facets" in config - assert "generate" in config + def test_entity_configurations(self): + with ( + patch( + "apps.common.management.commands.algolia_update_suggestions.settings" + ) as mock_settings, + patch("apps.common.index.is_indexable", return_value=True), + ): + mock_settings.ENVIRONMENT = "development" + call_command("algolia_update_suggestions") - assert any(facet["attribute"] == "idx_key" for facet in config["facets"]) - assert any(facet["attribute"] == "idx_name" for facet in config["facets"]) + projects_call = next( + call + for call in self.mock_client.update_config.call_args_list + if call[1]["index_name"] == "development_projects_suggestions" + ) - assert ["idx_name"] in config["generate"] - assert ["idx_tags"] in config["generate"] + config = projects_call[1]["configuration"]["sourceIndices"][0] + assert "facets" in config + assert "generate" in config + assert any(facet["attribute"] == "idx_key" for facet in config["facets"]) + assert any(facet["attribute"] == "idx_name" for facet in config["facets"]) From c997e7f829679ca65cbc876c947c13325c2e9f40 Mon Sep 17 00:00:00 2001 From: Arkadii Yakovets Date: Fri, 7 Feb 2025 11:31:40 -0800 Subject: [PATCH 2/5] Update IndexBase::get_total_count logic --- backend/apps/common/index.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/backend/apps/common/index.py b/backend/apps/common/index.py index 64671d9cc..4c6005ca6 100644 --- a/backend/apps/common/index.py +++ b/backend/apps/common/index.py @@ -194,13 +194,20 @@ def reindex_synonyms(app_name, index_name): @staticmethod @lru_cache(maxsize=1024) - def get_total_count(index_name): + def get_total_count(index_name, search_filters=None): """Get total count of records in index.""" client = IndexBase.get_client() try: + search_params = { + "analytics": False, + "hitsPerPage": 0, + "query": "", + } + if search_filters: + search_params["filters"] = search_filters return client.search_single_index( index_name=f"{settings.ENVIRONMENT.lower()}_{index_name}", - search_params={"query": "", "hitsPerPage": 0, "analytics": False}, + search_params=search_params, ).nb_hits except AlgoliaException: logger.exception("Error retrieving index count for '%s'", index_name) From fca531f67208a7c04f186b69147d71306c086a2b Mon Sep 17 00:00:00 2001 From: yashgoyal0110 <149111979+yashgoyal0110@users.noreply.github.com> Date: Sun, 9 Feb 2025 03:41:55 +0530 Subject: [PATCH 3/5] Update card layout (#780) * card-layout * card-layout * card layout * layout * margin fixed * icon validation moved to top * fixed icons * fixed icons --- frontend/src/components/Card.tsx | 19 +++++++------------ frontend/src/components/DisplayIcon.tsx | 20 ++++---------------- frontend/src/pages/Projects.tsx | 2 +- frontend/src/utils/data.ts | 14 +++++--------- frontend/src/utils/utility.ts | 4 +--- 5 files changed, 18 insertions(+), 41 deletions(-) diff --git a/frontend/src/components/Card.tsx b/frontend/src/components/Card.tsx index 51549b194..118ab91de 100644 --- a/frontend/src/components/Card.tsx +++ b/frontend/src/components/Card.tsx @@ -43,7 +43,7 @@ const Card = ({ return (
-
+
{/* Display project level badge (if available) */} {level && ( @@ -78,24 +78,19 @@ const Card = ({
{/* Icons associated with the project */} -
- {icons && - Object.keys(Icons).map((key, index) => + {icons && Object.keys(Icons).some((key) => icons[key]) ? ( +
+ {Object.keys(Icons).map((key, index) => icons[key] ? ( value))} // only pass in truthy meta data - idx={ - Object.keys(icons).findIndex((e) => e === key) === - Object.keys(icons).filter((key) => icons[key]).length - 1 - ? -1 - : Object.keys(icons).findIndex((e) => e === key) - } + icons={Object.fromEntries(Object.entries(icons).filter(([_, value]) => value))} /> ) : null )} -
+
+ ) : null}
{/* Link to project name if provided */} {projectName && ( diff --git a/frontend/src/components/DisplayIcon.tsx b/frontend/src/components/DisplayIcon.tsx index 19a2f4d19..e02b8e721 100644 --- a/frontend/src/components/DisplayIcon.tsx +++ b/frontend/src/components/DisplayIcon.tsx @@ -5,23 +5,11 @@ import { TooltipRecipe } from 'utils/theme' import FontAwesomeIconWrapper from 'wrappers/FontAwesomeIconWrapper' import { Tooltip } from 'components/ui/tooltip' -export default function DisplayIcon({ - item, - icons, - idx, -}: { - item: string - icons: IconType - idx: number -}) { +export default function DisplayIcon({ item, icons }: { item: string; icons: IconType }) { // className for the container const containerClassName = [ - 'flex flex-col items-center justify-center gap-1 border-b border-l border-t border-border px-4 pb-1 sm:border-t-0', - idx === 0 || (idx === -1 && Object.keys(icons).length === 1) - ? 'rounded-bl-none sm:rounded-bl-md' - : '', - idx === -1 ? 'border-r sm:border-r-0' : '', - item === 'updated_at' || item === 'stars_count' ? 'rotate-container' : '', + 'flex flex-row-reverse items-center justify-center gap-1 px-4 pb-1 -ml-2', + item === 'stars_count' ? 'rotate-container' : '', item === 'forks_count' || item === 'contributors_count' ? 'flip-container' : '', ] .filter(Boolean) @@ -30,7 +18,7 @@ export default function DisplayIcon({ // className for the FontAwesome icon const iconClassName = [ 'text-gray-600 dark:text-gray-300', - item === 'updated_at' || item === 'stars_count' ? 'icon-rotate' : '', + item === 'stars_count' ? 'icon-rotate' : '', item === 'forks_count' || item === 'contributors_count' ? 'icon-flip' : '', ] .filter(Boolean) diff --git a/frontend/src/pages/Projects.tsx b/frontend/src/pages/Projects.tsx index dd829bd2d..83500a1fc 100644 --- a/frontend/src/pages/Projects.tsx +++ b/frontend/src/pages/Projects.tsx @@ -30,7 +30,7 @@ const ProjectsPage = () => { const navigate = useNavigate() const renderProjectCard = (project: project) => { - const params: string[] = ['updated_at', 'forks_count', 'stars_count', 'contributors_count'] + const params: string[] = ['forks_count', 'stars_count', 'contributors_count'] const filteredIcons = getFilteredIcons(project, params) const handleButtonClick = () => { diff --git a/frontend/src/utils/data.ts b/frontend/src/utils/data.ts index 7568d7484..7dc195657 100644 --- a/frontend/src/utils/data.ts +++ b/frontend/src/utils/data.ts @@ -64,20 +64,16 @@ library.add( ) export const Icons = { - updated_at: { - label: 'Last update date', - icon: 'fa-solid fa-arrows-rotate', + stars_count: { + label: 'GitHub stars', + icon: 'fa-regular fa-star', }, forks_count: { - label: 'Total GitHub forks count', + label: 'GitHub forks', icon: 'fa-solid fa-code-fork', }, - stars_count: { - label: 'Total GitHub stars count', - icon: 'fa-regular fa-star', - }, contributors_count: { - label: 'Total GitHub contributors count', + label: 'GitHub contributors', icon: 'fa-regular fa-user', }, created_at: { diff --git a/frontend/src/utils/utility.ts b/frontend/src/utils/utility.ts index 387f847ea..2511a5959 100644 --- a/frontend/src/utils/utility.ts +++ b/frontend/src/utils/utility.ts @@ -20,9 +20,7 @@ type projectType = project | IssueType | CommitteeType export const getFilteredIcons = (project: projectType, params: string[]): IconType => { const filteredIcons = params.reduce((acc: IconType, key) => { if (Icons[key as IconKeys] && project[key as keyof typeof project] !== undefined) { - if (key === 'updated_at') { - acc[key] = dayjs.unix(project[key] as number).fromNow() - } else if (key === 'created_at') { + if (key === 'created_at') { acc[key] = dayjs.unix(project[key as keyof projectType] as number).fromNow() } else { acc[key] = project[key as keyof typeof project] as number From ab62377ad0e7f94814c4209ef549f2b877228780 Mon Sep 17 00:00:00 2001 From: yashgoyal0110 <149111979+yashgoyal0110@users.noreply.github.com> Date: Sun, 9 Feb 2025 05:29:35 +0530 Subject: [PATCH 4/5] Add schema URL validation (#774) * uri-validation * commit * uri validatn * uri validatn * deps-upgrade * uri-validation * Update code * uri-validation * Update code --------- Co-authored-by: Arkadii Yakovets Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --- schema/poetry.lock | 18 ++++++++- schema/project.json | 7 ++-- schema/pyproject.toml | 1 + schema/tests/chapter_test.py | 17 +++++---- .../data/chapter/negative/blog-invalid.yaml | 12 ++++++ .../{blog-none.yaml => blog-null.yaml} | 0 ...nique-urls.yaml => events-non-unique.yaml} | 0 ...il-missing.yaml => leader-email-null.yaml} | 0 ...rs-empty-list.yaml => sponsors-empty.yaml} | 0 ...sing.yaml => sponsors-name-undefined.yaml} | 0 ...l-missing.yaml => sponsors-undefined.yaml} | 0 .../{website-none.yaml => website-null.yaml} | 0 .../data/project/negative/audience-empty.yaml | 2 +- .../data/project/negative/audience-null.yaml | 14 +++++++ ...e-missing.yaml => audience-undefined.yaml} | 0 .../data/project/negative/blog-invalid.yaml | 16 ++++++++ .../{blog-none.yaml => blog-null.yaml} | 0 ...rs-name-missing.yaml => demo-invalid.yaml} | 5 +-- .../{demo-none.yaml => demo-null.yaml} | 0 .../project/negative/downloads-empty.yaml | 6 +-- .../project/negative/downloads-invalid.yaml | 14 +++++++ .../negative/downloads-non-unique.yaml | 6 +-- .../data/project/negative/downloads-null.yaml | 13 +++++++ .../data/project/negative/events-invalid.yaml | 14 +++++++ ...nique-urls.yaml => events-non-unique.yaml} | 6 +-- .../data/project/negative/events-null.yaml | 13 +++++++ ...il-empty.yaml => leaders-email-empty.yaml} | 0 ...l-missing.yaml => leaders-email-null.yaml} | 0 .../{name-none.yaml => name-null.yaml} | 0 ...issing-url.yaml => repositories-null.yaml} | 2 - ...rs-empty-list.yaml => sponsors-empty.yaml} | 2 +- .../data/project/negative/sponsors-null.yaml | 15 ++++++++ ...l-missing.yaml => sponsors-undefined.yaml} | 8 ++-- .../data/project/negative/website-empty.yaml | 15 ++++++++ .../{website-none.yaml => website-null.yaml} | 0 schema/tests/project_test.py | 37 ++++++++++++------- schema/utils/schema_validators.py | 17 +++++++++ schema/utils/validators.py | 9 ----- 38 files changed, 213 insertions(+), 56 deletions(-) create mode 100644 schema/tests/data/chapter/negative/blog-invalid.yaml rename schema/tests/data/chapter/negative/{blog-none.yaml => blog-null.yaml} (100%) rename schema/tests/data/chapter/negative/{events-non-unique-urls.yaml => events-non-unique.yaml} (100%) rename schema/tests/data/chapter/negative/{leader-email-missing.yaml => leader-email-null.yaml} (100%) rename schema/tests/data/chapter/negative/{sponsors-empty-list.yaml => sponsors-empty.yaml} (100%) rename schema/tests/data/chapter/negative/{sponsors-name-missing.yaml => sponsors-name-undefined.yaml} (100%) rename schema/tests/data/chapter/negative/{sponsors-url-missing.yaml => sponsors-undefined.yaml} (100%) rename schema/tests/data/chapter/negative/{website-none.yaml => website-null.yaml} (100%) create mode 100644 schema/tests/data/project/negative/audience-null.yaml rename schema/tests/data/project/negative/{audience-missing.yaml => audience-undefined.yaml} (100%) create mode 100644 schema/tests/data/project/negative/blog-invalid.yaml rename schema/tests/data/project/negative/{blog-none.yaml => blog-null.yaml} (100%) rename schema/tests/data/project/negative/{sponsors-name-missing.yaml => demo-invalid.yaml} (72%) rename schema/tests/data/project/negative/{demo-none.yaml => demo-null.yaml} (100%) create mode 100644 schema/tests/data/project/negative/downloads-invalid.yaml create mode 100644 schema/tests/data/project/negative/downloads-null.yaml create mode 100644 schema/tests/data/project/negative/events-invalid.yaml rename schema/tests/data/project/negative/{events-non-unique-urls.yaml => events-non-unique.yaml} (83%) create mode 100644 schema/tests/data/project/negative/events-null.yaml rename schema/tests/data/project/negative/{leader-email-empty.yaml => leaders-email-empty.yaml} (100%) rename schema/tests/data/project/negative/{leader-email-missing.yaml => leaders-email-null.yaml} (100%) rename schema/tests/data/project/negative/{name-none.yaml => name-null.yaml} (100%) rename schema/tests/data/project/negative/{repositories-missing-url.yaml => repositories-null.yaml} (85%) rename schema/tests/data/project/negative/{sponsors-empty-list.yaml => sponsors-empty.yaml} (100%) create mode 100644 schema/tests/data/project/negative/sponsors-null.yaml rename schema/tests/data/project/negative/{sponsors-url-missing.yaml => sponsors-undefined.yaml} (100%) create mode 100644 schema/tests/data/project/negative/website-empty.yaml rename schema/tests/data/project/negative/{website-none.yaml => website-null.yaml} (100%) create mode 100644 schema/utils/schema_validators.py delete mode 100644 schema/utils/validators.py diff --git a/schema/poetry.lock b/schema/poetry.lock index 28ce6ca68..2aa23f370 100644 --- a/schema/poetry.lock +++ b/schema/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.7.1 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. [[package]] name = "attrs" @@ -311,7 +311,21 @@ files = [ {file = "rpds_py-0.22.3.tar.gz", hash = "sha256:e32fee8ab45d3c2db6da19a5323bc3362237c8b653c70194414b892fd06a080d"}, ] +[[package]] +name = "validators" +version = "0.34.0" +description = "Python Data Validation for Humans™" +optional = false +python-versions = ">=3.8" +files = [ + {file = "validators-0.34.0-py3-none-any.whl", hash = "sha256:c804b476e3e6d3786fa07a30073a4ef694e617805eb1946ceee3fe5a9b8b1321"}, + {file = "validators-0.34.0.tar.gz", hash = "sha256:647fe407b45af9a74d245b943b18e6a816acf4926974278f6dd617778e1e781f"}, +] + +[package.extras] +crypto-eth-addresses = ["eth-hash[pycryptodome] (>=0.7.0)"] + [metadata] lock-version = "2.0" python-versions = "^3.13" -content-hash = "6a0371d19be66d1ccb40363672ae93d026ef61a33ba86c24d9b804fc012be381" +content-hash = "f27ce956821f7413aef555a68c03caa79aed2c4e46957c8274bd1d01e359f9f2" diff --git a/schema/project.json b/schema/project.json index ef3bbdc51..14fd3abc9 100644 --- a/schema/project.json +++ b/schema/project.json @@ -112,7 +112,7 @@ "blog": { "description": "Project's blog.", "format": "uri", - "type": ["string"] + "type": "string" }, "demo": { "description": "Optional URL to the project demo.", @@ -230,8 +230,9 @@ }, "website": { "description": "The official website of the project.", - "type": "string", - "format": "url" + "format": "url", + "minLength": 4, + "type": "string" } }, "required": [ diff --git a/schema/pyproject.toml b/schema/pyproject.toml index 75a04d7cb..8ba3af40c 100644 --- a/schema/pyproject.toml +++ b/schema/pyproject.toml @@ -11,6 +11,7 @@ jsonschema = "^4.23.0" pytest = "^8.3.4" python = "^3.13" pyyaml = "^6.0.2" +validators = "^0.34.0" [tool.poetry.group.dev.dependencies] diff --git a/schema/tests/chapter_test.py b/schema/tests/chapter_test.py index 7395aa839..80f4e3088 100644 --- a/schema/tests/chapter_test.py +++ b/schema/tests/chapter_test.py @@ -2,7 +2,7 @@ import pytest import yaml -from utils.validators import validate_data +from utils.schema_validators import validate_data from tests.conftest import tests_data_dir @@ -23,10 +23,11 @@ def test_positive(chapter_schema): @pytest.mark.parametrize( ("file_path", "error_message"), [ - ("blog-none.yaml", "None is not of type 'string'"), + ("blog-invalid.yaml", "'invalid-blog-uri' is not a 'uri'"), + ("blog-null.yaml", "None is not a 'uri'"), ("events-empty.yaml", "[] should be non-empty"), ( - "events-non-unique-urls.yaml", + "events-non-unique.yaml", "['https://example.com/event1', 'https://example.com/event1'] has non-unique elements", ), ( @@ -34,15 +35,15 @@ def test_positive(chapter_schema): "[{'email': '', 'github': 'leader-1-github', 'name': 'Leader 1 Name'}] is too short", ), ( - "leader-email-missing.yaml", + "leader-email-null.yaml", "[{'email': None, 'github': 'leader-1-github', 'name': 'Leader 1 Name'}] is too short", ), ("name-empty.yaml", "'' is too short"), ("name-none.yaml", "None is not of type 'string'"), - ("sponsors-empty-list.yaml", "[] should be non-empty"), - ("sponsors-name-missing.yaml", "'name' is a required property"), - ("sponsors-url-missing.yaml", "'url' is a required property"), - ("website-none.yaml", "None is not of type 'string'"), + ("sponsors-empty.yaml", "[] should be non-empty"), + ("sponsors-name-undefined.yaml", "'name' is a required property"), + ("sponsors-undefined.yaml", "'url' is a required property"), + ("website-null.yaml", "None is not of type 'string'"), ], ) def test_negative(chapter_schema, file_path, error_message): diff --git a/schema/tests/data/chapter/negative/blog-invalid.yaml b/schema/tests/data/chapter/negative/blog-invalid.yaml new file mode 100644 index 000000000..acae32f3d --- /dev/null +++ b/schema/tests/data/chapter/negative/blog-invalid.yaml @@ -0,0 +1,12 @@ +blog: "invalid-blog-uri" +country: India +leaders: + - github: leader-1-github + name: Leader 1 Name + - github: leader-2-github + name: Leader 2 Name +name: OWASP Chapter +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 diff --git a/schema/tests/data/chapter/negative/blog-none.yaml b/schema/tests/data/chapter/negative/blog-null.yaml similarity index 100% rename from schema/tests/data/chapter/negative/blog-none.yaml rename to schema/tests/data/chapter/negative/blog-null.yaml diff --git a/schema/tests/data/chapter/negative/events-non-unique-urls.yaml b/schema/tests/data/chapter/negative/events-non-unique.yaml similarity index 100% rename from schema/tests/data/chapter/negative/events-non-unique-urls.yaml rename to schema/tests/data/chapter/negative/events-non-unique.yaml diff --git a/schema/tests/data/chapter/negative/leader-email-missing.yaml b/schema/tests/data/chapter/negative/leader-email-null.yaml similarity index 100% rename from schema/tests/data/chapter/negative/leader-email-missing.yaml rename to schema/tests/data/chapter/negative/leader-email-null.yaml diff --git a/schema/tests/data/chapter/negative/sponsors-empty-list.yaml b/schema/tests/data/chapter/negative/sponsors-empty.yaml similarity index 100% rename from schema/tests/data/chapter/negative/sponsors-empty-list.yaml rename to schema/tests/data/chapter/negative/sponsors-empty.yaml diff --git a/schema/tests/data/chapter/negative/sponsors-name-missing.yaml b/schema/tests/data/chapter/negative/sponsors-name-undefined.yaml similarity index 100% rename from schema/tests/data/chapter/negative/sponsors-name-missing.yaml rename to schema/tests/data/chapter/negative/sponsors-name-undefined.yaml diff --git a/schema/tests/data/chapter/negative/sponsors-url-missing.yaml b/schema/tests/data/chapter/negative/sponsors-undefined.yaml similarity index 100% rename from schema/tests/data/chapter/negative/sponsors-url-missing.yaml rename to schema/tests/data/chapter/negative/sponsors-undefined.yaml diff --git a/schema/tests/data/chapter/negative/website-none.yaml b/schema/tests/data/chapter/negative/website-null.yaml similarity index 100% rename from schema/tests/data/chapter/negative/website-none.yaml rename to schema/tests/data/chapter/negative/website-null.yaml diff --git a/schema/tests/data/project/negative/audience-empty.yaml b/schema/tests/data/project/negative/audience-empty.yaml index fb308dfad..57ebbea42 100644 --- a/schema/tests/data/project/negative/audience-empty.yaml +++ b/schema/tests/data/project/negative/audience-empty.yaml @@ -1,4 +1,4 @@ -audience: "" +audience: '' leaders: - github: leader-name-1 name: Leader Name 1 diff --git a/schema/tests/data/project/negative/audience-null.yaml b/schema/tests/data/project/negative/audience-null.yaml new file mode 100644 index 000000000..7a1595865 --- /dev/null +++ b/schema/tests/data/project/negative/audience-null.yaml @@ -0,0 +1,14 @@ +audience: +leaders: + - github: leader-name-1 + name: Leader Name 1 + - github: leader-name-2 + name: Leader Name 2 +level: 2 +name: OWASP Incubator Code Project +pitch: A very brief, one-line description of your project +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code diff --git a/schema/tests/data/project/negative/audience-missing.yaml b/schema/tests/data/project/negative/audience-undefined.yaml similarity index 100% rename from schema/tests/data/project/negative/audience-missing.yaml rename to schema/tests/data/project/negative/audience-undefined.yaml diff --git a/schema/tests/data/project/negative/blog-invalid.yaml b/schema/tests/data/project/negative/blog-invalid.yaml new file mode 100644 index 000000000..135514b96 --- /dev/null +++ b/schema/tests/data/project/negative/blog-invalid.yaml @@ -0,0 +1,16 @@ +audience: breaker +blog: https://invalid/ +leaders: + - github: leader-1-github + name: Leader 1 Name + - github: leader-2-github + name: Leader 2 Name +level: 2 +name: OWASP Incubator Code Project +pitch: A very brief, one-line description of your project + +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code diff --git a/schema/tests/data/project/negative/blog-none.yaml b/schema/tests/data/project/negative/blog-null.yaml similarity index 100% rename from schema/tests/data/project/negative/blog-none.yaml rename to schema/tests/data/project/negative/blog-null.yaml diff --git a/schema/tests/data/project/negative/sponsors-name-missing.yaml b/schema/tests/data/project/negative/demo-invalid.yaml similarity index 72% rename from schema/tests/data/project/negative/sponsors-name-missing.yaml rename to schema/tests/data/project/negative/demo-invalid.yaml index ba2980fb5..7534042b2 100644 --- a/schema/tests/data/project/negative/sponsors-name-missing.yaml +++ b/schema/tests/data/project/negative/demo-invalid.yaml @@ -1,4 +1,5 @@ audience: breaker +demo: https://invalid/ leaders: - github: leader-name-1 name: Leader Name 1 @@ -12,7 +13,3 @@ tags: - example-tag-2 - example-tag-3 type: code -sponsors: - - description: A great sponsor - logo: https://sponsor1.com/logo.png - url: https://sponsor1.com diff --git a/schema/tests/data/project/negative/demo-none.yaml b/schema/tests/data/project/negative/demo-null.yaml similarity index 100% rename from schema/tests/data/project/negative/demo-none.yaml rename to schema/tests/data/project/negative/demo-null.yaml diff --git a/schema/tests/data/project/negative/downloads-empty.yaml b/schema/tests/data/project/negative/downloads-empty.yaml index ee7eb3b3c..6da9f0950 100644 --- a/schema/tests/data/project/negative/downloads-empty.yaml +++ b/schema/tests/data/project/negative/downloads-empty.yaml @@ -1,10 +1,10 @@ audience: builder downloads: [] -name: "Test Project" +name: Test Project level: 3 leaders: - - github: "testuser1" - - github: "testuser2" + - github: testuser1 + - github: testuser2 pitch: A very brief, one-line description of your project tags: - example-tag-1 diff --git a/schema/tests/data/project/negative/downloads-invalid.yaml b/schema/tests/data/project/negative/downloads-invalid.yaml new file mode 100644 index 000000000..2418e4f74 --- /dev/null +++ b/schema/tests/data/project/negative/downloads-invalid.yaml @@ -0,0 +1,14 @@ +audience: builder +downloads: + - xyz-abc +name: Test Project +level: 3 +leaders: + - github: testuser1 + - github: testuser2 +pitch: A very brief, one-line description of your project +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code diff --git a/schema/tests/data/project/negative/downloads-non-unique.yaml b/schema/tests/data/project/negative/downloads-non-unique.yaml index 3f1f87e11..85db59ecb 100644 --- a/schema/tests/data/project/negative/downloads-non-unique.yaml +++ b/schema/tests/data/project/negative/downloads-non-unique.yaml @@ -2,11 +2,11 @@ audience: builder downloads: - https://abc.com/download - https://abc.com/download -name: "Test Project" +name: Test Project level: 3 leaders: - - github: "testuser1" - - github: "testuser2" + - github: testuser1 + - github: testuser2 pitch: A very brief, one-line description of your project tags: - example-tag-1 diff --git a/schema/tests/data/project/negative/downloads-null.yaml b/schema/tests/data/project/negative/downloads-null.yaml new file mode 100644 index 000000000..29b440ecd --- /dev/null +++ b/schema/tests/data/project/negative/downloads-null.yaml @@ -0,0 +1,13 @@ +audience: builder +downloads: +name: Test Project +level: 3 +leaders: + - github: testuser1 + - github: testuser2 +pitch: A very brief, one-line description of your project +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code diff --git a/schema/tests/data/project/negative/events-invalid.yaml b/schema/tests/data/project/negative/events-invalid.yaml new file mode 100644 index 000000000..4fbc5e7dd --- /dev/null +++ b/schema/tests/data/project/negative/events-invalid.yaml @@ -0,0 +1,14 @@ +audience: builder +events: + - xyz-abc +name: Test Project +level: 3 +leaders: + - github: testuser1 + - github: testuser2 +pitch: A very brief, one-line description of your project +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code diff --git a/schema/tests/data/project/negative/events-non-unique-urls.yaml b/schema/tests/data/project/negative/events-non-unique.yaml similarity index 83% rename from schema/tests/data/project/negative/events-non-unique-urls.yaml rename to schema/tests/data/project/negative/events-non-unique.yaml index b50c031e5..32b6b2933 100644 --- a/schema/tests/data/project/negative/events-non-unique-urls.yaml +++ b/schema/tests/data/project/negative/events-non-unique.yaml @@ -1,4 +1,7 @@ audience: breaker +events: + - https://example.com/event1 + - https://example.com/event1 leaders: - github: leader-1-github name: Leader 1 Name @@ -13,6 +16,3 @@ tags: - example-tag-2 - example-tag-3 type: code -events: - - "https://example.com/event1" - - "https://example.com/event1" diff --git a/schema/tests/data/project/negative/events-null.yaml b/schema/tests/data/project/negative/events-null.yaml new file mode 100644 index 000000000..85e7c19df --- /dev/null +++ b/schema/tests/data/project/negative/events-null.yaml @@ -0,0 +1,13 @@ +audience: builder +events: +name: Test Project +level: 3 +leaders: + - github: testuser1 + - github: testuser2 +pitch: A very brief, one-line description of your project +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code diff --git a/schema/tests/data/project/negative/leader-email-empty.yaml b/schema/tests/data/project/negative/leaders-email-empty.yaml similarity index 100% rename from schema/tests/data/project/negative/leader-email-empty.yaml rename to schema/tests/data/project/negative/leaders-email-empty.yaml diff --git a/schema/tests/data/project/negative/leader-email-missing.yaml b/schema/tests/data/project/negative/leaders-email-null.yaml similarity index 100% rename from schema/tests/data/project/negative/leader-email-missing.yaml rename to schema/tests/data/project/negative/leaders-email-null.yaml diff --git a/schema/tests/data/project/negative/name-none.yaml b/schema/tests/data/project/negative/name-null.yaml similarity index 100% rename from schema/tests/data/project/negative/name-none.yaml rename to schema/tests/data/project/negative/name-null.yaml diff --git a/schema/tests/data/project/negative/repositories-missing-url.yaml b/schema/tests/data/project/negative/repositories-null.yaml similarity index 85% rename from schema/tests/data/project/negative/repositories-missing-url.yaml rename to schema/tests/data/project/negative/repositories-null.yaml index b1c333d75..805b35371 100644 --- a/schema/tests/data/project/negative/repositories-missing-url.yaml +++ b/schema/tests/data/project/negative/repositories-null.yaml @@ -9,8 +9,6 @@ level: 2 name: OWASP Incubator Code Project pitch: A very brief, one-line description of your project repositories: - - description: A repository without URL - name: test-repo tags: - example-tag-1 - example-tag-2 diff --git a/schema/tests/data/project/negative/sponsors-empty-list.yaml b/schema/tests/data/project/negative/sponsors-empty.yaml similarity index 100% rename from schema/tests/data/project/negative/sponsors-empty-list.yaml rename to schema/tests/data/project/negative/sponsors-empty.yaml index af9e922ef..9afdd2878 100644 --- a/schema/tests/data/project/negative/sponsors-empty-list.yaml +++ b/schema/tests/data/project/negative/sponsors-empty.yaml @@ -7,9 +7,9 @@ leaders: level: 2 name: OWASP Incubator Code Project pitch: A very brief, one-line description of your project +sponsors: [] tags: - example-tag-1 - example-tag-2 - example-tag-3 type: code -sponsors: [] diff --git a/schema/tests/data/project/negative/sponsors-null.yaml b/schema/tests/data/project/negative/sponsors-null.yaml new file mode 100644 index 000000000..add7e3f93 --- /dev/null +++ b/schema/tests/data/project/negative/sponsors-null.yaml @@ -0,0 +1,15 @@ +audience: breaker +leaders: + - github: leader-1-github + name: Leader 1 Name + - github: leader-2-github + name: Leader 2 Name +level: 2 +name: OWASP Incubator Code Project +pitch: A very brief, one-line description of your project +sponsors: +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code diff --git a/schema/tests/data/project/negative/sponsors-url-missing.yaml b/schema/tests/data/project/negative/sponsors-undefined.yaml similarity index 100% rename from schema/tests/data/project/negative/sponsors-url-missing.yaml rename to schema/tests/data/project/negative/sponsors-undefined.yaml index 3d21bf5af..8d57032d4 100644 --- a/schema/tests/data/project/negative/sponsors-url-missing.yaml +++ b/schema/tests/data/project/negative/sponsors-undefined.yaml @@ -7,12 +7,12 @@ leaders: level: 2 name: OWASP Incubator Code Project pitch: A very brief, one-line description of your project +sponsors: + - description: A great sponsor + logo: https://sponsor1.com/logo.png + name: Sponsor 1 tags: - example-tag-1 - example-tag-2 - example-tag-3 type: code -sponsors: - - description: A great sponsor - logo: https://sponsor1.com/logo.png - name: Sponsor 1 diff --git a/schema/tests/data/project/negative/website-empty.yaml b/schema/tests/data/project/negative/website-empty.yaml new file mode 100644 index 000000000..6458d4e03 --- /dev/null +++ b/schema/tests/data/project/negative/website-empty.yaml @@ -0,0 +1,15 @@ +audience: breaker +leaders: + - github: leader-1-github + name: Leader 1 Name + - github: leader-2-github + name: Leader 2 Name +level: 2 +name: OWASP Incubator Code Project +pitch: A very brief, one-line description of your project +tags: + - example-tag-1 + - example-tag-2 + - example-tag-3 +type: code +website: '' diff --git a/schema/tests/data/project/negative/website-none.yaml b/schema/tests/data/project/negative/website-null.yaml similarity index 100% rename from schema/tests/data/project/negative/website-none.yaml rename to schema/tests/data/project/negative/website-null.yaml diff --git a/schema/tests/project_test.py b/schema/tests/project_test.py index 0b6800eda..2f2afb3fe 100644 --- a/schema/tests/project_test.py +++ b/schema/tests/project_test.py @@ -2,7 +2,7 @@ import pytest import yaml -from utils.validators import validate_data +from utils.schema_validators import validate_data from tests.conftest import tests_data_dir @@ -28,25 +28,35 @@ def test_positive(project_schema): "'hacker' is not one of ['breaker', 'builder', 'defender']", ), ("audience-empty.yaml", "'' is not one of ['breaker', 'builder', 'defender']"), - ("audience-missing.yaml", "'audience' is a required property"), - ("blog-none.yaml", "None is not of type 'string'"), - ("demo-none.yaml", "None is not of type 'string'"), + ("audience-null.yaml", "None is not one of ['breaker', 'builder', 'defender']"), + ("audience-undefined.yaml", "'audience' is a required property"), + ("blog-invalid.yaml", "'https://invalid/' is not a 'uri'"), + ("blog-null.yaml", "None is not a 'uri'"), + ("demo-invalid.yaml", "'https://invalid/' is not a 'uri'"), + ("demo-null.yaml", "None is not a 'uri'"), ("downloads-empty.yaml", "[] should be non-empty"), + ( + "downloads-invalid.yaml", + "'xyz-abc' is not a 'uri'", + ), ( "downloads-non-unique.yaml", "['https://abc.com/download', 'https://abc.com/download'] has non-unique elements", ), + ("downloads-null.yaml", "None is not of type 'array'"), ("events-empty.yaml", "[] should be non-empty"), ( - "events-non-unique-urls.yaml", + "events-non-unique.yaml", "['https://example.com/event1', 'https://example.com/event1'] has non-unique elements", ), + ("events-invalid.yaml", "'xyz-abc' is not a 'uri'"), + ("events-null.yaml", "None is not of type 'array'"), ( - "leader-email-empty.yaml", + "leaders-email-empty.yaml", "[{'email': '', 'github': 'leader-1-github', 'name': 'Leader 1 Name'}] is too short", ), ( - "leader-email-missing.yaml", + "leaders-email-null.yaml", "[{'email': None, 'github': 'leader-1-github', 'name': 'Leader 1 Name'}] is too short", ), ("level-invalid.yaml", "2.5 is not one of [2, 3, 3.5, 4]"), @@ -55,17 +65,18 @@ def test_positive(project_schema): "'INVALID-LICENSE-VALUE' is not one of ['AGPL-3.0', 'Apache-2.0', 'BSD-2-Clause', 'BSD-3-Clause', 'CC-BY-4.0', 'CC-BY-SA-4.0', 'CC0-1.0', 'EUPL-1.2', 'GPL-2.0', 'GPL-3.0', 'LGPL-2.1', 'LGPL-3.0', 'MIT', 'MPL-2.0', 'OTHER']", ), ("name-empty.yaml", "'' is too short"), - ("name-none.yaml", "None is not of type 'string'"), + ("name-null.yaml", "None is not of type 'string'"), ("repositories-empty.yaml", "[] should be non-empty"), ( "repositories-non-unique.yaml", "['https://example.com/repo1', 'https://example.com/repo1'] has non-unique elements", ), - ("repositories-missing-url.yaml", "'url' is a required property"), - ("sponsors-empty-list.yaml", "[] should be non-empty"), - ("sponsors-name-missing.yaml", "'name' is a required property"), - ("sponsors-url-missing.yaml", "'url' is a required property"), - ("website-none.yaml", "None is not of type 'string'"), + ("repositories-null.yaml", "None is not of type 'array'"), + ("sponsors-empty.yaml", "[] should be non-empty"), + ("sponsors-null.yaml", "None is not of type 'array'"), + ("sponsors-undefined.yaml", "'url' is a required property"), + ("website-empty.yaml", "'' is too short"), + ("website-null.yaml", "None is not of type 'string'"), ], ) def test_negative(project_schema, file_path, error_message): diff --git a/schema/utils/schema_validators.py b/schema/utils/schema_validators.py new file mode 100644 index 000000000..b13a377bb --- /dev/null +++ b/schema/utils/schema_validators.py @@ -0,0 +1,17 @@ +from jsonschema import validate, FormatChecker +from jsonschema.exceptions import ValidationError +import validators + + +def validate_data(schema, data): + """Validate data against schema.""" + format_checker = FormatChecker() + + @format_checker.checks("uri") + def check_uri_format(value): + return validators.url(value) + + try: + validate(schema=schema, instance=data, format_checker=format_checker) + except ValidationError as e: + return e.message diff --git a/schema/utils/validators.py b/schema/utils/validators.py deleted file mode 100644 index 726a334a2..000000000 --- a/schema/utils/validators.py +++ /dev/null @@ -1,9 +0,0 @@ -from jsonschema import validate -from jsonschema.exceptions import ValidationError - - -def validate_data(schema, data): - try: - validate(schema=schema, instance=data) - except ValidationError as e: - return e.message From 473646fba59601636d999b58f3c15e9e4371d838 Mon Sep 17 00:00:00 2001 From: Arkadii Yakovets Date: Sat, 8 Feb 2025 19:19:33 -0800 Subject: [PATCH 5/5] Add NestBot event logging --- backend/apps/github/admin.py | 2 +- backend/apps/slack/admin.py | 19 +++++++ backend/apps/slack/apps.py | 24 +++++++++ backend/apps/slack/commands/gsoc.py | 2 +- backend/apps/slack/migrations/0001_initial.py | 45 ++++++++++++++++ ...el_id_alter_event_channel_name_and_more.py | 47 ++++++++++++++++ .../migrations/0003_remove_event_command.py | 16 ++++++ backend/apps/slack/migrations/__init__.py | 0 backend/apps/slack/models/__init__.py | 0 backend/apps/slack/models/event.py | 53 +++++++++++++++++++ 10 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 backend/apps/slack/admin.py create mode 100644 backend/apps/slack/migrations/0001_initial.py create mode 100644 backend/apps/slack/migrations/0002_alter_event_channel_id_alter_event_channel_name_and_more.py create mode 100644 backend/apps/slack/migrations/0003_remove_event_command.py create mode 100644 backend/apps/slack/migrations/__init__.py create mode 100644 backend/apps/slack/models/__init__.py create mode 100644 backend/apps/slack/models/event.py diff --git a/backend/apps/github/admin.py b/backend/apps/github/admin.py index fd3f5cc03..09bd769f6 100644 --- a/backend/apps/github/admin.py +++ b/backend/apps/github/admin.py @@ -1,4 +1,4 @@ -"""Repository app admin.""" +"""GitHub app admin.""" from django.contrib import admin from django.utils.safestring import mark_safe diff --git a/backend/apps/slack/admin.py b/backend/apps/slack/admin.py new file mode 100644 index 000000000..56fe710f2 --- /dev/null +++ b/backend/apps/slack/admin.py @@ -0,0 +1,19 @@ +"""Slack app admin.""" + +from django.contrib import admin + +from apps.slack.models.event import Event + + +class EventAdmin(admin.ModelAdmin): + search_fields = ( + "channel_id", + "channel_name", + "text", + "user_id", + "user_name", + ) + list_filter = ("trigger",) + + +admin.site.register(Event, EventAdmin) diff --git a/backend/apps/slack/apps.py b/backend/apps/slack/apps.py index 3be0751a3..e76c90cd1 100644 --- a/backend/apps/slack/apps.py +++ b/backend/apps/slack/apps.py @@ -1,7 +1,11 @@ +import logging + from django.apps import AppConfig from django.conf import settings from slack_bolt import App +logger = logging.getLogger(__name__) + class SlackConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" @@ -15,3 +19,23 @@ class SlackConfig(AppConfig): if settings.SLACK_BOT_TOKEN != "None" and settings.SLACK_SIGNING_SECRET != "None" # noqa: S105 else None ) + + +if SlackConfig.app: + + @SlackConfig.app.error + def error_handler(error, body, *args, **kwargs): # noqa: ARG001 + """Handle Slack application errors.""" + logger.exception(error, extra={"body": body}) + + @SlackConfig.app.use + def log_events(client, context, logger, payload, next): # noqa: A002, ARG001 + """Log Slack events.""" + from apps.slack.models.event import Event + + try: + Event.create(context, payload) + except Exception: + logger.exception("Could not log Slack event") + + next() diff --git a/backend/apps/slack/commands/gsoc.py b/backend/apps/slack/commands/gsoc.py index 33953a179..70a8c5505 100644 --- a/backend/apps/slack/commands/gsoc.py +++ b/backend/apps/slack/commands/gsoc.py @@ -13,7 +13,7 @@ SUPPORTED_YEAR_START = 2012 SUPPORTED_YEAR_END = 2025 -SUPPORTED_YEARS = set(range(SUPPORTED_YEAR_START, SUPPORTED_YEAR_END + 1)) # 2012-2025 +SUPPORTED_YEARS = set(range(SUPPORTED_YEAR_START, SUPPORTED_YEAR_END + 1)) SUPPORTED_ANNOUNCEMENT_YEARS = SUPPORTED_YEARS - {2012, 2013, 2014, 2015, 2016, 2018} diff --git a/backend/apps/slack/migrations/0001_initial.py b/backend/apps/slack/migrations/0001_initial.py new file mode 100644 index 000000000..823f85108 --- /dev/null +++ b/backend/apps/slack/migrations/0001_initial.py @@ -0,0 +1,45 @@ +# Generated by Django 5.1.6 on 2025-02-09 02:31 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="Event", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ("nest_created_at", models.DateTimeField(auto_now_add=True)), + ("nest_updated_at", models.DateTimeField(auto_now=True)), + ( + "channel_id", + models.TextField(default="", max_length=15, verbose_name="Channel ID"), + ), + ( + "channel_name", + models.TextField(default="", max_length=100, verbose_name="Channel name"), + ), + ("command", models.TextField(default="", max_length=15, verbose_name="Command")), + ("text", models.TextField(default="", max_length=1000, verbose_name="Text")), + ("trigger", models.TextField(default="", max_length=100, verbose_name="Trigger")), + ("user_id", models.TextField(max_length=15, verbose_name="User ID")), + ( + "user_name", + models.TextField(default="", max_length=100, verbose_name="User name"), + ), + ], + options={ + "verbose_name_plural": "Events", + "db_table": "slack_events", + }, + ), + ] diff --git a/backend/apps/slack/migrations/0002_alter_event_channel_id_alter_event_channel_name_and_more.py b/backend/apps/slack/migrations/0002_alter_event_channel_id_alter_event_channel_name_and_more.py new file mode 100644 index 000000000..b0270123d --- /dev/null +++ b/backend/apps/slack/migrations/0002_alter_event_channel_id_alter_event_channel_name_and_more.py @@ -0,0 +1,47 @@ +# Generated by Django 5.1.6 on 2025-02-09 02:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0001_initial"), + ] + + operations = [ + migrations.AlterField( + model_name="event", + name="channel_id", + field=models.CharField(default="", max_length=15, verbose_name="Channel ID"), + ), + migrations.AlterField( + model_name="event", + name="channel_name", + field=models.CharField(default="", max_length=100, verbose_name="Channel name"), + ), + migrations.AlterField( + model_name="event", + name="command", + field=models.CharField(default="", max_length=15, verbose_name="Command"), + ), + migrations.AlterField( + model_name="event", + name="text", + field=models.CharField(default="", max_length=1000, verbose_name="Text"), + ), + migrations.AlterField( + model_name="event", + name="trigger", + field=models.CharField(default="", max_length=100, verbose_name="Trigger"), + ), + migrations.AlterField( + model_name="event", + name="user_id", + field=models.CharField(max_length=15, verbose_name="User ID"), + ), + migrations.AlterField( + model_name="event", + name="user_name", + field=models.CharField(default="", max_length=100, verbose_name="User name"), + ), + ] diff --git a/backend/apps/slack/migrations/0003_remove_event_command.py b/backend/apps/slack/migrations/0003_remove_event_command.py new file mode 100644 index 000000000..eceba79a7 --- /dev/null +++ b/backend/apps/slack/migrations/0003_remove_event_command.py @@ -0,0 +1,16 @@ +# Generated by Django 5.1.6 on 2025-02-09 02:48 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("slack", "0002_alter_event_channel_id_alter_event_channel_name_and_more"), + ] + + operations = [ + migrations.RemoveField( + model_name="event", + name="command", + ), + ] diff --git a/backend/apps/slack/migrations/__init__.py b/backend/apps/slack/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/apps/slack/models/__init__.py b/backend/apps/slack/models/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/apps/slack/models/event.py b/backend/apps/slack/models/event.py new file mode 100644 index 000000000..6ea787dfa --- /dev/null +++ b/backend/apps/slack/models/event.py @@ -0,0 +1,53 @@ +"""Slack app event model.""" + +from django.db import models + +from apps.common.models import TimestampedModel +from apps.slack.commands.owasp import COMMAND as OWASP_COMMAND + + +class Event(TimestampedModel): + """Event model.""" + + class Meta: + db_table = "slack_events" + verbose_name_plural = "Events" + + channel_id = models.CharField(verbose_name="Channel ID", max_length=15, default="") + channel_name = models.CharField(verbose_name="Channel name", max_length=100, default="") + text = models.CharField(verbose_name="Text", max_length=1000, default="") + trigger = models.CharField(verbose_name="Trigger", max_length=100, default="") + user_id = models.CharField(verbose_name="User ID", max_length=15) + user_name = models.CharField(verbose_name="User name", max_length=100, default="") + + def __str__(self): + """Event human readable representation.""" + return f"Event from {self.user_name or self.user_id} triggered by {self.trigger}" + + def from_slack(self, context, payload): + """Create instance based on Slack data.""" + self.channel_id = context.get("channel_id", "") + self.channel_name = payload.get("channel_name", "") + + command = payload.get("command", "") + text = payload.get("text", "") + if command and command == OWASP_COMMAND: + command, *args = text.strip().split() + text = " ".join(args) + self.command = command.lstrip("/") + self.text = text + + # In this order. + self.trigger = self.command or payload.get("action_id", "") or payload.get("type", "") + self.user_id = context["user_id"] + self.user_name = payload.get("user_name", "") + + @staticmethod + def create(context, payload, save=True): + """Create event.""" + event = Event() + event.from_slack(context, payload) + if save: + event.save() + + return event