mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-05 06:45:09 +00:00
refactor: remove the skin system and the unused src/base_classes package (#615)
* refactor: remove the skin system Skins never rendered with the current scoreboard plugins: the only hook was SportsCore._render_game in src/base_classes, which no plugin builds on, so the UI and store already treated them as unsupported. The owner decided on 2026-09-23 to remove them outright. Removed src/skin_system/ (runtime, base class, fixtures), skins/, scripts/validate_skin.py and their tests; the store's "type": "skin" installer, uninstaller and hide/refuse filters (the official registry lists no skins); SchemaManager.inject_skin_selector; and GET /api/v3/skins. Stored skin/skin_options config values are handled in the next commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(config): drop retired skin/skin_options keys instead of validating them A config.json written while the skin system existed can carry skin and skin_options in any plugin section, and most plugin schemas set additionalProperties: false. They are no longer core plugin properties; RETIRED_PLUGIN_KEYS in schema_manager lists them and drop_retired_plugin_keys removes them (unless the plugin's own schema declares the name) in prepare_plugin_config, which loading, hot reload, GET /plugins/config and both web saves already share, and in validate_config_against_schema for callers that validate a raw section. POST /plugins/config and /config/main also drop them from the stored section they merge into, so they leave config.json on the next save. Tests cover the load path (real PluginManager.load_plugin: no schema warning, not degraded), raw and prepared validation, validate_all_plugin_configs, and the JSON, form and /config/main saves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor: remove the unused src/base_classes package No scoreboard plugin builds on src.base_classes: the nine monorepo scoreboards ship their own sports.py and share code through src/common (docs/SPORTS_UNIFICATION.md), and none of the third-party registry plugins imports it. The one import anywhere, baseball-scoreboard's rankings_manager.py, is a lazy import of ESPNDataSource in a class nothing instantiates. Removed the package and the eight test files that only tested it (test_api_extractors, test_data_sources, test_sports_base_characterization, test_sports_capabilities, test_sports_core_promotions, test_sports_logo_cache_bounded, test_sports_modes_promotions, test_sports_odds_fanout). test_common_is_hardware_free no longer lists src.base_classes as a forbidden import, and comments in sports_helpers.py and base_odds_manager.py stop pointing at it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: drop the skin system and src/base_classes from the docs Deletes docs/SKIN_SYSTEM.md and docs/CREATING_SKINS.md and every link to them (docs/README.md, README.md, PLUGIN_DEVELOPMENT_GUIDE.md, the /skins section of REST_API_REFERENCE.md), the skin section of CLAUDE.md and the term in PRODUCT.md. SPORTS_UNIFICATION.md now says src/base_classes was removed and shared code lives in src/common, in the Layering section and the view-model-contract rule. Other docs stop pointing at the removed package. CHANGELOG records both removals under Unreleased. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(store): hide and refuse registry entries that aren't plugins The skin filters went with the skin system, but a custom registry can still list "type": "skin" entries, and installing one as a plugin would unpack it into the plugins directory. PluginStoreManager.is_plugin_entry() (a missing type means plugin) now hides non-plugin entries from the store and custom-registry listings, and install refuses them, in the route with a clear 400 and in _install_plugin_impl for any other caller. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
@@ -53,6 +53,17 @@ class PluginStoreManager:
|
||||
# "..", "../x") into a filesystem path that purge_uninstalled_plugins
|
||||
# would delete — an empty id resolves to the plugins root itself.
|
||||
_PLUGIN_ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$")
|
||||
|
||||
@staticmethod
|
||||
def is_plugin_entry(entry) -> bool:
|
||||
"""Whether a registry entry is a plugin core can install.
|
||||
|
||||
A missing ``type`` means plugin. Anything else (registries used to
|
||||
carry ``"type": "skin"`` entries, and a custom registry still can) is
|
||||
hidden from the store and refused at install, rather than being
|
||||
unpacked into the plugins directory as if it were a plugin.
|
||||
"""
|
||||
return isinstance(entry, dict) and (entry.get('type') or 'plugin') == 'plugin'
|
||||
|
||||
def __init__(self, plugins_dir: str = "plugins",
|
||||
uninstalled_registry_path: Optional[str] = None):
|
||||
@@ -1314,18 +1325,10 @@ class PluginStoreManager:
|
||||
if not plugin_info:
|
||||
self.logger.error(f"Plugin not found in registry: {plugin_id}")
|
||||
return False
|
||||
|
||||
# Visual skins share the registry. _install_skin_from_info can put one
|
||||
# in skins/, but no current scoreboard plugin renders skins, so the
|
||||
# store refuses them rather than installing something that does
|
||||
# nothing (docs/SKIN_SYSTEM.md). Manual installs under skins/ and
|
||||
# uninstall_skin are unaffected.
|
||||
if (plugin_info.get('type') or 'plugin') == 'skin':
|
||||
from src.skin_system import SKINS_RENDER_SUPPORTED, SKINS_UNSUPPORTED_MESSAGE
|
||||
if not SKINS_RENDER_SUPPORTED:
|
||||
self.logger.error(f"Not installing skin {plugin_id}: {SKINS_UNSUPPORTED_MESSAGE}")
|
||||
return False
|
||||
return self._install_skin_from_info(plugin_id, plugin_info, branch)
|
||||
if not self.is_plugin_entry(plugin_info):
|
||||
self.logger.error(f"Not installing {plugin_id}: registry entry type "
|
||||
f"{plugin_info.get('type')!r} is not a plugin")
|
||||
return False
|
||||
|
||||
repo_url = plugin_info.get('repo')
|
||||
if not repo_url:
|
||||
@@ -2479,152 +2482,6 @@ class PluginStoreManager:
|
||||
continue
|
||||
return None
|
||||
|
||||
_SKIN_ID_PATTERN = re.compile(r'^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$')
|
||||
|
||||
def _resolve_skin_target(self, skin_id: str) -> Optional[Path]:
|
||||
"""Validate an externally-supplied skin id and resolve it to a path
|
||||
strictly inside the skins directory. Returns None (after logging)
|
||||
for ids that are malformed or would escape the directory — registry
|
||||
entries and manifests are external input and must not be able to
|
||||
write or delete outside skins/."""
|
||||
from src.skin_system import skin_runtime
|
||||
|
||||
if not isinstance(skin_id, str) or not self._SKIN_ID_PATTERN.match(skin_id) \
|
||||
or '..' in skin_id:
|
||||
self.logger.error(f"Rejecting unsafe skin id: {skin_id!r}")
|
||||
return None
|
||||
skins_dir = skin_runtime.get_skins_directory().resolve()
|
||||
target = (skins_dir / skin_id).resolve()
|
||||
if target.parent != skins_dir:
|
||||
self.logger.error(f"Skin id {skin_id!r} escapes the skins directory; rejecting")
|
||||
return None
|
||||
return target
|
||||
|
||||
def _install_skin_from_info(self, skin_id: str, skin_info: Dict,
|
||||
branch: Optional[str] = None) -> bool:
|
||||
"""Install a registry entry of type "skin" into skins/<id>/.
|
||||
|
||||
Reuses the plugin download machinery (git / monorepo zip / archive)
|
||||
but validates skin.json instead of manifest.json and never installs
|
||||
dependencies — skins are render-only (stdlib + PIL + the provided
|
||||
SkinContext), which is also what keeps them safe to iterate on.
|
||||
|
||||
Downloads into a staging directory and validates there; the
|
||||
existing installation is only replaced after the new one passes,
|
||||
so a failed download or bad manifest can't destroy a working skin.
|
||||
"""
|
||||
from src.skin_system import skin_runtime
|
||||
from src.skin_system.skin_base import SKIN_API_VERSION
|
||||
|
||||
repo_url = skin_info.get('repo')
|
||||
if not repo_url:
|
||||
self.logger.error(f"Skin {skin_id} missing repository URL")
|
||||
return False
|
||||
|
||||
target = self._resolve_skin_target(skin_id)
|
||||
if target is None:
|
||||
return False
|
||||
skins_dir = target.parent
|
||||
skins_dir.mkdir(parents=True, exist_ok=True)
|
||||
# Leading "_" keeps staging invisible to skin discovery
|
||||
staging = skins_dir / f"_staging-{skin_id}"
|
||||
if staging.exists() and not self._safe_remove_directory(staging):
|
||||
return False
|
||||
|
||||
subpath = skin_info.get('plugin_path')
|
||||
branch_candidates = self._distinct_sequence([
|
||||
branch,
|
||||
skin_info.get('branch'),
|
||||
skin_info.get('default_branch'),
|
||||
skin_info.get('last_commit_branch'),
|
||||
'main',
|
||||
'master'
|
||||
])
|
||||
|
||||
try:
|
||||
branch_used = None
|
||||
if subpath:
|
||||
for candidate in branch_candidates:
|
||||
download_url = f"{repo_url}/archive/refs/heads/{candidate}.zip"
|
||||
if self._install_from_monorepo(download_url, subpath, staging):
|
||||
branch_used = candidate
|
||||
break
|
||||
else:
|
||||
branch_used = self._install_via_git(repo_url, staging, branch_candidates)
|
||||
if branch_used is None and not staging.exists():
|
||||
for candidate in branch_candidates:
|
||||
download_url = f"{repo_url}/archive/refs/heads/{candidate}.zip"
|
||||
if self._install_via_download(download_url, staging):
|
||||
branch_used = candidate
|
||||
break
|
||||
|
||||
if branch_used is None and not staging.exists():
|
||||
self.logger.error(f"Failed to install skin {skin_id} via git or archive download")
|
||||
return False
|
||||
|
||||
try:
|
||||
with open(staging / 'skin.json', 'r', encoding='utf-8') as f:
|
||||
manifest = json.load(f)
|
||||
except (OSError, json.JSONDecodeError) as e:
|
||||
self.logger.error(f"Skin {skin_id} has no valid skin.json: {e}")
|
||||
return False
|
||||
|
||||
missing = [k for k in ('id', 'name', 'version', 'skin_api_version', 'class_name')
|
||||
if not manifest.get(k)]
|
||||
if missing:
|
||||
self.logger.error(f"Skin {skin_id} manifest missing fields: {missing}")
|
||||
return False
|
||||
|
||||
# Unlike plugins, a mismatched id is rejected rather than
|
||||
# renamed: the manifest id is external input, and the registry
|
||||
# id is what the user asked to install.
|
||||
if manifest['id'] != skin_id:
|
||||
self.logger.error(
|
||||
f"Skin manifest id {manifest['id']!r} doesn't match registry id "
|
||||
f"{skin_id!r}; not installing")
|
||||
return False
|
||||
|
||||
def _api_major(v):
|
||||
try:
|
||||
return int(str(v).split('.')[0])
|
||||
except (ValueError, IndexError):
|
||||
return None
|
||||
|
||||
if _api_major(manifest['skin_api_version']) != _api_major(SKIN_API_VERSION):
|
||||
self.logger.error(
|
||||
f"Skin {skin_id} targets skin API {manifest['skin_api_version']} but this "
|
||||
f"LEDMatrix provides {SKIN_API_VERSION}; not installing")
|
||||
return False
|
||||
|
||||
# Validated — swap into place
|
||||
if target.exists() and not self._safe_remove_directory(target):
|
||||
self.logger.error(f"Could not replace existing skin directory: {target}")
|
||||
return False
|
||||
shutil.move(str(staging), str(target))
|
||||
skin_runtime.discover_skins(force_refresh=True)
|
||||
self.logger.info(f"Successfully installed skin: {skin_id} (branch: {branch_used})")
|
||||
return True
|
||||
finally:
|
||||
if staging.exists():
|
||||
self._safe_remove_directory(staging)
|
||||
|
||||
def uninstall_skin(self, skin_id: str) -> bool:
|
||||
"""Remove an installed skin. Plugin configs referencing it keep
|
||||
validating; rendering falls back to the built-in layout."""
|
||||
from src.skin_system import skin_runtime
|
||||
|
||||
target = self._resolve_skin_target(skin_id)
|
||||
if target is None:
|
||||
return False
|
||||
if not target.exists():
|
||||
self.logger.info(f"Skin {skin_id} not found (already uninstalled)")
|
||||
return True
|
||||
if self._safe_remove_directory(target):
|
||||
skin_runtime.discover_skins(force_refresh=True)
|
||||
self.logger.info(f"Successfully uninstalled skin: {skin_id}")
|
||||
return True
|
||||
return False
|
||||
|
||||
def uninstall_plugin(self, plugin_id: str) -> bool:
|
||||
"""
|
||||
Uninstall a plugin by removing its directory.
|
||||
@@ -2638,12 +2495,6 @@ class PluginStoreManager:
|
||||
plugin_path = self._find_plugin_path(plugin_id)
|
||||
|
||||
if plugin_path is None or not plugin_path.exists():
|
||||
# A skin id passed to the plugin uninstall path (the store UI
|
||||
# uses one uninstall flow) removes the skin instead
|
||||
skin_target = self._resolve_skin_target(plugin_id) \
|
||||
if self._SKIN_ID_PATTERN.match(str(plugin_id)) else None
|
||||
if skin_target is not None and skin_target.exists():
|
||||
return self.uninstall_skin(plugin_id)
|
||||
self.logger.info(f"Plugin {plugin_id} not found (already uninstalled)")
|
||||
return True # Already uninstalled, consider this success
|
||||
|
||||
|
||||
Reference in New Issue
Block a user