diff --git a/database/models/reports.py b/database/models/reports.py index d727c494a..0a13a2efb 100644 --- a/database/models/reports.py +++ b/database/models/reports.py @@ -3,7 +3,6 @@ from decimal import Decimal from functools import cached_property -from shared.reports.types import ReportTotals from sqlalchemy import Column, ForeignKey, Table, UniqueConstraint, types from sqlalchemy.dialects import postgresql from sqlalchemy.dialects.postgresql import UUID @@ -11,9 +10,7 @@ from database.base import CodecovBaseModel, MixinBaseClass, MixinBaseClassNoExternalID from database.models.core import Commit, CompareCommit, Repository -from database.utils import ArchiveField from helpers.clock import get_utc_now -from helpers.config import should_write_data_to_storage_config_check from helpers.number import precise_round log = logging.getLogger(__name__) @@ -38,13 +35,6 @@ class CommitReport(CodecovBaseModel, MixinBaseClass): back_populates="reports_list", cascade="all, delete", ) - details = relationship( - "ReportDetails", - back_populates="report", - uselist=False, - cascade="all, delete", - passive_deletes=True, - ) totals = relationship( "ReportLevelTotals", back_populates="report", @@ -129,62 +119,6 @@ class UploadError(CodecovBaseModel, MixinBaseClass): error_params = Column(postgresql.JSON, default=dict) -class ReportDetails(CodecovBaseModel, MixinBaseClass): - __tablename__ = "reports_reportdetails" - report_id = Column(types.BigInteger, ForeignKey("reports_commitreport.id")) - report: CommitReport = relationship( - "CommitReport", foreign_keys=[report_id], back_populates="details" - ) - _files_array = Column("files_array", postgresql.ARRAY(postgresql.JSONB)) - _files_array_storage_path = Column( - "files_array_storage_path", types.Text, nullable=True - ) - - def get_repository(self): - return self.report.commit.repository - - def get_commitid(self): - return self.report.commit.commitid - - 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", []))), - "diff_totals": ( - ReportTotals(*v["diff_totals"]) if v["diff_totals"] else None - ), - } - for v in json_files_array - ] - - def _should_write_to_storage(self) -> bool: - # 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.owner is None - ): - return False - is_codecov_repo = self.report.commit.repository.owner.username == "codecov" - return should_write_data_to_storage_config_check( - "report_details_files_array", - is_codecov_repo, - self.report.commit.repository.repoid, - ) - - files_array = ArchiveField( - should_write_to_storage_fn=_should_write_to_storage, - rehydrate_fn=rehydrate_encoded_data, - default_value_class=list, - ) - - class AbstractTotals(MixinBaseClass): branches = Column(types.Integer) coverage = Column(types.Numeric(precision=8, scale=5)) diff --git a/database/tests/factories/core.py b/database/tests/factories/core.py index a3f5b9ddd..92ac521bd 100644 --- a/database/tests/factories/core.py +++ b/database/tests/factories/core.py @@ -212,14 +212,6 @@ class Meta: files = 0 -class ReportDetailsFactory(Factory): - class Meta: - model = models.ReportDetails - - report = factory.SubFactory(ReportFactory) - _files_array = factory.LazyFunction(list) - - class ReportResultsFactory(Factory): class Meta: model = models.ReportResults diff --git a/database/tests/unit/test_models.py b/database/tests/unit/test_models.py index 4f8825cf9..735706d92 100644 --- a/database/tests/unit/test_models.py +++ b/database/tests/unit/test_models.py @@ -1,11 +1,9 @@ import json -from unittest.mock import PropertyMock, call +from unittest.mock import PropertyMock from mock import MagicMock, patch from shared.plan.constants import DEFAULT_FREE_PLAN -from shared.reports.types import ReportTotals from shared.storage.exceptions import FileNotInStorageError -from shared.utils.ReportEncoder import ReportEncoder from sqlalchemy.orm import Session from database.models import ( @@ -22,7 +20,6 @@ AccountsUsers, GithubAppInstallation, ) -from database.models.reports import ReportDetails from database.tests.factories import ( BranchFactory, CommitFactory, @@ -32,7 +29,7 @@ PullFactory, RepositoryFactory, ) -from database.tests.factories.core import ReportDetailsFactory, UserFactory +from database.tests.factories.core import UserFactory class TestReprModels(object): @@ -292,263 +289,6 @@ def test_account_fks(self, dbsession): assert through_table_obj.account_id == account.id -class TestReportDetailsModel(object): - sample_files_array = [ - { - "filename": "file_1.go", - "file_index": 0, - "file_totals": ReportTotals( - *[0, 8, 5, 3, 0, "62.50000", 0, 0, 0, 0, 10, 2, 0] - ), - "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] - ), - "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], - "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], - "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] - ), - "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] - ), - "diff_totals": None, - }, - ] - rd: ReportDetails = ReportDetailsFactory() - assert ( - rd.rehydrate_encoded_data(self.sample_files_array) - == self.sample_files_array - ) - assert ( - rd.rehydrate_encoded_data(fully_encoded_sample) == self.sample_files_array - ) - assert rd.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 - factory_report_details._files_array_storage_path = None - dbsession.add(factory_report_details) - dbsession.flush() - - mock_archive_service = mocker.patch("database.utils.ArchiveService") - retrieved_instance = dbsession.query(ReportDetails).get( - factory_report_details.id_ - ) - assert retrieved_instance.external_id == factory_report_details.external_id - files_array = retrieved_instance.files_array - assert files_array == self.sample_files_array - mock_archive_service.assert_not_called() - - def test_get_files_array_from_storage(self, dbsession, mocker): - factory_report_details: ReportDetails = ReportDetailsFactory() - factory_report_details._files_array = None - factory_report_details._files_array_storage_path = ( - "https://storage-url/path/to/item.json" - ) - dbsession.add(factory_report_details) - dbsession.flush() - - mock_archive_service = mocker.patch("database.utils.ArchiveService") - mock_archive_service.return_value.read_file.return_value = json.dumps( - self.sample_files_array, cls=ReportEncoder - ) - retrieved_instance = dbsession.query(ReportDetails).get( - factory_report_details.id_ - ) - assert retrieved_instance.external_id == factory_report_details.external_id - files_array = retrieved_instance.files_array - assert files_array == self.sample_files_array - assert mock_archive_service.call_count == 1 - mock_archive_service.return_value.read_file.assert_has_calls( - [call("https://storage-url/path/to/item.json")] - ) - # Check that caching works within the instance - files_array = retrieved_instance.files_array - assert mock_archive_service.call_count == 1 - assert mock_archive_service.return_value.read_file.call_count == 1 - - def test_get_files_array_from_storage_error(self, dbsession, mocker): - factory_report_details: ReportDetails = ReportDetailsFactory() - factory_report_details._files_array = None - factory_report_details._files_array_storage_path = ( - "https://storage-url/path/to/item.json" - ) - dbsession.add(factory_report_details) - dbsession.flush() - - mocker.patch( - "database.utils.ArchiveField.read_timeout", - new_callable=PropertyMock, - return_value=0.1, - ) - mock_archive_service = mocker.patch("database.utils.ArchiveService") - - def side_effect(path): - assert path == "https://storage-url/path/to/item.json" - raise FileNotInStorageError() - - mock_archive_service.return_value.read_file.side_effect = side_effect - retrieved_instance = dbsession.query(ReportDetails).get( - factory_report_details.id_ - ) - files_array = retrieved_instance.files_array - assert files_array == [] - assert mock_archive_service.call_count == 1 - mock_archive_service.return_value.read_file.assert_has_calls( - [call("https://storage-url/path/to/item.json")] - ) - - def test__should_write_to_storage(self, dbsession, mocker, mock_configuration): - factory_report_details: ReportDetails = ReportDetailsFactory() - codecov_report_details: ReportDetails = ReportDetailsFactory( - report__commit__repository__owner__username="codecov" - ) - allowlisted_repo: ReportDetails = ReportDetailsFactory() - dbsession.add(factory_report_details) - dbsession.add(codecov_report_details) - dbsession.add(allowlisted_repo) - dbsession.flush() - - mock_configuration.set_params( - { - "setup": { - "save_report_data_in_storage": { - "repo_ids": [allowlisted_repo.report.commit.repository.repoid], - "report_details_files_array": "restricted_access", - }, - } - } - ) - assert factory_report_details._should_write_to_storage() == False - assert allowlisted_repo._should_write_to_storage() == True - assert codecov_report_details._should_write_to_storage() == True - mock_configuration.set_params( - { - "setup": { - "save_report_data_in_storage": { - "repo_ids": [], - "report_details_files_array": False, - }, - } - } - ) - assert factory_report_details._should_write_to_storage() == False - 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, 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.utils.ArchiveService") - - factory_report_details: ReportDetails = ReportDetailsFactory() - # Setting files_array. - factory_report_details.files_array = self.sample_files_array - dbsession.add(factory_report_details) - dbsession.flush() - - retrieved_instance = dbsession.query(ReportDetails).get( - factory_report_details.id_ - ) - assert retrieved_instance.external_id == factory_report_details.external_id - files_array = retrieved_instance.files_array - assert files_array == self.sample_files_array - mock_archive_service.assert_not_called() - - def test_set_files_array_to_storage(self, dbsession, mocker, mock_configuration): - mock_configuration.set_params( - { - "setup": { - "save_report_data_in_storage": { - "report_details_files_array": "general_access", - }, - } - } - ) - mock_archive_service = mocker.patch("database.utils.ArchiveService") - mock_archive_service.return_value.read_file.return_value = json.dumps( - self.sample_files_array, cls=ReportEncoder - ) - mock_archive_service.return_value.write_json_data_to_storage.return_value = ( - "https://storage-url/path/to/item.json" - ) - - factory_report_details: ReportDetails = ReportDetailsFactory() - dbsession.add(factory_report_details) - dbsession.flush() - - retrieved_instance = dbsession.query(ReportDetails).get( - factory_report_details.id_ - ) - # The default value from factory is [] - assert retrieved_instance.files_array == [] - assert mock_archive_service.call_count == 0 - assert mock_archive_service.return_value.read_file.call_count == 0 - # Set the new value - retrieved_instance.files_array = self.sample_files_array - assert mock_archive_service.call_count == 1 - mock_archive_service.return_value.write_json_data_to_storage.assert_has_calls( - [ - call( - commit_id=retrieved_instance.report.commit.commitid, - table="reports_reportdetails", - field="files_array", - external_id=retrieved_instance.external_id, - data=self.sample_files_array, - encoder=ReportEncoder, - ) - ] - ) - # Retrieve the set value - files_array = retrieved_instance.files_array - assert files_array == self.sample_files_array - assert mock_archive_service.call_count == 1 - # Check that caching works within the instance - files_array = retrieved_instance.files_array - assert mock_archive_service.call_count == 1 - assert mock_archive_service.return_value.read_file.call_count == 0 - - class TestCommitModel(object): sample_report = { "files": { diff --git a/services/archive.py b/services/archive.py index 63db1f02d..c54be2c81 100644 --- a/services/archive.py +++ b/services/archive.py @@ -4,7 +4,6 @@ from datetime import datetime from enum import Enum from hashlib import md5 -from uuid import uuid4 import sentry_sdk import shared.storage @@ -22,11 +21,8 @@ class MinioEndpoints(Enum): json_data_no_commit = ( "{version}/repos/{repo_hash}/json_data/{table}/{field}/{external_id}.json" ) - profiling_summary = "{version}/repos/{repo_hash}/profilingsummaries/{profiling_commit_id}/{location}" raw = "v4/raw/{date}/{repo_hash}/{commit_sha}/{reportid}.txt" - profiling_collection = "{version}/repos/{repo_hash}/profilingcollections/{profiling_commit_id}/{location}" computed_comparison = "{version}/repos/{repo_hash}/comparisons/{comparison_id}.json" - profiling_normalization = "{version}/repos/{repo_hash}/profilingnormalizations/{profiling_commit_id}/{location}" def get_path(self, **kwaargs) -> str: return self.value.format(**kwaargs) @@ -103,41 +99,6 @@ def write_computed_comparison(self, comparison, data) -> str: self.write_file(path, json.dumps(data)) return path - def write_profiling_collection_result(self, version_identifier, data): - location = uuid4().hex - path = MinioEndpoints.profiling_collection.get_path( - version="v4", - repo_hash=self.storage_hash, - profiling_commit_id=version_identifier, - location=location, - ) - - self.write_file(path, data) - return path - - def write_profiling_summary_result(self, version_identifier, data): - location = f"{uuid4().hex}.txt" - path = MinioEndpoints.profiling_summary.get_path( - version="v4", - repo_hash=self.storage_hash, - profiling_commit_id=version_identifier, - location=location, - ) - - self.write_file(path, data) - return path - - def write_profiling_normalization_result(self, version_identifier, data): - location = f"{uuid4().hex}.txt" - path = MinioEndpoints.profiling_normalization.get_path( - version="v4", - repo_hash=self.storage_hash, - profiling_commit_id=version_identifier, - location=location, - ) - self.write_file(path, data) - return path - def write_json_data_to_storage( self, commit_id, diff --git a/services/cleanup/models.py b/services/cleanup/models.py index 244823dc4..dcbd9233c 100644 --- a/services/cleanup/models.py +++ b/services/cleanup/models.py @@ -8,11 +8,9 @@ from shared.bundle_analysis import StoragePaths from shared.django_apps.compare.models import CommitComparison from shared.django_apps.core.models import Commit, Pull, Repository -from shared.django_apps.profiling.models import ProfilingUpload from shared.django_apps.reports.models import ( CommitReport, DailyTestRollup, - ReportDetails, TestInstance, ) from shared.django_apps.reports.models import ReportSession as Upload @@ -247,11 +245,9 @@ def unroll_subquery(context: CleanupContext, query: QuerySet) -> CleanupResult: ] = { Commit: partial(cleanup_archivefield, "report"), Pull: partial(cleanup_archivefield, "flare"), - ReportDetails: partial(cleanup_archivefield, "files_array"), CommitReport: cleanup_commitreport, Upload: cleanup_upload, CommitComparison: partial(cleanup_with_storage_field, "report_storage_path"), - ProfilingUpload: partial(cleanup_with_storage_field, "raw_upload_location"), StaticAnalysisSingleFileSnapshot: partial( cleanup_with_storage_field, "content_location" ), diff --git a/services/cleanup/regular.py b/services/cleanup/regular.py index 01353515e..37454c422 100644 --- a/services/cleanup/regular.py +++ b/services/cleanup/regular.py @@ -1,9 +1,6 @@ import logging import random -from shared.django_apps.profiling.models import ProfilingUpload -from shared.django_apps.reports.models import ReportDetails - from services.cleanup.cleanup import run_cleanup from services.cleanup.utils import CleanupResult, CleanupSummary, cleanup_context @@ -15,10 +12,7 @@ def run_regular_cleanup() -> CleanupSummary: complete_summary = CleanupSummary(CleanupResult(0), summary={}) # Usage of these model was removed, and we should clean up all its data before dropping the table for good. - cleanups_to_run = [ - ReportDetails.objects.all(), - ProfilingUpload.objects.all(), - ] + cleanups_to_run = [] # as we expect this job to have frequent retries, and cleanup to take a long time, # lets shuffle the various cleanups so that each one of those makes a little progress. diff --git a/services/cleanup/tests/snapshots/relations__leaf_table__leaf.txt b/services/cleanup/tests/snapshots/relations__leaf_table__leaf.txt index ff8577354..2b323e921 100644 --- a/services/cleanup/tests/snapshots/relations__leaf_table__leaf.txt +++ b/services/cleanup/tests/snapshots/relations__leaf_table__leaf.txt @@ -1,3 +1,3 @@ --- ReportDetails +-- UploadLevelTotals DELETE -FROM "reports_reportdetails"; +FROM "reports_uploadleveltotals"; diff --git a/services/cleanup/tests/test_regular_cleanup.py b/services/cleanup/tests/test_regular_cleanup.py index d160a863b..da52c30a2 100644 --- a/services/cleanup/tests/test_regular_cleanup.py +++ b/services/cleanup/tests/test_regular_cleanup.py @@ -1,32 +1,11 @@ import pytest -from shared.django_apps.reports.models import CommitReport, ReportDetails -from shared.django_apps.reports.tests.factories import ReportDetailsFactory from services.cleanup.regular import run_regular_cleanup from services.cleanup.utils import CleanupResult, CleanupSummary @pytest.mark.django_db -def test_deletes_reportdetails(mock_archive_storage): - mock_archive_storage.write_file("archive", "some_random_path", b"some random data") - - ReportDetailsFactory() - ReportDetailsFactory(_files_array_storage_path="some_random_path") - - # the parent which is being created by the factory: - assert CommitReport.objects.all().count() == 2 - assert ReportDetails.objects.all().count() == 2 - assert len(mock_archive_storage.storage["archive"]) == 1 - +def test_runs_regular_cleanup(): summary = run_regular_cleanup() - assert summary == CleanupSummary( - CleanupResult(2, 1), - { - ReportDetails: CleanupResult(2, 1), - }, - ) - - assert CommitReport.objects.all().count() == 2 - assert ReportDetails.objects.all().count() == 0 - assert len(mock_archive_storage.storage["archive"]) == 0 + assert summary == CleanupSummary(CleanupResult(0, 0), {}) diff --git a/services/cleanup/tests/test_relations.py b/services/cleanup/tests/test_relations.py index 586331165..865df7bff 100644 --- a/services/cleanup/tests/test_relations.py +++ b/services/cleanup/tests/test_relations.py @@ -6,7 +6,7 @@ from shared.django_apps.codecov_auth.tests.factories import OwnerFactory from shared.django_apps.core.models import Repository from shared.django_apps.core.tests.factories import RepositoryFactory -from shared.django_apps.reports.models import ReportDetails, TestInstance +from shared.django_apps.reports.models import TestInstance, UploadLevelTotals from services.cleanup.relations import ( build_relation_graph, @@ -64,7 +64,7 @@ def test_can_simplify_queries(): @pytest.mark.django_db def test_leaf_table(snapshot): - query = ReportDetails.objects.all() + query = UploadLevelTotals.objects.all() assert dump_delete_queries(query) == snapshot("leaf.txt")