From 432a7e1a9dfbfd51e7cb046a305041f7c918dea6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0oltis?= Date: Thu, 28 Nov 2024 12:39:13 +0100 Subject: [PATCH] yarn v1: Update workspace metadata extraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the time of implementing Workspace model, there was no PackageJson class, therefore only the raw dictionary was availible for use inside the model. Replace raw dictionary access for package.json data with the PackageJson class to enhance type safety and readability. The Workspace model now uses PackageJson instead of a dictionary for package_contents, with updated validators. Signed-off-by: Michal Šoltis --- .../package_managers/yarn_classic/resolver.py | 4 +-- .../yarn_classic/workspaces.py | 35 ++++++++++++------- .../yarn_classic/test_resolver.py | 15 +++++--- .../yarn_classic/test_workspaces.py | 33 +++++++++-------- 4 files changed, 54 insertions(+), 33 deletions(-) diff --git a/cachi2/core/package_managers/yarn_classic/resolver.py b/cachi2/core/package_managers/yarn_classic/resolver.py index bb747c68a..4949bd11b 100644 --- a/cachi2/core/package_managers/yarn_classic/resolver.py +++ b/cachi2/core/package_managers/yarn_classic/resolver.py @@ -218,8 +218,8 @@ def _get_workspace_packages( """Return a WorkspacePackage for each Workspace.""" return [ WorkspacePackage( - name=ws.package_contents["name"], - version=ws.package_contents.get("version"), + name=ws.package_json.data.get("name"), + version=ws.package_json.data.get("version"), relpath=ws.path.relative_to(source_dir.path), ) for ws in workspaces diff --git a/cachi2/core/package_managers/yarn_classic/workspaces.py b/cachi2/core/package_managers/yarn_classic/workspaces.py index b5f558498..02f091c3c 100644 --- a/cachi2/core/package_managers/yarn_classic/workspaces.py +++ b/cachi2/core/package_managers/yarn_classic/workspaces.py @@ -6,20 +6,27 @@ import pydantic from cachi2.core.errors import PackageRejected +from cachi2.core.package_managers.yarn_classic.project import PackageJson from cachi2.core.rooted_path import RootedPath class Workspace(pydantic.BaseModel): - """Workspace model.""" + """ + Workspace model. + + Attributes: + path: Path to workspace directory. + package_json: Content of package.json file. + """ - path: Path # path to a workspace. - package_contents: dict # package data extracted from path/"package.json". + path: Path + package_json: PackageJson - @pydantic.field_validator("package_contents") - def _ensure_package_is_named(cls, package_contents: dict) -> dict: - if "name" not in package_contents: + @pydantic.field_validator("package_json") + def _ensure_package_is_named(cls, package_json: PackageJson) -> PackageJson: + if "name" not in package_json.data: raise ValueError("Workspaces must contain 'name' field.") - return package_contents + return package_json def ensure_no_path_leads_out( @@ -83,21 +90,23 @@ def _read_package_from(path: RootedPath) -> dict[str, Any]: return json.loads(path.join_within_root("package.json").path.read_text()) -def extract_workspace_metadata( - package_path: RootedPath, -) -> list[Workspace]: +def extract_workspace_metadata(package_path: RootedPath) -> list[Workspace]: """Extract workspace metadata from a package.""" - processed_package = _read_package_from(package_path) - workspaces_globs = _extract_workspaces_globs(processed_package) + package_json = PackageJson.from_file(package_path.join_within_root("package.json")) + workspaces_globs = _extract_workspaces_globs(package_json.data) workspaces_paths = _get_workspace_paths(workspaces_globs, package_path) ensure_no_path_leads_out(workspaces_paths, package_path) _ensure_workspaces_are_well_formed(workspaces_paths) + parsed_workspaces = [] for wp in workspaces_paths: parsed_workspaces.append( Workspace( path=wp, - package_contents=_read_package_from(package_path.join_within_root(wp)), + package_json=PackageJson.from_file( + package_path.join_within_root(wp, "package.json") + ), ) ) + return parsed_workspaces diff --git a/tests/unit/package_managers/yarn_classic/test_resolver.py b/tests/unit/package_managers/yarn_classic/test_resolver.py index 1b5ec44b2..af96349a5 100644 --- a/tests/unit/package_managers/yarn_classic/test_resolver.py +++ b/tests/unit/package_managers/yarn_classic/test_resolver.py @@ -324,16 +324,23 @@ def test__get_main_package_no_name(rooted_tmp_path: RootedPath) -> None: def test__get_workspace_packages(rooted_tmp_path: RootedPath) -> None: - workspace_path = rooted_tmp_path.join_within_root("foo").path + workspace_path = rooted_tmp_path.join_within_root("foo") + workspace_path.path.mkdir() + + package_json_path = workspace_path.join_within_root("package.json") + package_json_path.path.write_text('{"name": "foo", "version": "1.0.0"}') + + package_json = PackageJson.from_file(package_json_path) workspace = Workspace( - path=workspace_path, - package_contents={"name": "foo", "version": "1.0.0"}, + path=workspace_path.path, + package_json=package_json, ) + expected = [ WorkspacePackage( name="foo", version="1.0.0", - relpath=workspace_path.relative_to(rooted_tmp_path.path), + relpath=workspace_path.path.relative_to(rooted_tmp_path.path), ) ] diff --git a/tests/unit/package_managers/yarn_classic/test_workspaces.py b/tests/unit/package_managers/yarn_classic/test_workspaces.py index 31623484c..3b16ebfcd 100644 --- a/tests/unit/package_managers/yarn_classic/test_workspaces.py +++ b/tests/unit/package_managers/yarn_classic/test_workspaces.py @@ -4,6 +4,7 @@ import pytest from cachi2.core.errors import PathOutsideRoot +from cachi2.core.package_managers.yarn_classic.project import PackageJson from cachi2.core.package_managers.yarn_classic.workspaces import ( Workspace, _extract_workspaces_globs, @@ -13,21 +14,19 @@ from cachi2.core.rooted_path import RootedPath -@mock.patch("cachi2.core.package_managers.yarn_classic.workspaces._read_package_from") @mock.patch("cachi2.core.package_managers.yarn_classic.workspaces._get_workspace_paths") def test_packages_with_workspaces_outside_source_dir_are_rejected( mock_get_ws_paths: mock.Mock, - mock_read_package_from: mock.Mock, + rooted_tmp_path: RootedPath, ) -> None: - mock_read_package_from.return_value = {"workspaces": ["../../usr"]} - mock_get_ws_paths.return_value = [Path("/tmp/foo/bar"), Path("/usr")] - package_path = RootedPath("/tmp/foo") + package_json_path = rooted_tmp_path.join_within_root("package.json") + package_json_path.path.write_text('{"workspaces": ["../../usr"]}') + mock_get_ws_paths.return_value = [Path("/usr")] with pytest.raises(PathOutsideRoot): - extract_workspace_metadata(package_path) + extract_workspace_metadata(rooted_tmp_path) -@mock.patch("cachi2.core.package_managers.yarn_classic.workspaces._read_package_from") @mock.patch("cachi2.core.package_managers.yarn_classic.workspaces._get_workspace_paths") @mock.patch( "cachi2.core.package_managers.yarn_classic.workspaces._ensure_workspaces_are_well_formed" @@ -35,19 +34,25 @@ def test_packages_with_workspaces_outside_source_dir_are_rejected( def test_workspaces_could_be_parsed( mock_workspaces_ok: mock.Mock, mock_get_ws_paths: mock.Mock, - mock_read_package_from: mock.Mock, + rooted_tmp_path: RootedPath, ) -> None: - mock_read_package_from.side_effect = [{"workspaces": ["quux"]}, {"name": "inner_package"}] - mock_get_ws_paths.return_value = [Path("/tmp/foo/bar")] - package_path = RootedPath("/tmp/foo") + package_json_path = rooted_tmp_path.join_within_root("package.json") + package_json_path.path.write_text('{"name": "outer_package", "workspaces": ["foo"]}') + + workspace_path = rooted_tmp_path.join_within_root("foo") + workspace_path.path.mkdir() + workspace_package_json_path = workspace_path.join_within_root("package.json") + workspace_package_json_path.path.write_text('{"name": "inner_package"}') + + mock_get_ws_paths.return_value = [workspace_path.path] expected_result = [ Workspace( - path="/tmp/foo/bar", - package_contents={"name": "inner_package"}, + path=workspace_path.path, + package_json=PackageJson.from_file(workspace_package_json_path), ), ] - result = extract_workspace_metadata(package_path) + result = extract_workspace_metadata(rooted_tmp_path) assert result == expected_result