fix scope logic

openupm · Sep 15, 2024 · 63a3d22 · 63a3d22
1 parent ea19990
commit 63a3d22
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 16 deletions.
diff --git a/src/app/add-dependencies.ts b/src/app/add-dependencies.ts
@@ -31,6 +31,7 @@ import {
   mapScopedRegistry,
   setDependency,
 } from "../domain/project-manifest";
+import { recordEntries } from "../domain/record-utils";
 import { type Registry } from "../domain/registry";
 import { type RegistryUrl, unityRegistryUrl } from "../domain/registry-url";
 import {
@@ -228,9 +229,9 @@ export async function addDependenciesUsing(
   async function resolveScopesFor(
     packageName: DomainName,
     verison: SemanticVersion,
-    isUnityPackage: boolean
-  ): Promise<ReadonlyArray<DomainName>> {
-    if (isUnityPackage) return [packageName];
+    source: RegistryUrl
+  ): Promise<Readonly<Record<RegistryUrl, ReadonlyArray<DomainName>>>> {
+    if (source === unityRegistryUrl) return {};
 
     await debugLog(`fetch: ${makePackageReference(packageName, verison)}`);
     const dependencyGraph = await resolveDependenciesUsing(
@@ -243,7 +244,7 @@ export async function addDependenciesUsing(
     );
 
     const unresolvedDependencies = Array.of<UnresolvedDependency>();
-    const scopes = Array.of<DomainName>();
+    const scopes: Record<RegistryUrl, DomainName[]> = {};
     for (const [
       dependencyName,
       dependencyVersion,
@@ -273,8 +274,8 @@ export async function addDependenciesUsing(
         dependency.source === unityRegistryUrl;
       if (isUnityPackage) continue;
 
-      // add depsValid to packagesInScope.
-      scopes.push(dependencyName);
+      if (!(dependency.source in scopes)) scopes[dependency.source] = [];
+      scopes[dependency.source]!.push(dependencyName);
     }
 
     // print suggestion for depsInvalid
@@ -401,23 +402,22 @@ export async function addDependenciesUsing(
       requestedVersion
     );
 
-    const isUnityPackage = source === unityRegistryUrl;
-
-    const packagesInScope = await resolveScopesFor(
+    const scopesBySource = await resolveScopesFor(
       packageName,
       versionToAdd,
-      isUnityPackage
+      source
     );
 
-    if (!isUnityPackage && packagesInScope.length > 0) {
-      manifest = mapScopedRegistry(manifest, source, (initial) => {
-        let updated = initial ?? makeEmptyScopedRegistryFor(source);
+    recordEntries(scopesBySource).forEach(([scopeSource, scopes]) => {
+      manifest = mapScopedRegistry(manifest, scopeSource, (initial) => {
+        let updated = initial ?? makeEmptyScopedRegistryFor(scopeSource);
 
-        updated = packagesInScope.reduce(addScope, updated!);
+        updated = scopes.reduce(addScope, updated!);
 
         return updated;
       });
-    }
+    });
+
     if (shouldAddTestable) manifest = addTestable(manifest, packageName);
 
     return addDependencyToManifest(manifest, packageName, versionToAdd);

diff --git a/test/integration/app/add-dependencies.test.ts b/test/integration/app/add-dependencies.test.ts
@@ -20,7 +20,12 @@ import { getRegistryPackumentUsing } from "../../../src/io/registry";
 import { fetchCheckUrlExists } from "../../../src/io/www";
 import { buildPackument } from "../../common/data-packument";
 import { buildProjectManifest } from "../../common/data-project-manifest";
-import { someRegistry, someRegistryUrl } from "../../common/data-registry";
+import {
+  otherRegistry,
+  otherRegistryUrl,
+  someRegistry,
+  someRegistryUrl,
+} from "../../common/data-registry";
 import { makeMockLogger } from "../../common/log.mock";
 import { MockFs } from "../fs.mock";
 import { mockRegistryPackuments } from "../registry.mock";
@@ -34,6 +39,7 @@ describe("add dependencies", () => {
       version.set("unity", "2022.2")
     )
   );
+
   const somePackument = buildPackument("com.some.package", (packument) =>
     packument.addVersion(someVersion, (version) =>
       version
@@ -42,6 +48,14 @@ describe("add dependencies", () => {
     )
   );
 
+  const anotherPackument = buildPackument("com.another.package", (packument) =>
+    packument.addVersion(someVersion, (version) =>
+      version
+        .set("unity", "2022.2")
+        .addDependency(somePackument.name, someVersion)
+    )
+  );
+
   const badEditorPackument = buildPackument("com.bad.package", (packument) =>
     packument.addVersion(someVersion, (version) =>
       // eslint-disable-next-line @typescript-eslint/ban-ts-comment
@@ -100,6 +114,7 @@ describe("add dependencies", () => {
   });
 
   beforeEach(() => {
+    mockRegistryPackuments(otherRegistryUrl, [anotherPackument]);
     mockRegistryPackuments(someRegistryUrl, [
       somePackument,
       packumentWithBadDependency,
@@ -321,6 +336,38 @@ describe("add dependencies", () => {
     );
   });
 
+  it("should add package with dependencies in multiple registries", async () => {
+    const { addDependencies, mockFs } = makeDependencies();
+
+    await addDependencies(
+      someProjectDir,
+      null,
+      [someRegistry, otherRegistry, unityRegistry],
+      false,
+      false,
+      [anotherPackument.name]
+    );
+
+    const actual = mockFs.tryGetUnityProject(someProjectDir);
+    expect(actual).toEqual(
+      expect.objectContaining({
+        dependencies: { [anotherPackument.name]: someVersion },
+        scopedRegistries: expect.arrayContaining([
+          {
+            name: expect.any(String),
+            url: otherRegistryUrl,
+            scopes: [anotherPackument.name],
+          },
+          {
+            name: expect.any(String),
+            url: someRegistryUrl,
+            scopes: [somePackument.name],
+          },
+        ]),
+      })
+    );
+  });
+
   it("should be atomic", async () => {
     const { addDependencies, mockFs } = makeDependencies();