Skip to content

Commit aaf2de6

Browse files
authored
Scope Python RPC marketplace responses to the requested distribution (#7512)
* Scope Python RPC marketplace responses to the requested distribution `discover_recipes()` eagerly walked every `openrewrite.recipes` entry point on `_get_marketplace()` init and `handle_get_marketplace()` returned the full singleton, so installing any pip package had its `RecipeBundle` tagged with every recipe in the process - including the eight built-in `org.openrewrite.python.*` recipes activated by the `openrewrite` distribution itself. A visualization-only package like `moderne-visualizations-misc` ended up "owning" every Python recipe. Track per-distribution attribution at activation time (`{distribution_name -> set(recipe_name)}`), have `handle_install_recipes` return only the rows for the requested package, and let `handle_get_marketplace` filter by `packageName` when supplied. `PipRecipeBundleReader` now uses the install response directly instead of issuing a follow-up `GetMarketplace` call that would still see the full singleton. `InstallRecipesResponse` carries the new `recipes` field backwards-compatibly: older Python servers that don't include it deserialize to `null`, accessed through `recipesOrEmpty()`. * Avoid baking the recipe count into prose The number of built-in `org.openrewrite.python.*` recipes will change over time. Drop the hard-coded "eight" from the comments and test docstrings so they don't decay. * Wrap attribution map in a RecipeAttribution type `Optional[Dict[str, Set[str]]]` told the reader nothing about what the keys or values mean, and pushed PEP 503 normalization out to every call site. Replace it with a small `RecipeAttribution` dataclass that encapsulates the storage, normalizes distribution names on both record and lookup, and exposes named `record(...)` / `recipes_for(...)` methods. Export it from the package since it's now part of the `discover_recipes` signature. * Address review feedback on attribution typing + recipesInstalled semantic - `recipesInstalled` is back to the diff semantic (recipes added by THIS call, zero on idempotent reinstalls), matching the field name's promise and the prior wire contract. The new `recipes` field carries the cumulative per-distribution row list separately. Audit of other language servers (Java in-process, C# LanguageServer, JS, Go) confirms Python is the sole InstallRecipes producer and no consumer reads `recipesInstalled` today, so the diff semantic is the safe default. - Drop `@dataclass` on `RecipeAttribution`. The auto-generated `__init__(_by_package=...)` exposed the internal map through the constructor signature and we don't use the auto `__eq__`/`__repr__`. Plain class with `__init__(self) -> None` keeps the storage truly private. - Replace bare `Dict[str, Set[str]]` with NewType-typed `Dict[NormalizedDistName, Set[RecipeName]]`. `NormalizedDistName` can only be built via `_normalize_package_name`; `RecipeName` propagates through `recipe_name_set`, the attribution API, and `_collect_marketplace_rows`'s filter parameter. Zero runtime cost, type-checker discipline at every site.
1 parent c229022 commit aaf2de6

7 files changed

Lines changed: 367 additions & 34 deletions

File tree

rewrite-python/rewrite/src/rewrite/__init__.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,13 @@
99

1010
from .category import CategoryDescriptor, LOWEST_PRECEDENCE, DEFAULT_PRECEDENCE, HIGHEST_PRECEDENCE
1111
from .decorators import categorize, get_recipe_category
12-
from .discovery import discover_recipes, discover_decorated_recipes_in_module
12+
from .discovery import (
13+
discover_recipes,
14+
discover_decorated_recipes_in_module,
15+
RecipeAttribution,
16+
RecipeName,
17+
NormalizedDistName,
18+
)
1319
from .execution import ExecutionContext, DelegatingExecutionContext, InMemoryExecutionContext, \
1420
RecipeRunException, LargeSourceSet, InMemoryLargeSourceSet, Result
1521
from .marketplace import RecipeMarketplace, Python
@@ -68,6 +74,9 @@
6874
'get_recipe_category',
6975
'discover_recipes',
7076
'discover_decorated_recipes_in_module',
77+
'RecipeAttribution',
78+
'RecipeName',
79+
'NormalizedDistName',
7180
'activate',
7281

7382
# Markers

rewrite-python/rewrite/src/rewrite/discovery.py

Lines changed: 85 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,81 @@
1818

1919
import inspect
2020
from importlib.metadata import entry_points
21-
from typing import List, Tuple, Type
21+
from typing import Dict, List, NewType, Optional, Set, Tuple, Type
2222

2323
from rewrite.category import CategoryDescriptor
2424
from rewrite.decorators import get_recipe_category
2525
from rewrite.marketplace import RecipeMarketplace
2626
from rewrite.recipe import Recipe
2727

2828

29-
def discover_recipes() -> RecipeMarketplace:
29+
# A recipe's fully qualified name (e.g., ``org.openrewrite.python.RemovePass``).
30+
# NewType is a static-only distinction — at runtime these are plain ``str`` —
31+
# but it lets type checkers refuse a bare string where a recipe-name set is
32+
# expected, which catches the kind of "did I just lookup the wrong key" bug
33+
# that motivated this module's per-distribution attribution in the first place.
34+
RecipeName = NewType("RecipeName", str)
35+
36+
# A python distribution name in its PEP 503 normalized form (hyphens,
37+
# underscores, dots all folded to ``_`` and lowercased). Used as the
38+
# attribution map key. Construct only via :func:`_normalize_package_name`.
39+
NormalizedDistName = NewType("NormalizedDistName", str)
40+
41+
42+
class RecipeAttribution:
43+
"""Tracks which distribution's entry point activated which recipes.
44+
45+
Used to answer "which recipes came from package X?" so callers can scope
46+
a marketplace response to a specific distribution instead of returning
47+
the whole singleton. Distribution names are PEP 503 normalized (hyphen,
48+
underscore, and case folded together) on both write and read, so callers
49+
can use any common spelling.
50+
"""
51+
52+
def __init__(self) -> None:
53+
self._by_package: Dict[NormalizedDistName, Set[RecipeName]] = {}
54+
55+
def record(self, distribution_name: str, recipe_names: Set[RecipeName]) -> None:
56+
"""Attribute ``recipe_names`` to ``distribution_name``.
57+
58+
No-op when ``recipe_names`` is empty; multiple calls for the same
59+
distribution accumulate.
60+
"""
61+
if not recipe_names:
62+
return
63+
key = _normalize_package_name(distribution_name)
64+
self._by_package.setdefault(key, set()).update(recipe_names)
65+
66+
def recipes_for(self, distribution_name: str) -> Set[RecipeName]:
67+
"""Return the (possibly empty) set of recipe names attributed to a
68+
distribution. The returned set is a snapshot — mutating it does not
69+
change the attribution.
70+
"""
71+
return set(self._by_package.get(_normalize_package_name(distribution_name), ()))
72+
73+
74+
def discover_recipes(
75+
marketplace: Optional[RecipeMarketplace] = None,
76+
attribution: Optional[RecipeAttribution] = None,
77+
) -> RecipeMarketplace:
3078
"""
3179
Discover all recipes from installed packages via entry points.
3280
3381
Looks for packages that declare entry points under
3482
[project.entry-points."openrewrite.recipes"] and calls their
3583
activate function to install recipes into the marketplace.
3684
85+
Args:
86+
marketplace: Optional existing marketplace to install into; a new one
87+
is created when None.
88+
attribution: Optional sink that records which distribution activated
89+
which recipes. When supplied, each entry point's contribution is
90+
recorded against its distribution name. Recipes already in the
91+
marketplace before this call (e.g., deduped by an earlier entry
92+
point) are not re-recorded for the current entry point.
93+
3794
Returns:
38-
A RecipeMarketplace containing all discovered recipes.
95+
The marketplace containing all discovered recipes.
3996
4097
Example pyproject.toml entry point:
4198
[project.entry-points."openrewrite.recipes"]
@@ -45,23 +102,46 @@ def discover_recipes() -> RecipeMarketplace:
45102
def activate(marketplace: RecipeMarketplace) -> None:
46103
marketplace.install(MyRecipe, [Python, Cleanup])
47104
"""
48-
marketplace = RecipeMarketplace()
105+
if marketplace is None:
106+
marketplace = RecipeMarketplace()
49107

50108
# Python 3.10+ uses select parameter via SelectableGroups
51109
eps = entry_points(group="openrewrite.recipes")
52110

53111
for ep in eps:
54112
try:
55113
module = ep.load()
56-
if hasattr(module, "activate") and callable(module.activate):
114+
if not hasattr(module, "activate") or not callable(module.activate):
115+
continue
116+
if attribution is None:
57117
module.activate(marketplace)
118+
continue
119+
dist_name = ep.dist.name if ep.dist is not None else None
120+
before = recipe_name_set(marketplace)
121+
module.activate(marketplace)
122+
if dist_name:
123+
attribution.record(dist_name, recipe_name_set(marketplace) - before)
58124
except Exception:
59125
# Log or handle the error - for now, skip failed activations
60126
pass
61127

62128
return marketplace
63129

64130

131+
def recipe_name_set(marketplace: RecipeMarketplace) -> Set[RecipeName]:
132+
"""Snapshot the set of recipe names currently in the marketplace."""
133+
return {RecipeName(r.name) for r in marketplace.all_recipes()}
134+
135+
136+
def _normalize_package_name(name: str) -> NormalizedDistName:
137+
"""Normalize a python distribution name for attribution lookup.
138+
139+
PyPI/pip treats hyphens, underscores, and case as equivalent when
140+
resolving distribution identity.
141+
"""
142+
return NormalizedDistName(name.replace("-", "_").replace(".", "_").lower())
143+
144+
65145
def discover_decorated_recipes_in_module(
66146
module,
67147
) -> List[Tuple[Type[Recipe], List[CategoryDescriptor]]]:

rewrite-python/rewrite/src/rewrite/rpc/server.py

Lines changed: 100 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@
3333
import threading
3434

3535
from pathlib import Path
36-
from typing import Dict, Any, Optional, List, Callable
36+
from typing import Dict, Any, Optional, List, Callable, Set
3737
from uuid import uuid4
3838

39+
from rewrite.discovery import RecipeAttribution, RecipeName
40+
3941
# Deeply nested LST nodes (e.g., 256 implicitly concatenated strings) can
4042
# overflow the default recursion limit (1000) during RPC serialization.
4143
sys.setrecursionlimit(sys.getrecursionlimit() * 2)
@@ -584,21 +586,43 @@ def handle_reset(params: dict) -> bool:
584586
# Global marketplace instance (lazily initialized)
585587
_marketplace = None
586588

589+
# Tracks which distribution's entry point activated which recipes. Used by
590+
# handle_install_recipes to scope its response to the requested distribution
591+
# and by handle_get_marketplace to filter the singleton marketplace down to
592+
# a single package.
593+
_attribution = RecipeAttribution()
594+
587595

588596
def _get_marketplace():
589-
"""Get or create the global marketplace instance."""
597+
"""Get or create the global marketplace instance.
598+
599+
Discovery populates ``_attribution`` so each recipe is attributed to the
600+
distribution whose entry point activated it. Without that attribution, a
601+
later GetMarketplace or InstallRecipes for package X would incorrectly
602+
return every recipe in the singleton, including the built-in
603+
``org.openrewrite.python.*`` recipes activated by the ``openrewrite``
604+
distribution itself.
605+
"""
590606
global _marketplace
591607
if _marketplace is None:
592-
from rewrite.discovery import discover_recipes
608+
from rewrite.discovery import discover_recipes, recipe_name_set
593609
from rewrite.marketplace import RecipeMarketplace
594610
from rewrite import activate
595611

596-
# First try to discover from installed packages
597-
_marketplace = discover_recipes()
612+
_marketplace = RecipeMarketplace()
598613

599-
# Also activate local recipes (in case package isn't installed)
600-
# This ensures recipes work during development
614+
# Discover from installed packages, tracking which distribution
615+
# contributed each recipe.
616+
discover_recipes(marketplace=_marketplace, attribution=_attribution)
617+
618+
# Also activate local recipes (in case the openrewrite distribution
619+
# isn't pip-installed, e.g., when running from source). When it is
620+
# installed, discovery already covered these and install() will dedupe
621+
# by name; attribute the source-mode additions to "openrewrite" so
622+
# they're returned by GetMarketplace/InstallRecipes for that package.
623+
before = recipe_name_set(_marketplace)
601624
activate(_marketplace)
625+
_attribution.record("openrewrite", recipe_name_set(_marketplace) - before)
602626

603627
return _marketplace
604628

@@ -616,16 +640,24 @@ def handle_install_recipes(params: dict) -> dict:
616640
- 'recipes': {'packageName': str, 'version': str|None} - A package spec
617641
618642
Returns:
619-
Dict with 'recipesInstalled' count and 'version' (if resolved)
643+
Dict with:
644+
- 'recipesInstalled': count of recipes added to the marketplace by
645+
this call (zero on idempotent reinstalls).
646+
- 'version': resolved version (if known).
647+
- 'recipes': cumulative list of {descriptor, categoryPaths} rows
648+
for recipes attributed to this distribution. Stable across
649+
reinstalls; this is what the caller binds to its bundle.
620650
"""
621651
import importlib
622652
import importlib.util
653+
from rewrite.discovery import recipe_name_set
623654

624655
marketplace = _get_marketplace()
625-
before_count = len(list(marketplace.all_recipes()))
626656

627657
recipes = params.get('recipes')
628658
installed_version = None
659+
package_name: Optional[str] = None
660+
recipes_added = 0
629661

630662
if isinstance(recipes, str):
631663
# Local file path - package should already be installed by caller
@@ -636,7 +668,11 @@ def handle_install_recipes(params: dict) -> dict:
636668
# For local paths, we look for the package name from setup.py/pyproject.toml
637669
package_name = _find_package_name(local_path)
638670
if package_name:
671+
before = recipe_name_set(marketplace)
639672
_import_and_activate_package(package_name, marketplace, local_path)
673+
added = recipe_name_set(marketplace) - before
674+
_attribution.record(package_name, added)
675+
recipes_added = len(added)
640676

641677
elif isinstance(recipes, dict):
642678
# Package spec with name and optional version - package should already be installed
@@ -655,17 +691,31 @@ def handle_install_recipes(params: dict) -> dict:
655691
except Exception:
656692
pass
657693

694+
before = recipe_name_set(marketplace)
658695
_import_and_activate_package(package_name, marketplace)
696+
added = recipe_name_set(marketplace) - before
697+
_attribution.record(package_name, added)
698+
recipes_added = len(added)
659699
else:
660700
raise ValueError(f"Invalid recipes parameter: {recipes}")
661701

662-
after_count = len(list(marketplace.all_recipes()))
663-
recipes_installed = after_count - before_count
664-
665-
logger.info(f"InstallRecipes: installed {recipes_installed} recipes")
702+
package_rows: List[dict] = []
703+
if package_name:
704+
# recipes_for returns an empty set (not None) when the package activated
705+
# nothing, so the filter scopes the result to "this package's recipes"
706+
# (zero of them) instead of falling back to "no filter" and returning
707+
# everything.
708+
package_rows = _collect_marketplace_rows(
709+
marketplace, recipe_filter=_attribution.recipes_for(package_name)
710+
)
711+
712+
logger.info(
713+
f"InstallRecipes: {recipes_added} new, {len(package_rows)} cumulative for {package_name}"
714+
)
666715
return {
667-
'recipesInstalled': recipes_installed,
668-
'version': installed_version
716+
'recipesInstalled': recipes_added,
717+
'version': installed_version,
718+
'recipes': package_rows,
669719
}
670720

671721

@@ -828,22 +878,48 @@ def normalize(name: str) -> str:
828878
def handle_get_marketplace(params: dict) -> List[dict]:
829879
"""Handle a GetMarketplace RPC request.
830880
831-
Returns all recipes organized by category in a format compatible with Java.
881+
Returns recipes organized by category in a format compatible with Java.
882+
883+
Args:
884+
params: Optional dict with a 'packageName' key. When supplied, the
885+
response is filtered to recipes attributed to that distribution,
886+
so callers requesting a specific bundle don't get every recipe
887+
in the singleton marketplace.
832888
833889
Returns:
834890
List of dicts with 'descriptor' and 'categoryPaths' for each recipe.
835891
"""
836-
from dataclasses import asdict
837-
838892
marketplace = _get_marketplace()
893+
894+
recipe_filter: Optional[Set[RecipeName]] = None
895+
if isinstance(params, dict):
896+
package_name = params.get('packageName')
897+
if isinstance(package_name, str) and package_name:
898+
recipe_filter = _attribution.recipes_for(package_name)
899+
900+
rows = _collect_marketplace_rows(marketplace, recipe_filter=recipe_filter)
901+
logger.info(f"GetMarketplace: returning {len(rows)} recipes")
902+
return rows
903+
904+
905+
def _collect_marketplace_rows(
906+
marketplace,
907+
recipe_filter: Optional[Set[RecipeName]] = None,
908+
) -> List[dict]:
909+
"""Walk the marketplace and return recipe rows in GetMarketplaceResponse shape.
910+
911+
A recipe that appears in multiple categories produces one row whose
912+
``categoryPaths`` lists each path. When ``recipe_filter`` is provided,
913+
recipes whose name isn't in the set are skipped.
914+
"""
839915
rows: List[dict] = []
840916

841-
def collect_recipes(category, category_path: List[dict]):
842-
"""Recursively collect recipes from a category and its subcategories."""
917+
def collect(category, category_path: List[dict]) -> None:
843918
current_path = [*category_path, _category_descriptor_to_dict(category.descriptor)]
844919

845-
for recipe_name, (recipe_desc, _recipe_class) in category.recipes.items():
846-
# Check if we already have this recipe (it can appear in multiple categories)
920+
for _recipe_name, (recipe_desc, _recipe_class) in category.recipes.items():
921+
if recipe_filter is not None and recipe_desc.name not in recipe_filter:
922+
continue
847923
existing = next((r for r in rows if r['descriptor']['name'] == recipe_desc.name), None)
848924
if existing:
849925
existing['categoryPaths'].append(current_path)
@@ -854,13 +930,11 @@ def collect_recipes(category, category_path: List[dict]):
854930
})
855931

856932
for subcategory in category.categories:
857-
collect_recipes(subcategory, current_path)
933+
collect(subcategory, current_path)
858934

859-
# Start from the root's children (skip the root itself)
860935
for category in marketplace.categories():
861-
collect_recipes(category, [])
936+
collect(category, [])
862937

863-
logger.info(f"GetMarketplace: returning {len(rows)} recipes")
864938
return rows
865939

866940

0 commit comments

Comments
 (0)