Skip to content

refactor: migrate ExtensionCatalog to shared catalog stack base#2576

Open
WOLIKIMCHENG wants to merge 1 commit into
github:mainfrom
WOLIKIMCHENG:refactor-extension-catalog-stack
Open

refactor: migrate ExtensionCatalog to shared catalog stack base#2576
WOLIKIMCHENG wants to merge 1 commit into
github:mainfrom
WOLIKIMCHENG:refactor-extension-catalog-stack

Conversation

@WOLIKIMCHENG
Copy link
Copy Markdown
Contributor

Summary

  • Migrate ExtensionCatalog to inherit CatalogStackBase
  • Remove duplicate extension catalog URL validation and YAML stack parsing
  • Preserve extension catalog source ordering and public CatalogEntry import compatibility
  • Add focused tests for malformed extension catalog config hardening

Behavior Notes

Valid extension catalog configs should behave the same. Malformed configs now align with shared base behavior:

  • non-mapping YAML roots raise ValidationError
  • boolean priorities are rejected
  • blank/null names normalize to catalog-N
  • invalid catalog URLs include config path and entry index context

Tests

  • uv run --with ruff ruff check src/specify_cli/extensions.py tests/test_extensions.py
    • Passed: all checks passed
  • Focused pytest selection for extension catalog stack, ExtensionCatalog, CLI path output, update hardening, and touched remediation test
    • Passed: 49 passed

Closes #2437

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates ExtensionCatalog to inherit from the shared CatalogStackBase, dropping its private duplicates of catalog URL validation and YAML config parsing. CatalogEntry becomes a thin subclass of the shared BaseCatalogEntry to preserve public import compatibility, and built-in/SPECKIT_CATALOG_URL/project/user catalog paths now go through self._entry() and self.CONFIG_FILENAME. Test suite gains focused coverage for malformed extension catalog configs (non-mapping roots, boolean priorities, blank/null names, invalid URLs with context) and applies a couple of unrelated lint cleanups.

Changes:

  • ExtensionCatalog now subclasses CatalogStackBase, declaring CONFIG_FILENAME/ENTRY_CLASS/ERROR_TYPE/VALIDATION_ERROR_TYPE, and removes its in-class _validate_catalog_url and _load_catalog_config.
  • CatalogEntry is redefined as class CatalogEntry(BaseCatalogEntry) to keep the existing public symbol while inheriting fields from the shared dataclass.
  • New parametrized tests cover non-mapping YAML roots, boolean priorities, blank/null name normalization, and URL error context for the extension catalog config; plus minor lint fixes (unused variable, split combined import).
Show a summary per file
File Description
src/specify_cli/extensions.py Reparent ExtensionCatalog on CatalogStackBase, switch entry construction to self._entry, drop duplicated URL/config helpers.
tests/test_extensions.py Add hardening tests for malformed extension-catalog configs; small lint-only cleanups.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(catalogs): reduce duplicated catalog stack handling

3 participants