Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and optimize querying pathContents #1141

Merged
merged 5 commits into from
Feb 13, 2025
Merged

Refactor and optimize querying pathContents #1141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5f2cb34
Refactor and optimize querying `pathContents`
Swatinem Feb 7, 2025
efdc87b
avoid string building in `PrefixedPath.basename`
Swatinem Feb 7, 2025
6dc1b8c
fix type annotation
Swatinem Feb 7, 2025
ca99ee9
fix test mocks, and revert basename changes
Swatinem Feb 7, 2025
3929f05
review
Swatinem Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 3 additions & 31 deletions graphql_api/actions/path_contents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,46 +1,18 @@
from typing import Iterable, Union

import sentry_sdk

from graphql_api.types.enums import PathContentDisplayType
from services.path import Dir, File


def partition_list_into_files_and_directories(
items: Iterable[Union[File, Dir]],
) -> tuple[Union[File, Dir]]:
files = []
directories = []

# Separate files and directories
for item in items:
if isinstance(item, Dir):
directories.append(item)
else:
files.append(item)

return (files, directories)


def sort_list_by_directory(
items: Iterable[Union[File, Dir]],
) -> Iterable[Union[File, Dir]]:
(files, directories) = partition_list_into_files_and_directories(items=items)
return directories + files


@sentry_sdk.trace
def sort_path_contents(
items: Iterable[Union[File, Dir]], filters={}
) -> Iterable[Union[File, Dir]]:
def sort_path_contents(items: list[File | Dir], filters: dict = {}) -> list[File | Dir]:
filter_parameter = filters.get("ordering", {}).get("parameter")
filter_direction = filters.get("ordering", {}).get("direction")

if filter_parameter and filter_direction:
parameter_value = filter_parameter.value
direction_value = filter_direction.value
items = sorted(
items,
items.sort(
key=lambda item: getattr(item, parameter_value),
reverse=direction_value == "descending",
)
Expand All @@ -49,6 +21,6 @@ def sort_path_contents(
parameter_value == "name"
and display_type is not PathContentDisplayType.LIST
):
items = sort_list_by_directory(items=items)
items.sort(key=lambda item: isinstance(item, File))

return items
13 changes: 10 additions & 3 deletions graphql_api/tests/test_branch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ class MockSession(object):
pass


class MockFile:
def __init__(self, name: str):
self.name = name


class MockReport(object):
def __init__(self):
self.sessions = {1: MockSession()}
Expand All @@ -171,6 +176,10 @@ def files(self):
"folder/subfolder/fileD.py",
]

def __iter__(self):
for name in self.files:
yield MockFile(name)

