From 60263fed4843f3d1199fe3124eb96827b85632c5 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Thu, 16 May 2024 17:47:09 +0200 Subject: [PATCH] Add database migration for #8045. --- .../api_server/src/initialization/database.py | 66 ++++++- .../tests/initialization/test_database.py | 176 +++++++++++++++--- docs/src/changelog.md | 4 +- 3 files changed, 213 insertions(+), 33 deletions(-) diff --git a/components/api_server/src/initialization/database.py b/components/api_server/src/initialization/database.py index 032ea05b88..e18c572f4f 100644 --- a/components/api_server/src/initialization/database.py +++ b/components/api_server/src/initialization/database.py @@ -53,12 +53,13 @@ def create_indexes(database: Database) -> None: database.measurements.create_indexes([period_index, latest_measurement_index, latest_successful_measurement_index]) -def perform_migrations(database: Database) -> None: # pragma: no cover-behave +def perform_migrations(database: Database) -> None: # pragma: no feature-test-cover """Perform database migrations.""" change_accessibility_violation_metrics_to_violations(database) + fix_branch_parameters_without_value(database) -def change_accessibility_violation_metrics_to_violations(database: Database) -> None: # pragma: no cover-behave +def change_accessibility_violation_metrics_to_violations(database: Database) -> None: # pragma: no feature-test-cover """Replace accessibility metrics with the violations metric.""" # Added after Quality-time v5.5.0, see https://github.com/ICTU/quality-time/issues/562 for report in database.reports.find(filter={"last": True, "deleted": {"$exists": False}}): @@ -72,17 +73,70 @@ def change_accessibility_violation_metrics_to_violations(database: Database) -> changed = True if changed: logging.info("Updating report to change its accessibility metrics to violations metrics: %s", report_uuid) - report_id = report["_id"] - del report["_id"] - database.reports.replace_one({"_id": report_id}, report) + replace_report(database, report) else: logging.info("No accessibility metrics found in report: %s", report_uuid) -def change_accessibility_violations_metric_to_violations(metric: dict) -> None: # pragma: no cover-behave +def change_accessibility_violations_metric_to_violations(metric: dict) -> None: # pragma: no feature-test-cover """Change the accessibility violations metric to violations metric.""" metric["type"] = "violations" if not metric.get("name"): metric["name"] = "Accessibility violations" if not metric.get("unit"): metric["unit"] = "accessibility violations" + + +def fix_branch_parameters_without_value(database: Database) -> None: # pragma: no feature-test-cover + """Set the branch parameter of sources to 'master' (the previous default) if they have no value.""" + # Added after Quality-time v5.11.0, see https://github.com/ICTU/quality-time/issues/8045 + for report in database.reports.find(filter={"last": True, "deleted": {"$exists": False}}): + report_uuid = report["report_uuid"] + logging.info("Checking report for sources with empty branch parameters: %s", report_uuid) + changed = False + for source in sources_with_branch_parameter(report): + if not source["parameters"].get("branch"): + source["parameters"]["branch"] = "master" + changed = True + if changed: + logging.info("Updating report to change sources with empty branch parameter: %s", report_uuid) + replace_report(database, report) + else: + logging.info("No sources with empty branch parameters found in report: %s", report_uuid) + + +METRICS_WITH_SOURCES_WITH_BRANCH_PARAMETER = { + "commented_out_code": ["sonarqube"], + "complex_units": ["sonarqube"], + "duplicated_lines": ["sonarqube"], + "loc": ["sonarqube"], + "long_units": ["sonarqube"], + "many_parameters": ["sonarqube"], + "remediation_effort": ["sonarqube"], + "security_warnings": ["sonarqube"], + "software_version": ["sonarqube"], + "source_up_to_dateness": ["azure_devops", "gitlab", "sonarqube"], + "suppressed_violations": ["sonarqube"], + "tests": ["sonarqube"], + "todo_and_fixme_comments": ["sonarqube"], + "uncovered_branches": ["sonarqube"], + "uncovered_lines": ["sonarqube"], + "violations": ["sonarqube"], +} + + +def sources_with_branch_parameter(report: dict): # pragma: no feature-test-cover + """Return the sources with a branch parameter.""" + for subject in report["subjects"].values(): + for metric in subject["metrics"].values(): + if metric["type"] in METRICS_WITH_SOURCES_WITH_BRANCH_PARAMETER: + for source in metric.get("sources", {}).values(): + if source["type"] in METRICS_WITH_SOURCES_WITH_BRANCH_PARAMETER[metric["type"]]: + yield source + + +def replace_report(database: Database, report) -> None: # pragma: no feature-test-cover + """Replace the report in the database.""" + report_id = report["_id"] + del report["_id"] + database.reports.replace_one({"_id": report_id}, report) diff --git a/components/api_server/tests/initialization/test_database.py b/components/api_server/tests/initialization/test_database.py index 08c4c58693..fdd26bccc3 100644 --- a/components/api_server/tests/initialization/test_database.py +++ b/components/api_server/tests/initialization/test_database.py @@ -6,15 +6,25 @@ from initialization.database import init_database, perform_migrations from tests.base import DataModelTestCase +from tests.fixtures import REPORT_ID, SUBJECT_ID, METRIC_ID, METRIC_ID2, METRIC_ID3, SOURCE_ID, SOURCE_ID2 -class DatabaseInitTest(DataModelTestCase): +class DatabaseInitializationTestCase(DataModelTestCase): + """Base class for database unittests.""" + + def setUp(self): + """Extend to set up the database fixture.""" + super().setUp() + self.database = Mock() + + +class DatabaseInitTest(DatabaseInitializationTestCase): """Unit tests for database initialization.""" def setUp(self): - """Override to set up the database fixture.""" + """Extend to set up the Mongo client and database contents.""" + super().setUp() self.mongo_client = Mock() - self.database = Mock() self.database.reports.find.return_value = [] self.database.reports.distinct.return_value = [] self.database.datamodels.find_one.return_value = None @@ -67,12 +77,8 @@ def test_skip_loading_example_reports(self): self.database.reports_overviews.insert_one.assert_called_once() -class DatabaseMigrationsTest(DataModelTestCase): - """Unit tests for the database migration.""" - - def setUp(self): - """Override to set up the database fixture.""" - self.database = Mock() +class DatabaseMigrationsChangeAccessibilityViolationsTest(DatabaseInitializationTestCase): + """Unit tests for the accessibility violations database migration.""" def test_change_accessibility_violations_to_violations_without_reports(self): """Test that the migration succeeds without reports.""" @@ -83,7 +89,7 @@ def test_change_accessibility_violations_to_violations_without_reports(self): def test_change_accessibility_violations_to_violations_when_report_has_no_accessibility_metrics(self): """Test that the migration succeeds wtih reports, but without accessibility metrics.""" self.database.reports.find.return_value = [ - {"report_uuid": "report_uuid", "subjects": {"subject_uuid": {"metrics": {"metric_uuid": {"type": "loc"}}}}} + {"report_uuid": REPORT_ID, "subjects": {SUBJECT_ID: {"metrics": {METRIC_ID: {"type": "loc"}}}}} ] perform_migrations(self.database) self.database.reports.replace_one.assert_not_called() @@ -93,19 +99,19 @@ def test_change_accessibility_violations_to_violations_when_report_has_an_access self.database.reports.find.return_value = [ { "_id": "id", - "report_uuid": "report_uuid", - "subjects": {"subject_uuid": {"metrics": {"metric_uuid": {"type": "accessibility"}}}}, + "report_uuid": REPORT_ID, + "subjects": {SUBJECT_ID: {"metrics": {METRIC_ID: {"type": "accessibility"}}}}, }, ] perform_migrations(self.database) self.database.reports.replace_one.assert_called_once_with( {"_id": "id"}, { - "report_uuid": "report_uuid", + "report_uuid": REPORT_ID, "subjects": { - "subject_uuid": { + SUBJECT_ID: { "metrics": { - "metric_uuid": { + METRIC_ID: { "type": "violations", "name": "Accessibility violations", "unit": "accessibility violations", @@ -116,18 +122,48 @@ def test_change_accessibility_violations_to_violations_when_report_has_an_access }, ) + def test_change_accessibility_violations_to_violations_when_metric_has_name_and_unit(self): + """Test that the migration succeeds with an accessibility metric, and existing name and unit are kept.""" + self.database.reports.find.return_value = [ + { + "_id": "id", + "report_uuid": REPORT_ID, + "subjects": { + SUBJECT_ID: {"metrics": {METRIC_ID: {"type": "accessibility", "name": "name", "unit": "unit"}}}, + }, + }, + ] + perform_migrations(self.database) + self.database.reports.replace_one.assert_called_once_with( + {"_id": "id"}, + { + "report_uuid": REPORT_ID, + "subjects": { + SUBJECT_ID: { + "metrics": { + METRIC_ID: { + "type": "violations", + "name": "name", + "unit": "unit", + } + } + } + }, + }, + ) + def test_change_accessibility_violations_to_violations_when_report_has_accessibility_metric_and_other_types(self): """Test that the migration succeeds with an accessibility metric and other metric types.""" self.database.reports.find.return_value = [ { "_id": "id", - "report_uuid": "report_uuid", + "report_uuid": REPORT_ID, "subjects": { - "subject_uuid": { + SUBJECT_ID: { "metrics": { - "metric_uuid": {"type": "accessibility"}, - "metric_uuid2": {"type": "violations"}, - "metric_uuid3": {"type": "security_warnings"}, + METRIC_ID: {"type": "accessibility"}, + METRIC_ID2: {"type": "violations"}, + METRIC_ID3: {"type": "security_warnings"}, } } }, @@ -137,19 +173,19 @@ def test_change_accessibility_violations_to_violations_when_report_has_accessibi self.database.reports.replace_one.assert_called_once_with( {"_id": "id"}, { - "report_uuid": "report_uuid", + "report_uuid": REPORT_ID, "subjects": { - "subject_uuid": { + SUBJECT_ID: { "metrics": { - "metric_uuid": { + METRIC_ID: { "type": "violations", "name": "Accessibility violations", "unit": "accessibility violations", }, - "metric_uuid2": { + METRIC_ID2: { "type": "violations", }, - "metric_uuid3": { + METRIC_ID3: { "type": "security_warnings", }, } @@ -157,3 +193,93 @@ def test_change_accessibility_violations_to_violations_when_report_has_accessibi }, }, ) + + +class DatabaseMigrationsBranchParameterTest(DatabaseInitializationTestCase): + """Unit tests for the branch parameter database migration.""" + + def test_migration_without_reports(self): + """Test that the migration succeeds without reports.""" + self.database.reports.find.return_value = [] + perform_migrations(self.database) + self.database.reports.replace_one.assert_not_called() + + def test_migration_when_report_has_no_metrics_with_sources_with_branch_parameter(self): + """Test that the migration succeeds with reports, but without metrics with a branch parameter.""" + self.database.reports.find.return_value = [ + { + "report_uuid": REPORT_ID, + "subjects": { + SUBJECT_ID: { + "metrics": { + METRIC_ID: {"type": "issues"}, + METRIC_ID2: {"type": "loc"}, + }, + }, + }, + }, + ] + perform_migrations(self.database) + self.database.reports.replace_one.assert_not_called() + + def test_migration_when_report_has_source_with_branch(self): + """Test that the migration succeeds when the branch parameter is not empty.""" + self.database.reports.find.return_value = [ + { + "report_uuid": REPORT_ID, + "subjects": { + SUBJECT_ID: { + "metrics": { + METRIC_ID: { + "type": "loc", + "sources": {SOURCE_ID: {"type": "sonarqube", "parameters": {"branch": "main"}}}, + }, + }, + }, + }, + }, + ] + perform_migrations(self.database) + self.database.reports.replace_one.assert_not_called() + + def test_migration_when_report_has_sonarqube_metric_with_branch_without_value(self): + """Test that the migration succeeds when a source has an empty branch parameter.""" + self.database.reports.find.return_value = [ + { + "_id": "id", + "report_uuid": REPORT_ID, + "subjects": { + SUBJECT_ID: { + "metrics": { + METRIC_ID: { + "type": "source_up_to_dateness", + "sources": { + SOURCE_ID: {"type": "gitlab", "parameters": {"branch": ""}}, + SOURCE_ID2: {"type": "cloc", "parameters": {"branch": ""}}, + }, + }, + }, + }, + }, + }, + ] + perform_migrations(self.database) + self.database.reports.replace_one.assert_called_once_with( + {"_id": "id"}, + { + "report_uuid": REPORT_ID, + "subjects": { + SUBJECT_ID: { + "metrics": { + METRIC_ID: { + "type": "source_up_to_dateness", + "sources": { + SOURCE_ID: {"type": "gitlab", "parameters": {"branch": "master"}}, + SOURCE_ID2: {"type": "cloc", "parameters": {"branch": ""}}, + }, + }, + }, + }, + }, + }, + ) diff --git a/docs/src/changelog.md b/docs/src/changelog.md index 7e5ddd786e..045bc41210 100644 --- a/docs/src/changelog.md +++ b/docs/src/changelog.md @@ -8,7 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) -## v5.12.0-rc.3 - 2024-05-16 +## [Unreleased] ### Deployment notes @@ -22,7 +22,7 @@ If your currently installed *Quality-time* version is v4.10.0 or older, please r ### Added - Allow for hiding the legend card via the Settings panel. -- Explain difference between security warnings and violations in a new FAQ section of the documentation. Closes [#7797](https://github.com/ICTU/quality-time/issues/7797). +- Explain the difference between security warnings and violations in a new FAQ section of the documentation. Closes [#7797](https://github.com/ICTU/quality-time/issues/7797). - When adding a metric to a subject, add an option to hide metric types already used. Closes [#7992](https://github.com/ICTU/quality-time/issues/7992). - When adding a metric to a subject, add an option to choose from all metric types in addition to the metric types officially supported by the subject type. Closes [#8176](https://github.com/ICTU/quality-time/issues/8176). - When a report has a configured issue tracker, show a card in the dashboard with the number of issues per issue status (open, in progress, done). The card can be hidden via the Settings panel. Clicking the card or setting "Visible metrics" to "Metrics with issues" in the Settings panel hides metrics without issues. Closes [#8222](https://github.com/ICTU/quality-time/issues/8222).