Skip to content

Commit

Permalink
Add database migration for #8045.
Browse files Browse the repository at this point in the history
  • Loading branch information
fniessink committed May 17, 2024
1 parent 9995e10 commit 60263fe
Show file tree
Hide file tree
Showing 3 changed files with 213 additions and 33 deletions.
66 changes: 60 additions & 6 deletions components/api_server/src/initialization/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}}):
Expand All @@ -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)
176 changes: 151 additions & 25 deletions components/api_server/tests/initialization/test_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""
Expand All @@ -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()
Expand All @@ -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",
Expand All @@ -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"},
}
}
},
Expand All @@ -137,23 +173,113 @@ 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",
},
}
}
},
},
)


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": ""}},
},
},
},
},
},
},
)
4 changes: 2 additions & 2 deletions docs/src/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

<!-- The line "## <square-bracket>Unreleased</square-bracket>" is replaced by the release/release.py script with the new release version and release date. -->

## v5.12.0-rc.3 - 2024-05-16
## [Unreleased]

### Deployment notes

Expand All @@ -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).
Expand Down

0 comments on commit 60263fe

Please sign in to comment.