From aaee25008fdd8f5f5bdf8e2c416039615cf004e4 Mon Sep 17 00:00:00 2001 From: Bill Wei Date: Tue, 29 Oct 2024 11:47:34 -0400 Subject: [PATCH] fix: set error message for project importing from a folder with no rulebooks (#1128) --- src/aap_eda/services/project/imports.py | 54 ++++----- .../extensions/eda/rulebooks/README | 1 + .../services/test_project_import.py | 105 ++++++++++-------- 3 files changed, 83 insertions(+), 77 deletions(-) create mode 100644 tests/integration/services/data/project-06/extensions/eda/rulebooks/README diff --git a/src/aap_eda/services/project/imports.py b/src/aap_eda/services/project/imports.py index 717ce9891..a57872788 100644 --- a/src/aap_eda/services/project/imports.py +++ b/src/aap_eda/services/project/imports.py @@ -16,6 +16,7 @@ import logging import os import tempfile +from contextlib import contextmanager from dataclasses import dataclass from functools import wraps from typing import Any, Callable, Final, Iterator, Optional, Type @@ -89,9 +90,6 @@ def wrapper(self: ProjectImportService, project: models.Project): return wrapper -# TODO(cutwater): The project import and project sync are mostly -# similar operations. Current implementation has some code duplication. -# This needs to be refactored in the future. class ProjectImportService: def __init__(self, scm_cls: Optional[Type[ScmRepository]] = None): if scm_cls is None: @@ -100,27 +98,27 @@ def __init__(self, scm_cls: Optional[Type[ScmRepository]] = None): @_project_import_wrapper def import_project(self, project: models.Project) -> None: - with self._temporary_directory() as tempdir: - repo_dir = os.path.join(tempdir, "src") - - proxy = project.proxy.get_secret_value() if project.proxy else None - repo = self._scm_cls.clone( - project.url, - repo_dir, - credential=project.eda_credential, - gpg_credential=project.signature_validation_credential, - depth=1, - verify_ssl=project.verify_ssl, - branch=project.scm_branch, - refspec=project.scm_refspec, - proxy=proxy, - ) - project.git_hash = repo.rev_parse("HEAD") - + with self._clone_and_process(project) as (repo_dir, git_hash): + project.git_hash = git_hash self._import_rulebooks(project, repo_dir) @_project_import_wrapper def sync_project(self, project: models.Project) -> None: + with self._clone_and_process(project) as (repo_dir, git_hash): + if project.git_hash == git_hash: + logger.info( + "Project (id=%s, name=%s) is up to date. Nothing to sync.", + project.id, + project.name, + ) + return + + project.git_hash = git_hash + + self._sync_rulebooks(project, repo_dir, git_hash) + + @contextmanager + def _clone_and_process(self, project: models.Project): with self._temporary_directory() as tempdir: repo_dir = os.path.join(tempdir, "src") @@ -136,19 +134,9 @@ def sync_project(self, project: models.Project) -> None: refspec=project.scm_refspec, proxy=proxy, ) - git_hash = repo.rev_parse("HEAD") - - if project.git_hash == git_hash: - logger.info( - "Project (id=%s, name=%s) is up to date. Nothing to sync.", - project.id, - project.name, - ) - return - - project.git_hash = git_hash - - self._sync_rulebooks(project, repo_dir, git_hash) + yield repo_dir, repo.rev_parse("HEAD") + if project.rulebook_set.count() == 0: + raise ScmEmptyError("This project contains no rulebooks.") def _temporary_directory(self) -> tempfile.TemporaryDirectory: return tempfile.TemporaryDirectory(prefix=TMP_PREFIX) diff --git a/tests/integration/services/data/project-06/extensions/eda/rulebooks/README b/tests/integration/services/data/project-06/extensions/eda/rulebooks/README new file mode 100644 index 000000000..6a2f6eade --- /dev/null +++ b/tests/integration/services/data/project-06/extensions/eda/rulebooks/README @@ -0,0 +1 @@ +This folder purposely contains no rulebooks \ No newline at end of file diff --git a/tests/integration/services/test_project_import.py b/tests/integration/services/test_project_import.py index 534c7634c..4d49b1060 100644 --- a/tests/integration/services/test_project_import.py +++ b/tests/integration/services/test_project_import.py @@ -64,25 +64,18 @@ def storage_save_patch(): yield save_mock -@pytest.mark.django_db -def test_project_import( - default_organization: models.Organization, - storage_save_patch, - service_tempdir_patch, -): - def clone_project(_url, path, *_args, **_kwargs): - src = DATA_DIR / "project-01" - shutil.copytree(src, path, symlinks=False) - return repo_mock - - repo_mock = mock.Mock(name="ScmRepository()") - repo_mock.rev_parse.return_value = ( - "adc83b19e793491b1c6ea0fd8b46cd9f32e592fc" +@pytest.fixture +def project(default_organization: models.Organization): + return models.Project.objects.create( + name="test-project-01", + url="https://git.example.com/repo.git", + organization=default_organization, ) - git_mock = mock.Mock(name="ScmRepository", spec=ScmRepository) - git_mock.clone.side_effect = clone_project - projects = models.Project.objects.bulk_create( + +@pytest.fixture +def projects(default_organization: models.Organization): + return models.Project.objects.bulk_create( [ models.Project( name="test-project-01", @@ -102,6 +95,25 @@ def clone_project(_url, path, *_args, **_kwargs): ] ) + +@pytest.mark.django_db +def test_project_import( + projects: list[models.Project], + storage_save_patch, + service_tempdir_patch, +): + def clone_project(_url, path, *_args, **_kwargs): + src = DATA_DIR / "project-01" + shutil.copytree(src, path, symlinks=False) + return repo_mock + + repo_mock = mock.Mock(name="ScmRepository()") + repo_mock.rev_parse.return_value = ( + "adc83b19e793491b1c6ea0fd8b46cd9f32e592fc" + ) + git_mock = mock.Mock(name="ScmRepository", spec=ScmRepository) + git_mock.clone.side_effect = clone_project + for project in projects: service = ProjectImportService(scm_cls=git_mock) service.import_project(project) @@ -134,7 +146,7 @@ def clone_project(_url, path, *_args, **_kwargs): @pytest.mark.django_db def test_project_import_with_new_layout( - default_organization: models.Organization, + project: models.Project, storage_save_patch, service_tempdir_patch, ): @@ -151,12 +163,6 @@ def clone_project(_url, path, *_args, **_kwargs): git_mock = mock.Mock(name="ScmRepository", spec=ScmRepository) git_mock.clone.side_effect = clone_project - project = models.Project.objects.create( - name="test-project-01", - url="https://git.example.com/repo.git", - organization=default_organization, - ) - service = ProjectImportService(scm_cls=git_mock) service.import_project(project) project.refresh_from_db() @@ -167,7 +173,7 @@ def clone_project(_url, path, *_args, **_kwargs): @pytest.mark.django_db def test_project_import_rulebook_directory_missing( - default_organization: models.Organization, + project: models.Project, storage_save_patch, service_tempdir_patch, ): @@ -178,11 +184,6 @@ def test_project_import_rulebook_directory_missing( git_mock = mock.Mock(name="ScmRepository", spec=ScmRepository) git_mock.clone.return_value = repo_mock - project = models.Project.objects.create( - name="test-project-01", - url="https://git.example.com/repo.git", - organization=default_organization, - ) message_expected = ( "The 'extensions/eda/rulebooks' or 'rulebooks'" + " directory doesn't exist within the project root." @@ -201,13 +202,13 @@ def test_project_import_rulebook_directory_missing( @pytest.mark.django_db -def test_project_import_with_vaulted_data( - default_organization: models.Organization, +def test_project_import_with_no_rulebooks( + project: models.Project, storage_save_patch, service_tempdir_patch, ): def clone_project(_url, path, *_args, **_kwargs): - src = DATA_DIR / "project-04" + src = DATA_DIR / "project-06" shutil.copytree(src, path, symlinks=False) return repo_mock @@ -219,12 +220,34 @@ def clone_project(_url, path, *_args, **_kwargs): git_mock = mock.Mock(name="ScmRepository", spec=ScmRepository) git_mock.clone.side_effect = clone_project - project = models.Project.objects.create( - name="test-project-04", - url="https://git.example.com/repo.git", - organization=default_organization, + service = ProjectImportService(scm_cls=git_mock) + service.import_project(project) + project.refresh_from_db() + + assert project.git_hash == "adc83b19e793491b1c6ea0fd8b46cd9f32e592fc" + assert project.import_state == models.Project.ImportState.COMPLETED + assert project.import_error == "This project contains no rulebooks." + + +@pytest.mark.django_db +def test_project_import_with_vaulted_data( + project: models.Project, + storage_save_patch, + service_tempdir_patch, +): + def clone_project(_url, path, *_args, **_kwargs): + src = DATA_DIR / "project-04" + shutil.copytree(src, path, symlinks=False) + return repo_mock + + repo_mock = mock.Mock(name="ScmRepository()") + repo_mock.rev_parse.return_value = ( + "adc83b19e793491b1c6ea0fd8b46cd9f32e592fc" ) + git_mock = mock.Mock(name="ScmRepository", spec=ScmRepository) + git_mock.clone.side_effect = clone_project + service = ProjectImportService(scm_cls=git_mock) service.import_project(project) project.refresh_from_db() @@ -342,7 +365,7 @@ def clone_project(_url, path, *_args, **_kwargs): @pytest.mark.django_db def test_project_import_with_invalid_rulebooks( - default_organization: models.Organization, + project: models.Project, storage_save_patch, service_tempdir_patch, caplog, @@ -360,12 +383,6 @@ def clone_project(_url, path, *_args, **_kwargs): git_mock = mock.Mock(name="ScmRepository", spec=ScmRepository) git_mock.clone.side_effect = clone_project - project = models.Project.objects.create( - name="test-project-04", - url="https://git.example.com/repo.git", - organization=default_organization, - ) - logger = logging.getLogger("aap_eda") propagate = logger.propagate logger.propagate = True