@property
def flags(self):
return {"flag-a": MockFlag()}
Expand Down Expand Up @@ -774,11 +783,9 @@ def test_fetch_path_contents_component_filter_missing_coverage(
@patch("services.components.component_filtered_report")
@patch("services.components.commit_components")
@patch("shared.reports.api_report_service.build_report_from_commit")
@patch("services.report.files_in_sessions")
def test_fetch_path_contents_component_filter_has_coverage(
self, session_files_mock, report_mock, commit_components_mock, filtered_mock
self, report_mock, commit_components_mock, filtered_mock
):
session_files_mock.return_value = MockReport().files
components = ["Global"]
variables = {
"org": self.org.username,
Expand Down
156 changes: 57 additions & 99 deletions graphql_api/types/commit/commit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import services.components as components_service
import services.path as path_service
from codecov.db import sync_to_async
from codecov_auth.models import Owner
from core.models import Commit
from graphql_api.actions.commits import commit_status, commit_uploads
from graphql_api.actions.comparison import validate_commit_comparison
Expand Down Expand Up @@ -44,7 +45,7 @@
from services.bundle_analysis import BundleAnalysisComparison, BundleAnalysisReport
from services.comparison import Comparison, ComparisonReport
from services.components import Component
from services.path import ReportPaths
from services.path import Dir, File, ReportPaths
from services.profiling import CriticalFile, ProfilingSummary
from services.yaml import (
YamlStates,
Expand Down Expand Up @@ -146,23 +147,19 @@ def resolve_critical_files(commit: Commit, info, **kwargs) -> List[CriticalFile]
return profiling_summary.critical_files


@sentry_sdk.trace
@commit_bindable.field("pathContents")
@sync_to_async
def resolve_path_contents(commit: Commit, info, path: str = None, filters=None):
"""
The file directory tree is a list of all the files and directories
extracted from the commit report of the latest, head commit.
The is resolver results in a list that represent the tree with files
and nested directories.
"""
current_owner = info.context["request"].current_owner

def get_sorted_path_contents(
current_owner: Owner,
commit: Commit,
path: str | None = None,
filters: dict | None = None,
) -> (
list[File | Dir] | MissingHeadReport | MissingCoverage | UnknownFlags | UnknownPath
):
# TODO: Might need to add reports here filtered by flags in the future
commit_report = report_service.build_report_from_commit(
report = report_service.build_report_from_commit(
commit, report_class=ReadOnlyReport
)
if not commit_report:
if not report:
return MissingHeadReport()

if filters is None:
Expand All @@ -189,9 +186,9 @@ def resolve_path_contents(commit: Commit, info, path: str = None, filters=None):

for component in filtered_components:
component_paths.extend(component.paths)
if commit_report.flags:
if report.flags:
component_flags.extend(
component.get_matching_flags(commit_report.flags.keys())
component.get_matching_flags(report.flags.keys())
)

if component_flags:
Expand All @@ -200,11 +197,11 @@ def resolve_path_contents(commit: Commit, info, path: str = None, filters=None):
else:
flags_filter = component_flags

if flags_filter and not commit_report.flags:
if flags_filter and not report.flags:
return UnknownFlags(f"No coverage with chosen flags: {flags_filter}")

report_paths = ReportPaths(
report=commit_report,
report=report,
path=path,
search_term=search_value,
filter_flags=flags_filter,
Expand All @@ -221,25 +218,23 @@ def resolve_path_contents(commit: Commit, info, path: str = None, filters=None):
# we're just missing coverage for the file
return MissingCoverage(f"missing coverage for path: {path}")

items: list[File | Dir]
if search_value or display_type == PathContentDisplayType.LIST:
items = report_paths.full_filelist()
else:
items = report_paths.single_directory()
return {"results": sort_path_contents(items, filters)}
return sort_path_contents(items, filters)


@commit_bindable.field("deprecatedPathContents")
@sentry_sdk.trace
@commit_bindable.field("pathContents")
@sync_to_async
def resolve_deprecated_path_contents(
def resolve_path_contents(
commit: Commit,
info,
path: str = None,
filters=None,
first=None,
after=None,
last=None,
before=None,
):
info: GraphQLResolveInfo,
path: str | None = None,
filters: dict | None = None,
) -> Any:
"""
The file directory tree is a list of all the files and directories
extracted from the commit report of the latest, head commit.
Expand All @@ -248,80 +243,43 @@ def resolve_deprecated_path_contents(
"""
current_owner = info.context["request"].current_owner

# TODO: Might need to add reports here filtered by flags in the future
commit_report = report_service.build_report_from_commit(
commit, report_class=ReadOnlyReport
)
if not commit_report:
return MissingHeadReport()

if filters is None:
filters = {}
search_value = filters.get("search_value")
display_type = filters.get("display_type")

flags_filter = filters.get("flags", [])
component_filter = filters.get("components", [])

component_paths = []
component_flags = []

if component_filter:
all_components = components_service.commit_components(commit, current_owner)
filtered_components = components_service.filter_components_by_name_or_id(
all_components, component_filter
)

if not filtered_components:
return MissingCoverage(
f"missing coverage for report with components: {component_filter}"
)

for component in filtered_components:
component_paths.extend(component.paths)
if commit_report.flags:
component_flags.extend(
component.get_matching_flags(commit_report.flags.keys())
)

if component_flags:
if flags_filter:
flags_filter = list(set(flags_filter) & set(component_flags))
else:
flags_filter = component_flags
contents = get_sorted_path_contents(current_owner, commit, path, filters)
if isinstance(contents, list):
return {"results": contents}
return contents

if flags_filter and not commit_report.flags:
return UnknownFlags(f"No coverage with chosen flags: {flags_filter}")

report_paths = ReportPaths(
report=commit_report,
path=path,
search_term=search_value,
filter_flags=flags_filter,
filter_paths=component_paths,
)

if len(report_paths.paths) == 0:
# we do not know about this path

if path_service.provider_path_exists(path, commit, current_owner) is False:
# file doesn't exist
return UnknownPath(f"path does not exist: {path}")

# we're just missing coverage for the file
return MissingCoverage(f"missing coverage for path: {path}")

if search_value or display_type == PathContentDisplayType.LIST:
items = report_paths.full_filelist()
else:
items = report_paths.single_directory()

sorted_items = sort_path_contents(items, filters)
@commit_bindable.field("deprecatedPathContents")
@sync_to_async
def resolve_deprecated_path_contents(
commit: Commit,
info: GraphQLResolveInfo,
path: str | None = None,
filters: dict | None = None,
first: Any = None,
after: Any = None,
last: Any = None,
before: Any = None,
) -> Any:
"""
The file directory tree is a list of all the files and directories
extracted from the commit report of the latest, head commit.
The is resolver results in a list that represent the tree with files
and nested directories.
"""
current_owner = info.context["request"].current_owner

kwargs = {"first": first, "after": after, "last": last, "before": before}
contents = get_sorted_path_contents(current_owner, commit, path, filters)
if not isinstance(contents, list):
return contents

return queryset_to_connection_sync(
sorted_items, ordering_direction=OrderingDirection.ASC, **kwargs
contents,
ordering_direction=OrderingDirection.ASC,
first=first,
last=last,
before=before,
after=after,
)


Expand Down
Loading
Loading