diff --git a/database/models/reports.py b/database/models/reports.py index 9e9348b20..872f08cda 100644 --- a/database/models/reports.py +++ b/database/models/reports.py @@ -3,6 +3,7 @@ from functools import cached_property, lru_cache from shared.config import get_config +from shared.reports.types import ReportTotals, SessionTotalsArray from shared.storage.exceptions import FileNotInStorageError from sqlalchemy import Column, ForeignKey, Table, types from sqlalchemy.dialects import postgresql @@ -127,16 +128,35 @@ class ReportDetails(CodecovBaseModel, MixinBaseClass): "files_array_storage_path", types.Text, nullable=True ) + @classmethod + def rehydrate_encoded_data(self, json_files_array): + """This ensures that we always use the files_array with the correct underlying classes. + No matter where the data comes from. + """ + return [ + { + **v, + "file_totals": ReportTotals(*(v.get("file_totals", []))), + "session_totals": SessionTotalsArray.build_from_encoded_data( + v.get("session_totals") + ), + "diff_totals": ReportTotals(*v["diff_totals"]) + if v["diff_totals"] + else None, + } + for v in json_files_array + ] + @lru_cache(maxsize=1) def _get_files_array(self): # Get files_array from the proper source if self._files_array is not None: - return self._files_array + return ReportDetails.rehydrate_encoded_data(self._files_array) repository = self.report.commit.repository archive_service = ArchiveService(repository=repository) try: file_str = archive_service.read_file(self._files_array_storage_path) - return json.loads(file_str) + return ReportDetails.rehydrate_encoded_data(json.loads(file_str)) except FileNotInStorageError: log.error( "files_array not in storage", @@ -151,23 +171,36 @@ def _get_files_array(self): return [] def _should_write_to_storage(self): + # Safety check to see if the path to repository is valid + # Because we had issues around this before + if ( + self.report is None + or self.report.commit is None + or self.report.commit.repository is None + or self.report.commit.repository.slug is None + ): + return False report_builder_repo_ids = get_config( "setup", "save_report_data_in_storage", "repo_ids", default=[] ) - return get_config( + master_write_switch = get_config( "setup", "save_report_data_in_storage", "report_details_files_array", default=False, - ) and ( - not get_config( - "setup", - "save_report_data_in_storage", - "only_codecov", - default=True, - ) - or self.report.commit.repository.slug.startswith("codecov/") - or self.report.commit.repository.repoid in report_builder_repo_ids + ) + only_codecov = get_config( + "setup", + "save_report_data_in_storage", + "only_codecov", + default=True, + ) + is_codecov_repo = self.report.commit.repository.slug.startswith("codecov/") + is_repo_allowed = ( + self.report.commit.repository.repoid in report_builder_repo_ids + ) + return master_write_switch and ( + not only_codecov or is_codecov_repo or is_repo_allowed ) def _set_files_array(self, files_array: dict): diff --git a/database/tests/unit/test_models.py b/database/tests/unit/test_models.py index 8ff15f632..13a4fa752 100644 --- a/database/tests/unit/test_models.py +++ b/database/tests/unit/test_models.py @@ -2,6 +2,7 @@ from unittest.mock import call import pytest +from shared.reports.types import ReportTotals, SessionTotalsArray from shared.storage.exceptions import FileNotInStorageError from shared.utils.ReportEncoder import ReportEncoder @@ -134,25 +135,95 @@ class TestReportDetailsModel(object): { "filename": "file_1.go", "file_index": 0, - "file_totals": [0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2, 0], - "session_totals": { - "0": [0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2], - "meta": {"session_count": 1}, - }, + "file_totals": ReportTotals( + *[0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2, 0] + ), + "session_totals": SessionTotalsArray.build_from_encoded_data( + { + "0": [0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2], + "meta": {"session_count": 1}, + } + ), "diff_totals": None, }, { "filename": "file_2.py", "file_index": 1, - "file_totals": [0, 2, 1, 0, 1, "50.00000", 1, 0, 0, 0, 0, 0, 0], - "session_totals": { - "0": [0, 2, 1, 0, 1, "50.00000", 1], - "meta": {"session_count": 1}, - }, + "file_totals": ReportTotals( + *[0, 2, 1, 0, 1, "50.00000", 1, 0, 0, 0, 0, 0, 0] + ), + "session_totals": SessionTotalsArray.build_from_encoded_data( + { + "0": [0, 2, 1, 0, 1, "50.00000", 1], + "meta": {"session_count": 1}, + } + ), "diff_totals": None, }, ] + def test_rehydrate_already_hydrated(self): + fully_encoded_sample = [ + { + "filename": "file_1.go", + "file_index": 0, + "file_totals": [0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2, 0], + "session_totals": { + "0": [0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2], + "meta": {"session_count": 1}, + }, + "diff_totals": None, + }, + { + "filename": "file_2.py", + "file_index": 1, + "file_totals": [0, 2, 1, 0, 1, "50.00000", 1, 0, 0, 0, 0, 0, 0], + "session_totals": { + "0": [0, 2, 1, 0, 1, "50.00000", 1], + "meta": {"session_count": 1}, + }, + "diff_totals": None, + }, + ] + half_encoded_sample = [ + { + "filename": "file_1.go", + "file_index": 0, + "file_totals": ReportTotals( + *[0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2, 0] + ), + "session_totals": { + "0": [0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2], + "meta": {"session_count": 1}, + }, + "diff_totals": None, + }, + { + "filename": "file_2.py", + "file_index": 1, + "file_totals": ReportTotals( + *[0, 2, 1, 0, 1, "50.00000", 1, 0, 0, 0, 0, 0, 0] + ), + "session_totals": { + "0": [0, 2, 1, 0, 1, "50.00000", 1], + "meta": {"session_count": 1}, + }, + "diff_totals": None, + }, + ] + assert ( + ReportDetails.rehydrate_encoded_data(self.sample_files_array) + == self.sample_files_array + ) + assert ( + ReportDetails.rehydrate_encoded_data(fully_encoded_sample) + == self.sample_files_array + ) + assert ( + ReportDetails.rehydrate_encoded_data(half_encoded_sample) + == self.sample_files_array + ) + def test_get_files_array_from_db(self, dbsession, mocker): factory_report_details: ReportDetails = ReportDetailsFactory() factory_report_details._files_array = self.sample_files_array @@ -263,7 +334,17 @@ def test__should_write_to_storage(self, dbsession, mocker, mock_configuration): assert allowlisted_repo._should_write_to_storage() == False assert codecov_report_details._should_write_to_storage() == False - def test_set_files_array_to_db(self, dbsession, mocker): + def test_set_files_array_to_db(self, dbsession, mocker, mock_configuration): + mock_configuration.set_params( + { + "setup": { + "save_report_data_in_storage": { + "only_codecov": False, + "report_details_files_array": False, + }, + } + } + ) mock_archive_service = mocker.patch("database.models.reports.ArchiveService") factory_report_details: ReportDetails = ReportDetailsFactory() diff --git a/services/report/__init__.py b/services/report/__init__.py index 29a92bf12..e166fa4fd 100644 --- a/services/report/__init__.py +++ b/services/report/__init__.py @@ -159,7 +159,9 @@ async def initialize_and_save_report( ) if report_details is None: report_details = ReportDetails( - report_id=current_report_row.id_, files_array=[] + report_id=current_report_row.id_, + _files_array=[], + report=current_report_row, ) db_session.add(report_details) db_session.flush() diff --git a/services/tests/test_report.py b/services/tests/test_report.py index 43136359d..ae5eea0f5 100644 --- a/services/tests/test_report.py +++ b/services/tests/test_report.py @@ -1,7 +1,6 @@ import pprint from asyncio import Future from decimal import Decimal -from itertools import chain, combinations import mock import pytest @@ -3169,7 +3168,19 @@ async def test_create_new_report_for_commit_parent_has_no_report( r = await report_service.create_new_report_for_commit(commit) assert r.files == [] - def test_save_full_report(self, dbsession, mock_storage, sample_report): + def test_save_full_report( + self, dbsession, mock_storage, sample_report, mock_configuration + ): + mock_configuration.set_params( + { + "setup": { + "save_report_data_in_storage": { + "only_codecov": False, + "report_details_files_array": True, + }, + } + } + ) commit = CommitFactory.create() dbsession.add(commit) dbsession.flush() @@ -3990,10 +4001,13 @@ async def test_initialize_and_save_report_report_but_no_details_carryforward_nee @pytest.mark.asyncio async def test_initialize_and_save_report_needs_backporting( - self, dbsession, sample_commit_with_report_big, mock_storage + self, dbsession, sample_commit_with_report_big, mock_storage, mocker ): commit = sample_commit_with_report_big report_service = ReportService({}) + mocker.patch.object( + ReportDetails, "_should_write_to_storage", return_value=True + ) r = await report_service.initialize_and_save_report(commit) assert r is not None assert r.details is not None @@ -4020,7 +4034,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_00.py", "file_index": 0, - "file_totals": [0, 14, 12, 0, 2, "85.71429", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 14, 12, 0, 2, "85.71429", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4034,7 +4050,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_01.py", "file_index": 1, - "file_totals": [0, 11, 8, 0, 3, "72.72727", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 11, 8, 0, 3, "72.72727", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4048,7 +4066,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_10.py", "file_index": 10, - "file_totals": [0, 10, 6, 1, 3, "60.00000", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 10, 6, 1, 3, "60.00000", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4062,7 +4082,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_11.py", "file_index": 11, - "file_totals": [0, 23, 15, 1, 7, "65.21739", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 23, 15, 1, 7, "65.21739", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4076,7 +4098,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_12.py", "file_index": 12, - "file_totals": [0, 14, 8, 0, 6, "57.14286", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 14, 8, 0, 6, "57.14286", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4090,7 +4114,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_13.py", "file_index": 13, - "file_totals": [0, 15, 9, 0, 6, "60.00000", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 15, 9, 0, 6, "60.00000", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4104,7 +4130,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_14.py", "file_index": 14, - "file_totals": [0, 23, 13, 0, 10, "56.52174", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 23, 13, 0, 10, "56.52174", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4118,7 +4146,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_02.py", "file_index": 2, - "file_totals": [0, 13, 9, 0, 4, "69.23077", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 13, 9, 0, 4, "69.23077", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4132,7 +4162,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_03.py", "file_index": 3, - "file_totals": [0, 16, 8, 0, 8, "50.00000", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 16, 8, 0, 8, "50.00000", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4146,7 +4178,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_04.py", "file_index": 4, - "file_totals": [0, 10, 6, 0, 4, "60.00000", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 10, 6, 0, 4, "60.00000", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4160,7 +4194,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_05.py", "file_index": 5, - "file_totals": [0, 14, 10, 0, 4, "71.42857", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 14, 10, 0, 4, "71.42857", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4174,7 +4210,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_06.py", "file_index": 6, - "file_totals": [0, 9, 7, 1, 1, "77.77778", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 9, 7, 1, 1, "77.77778", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4188,7 +4226,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_07.py", "file_index": 7, - "file_totals": [0, 11, 9, 0, 2, "81.81818", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 11, 9, 0, 2, "81.81818", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4202,7 +4242,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_08.py", "file_index": 8, - "file_totals": [0, 11, 6, 0, 5, "54.54545", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 11, 6, 0, 5, "54.54545", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4216,7 +4258,9 @@ async def test_initialize_and_save_report_needs_backporting( { "filename": "file_09.py", "file_index": 9, - "file_totals": [0, 14, 10, 1, 3, "71.42857", 0, 0, 0, 0, 0, 0, 0], + "file_totals": ReportTotals( + *[0, 14, 10, 1, 3, "71.42857", 0, 0, 0, 0, 0, 0, 0] + ), "session_totals": SessionTotalsArray.build_from_encoded_data( [ None, @@ -4228,7 +4272,8 @@ async def test_initialize_and_save_report_needs_backporting( "diff_totals": None, }, ] - assert len(mock_storage.storage["archive"]) == 1 + # The chunks and the ReportDetails.files_array + assert len(mock_storage.storage["archive"]) == 2 @pytest.mark.asyncio async def test_initialize_and_save_report_existing_report(