Formatting fix for toolshed enhancements.

This commit is contained in:
John Chilton
2026-02-05 21:35:07 -05:00
parent 35f6bd847a
commit 2579fa55fb
8 changed files with 43 additions and 84 deletions
+9 -11
View File
@@ -478,9 +478,7 @@ def get_repository_revision_metadata_dict(
def safe_encode_id(id_value):
return app.security.encode_id(id_value) if id_value is not None else None
metadata_dict = metadata.to_dict(
value_mapper={"id": safe_encode_id, "repository_id": safe_encode_id}
)
metadata_dict = metadata.to_dict(value_mapper={"id": safe_encode_id, "repository_id": safe_encode_id})
metadata_dict["repository"] = repository.to_dict(
value_mapper={"id": app.security.encode_id, "user_id": app.security.encode_id}
)
@@ -520,9 +518,7 @@ def reset_metadata_on_repository(
) -> ResetMetadataOnRepositoryResponse:
app: ToolShedApp = trans.app
def handle_repository(
trans, start_time, repository, dry_run: bool, verbose: bool, clone_url: Optional[str] = None
):
def handle_repository(trans, start_time, repository, dry_run: bool, verbose: bool, clone_url: Optional[str] = None):
results: dict = dict(start_time=start_time, repository_status=[], dry_run=dry_run)
regenerated_metadata = {}
try:
@@ -547,9 +543,7 @@ def reset_metadata_on_repository(
results["status"] = "warning"
else:
action_desc = "Would reset" if dry_run else "Successfully reset"
message = (
f"{action_desc} metadata on repository {repository.name} owned by {repository.user.username}"
)
message = f"{action_desc} metadata on repository {repository.name} owned by {repository.user.username}"
results["status"] = "ok"
except Exception as e:
message = (
@@ -568,10 +562,14 @@ def reset_metadata_on_repository(
# Capture before state if verbose
metadata_before = None
if verbose:
as_dict = get_repository_metadata_dict_for_repository(app, repository, recursive=False, downloadable_only=False)
as_dict = get_repository_metadata_dict_for_repository(
app, repository, recursive=False, downloadable_only=False
)
metadata_before = RepositoryMetadataPreview(root=as_dict)
results, regenerated_metadata = handle_repository(trans, start_time, repository, dry_run, verbose, repository_clone_url)
results, regenerated_metadata = handle_repository(
trans, start_time, repository, dry_run, verbose, repository_clone_url
)
# Capture after state if verbose - use regenerated metadata for accurate diff
metadata_after = None
@@ -919,8 +919,7 @@ class RepositoryMetadataManager(ToolShedMetadataGenerator):
comparison_result=comparison,
record_operation=record_operation,
has_tools="tools" in (metadata_dict or {}),
has_repository_dependencies="repository_dependencies"
in (metadata_dict or {}),
has_repository_dependencies="repository_dependencies" in (metadata_dict or {}),
has_tool_dependencies="tool_dependencies" in (metadata_dict or {}),
)
)
@@ -937,8 +936,7 @@ class RepositoryMetadataManager(ToolShedMetadataGenerator):
numeric_revision=numeric_rev,
comparison_result=self.INITIAL,
has_tools="tools" in (self.metadata_dict or {}),
has_repository_dependencies="repository_dependencies"
in (self.metadata_dict or {}),
has_repository_dependencies="repository_dependencies" in (self.metadata_dict or {}),
has_tool_dependencies="tool_dependencies" in (self.metadata_dict or {}),
)
)
+1 -3
View File
@@ -402,9 +402,7 @@ class ToolShedPopulator:
self, repository: HasRepositoryId, dry_run: bool = False, verbose: bool = False
) -> ResetMetadataOnRepositoryResponse:
repository_id = self._repository_id(repository)
request = ResetMetadataOnRepositoryRequest(
repository_id=repository_id, dry_run=dry_run, verbose=verbose
)
request = ResetMetadataOnRepositoryRequest(repository_id=repository_id, dry_run=dry_run, verbose=verbose)
reset_response = self._api_interactor.post(
"repositories/reset_metadata_on_repository", json=request.model_dump()
)
@@ -6,6 +6,7 @@ conditions (e.g., corrupted metadata) that can't be created through normal API u
Unlike pure API tests in test_shed_repositories.py, these tests verify the API
correctly handles edge cases that require backend manipulation to reproduce.
"""
import copy
from dataclasses import dataclass
@@ -116,23 +117,21 @@ class TestRepositoriesIntegration(ShedApiTestCase):
before = result["repository_metadata_before"]
after = result["repository_metadata_after"]
assert metadata_contains_path(before, fixture.corrupted_path), (
f"Expected before snapshot to contain corrupted path '{fixture.corrupted_path}'"
)
assert not metadata_contains_path(after, fixture.corrupted_path), (
"After snapshot should not contain corrupted path"
)
assert metadata_has_valid_paths(after), (
"Expected after snapshot to contain fixed path"
)
assert metadata_contains_path(
before, fixture.corrupted_path
), f"Expected before snapshot to contain corrupted path '{fixture.corrupted_path}'"
assert not metadata_contains_path(
after, fixture.corrupted_path
), "After snapshot should not contain corrupted path"
assert metadata_has_valid_paths(after), "Expected after snapshot to contain fixed path"
# Verify DB still has corrupted data (dry_run shouldn't persist)
sa_session = test_db_util.sa_session()
sa_session.expire_all()
metadata_revision = sa_session.get(model.RepositoryMetadata, fixture.metadata_revision_id)
assert metadata_revision.metadata["tools"][0]["tool_config"] == fixture.corrupted_path, (
"dry_run should not have modified the database"
)
assert (
metadata_revision.metadata["tools"][0]["tool_config"] == fixture.corrupted_path
), "dry_run should not have modified the database"
@skip_if_api_v1
def test_reset_metadata_fixes_corrupted_path_when_not_dry_run(self):
@@ -153,15 +152,10 @@ class TestRepositoriesIntegration(ShedApiTestCase):
# Verify DB now has fixed path
sa_session = test_db_util.sa_session()
sa_session.expire_all()
stmt = (
select(model.RepositoryMetadata)
.where(model.RepositoryMetadata.repository_id == fixture.db_repo.id)
)
stmt = select(model.RepositoryMetadata).where(model.RepositoryMetadata.repository_id == fixture.db_repo.id)
revisions = sa_session.scalars(stmt).all()
assert len(revisions) >= 1
# Build metadata dict from DB revisions for reuse of helper
db_metadata = {str(rev.id): rev.metadata for rev in revisions if rev.metadata}
assert metadata_has_valid_paths(db_metadata), (
"Expected reset to fix the corrupted tool_config path"
)
assert metadata_has_valid_paths(db_metadata), "Expected reset to fix the corrupted tool_config path"
@@ -534,15 +534,11 @@ class TestShedRepositoriesApi(ShedApiTestCase):
# 2. RepositoryMetadata - bismark (with invalid_tools)
bismark_repo = populator.setup_bismark_repo()
assert not isinstance(bismark_repo, str) # type narrowing for mypy
bismark_response = self.api_interactor.get(
f"repositories/{bismark_repo.id}/metadata?downloadable_only=false"
)
bismark_response = self.api_interactor.get(f"repositories/{bismark_repo.id}/metadata?downloadable_only=false")
api_asserts.assert_status_code_is_ok(bismark_response)
bismark_metadata = bismark_response.json()
# Verify it has invalid_tools
has_invalid = any(
rev.get("invalid_tools") for rev in bismark_metadata.values()
)
has_invalid = any(rev.get("invalid_tools") for rev in bismark_metadata.values())
assert has_invalid, "bismark should have invalid_tools"
with open(os.path.join(output_dir, "repository_metadata_bismark.json"), "w") as f:
@@ -601,10 +597,7 @@ class TestShedRepositoriesApi(ShedApiTestCase):
assert unchanged_reset_data["dry_run"] is True
assert unchanged_reset_data["changeset_details"] is not None
# Verify we got an "equal" comparison - this is the whole point of this fixture
has_equal = any(
d.get("comparison_result") == "equal"
for d in unchanged_reset_data["changeset_details"]
)
has_equal = any(d.get("comparison_result") == "equal" for d in unchanged_reset_data["changeset_details"])
assert has_equal, "column_maker_unchanged should produce 'equal' comparison_result"
with open(os.path.join(output_dir, "reset_metadata_unchanged.json"), "w") as f:
@@ -623,10 +616,7 @@ class TestShedRepositoriesApi(ShedApiTestCase):
assert subset_reset_data["dry_run"] is True
assert subset_reset_data["changeset_details"] is not None
# Verify we got a "subset" comparison - this is the whole point of this fixture
has_subset = any(
d.get("comparison_result") == "subset"
for d in subset_reset_data["changeset_details"]
)
has_subset = any(d.get("comparison_result") == "subset" for d in subset_reset_data["changeset_details"])
assert has_subset, "column_maker_subset should produce 'subset' comparison_result"
with open(os.path.join(output_dir, "reset_metadata_subset.json"), "w") as f:
@@ -694,8 +684,7 @@ Now with improved help text!
assert direct_push_reset_data["changeset_details"] is not None
# Verify we got a "created" record_operation - this is the whole point of this fixture
has_created = any(
d.get("record_operation") == "created"
for d in direct_push_reset_data["changeset_details"]
d.get("record_operation") == "created" for d in direct_push_reset_data["changeset_details"]
)
assert has_created, "direct_push should produce 'created' record_operation"
+2 -6
View File
@@ -298,12 +298,8 @@ RequiredRepositoryChangesetRevisionParam: str = Query(..., title="Changeset Revi
OptionalRepositoryIdParam: Optional[str] = Query(None, title="TSR ID")
OptionalHexlifyParam: Optional[bool] = Query(True, title="Hexlify response")
DryRunQueryParam: bool = Query(
False, title="Dry Run", description="Preview changes without persisting to database"
)
VerboseQueryParam: bool = Query(
False, title="Verbose", description="Return detailed per-changeset information"
)
DryRunQueryParam: bool = Query(False, title="Dry Run", description="Preview changes without persisting to database")
VerboseQueryParam: bool = Query(False, title="Verbose", description="Return detailed per-changeset information")
CategoryIdPathParam: str = Path(
..., title="Category ID", description="The encoded database identifier of the category."
+1 -3
View File
@@ -305,9 +305,7 @@ class FastAPIRepositories:
dry_run: bool = DryRunQueryParam,
verbose: bool = VerboseQueryParam,
) -> ResetMetadataOnRepositoryResponse:
return reset_metadata_on_repository(
trans, encoded_repository_id, dry_run=dry_run, verbose=verbose
)
return reset_metadata_on_repository(trans, encoded_repository_id, dry_run=dry_run, verbose=verbose)
@router.post(
"/api/repositories/reset_metadata_on_repositories",
@@ -62,9 +62,7 @@ def test_reset_dm_with_uninstallable_revisions(
assert len(new_repository.downloadable_revisions) == 2
def test_reset_dry_run_does_not_persist(
provides_repositories: ProvidesRepositoriesContext, new_repository: Repository
):
def test_reset_dry_run_does_not_persist(provides_repositories: ProvidesRepositoriesContext, new_repository: Repository):
"""Verify dry_run=True doesn't modify DB."""
shed_app = provides_repositories.app
upload_directories_to_repository(provides_repositories, new_repository, "column_maker")
@@ -113,9 +111,7 @@ def test_reset_verbose_returns_changeset_details(
persist=False,
)
repo_path = new_repository.repo_path(app=shed_app)
result = rmm.reset_all_metadata_on_repository_in_tool_shed(
repository_clone_url=repo_path, verbose=True
)
result = rmm.reset_all_metadata_on_repository_in_tool_shed(repository_clone_url=repo_path, verbose=True)
assert result.changeset_details is not None
assert len(result.changeset_details) >= 1
@@ -139,16 +135,12 @@ def test_reset_without_verbose_omits_details(
persist=False,
)
repo_path = new_repository.repo_path(app=shed_app)
result = rmm.reset_all_metadata_on_repository_in_tool_shed(
repository_clone_url=repo_path, verbose=False
)
result = rmm.reset_all_metadata_on_repository_in_tool_shed(repository_clone_url=repo_path, verbose=False)
assert result.changeset_details is None
def test_reset_dry_run_with_gaps(
provides_repositories: ProvidesRepositoriesContext, new_repository: Repository
):
def test_reset_dry_run_with_gaps(provides_repositories: ProvidesRepositoriesContext, new_repository: Repository):
"""Test dry_run with repo that has uninstallable revisions."""
shed_app = provides_repositories.app
upload_directories_to_repository(provides_repositories, new_repository, "column_maker_with_download_gaps")
@@ -194,11 +186,9 @@ def test_reset_metadata_fixes_tool_config_path(
original_tool_config = tools[0]["tool_config"]
# Verify it's an absolute path based on repo_path
assert original_tool_config.startswith(repo_path), (
f"Expected tool_config to start with repo_path.\n"
f" tool_config: {original_tool_config}\n"
f" repo_path: {repo_path}"
)
assert original_tool_config.startswith(
repo_path
), f"Expected tool_config to start with repo_path.\n tool_config: {original_tool_config}\n repo_path: {repo_path}"
# 2. Corrupt the tool_config path (simulate repo location change)
corrupted_path = "/old/wrong/path/column_maker.xml"
@@ -339,8 +329,6 @@ def test_reset_metadata_dry_run_shows_visual_diff(
provides_repositories.sa_session.expire_all()
metadata_revision = new_repository.metadata_revisions[0]
db_tool_config = metadata_revision.metadata["tools"][0]["tool_config"]
assert db_tool_config == corrupted_path, (
f"dry_run should not modify database.\n"
f" Expected: {corrupted_path}\n"
f" Got: {db_tool_config}"
)
assert (
db_tool_config == corrupted_path
), f"dry_run should not modify database.\n Expected: {corrupted_path}\n Got: {db_tool_config}"